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

Inconsistent and sometimes incorrect false positive of no-member error #9883

Closed
akamat10 opened this issue Aug 19, 2024 · 9 comments · Fixed by #9911
Closed

Inconsistent and sometimes incorrect false positive of no-member error #9883

akamat10 opened this issue Aug 19, 2024 · 9 comments · Fixed by #9911
Assignees
Labels
Bug 🪲 Good first issue Friendly and approachable by new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@akamat10
Copy link
Contributor

akamat10 commented Aug 19, 2024

Bug description

I am seeing an issue with a sometimes false positive when using two modules with the same name but one of them under separate namespace. The following is the directory structure and contents:

├── module
│   └── __init__.py
├── package1
│   └── module
│       └── __init__.py
└── test.py

$ cat module/__init__.py
def main():
    pass

$ cat package1/module/__init__.py
def dummyfunc():
    pass

$ cat test.py
"""Test module"""
import module
import package1.module

module.main() 

Command run:

python3.12 -m pylint package1/module/ test.py

There seems to be some kind of module name conflict with module happening and sometimes I see a no-member error in the output even though the main function is defined in module/__init__.py

test.py:5:0: E1101: Module 'module' has no 'main' member (no-member)

When taking a closer look and trying to debug this, at least the non-deterministic behavior seems to stem from https://github.com/pylint-dev/pylint/blob/main/pylint/lint/pylinter.py#L667 where the code is trying to convert a set of paths to a list.



### Configuration

