Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeError exception when method named "format" replaces itself with different method #3686

Closed
BlankSpruce opened this issue Jun 13, 2020 · 16 comments
Assignees

Comments

@BlankSpruce
Copy link

Steps to reproduce

  1. Create a file with following class in either of these two variants:
class Foo:
    def _format(self):
        pass

    def format(self):
        self.format = self._format
        self.format()

or

class Foo:
    def _format(self):
        pass

    def format(self):
        self.format = self._format
        self._format()
  1. Run pylint on such file.

Current behavior

$ pylint reproduce.py
************* Module reproduce
reproduce.py:1:0: C0114: Missing module docstring (missing-module-docstring)
reproduce.py:1:0: C0115: Missing class docstring (missing-class-docstring)
reproduce.py:2:4: R0201: Method could be a function (no-self-use)
reproduce.py:5:4: C0116: Missing function or method docstring (missing-function-docstring)
Traceback (most recent call last):
  File "/home/foobar/.local/bin/pylint", line 8, in <module>
    sys.exit(run_pylint())
  File "/home/foobar/.local/lib/python3.8/site-packages/pylint/__init__.py", line 22, in run_pylint
    PylintRun(sys.argv[1:])
  File "/home/foobar/.local/lib/python3.8/site-packages/pylint/lint/run.py", line 344, in __init__
    linter.check(args)
  File "/home/foobar/.local/lib/python3.8/site-packages/pylint/lint/pylinter.py", line 870, in check
    self._check_files(
  File "/home/foobar/.local/lib/python3.8/site-packages/pylint/lint/pylinter.py", line 904, in _check_files
    self._check_file(get_ast, check_astroid_module, name, filepath, modname)
  File "/home/foobar/.local/lib/python3.8/site-packages/pylint/lint/pylinter.py", line 930, in _check_file
    check_astroid_module(ast_node)
  File "/home/foobar/.local/lib/python3.8/site-packages/pylint/lint/pylinter.py", line 1062, in check_astroid_module
    retval = self._check_astroid_module(
  File "/home/foobar/.local/lib/python3.8/site-packages/pylint/lint/pylinter.py", line 1107, in _check_astroid_module
    walker.walk(ast_node)
  File "/home/foobar/.local/lib/python3.8/site-packages/pylint/utils/ast_walker.py", line 75, in walk
    self.walk(child)
  File "/home/foobar/.local/lib/python3.8/site-packages/pylint/utils/ast_walker.py", line 75, in walk
    self.walk(child)
  File "/home/foobar/.local/lib/python3.8/site-packages/pylint/utils/ast_walker.py", line 72, in walk
    callback(astroid)
  File "/home/foobar/.local/lib/python3.8/site-packages/pylint/checkers/classes.py", line 992, in visit_functiondef
    for obj in ancestor.lookup(node.name)[1]:
  File "/home/foobar/.local/lib/python3.8/site-packages/astroid/node_classes.py", line 1067, in lookup
    return self.scope().scope_lookup(self, name)
  File "/home/foobar/.local/lib/python3.8/site-packages/astroid/scoped_nodes.py", line 2239, in scope_lookup
    return frame._scope_lookup(node, name, offset)
  File "/home/foobar/.local/lib/python3.8/site-packages/astroid/scoped_nodes.py", line 209, in _scope_lookup
    return pscope.scope_lookup(node, name)
  File "/home/foobar/.local/lib/python3.8/site-packages/astroid/scoped_nodes.py", line 514, in scope_lookup
    return self._scope_lookup(node, name, offset)
  File "/home/foobar/.local/lib/python3.8/site-packages/astroid/scoped_nodes.py", line 198, in _scope_lookup
    stmts = node._filter_stmts(self.locals[name], self, offset)
  File "/home/foobar/.local/lib/python3.8/site-packages/astroid/node_classes.py", line 1155, in _filter_stmts
    if stmt.fromlineno > mylineno > 0:
TypeError: '>' not supported between instances of 'NoneType' and 'int'

Expected behavior

$ pylint reproduce.py
************* Module reproduce
reproduce.py:1:0: C0114: Missing module docstring (missing-module-docstring)
reproduce.py:1:0: C0115: Missing class docstring (missing-class-docstring)
reproduce.py:5:4: C0116: Missing function or method docstring (missing-function-docstring)
reproduce.py:5:4: E0202: An attribute defined in reproduce line 6 hides this method (method-hidden)
reproduce.py:1:0: R0903: Too few public methods (1/2) (too-few-public-methods)

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

Additional info

The issue isn't reproduced if replaced method has different name:

class Foo:
    def _format(self):
        pass

    def definitely_not_a_format(self):
        self.definitely_not_a_format = self._format
        self.definitely_not_a_format()

or it doesn't replace itself:

class Foo:
    def _format(self):
        pass

    def format(self):
        self._format()

pylint --version output

pylint 2.5.3
astroid 2.4.0
Python 3.8.3 (default, May 17 2020, 18:15:42)
[GCC 10.1.0]
@bhack
Copy link

bhack commented Jan 26, 2021

Do we need to update the minimal astroid version in https://github.com/PyCQA/pylint/blob/master/DEPENDS (and in debian control)?

copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this issue Jan 27, 2021
They are caused by pylint-dev/pylint#3686

Ping #46046

PiperOrigin-RevId: 353980227
Change-Id: I7a33be622a752f0be514da495782cf04dd8010a3
@hippo91
Copy link
Contributor

hippo91 commented Jan 31, 2021

@PCManticore what do you think about it?

@PCManticore
Copy link
Contributor

@hippo91 and @bhack Let's do it. Those are really old versions.

@bhack
Copy link

bhack commented Feb 7, 2021

We Need a new astroid release

@bhack
Copy link

bhack commented Feb 7, 2021

Check pylint-dev/astroid#886

@hippo91
Copy link
Contributor

hippo91 commented Feb 16, 2021

@bhack the new astroid version (1.5) has been released.

@bhack
Copy link

bhack commented Feb 16, 2021

@hippo91 The minimal required version is still astroid 1.3.6 https://github.com/PyCQA/pylint/blob/master/DEPENDS

@hippo91
Copy link
Contributor

hippo91 commented Mar 3, 2021

@bhack sorry for the delay. I'm not aware of this kind of DEPENDS file. Can you explain me what is its purpose please?

@bhack
Copy link

bhack commented Mar 3, 2021

Probably it is only for the MANIFEST https://github.com/PyCQA/pylint/blob/master/MANIFEST.in

@hippo91
Copy link
Contributor

hippo91 commented Mar 5, 2021

@bhack i have no idea of the possible use of the DEPENDS file. It has been added 15 years ago and i don't know if we still need it. @PCManticore @AWhetter @Pierre-Sassoulas @cdce8p have you any idea about this?

@Pierre-Sassoulas
Copy link
Member

I think we're going to need to dust up the packaging at some point, we still have a compatibility with distutils in the setup.py who was replaced by setuptools a decade ago, and it seems there are also outdated confusing files all over the place. Right now the astroid dependencie version is defined in https://github.com/PyCQA/pylint/blob/master/pylint/__pkginfo__.py#L42. How about a clean new issue for moving to poetry/pyproject.toml and removing all this?

@bhack
Copy link

bhack commented Mar 5, 2021

We have also Astroid version for Debian package in https://github.com/PyCQA/pylint/blob/master/debian/control#L22

@cdce8p
Copy link
Member

cdce8p commented Mar 5, 2021

@sandrotosi Maybe you can help here. What files are needed for including pylint in debian?
Most of the files in pylint/debian haven't been updated in over a decade. Are they even necessary or can they be removed entirely?

@sandrotosi
Copy link
Contributor

not sure what you mean by "including pylint in debian". if it's having pylint available via debian repositories, then that's done via this repo https://salsa.debian.org/python-team/packages/pylint where (tbh) i completely disregard the debian/ directory provided upstream (that debianization is released under the same license of pylint, so feel free to use it accordingly).

if you mean creating a debian package once closinig PyCQA/pylint, then that's a bit more complicated, and as someone else already pointed out, the content of debian/ has not been kept up-to-date with debian packaging standards, and may need a serious revamp, but at that point i'd ask if it's really worth to keep it around at all (a question you guys should answer :) )

@AWhetter
Copy link
Contributor

AWhetter commented Mar 5, 2021

I'm all for moving to poetry and pyproject.toml.
If debian aren't using the files from the debian folder (which @sandrotosi has confirmed) then let's remove those.

@Pierre-Sassoulas
Copy link
Member

Just opened #4193 to follow through.

Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Apr 4, 2021
Pierre-Sassoulas added a commit that referenced this issue Apr 4, 2021
See discussion here: #3686
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants