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

import resolution incorrect with same-named file and external module #5151

Closed
silverwind opened this issue Oct 13, 2021 · 12 comments
Closed

Comments

@silverwind
Copy link

silverwind commented Oct 13, 2021

Bug description

When a python file and a external module have the same name, python resolves a import for that name to the external module, but pylint resolves it to the current file which causes false positives in rules import-self, no-name-in-module, no-member and more module-related rules.

To reproduce:

  • Install redis pypi module using your favourite package manager
  • Create a file redis.py with content import redis
  • Lint the file using pylint redis.py

Pylint output

************* Module redis
redis.py:1:0: W0406: Module import itself (import-self)

Expected behavior

No error

Pylint version

pylint 2.11.1
astroid 2.8.2
Python 3.9.7 (default, Sep  3 2021, 12:45:31)
[Clang 12.0.0 (clang-1200.0.32.29)]

OS / Environment

macOS 11

Additional dependencies

redis==3.2.0

@silverwind silverwind added Bug 🪲 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Oct 13, 2021
@DanielNoord DanielNoord added False Positive 🦟 A message is emitted but nothing is wrong with the code Import system and removed Bug 🪲 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Feb 9, 2022
@timmartin
Copy link
Contributor

I think if you create an file called redis.py in the current directory, with the content:

import redis

and that file is not part of a package then that file is importing itself, because the directory of the currently-executing script will always be first on sys.path, and so the import will find the local file redis.py before it searches libraries.

Where there might be an issue is with a file called redis.py within a package. In that case, the directory containing the redis.py will no longer be on that path. But in that case, I can't reproduce a case where pylint will complain about it importing itself. If I have the file be in a package (just add an __init__.py to the directory) then pylint stops complaining about importing self.

I suppose if the file in question is imported from another Python file, and is not in a package but is imported because its directory has been added to sys.path after the standard import locations, then pylint will report it as import self and Python will not, although I haven't tested this. I don't think pylint can know about runtime modification to sys.path.

Is there a more specific case that we should be investigating here?

@jacobtylerwalls jacobtylerwalls added Question Waiting on author Indicate that maintainers are waiting for a message of the author and removed False Positive 🦟 A message is emitted but nothing is wrong with the code labels May 7, 2022
@jacobtylerwalls
Copy link
Member

I suggest closing this in favor of #7289 and retitling it -- that one focuses on the case where we have redis or gzip instead of redis.py.

@DanielNoord
Copy link
Collaborator

DanielNoord commented Aug 22, 2022

Sorry, I missed the investigation by Tim. That said, I can still get an incorrect import-self to trigger when I create a pylint.py file in the root of the pylint project with a import pylint line.

pylint-dev/astroid#1747 fixes this.

Edit: Never mind, pylint-dev/astroid#1747 doesn't fix this. Needs some investigation.

@jacobtylerwalls
Copy link
Member

Thanks @DanielNoord. Can you include a snippet from a python session that verifies that python is not importing self? E.g. import pylint; print(pylint) and show the output?

@DanielNoord
Copy link
Collaborator

cat pylint.py
import pylint

print(pylint.modify_sys_path)
❯ python pylint.py
<function modify_sys_path at 0x101dde050>

@DanielNoord
Copy link
Collaborator

Oh wait, pylint-dev/astroid#1747 does fix this. As the no-member warnings go away. As I explained in #7289 (comment) for the import-self to go away we need to actually change the import-self check.

But the successful removal of no-member warnings on pylint.modify_sys_path shows that this works.

@jacobtylerwalls
Copy link
Member

I see something different:

$ cat gzip.py
import gzip
print(gzip)

$ python3 gzip.py
<module 'gzip' from '/Users/jwalls/somefolder/gzip.py'>
<module 'gzip' from '/Users/jwalls/somefolder/gzip.py'>

@DanielNoord
Copy link
Collaborator

Yeah, this is different than the original issue but I think there is still an issue here.
If there is a folder pylint which is a normal python package and there is a pylint.py file that imports pylint then the pylint directory gets precedence. However, due to how we cache modules this doesn't work in astroid. pylint-dev/astroid#1747 does fix this.

@DanielNoord
Copy link
Collaborator

You can also recreate this with:

cd /tmp
mkdir pylint
cd pylint
echo -e "X = 1" > __init__.py
cd ..
echo -e "import pylint\nprint(pylint.X)" > pylint.py

Without the fix in astroid this will warn about no-member, while Python executes this fine.

@DanielNoord
Copy link
Collaborator

Proposal, let pylint-dev/astroid#1747 fix this issue as this one was originally about import order resolution which I clearly what we are fixing there.

Rescope #7289 to fix import-self after we have access to that astroid update so that we can use the Module object instead of doing a string comparison for import-self.

@jacobtylerwalls Does that sound right?

@jacobtylerwalls jacobtylerwalls added Bug 🪲 and removed Question Waiting on author Indicate that maintainers are waiting for a message of the author labels Aug 22, 2022
@jacobtylerwalls jacobtylerwalls added this to the 2.16.0 milestone Aug 22, 2022
@DanielNoord
Copy link
Collaborator

This was fixed in pylint-dev/astroid#1747. We don't need regression tests in pylint but this should be closed with the bump to astroid 2.13 in pylint at some point.

@jacobtylerwalls jacobtylerwalls modified the milestones: 2.16.0, 2.15.0 Aug 25, 2022
@jacobtylerwalls
Copy link
Member

This was fixed with pylint 2.16's upgrade to astroid 2.13; please reopen if still an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants