pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.32k stars 1.14k forks source link

False Negative function-redefined when function named with leading underscore #9894

Open daviderie opened 2 months ago

daviderie commented 2 months ago

Bug description

When defining a function that is named with a leading underscore, Pylint does not raise the function-redefined error as expected.

E.g.:

def _my_func(self):
    ...

def _my_func(self):
    ...

Configuration

defaults

Command used

pylint my_package

Pylint output

-------------------------------------------------------------------
Your code has been rated at 10.00/10

Expected behavior

E0102: method already defined line 86 (function-redefined)

Pylint version

pylint 3.2.6
astroid 3.2.4
Python 3.11.3 (main, May  4 2023, 15:16:43) [Clang 14.0.3 (clang-1403.0.22.14.1)]

OS / Environment

No response

Additional dependencies

No response

Pierre-Sassoulas commented 2 months ago

Thank you for opening the issue. We have some mechanisms so that name with underscore are ignored, they should probably not be ignored for function-redefined though, there's no good reasons to ignore that one imo.

nickdrozd commented 2 months ago

basic_error_checker.py has:

            dummy_variables_rgx = self.linter.config.dummy_variables_rgx
            if dummy_variables_rgx and dummy_variables_rgx.match(node.name):
                return
            self.add_message(
                "function-redefined",
                node=node,
                args=(redeftype, defined_self.fromlineno),
            )

Test case:

def dummy_func():
    """First dummy function"""
    pass

def dummy_func():
    """Second dummy function, don't emit function-redefined message
    because of the dummy name"""
    pass

I guess the idea is to skip this check for certain names? But I don't know why that would be desirable.

Pierre-Sassoulas commented 2 months ago

It's useful when you want to ignore that a variable is not used for example, for for _ in range(3): or in a function definition for an API you can't change (without a pylint: disable). It's been extended too much here, function-redefined has no reason to be disabled with a shorthand (I don't see why it would ever be disabled with a pragma either?). The semantic between variable and function prefixed by _ is very different (protected function vs known unused variable) and are not interchangeable.