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-self only compares the name of a module and doesn't consider the actual module being imported #7289

Open
vapier opened this issue Aug 11, 2022 · 9 comments
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Import system Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@vapier
Copy link
Contributor

vapier commented Aug 11, 2022

Bug description

# This module is named "gzip".
#!/usr/bin/python3
"""Test"""
import gzip
gzip.main()

Configuration

No response

Command used

pylint gzip

Pylint output

************* Module gzip
gzip:5:0: W0406: Module import itself (import-self)
gzip:7:0: E1101: Module 'gzip' has no 'main' member (no-member)

Expected behavior

pylint shouldn't think a program named "foo" can be imported as "foo" when it doesn't have a .py extension

Pylint version

pylint 2.14.5
astroid 2.11.7
Python 3.9.2 (default, Feb 28 2021, 17:03:44) 
[GCC 10.2.1 20210110]

OS / Environment

Linux

Additional dependencies

No response

@vapier vapier added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Aug 11, 2022
@jacobtylerwalls
Copy link
Member

Hi @vapier thanks for the inquiry. This predates my involvement by a lot, but the blame tells me this is intentional since at least #219.

@jacobtylerwalls jacobtylerwalls closed this as not planned Won't fix, can't repro, duplicate, stale Aug 20, 2022
@jacobtylerwalls jacobtylerwalls added Invalid Not a bug, already exists or already fixed and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Aug 20, 2022
@vapier
Copy link
Contributor Author

vapier commented Aug 21, 2022

i don't think that conclusion is correct. being able to lint programs doesn't mean they have to be registered as importable modules.

@jacobtylerwalls
Copy link
Member

Thanks, I was having trouble understanding what you meant by "registered as an importable module." Do you mean import-self should not be emitted? (And not that the entire file should be ignored?)

In that case, having had a look at fac0b37, I guess we do have a flag we could check to see if this "was just some file" and not something importable ending in .py. (Although with some effort you could still import it, that's not relevant here, since a user who takes great effort to do that is probably not going to complain if we make import-self go away.)

If you don't mind, I'll retitle the issue to focus on import-self.

@jacobtylerwalls jacobtylerwalls changed the title pylint incorrectly registers a program as an importable module import-self emitted when a script does not end in .py (not importable) Aug 21, 2022
@jacobtylerwalls jacobtylerwalls added False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Invalid Not a bug, already exists or already fixed labels Aug 21, 2022
@jacobtylerwalls
Copy link
Member

jacobtylerwalls commented Aug 21, 2022

Regarding the no-member in this example, we have #5151 for that.

@jacobtylerwalls
Copy link
Member

jacobtylerwalls commented Aug 21, 2022

Maybe we should close #5151 in favor of this one and broaden the scope back out to all the other module-related checks (no-member, no-name-in-module) since we never got a response to @timmartin's reply. What do you think @vapier?

@vapier
Copy link
Contributor Author

vapier commented Aug 21, 2022

as long as the issue is resolved (where a program cannot poison the set of importable modules), i'm fine with any approach

@DanielNoord
Copy link
Collaborator

@jacobtylerwalls Did you look at the code already?

no-member seems to come from:
https://github.com/PyCQA/pylint/blob/e142ba8247276d25b3ff47e8f9fed06d0cc4753a/pylint/checkers/typecheck.py#L1063

Which returns the Module object and should probably be fixed in astroid somewhere.

import-self is much simpler:
https://github.com/PyCQA/pylint/blob/e142ba8247276d25b3ff47e8f9fed06d0cc4753a/pylint/checkers/imports.py#L843-L844

That's just two strings being compared.

Although import-self might actually need to start using the Module object which would then make this dependent (probably) on the same future fix in astroid.

@DanielNoord
Copy link
Collaborator

DanielNoord commented Aug 21, 2022

Some very relevant (and funny in hindsight) comments:
pylint-dev/astroid@6d59ad0
pylint-dev/astroid@9684d0f

Currently relevant code is stored here:
https://github.com/PyCQA/astroid/blob/16530f78c13863f584abcd874de9070d6f275072/astroid/nodes/_base_nodes.py#L140-L145

Edit: Found a fix 😄 Publishing PR to astroid. > pylint-dev/astroid#1747

@DanielNoord DanielNoord added the Needs astroid update Needs an astroid update (probably a release too) before being mergable label Aug 22, 2022
@DanielNoord DanielNoord changed the title import-self emitted when a script does not end in .py (not importable) import-self only compares the name of a module and doesn't consider the actual module being imported Aug 25, 2022
@DanielNoord DanielNoord removed the Needs astroid update Needs an astroid update (probably a release too) before being mergable label Aug 25, 2022
@DanielNoord
Copy link
Collaborator

DanielNoord commented Aug 25, 2022

This issue has been slightly rescoped, see #5151 (comment).

The issue about importing the correct module has been fixed and we should now follow this up in pylint by no longer doing the string comparison, but looking at the actual module. I'll add this to the 2.16 milestone so we don't forget to look at this when we bump astroid to 2.13.

Edit: A potential fix should also consider the following edge case and fix it: #7289

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Import system Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

4 participants