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

Plugin appears to broken by astroid 2.9.1 #343

Closed
amdw opened this issue Jan 1, 2022 · 16 comments · Fixed by #344
Closed

Plugin appears to broken by astroid 2.9.1 #343

amdw opened this issue Jan 1, 2022 · 16 comments · Fixed by #344

Comments

@amdw
Copy link

amdw commented Jan 1, 2022

With astroid 2.9.1, pylint-django appears not to work correctly.

For example, it produces errors like Class 'MyModel' has no 'objects' member (no-member), even when it should be able to infer that the objects member exists.

Full repro steps, including Pipenv.lock file with precise package versions, is available in the following repo:

https://github.com/amdw/pylint-django-repro

@amdw amdw changed the title Plugin appears to b Plugin appears to broken by astroid 2.9.1 Jan 1, 2022
@jhofeditz
Copy link

I have the same problem. Downgrading to 2.11 fixes it.
• Updating astroid (2.9.1 -> 2.8.6)
• Updating pylint (2.12.2 -> 2.11.1)

@amdw
Copy link
Author

amdw commented Jan 1, 2022

With my repro, the problem can be fixed just by downgrading to astroid 2.9.0 (full instructions in the readme).

I'm unsure whether the actual problem here is in pylint-django, astroid or pylint.

It was quite difficult to debug this, as there were no error messages, just a silent failure to behave as expected. Fortunately I had a working project available, and I was able to start with the simple repro and downgrade dependencies one-by-one to match that project until I pinpointed that the problem was with astroid.

Also fortunately, astroid starts with an "a", so was one of the first dependencies I tried. :)

@amdw
Copy link
Author

amdw commented Jan 2, 2022

I should probably have also explicitly said that 2.9.1 is the latest version of astroid at the time of writing, released on 31st December.

So if people try to install pylint-django from a package manager like pip right now, they will run into this problem.

@grnxp
Copy link

grnxp commented Jan 2, 2022

The problem might be in astroid, in the scoped_nodes.py file or scoped_nodes module.

When pylint-django defines its register method, it tries from pylint_django.augmentations apply_augmentations.
At this point, we have an from astroid.scoped_nodes import ClassDef as ScopedClass, which crashes.

In Astroid version 2.9.1, the _is_metaclass function cannot be imported from astroid.nodes.scoped_nodes:

>>> from astroid.scoped_nodes import ClassDef
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/fred/.venvs/ultron/lib/python3.9/site-packages/astroid/scoped_nodes.py", line 5, in <module>
    from astroid.nodes.scoped_nodes import (
ImportError: cannot import name '_is_metaclass' from 'astroid.nodes.scoped_nodes' (/home/fred/.venvs/ultron/lib/python3.9/site-packages/astroid/nodes/scoped_nodes/__init__.py)

There might have been a change on this module between v2.9.0 and v2.9.1.

(I'll continue later this afternoon).

@carlio
Copy link
Member

carlio commented Jan 2, 2022

There is a deprecation warning that scoped_nodes is going away in astroid 3.0 - maybe that was brought forward.

@carlio
Copy link
Member

carlio commented Jan 2, 2022

@GrimBox can you try with the code on this branch https://github.com/PyCQA/pylint-django/tree/343-astroid-2.9.1-compat

That seems to fix things - at least the tests are passing. (except the code formatting one I will fix that ;-) )

@carlio
Copy link
Member

carlio commented Jan 2, 2022

I tested it with the repro repository, it works too so I will finish up some other bugs/PRs and get a release with the fix in.

Thanks for the awesome repro steps @amdw!

carlio added a commit that referenced this issue Jan 2, 2022
@grnxp
Copy link

grnxp commented Jan 2, 2022

@carlio Yes, it works for me too 👍 Thanks !

@carlio
Copy link
Member

carlio commented Jan 2, 2022

Cool!

So version 2.5.0 was just pushed to PyPI with this fix (and a couple of other improvements)

@amdw
Copy link
Author

amdw commented Jan 2, 2022

Many thanks for the rapid fix!

I guess there are two things that surprise me about this bug: (1) upgrading from 2.9.0 to 2.9.1 of a dependency - just a single bugfix version - caused breakage, and (2) depending on a nonexistent class in that dependency did not cause any clear exception or error messages related to that problem, it just caused incorrect lint errors to be reported.

I guess (1) is more of a problem with astroid's version numbering, with a backwards-incompatible change being mislabelled as a bugfix, but might (2) be helped by error handling improvements in pylint-django? Is there anything that can be done to help identify the root cause more easily if something like this happens again?

@carlio
Copy link
Member

carlio commented Jan 2, 2022

I think that (1) is just a backwards-incompatible fix being missed in PR review. scoped_nodes was slated for removal in version 3.0 however, this PR included in the 2.9.1 release seems to have moved that forward a bit quicker.

All in all, this fix was something that needed to be done anyway in pylint-django, it just accelerated a bit!

Regarding (2) - the tests did reveal this problem. Only last week was this repo moved to using github actions (RIP travis!) and the test coverage already checks against latest main branches for dependencies like astroid. However that only triggers on a commit, so to be more pro-active and hopefully find bugs before end users do, I added a scheduled build to try to detect problems with the latest upstream code.

As for why it was not more obvious or throwing an exception, I don't really know to be honest. The tests showed a problem because test output was unexpected, but not due to being unable to import _is_metaclass but rather just different warnings raised about code. So that's a bit of a strange one.

@DanielNoord
Copy link

Just chipping in here to say that this was indeed a mistake in review and from me when moving these files.

There was no intention of deprecating anything in that move. I simply didn't check the imports well enough. Sorry about any trouble caused, we're discussing how to resolve this in pylint-dev/astroid#1325 and I hope to release a new version of astroid soon that should avoid this problem for any other packages!

@carlio
Copy link
Member

carlio commented Jan 3, 2022

No worries @DanielNoord , it was something I knew about for a while (see #339) I had just been a bit lazy about doing anything about it :-)

@MattiasMartens
Copy link

MattiasMartens commented May 17, 2022

hi, i seem to have this closed issue haunting me... running pylint I get this error:

cannot import name 'scoped_nodes' from 'astroid.nodes'

pylint-django version (per Pipfile.lock): 2.5.3
astroid version (per Pipfile.lock): 2.5

behaviour seems to be inconsistent; i can run the command in at least three ways—A) Dockerized Gitlab runner; B) local Docker container; C) local bare metal; and it fails on A and C but not on B.

per the source, the import statement appears to rely on an implicit reexport:

from astroid.scoped_nodes import (
    Module,
    GeneratorExp,
    Lambda,
    DictComp,
    ListComp,
    SetComp,
    FunctionDef,
    ClassDef,
    AsyncFunctionDef,
)

maybe this is the issue. still researching but wanted to log it.

UPDATE: it turns out i can fix this by updating pylint itself, from 2.6.0 to 2.13.7.

not quite sure why it was working in my local Docker container. probably just an issue with lockfile.

@gamesbook
Copy link

This is still failing in a miniconda3 (Python 3.9) environment. The pylint version used by @MattiasMartens seems not to be available?

Can someone please confirm a known list of package versions that do work together for :

  • pylint
  • pylint-django
  • astroid

(ideally in a miniconda environment)

@DanielNoord
Copy link

@gamesbook I have no experience with Anaconda and miniconda but it seems like they only have pylint 2.12 available:
https://anaconda.org/anaconda/pylint

I'm not sure what the process is by you should probably look into asking the maintainers for the Anaconda repositories to upload a new version to their package index.

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

Successfully merging a pull request may close this issue.

7 participants