False negative for `bad-thread-instantiation`

[clavedeluna]

Bug description

# module test.py

import threading


def thread_target(n):
    print(n ** 2)


thread = threading.Thread(thread_target, args=(10,))
thread.start()

I actually saw this example in pylint-errors and expected it to work.

Configuration

None

Command used

pylint test.py

Pylint output

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

Expected behavior

test.py:8:9: W1506: threading.Thread needs the target function (bad-thread-instantiation)

Pylint version

pylint 2.16.0-dev
astroid 2.12.10
Python 3.8.9 (default, Apr 13 2022, 08:48:07)

OS / Environment

Mac

Additional dependencies

No response

[clavedeluna]

A quick investigation shows that this is the relevant code

    def _check_bad_thread_instantiation(self, node: nodes.Call) -> None:
        if not node.kwargs and not node.keywords and len(node.args) <= 1:
            self.add_message("bad-thread-instantiation", node=node)

The message is not emitted if there are at least 2 args passed in. However, I would argue in these test cases:
thread = threading.Thread(thread_target, args=(10,))
threading.Thread(None, lambda: None)

which have 2 args and would currently not emit the message, we should in fact warn the user because they have mistakenly believed the first arg is the target func. I think this is in fact the spirit of this message "By default, the first parameter is the group param, not the target param.", So, I think we should always warn the user if the target= kwarg is not specified.

If not, this issue can be closed, or even better, this test case can be added to demonstrate why we're OK with it not emitting the message.

[Pierre-Sassoulas]

Thank you for the investigation @clavedeluna, I think the check is naive right now and making it more clever make sense.