```ini
No configuration used.

Command used

python3.12 -m pylint package1/module/ test.py

Pylint output

Depending on the run, I see different outputs (one is correct and the other is incorrect):
Output 1 (correct):

************* Module package1.module
package1/module/__init__.py:1:0: C0114: Missing module docstring (missing-module-docstring)
package1/module/__init__.py:1:0: C0116: Missing function or method docstring (missing-function-docstring)
************* Module test
test.py:3:0: W0611: Unused import package1.module (unused-import)

Output 2 (false no-member error):

************* Module module
package1/module/__init__.py:1:0: C0114: Missing module docstring (missing-module-docstring)
package1/module/__init__.py:1:0: C0116: Missing function or method docstring (missing-function-docstring)
************* Module test
test.py:5:0: E1101: Module 'module' has no 'main' member (no-member)
test.py:3:0: W0611: Unused import package1.module (unused-import)

Expected behavior

************* Module package1.module
package1/module/init.py:1:0: C0114: Missing module docstring (missing-module-docstring)
package1/module/init.py:1:0: C0116: Missing function or method docstring (missing-function-docstring)
************* Module test
test.py:3:0: W0611: Unused import package1.module (unused-import)

Pylint version

pylint 3.2.6
astroid 3.2.4
Python 3.12.3 (v3.12.3:f6650f9ad7, Apr  9 2024, 08:18:47) [Clang 13.0.0 (clang-1300.0.29.30)]

OS / Environment

Darwin Kernel Version 23.4.0: Fri Mar 15 00:19:22 PDT 2024; root:xnu-10063.101.17~1/RELEASE_ARM64_T8112 arm64

I am also able to reproduce on Windows 10 WSL 2 (Ubuntu 22.04)

Additional dependencies

No response

@akamat10 akamat10 added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Aug 19, 2024
@akamat10 akamat10 changed the title Inconsistent and sometimes incorrect false positive of no-member error Inconsistent and sometimes incorrect false positive of no-member error Aug 19, 2024
@jacobtylerwalls
Copy link
Member

jacobtylerwalls commented Aug 20, 2024

Thanks for the analysis.

at least the non-deterministic behavior seems to stem from https://github.com/pylint-dev/pylint/blob/main/pylint/lint/pylinter.py#L667 where the code is trying to convert a set of paths to a list.

We should definitely remove that indeterminacy. Would you like to prepare a PR? I don't think a regression test is needed in this case.

@jacobtylerwalls jacobtylerwalls added Good first issue Friendly and approachable by new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Aug 20, 2024
@akamat10
Copy link
Contributor Author

Will send one sometime this week.

@sknaumov
Copy link

sknaumov commented Aug 28, 2024

This could be much more broad error than in description.

In my case pylint 3.2.6 sometimes fails to find symbols from common module - the only one with that name in my project.
I run pylint in docker and setting jobs=1 instead of jobs=0 fixed the problem for me, but not in Jenkins runs or for my colleague - in their case it still was 50/50. Then we tried the latest master and the problem gone completely.

@akamat10
Copy link
Contributor Author

akamat10 commented Aug 28, 2024

I think the fix needed is to provide the context file (ie mymodule.path[0]) when handling the import statement here in the astroid codebase. This should ensure the correct module is processed. This seems to work at least when I try it locally.

@rogersheu
Copy link
Contributor

rogersheu commented Aug 29, 2024

I'm looking at the toy example you provided above and I wanted to confirm that the issue is present, but I think the issue is also not 100% solved by the associated PR alone (but maybe it'll be solved with the accompanying PR in astroid?).

Commit tested: 8314743 (current main).
Also, testing with the astroid version in akamat10:main.

For simplicity, I disabled the docstring and import warnings just to reduce the noise.

Example 1 (directory before test.py):

command input

pylint package1/module/ test.py

pylint output (Output 2 in the original issue)

************* Module test
test.py:5:0: E1101: Module 'module' has no 'main' member (no-member)

Contents of extra_package_paths

['~/toy_example/package1', '~/toy_example'] (slightly modified to hide local pathing)

Example 2 (test.py before the directory):

command input

pylint test.py package1/module/ 

pylint output (Output 1 in the original isuse)

(no messages)

Contents of extra_package_paths

['~/toy_example', '~/toy_example/package1'] (slightly modified to hide local pathing)

Example 3 (test.py only):

command input

pylint test.py

pylint output

(no messages)

Contents of extra_package_paths

['~/toy_example']

Example 4 (package1/module/ -> package1/module_foo)

command input

pylint package1/module_foo/ test.py

pylint output

(no messages)

Contents of extra_package_paths

['/Users/rogersheu/Documents/GitHub/toy_example/package1', '/Users/rogersheu/Documents/GitHub/toy_example']


So the overlap in name definitely matters and is the reason this issue arises. However, it also appears that the currently proposed solution does not lead to the expected behavior either (Example 1), because we should expect all of these to not output messages.


I think it might also come down to how with augmented_sys_path(extra_packages_path) is being applied.

It looks like the extra_package_paths is being inserted at the beginning of sys.path. The order of paths in sys.path matters when considering modules with overlapping names.

@sknaumov
Copy link

sknaumov commented Aug 30, 2024

This could be much more broad error than in description.

Now I can confirm that hypothesis. Tested previous commit (724fd5e) and had E0602 in 30 out of 50 pylint invocations. On 8314743 the error is gone.

So it is not only about two modules with the same name, but about modules order in general - the issue is much more broad and severe! In my case I use wildcard imports (disabled wildcard-import and unused-wildcard-import), and without the fix pylint sometimes fails to see symbols of imported module => it results in E0602 undefined-variable.

When new version of pylint to be released? Are there plans to backport this fix to v3.2.x and release new bugfix version? Because of this bug it is impossible to use pylint v3.2.6 in CI for now.

@akamat10
Copy link
Contributor Author

akamat10 commented Sep 4, 2024

I'm looking at the toy example you provided above and I wanted to confirm that the issue is present, but I think the issue is also not 100% solved by the associated PR alone (but maybe it'll be solved with the accompanying PR in astroid?).

So the overlap in name definitely matters and is the reason this issue arises. However, it also appears that the currently proposed solution does not lead to the expected behavior either (Example 1), because we should expect all of these to not output messages.

I think it might also come down to how with augmented_sys_path(extra_packages_path) is being applied.

It looks like the extra_package_paths is being inserted at the beginning of sys.path. The order of paths in sys.path matters when considering modules with overlapping names.

That is correct. The order matters and the fix (#9887) that got merged in only addresses the randomness of the error. The full fix may be tricky to fix and will need further discussion. I am trying to fix the issue in #9910 but have run into regression test failures. These failures will need further discussion that I will open an issue for to discuss further in the coming days.

@akamat10
Copy link
Contributor Author

akamat10 commented Sep 5, 2024

Created #9915 to discuss the issue I ran into.

@akamat10
Copy link
Contributor Author

As discussed in the above issue #9915, this is tied to the current behavior of module search paths. When an argument is passed to pylint, pylint will walk up the argument's directory structure to find the first one that doesn't have __init__.py. If there is no directory found without __init__.py, then the current directory is returned. The qualified module name is also relative to this search path. This qualified module is then cached for the rest for arguments and module discoveries. Overall, this logic can lead to surprising behaviors from the user perspective. The module search discovery logic needs to looked at to make it more intuitive to the user and the behavior IMO should be made closer to how module discovery works with the python interpreter. I have made a proposal in #9915. Would be interested in hearing more people's opinions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 Good first issue Friendly and approachable by new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
5 participants