Fix the failure to lint modules contained under an identically named directory #7114

jacobtylerwalls · 2022-07-03T04:36:40Z

Type of Changes

	Type
✓	🐛 Bug fix

Description

Fixes #4444: modules contained under an identically named directory weren't linted (failed with parse-error).

Notes

This PR causes a test failure in TestImportsChecker.test_relative_beyond_top_level_two, but that test uses epylint to excute pylint (!) which does additional manipulation of cwd, so I don't think it's correct.

Notice that linting the package in the test fails using regular pylint, but the failure is now more accurate on this branch:

main

$ pylint tests/regrtest_data/beyond_top_two
************* Module beyond_top_two
tests/regrtest_data/beyond_top_two/__init__.py:1:0: F0010: error while code parsing: Unable to load file tests/regrtest_data/beyond_top_two/__init__.py:
[Errno 2] No such file or directory: 'tests/regrtest_data/beyond_top_two/__init__.py' (parse-error)

PR

$ pylint tests/regrtest_data/beyond_top_two
************* Module tests/regrtest_data/beyond_top_two
tests/regrtest_data/beyond_top_two:1:0: F0001: No module named tests/regrtest_data/beyond_top_two (fatal)

…directory

It uses epylint to run these tests, but since epylint manipulates cwd, that confounds what is under test

coveralls · 2022-07-31T20:46:44Z

Pull Request Test Coverage Report for Build 2770829476

2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.006%) to 95.251%

Totals
Change from base Build 2770792790:	0.006%
Covered Lines:	16829
Relevant Lines:	17668

💛 - Coveralls

Pierre-Sassoulas

Looks pretty good already, I have a question regarding the xfailed test.

Pierre-Sassoulas · 2022-07-31T20:45:03Z

pylint/lint/expand_modules.py

@@ -103,9 +105,7 @@ def expand_modules(
                )
                if filepath is None:
                    continue
-            except (ImportError, SyntaxError) as ex:
-                # The SyntaxError is a Python bug and should be
-                # removed once we move away from imp.find_module: https://bugs.python.org/issue10588


Pierre-Sassoulas · 2022-07-31T20:45:56Z

tests/checkers/unittest_imports.py

@@ -40,6 +41,9 @@ def test_relative_beyond_top_level(self) -> None:
            self.checker.visit_importfrom(module.body[2].body[0])

    @staticmethod
+    @pytest.mark.xfail(
+        reason="epylint manipulates cwd; these tests should not be using epylint"


Shouldn't we fix it or remove it ? What's the benefit of keeping xfailed tests ?

Yeah, I just didn't want to block this PR on resolving that. All of the tests in that file probably shouldn't be using epylint. Would you like me to open an issue?

Yeah let's open an issue, I'm not sure emacs is used a lot so it's probably not worth a fix, no one read the emacs documentation at least.

Thank you !

Pierre-Sassoulas · 2022-07-31T20:46:37Z

tests/lint/unittest_lint.py

+    with tempdir():
+        create_files(["identical/identical.py"])
+        with open("identical/identical.py", "w", encoding="utf-8") as f:
+            f.write("import imp")


I wonder where the idea for this example came from 😄

github-actions · 2022-07-31T21:09:41Z

🤖 Effect of this PR on checked open source code: 🤖

Effect on django:
The following messages are no longer emitted:

super-init-not-called:
init method from base class 'UploadFileException' is not called
https://github.com/django/django/blob/main/django/core/files/uploadhandler.py#L36
super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/django/django/blob/main/django/core/serializers/base.py#L66
super-init-not-called:
init method from base class 'TextIOBase' is not called
https://github.com/django/django/blob/main/django/core/management/base.py#L150
super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/django/django/blob/main/django/middleware/csrf.py#L132
super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/django/django/blob/main/django/middleware/csrf.py#L167
super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/django/django/blob/main/django/template/base.py#L102
super-init-not-called:
init method from base class 'LookupError' is not called
https://github.com/django/django/blob/main/django/db/migrations/exceptions.py#L43
super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/django/django/blob/main/django/db/models/sql/datastructures.py#L15
super-init-not-called:
init method from base class 'str' is not called
https://github.com/django/django/blob/main/django/conf/__init__.py#L52

Effect on flask:
The following messages are no longer emitted:

super-init-not-called:
init method from base class 'KeyError' is not called
https://github.com/pallets/flask/blob/main/src/flask/debughelpers.py#L19
super-init-not-called:
init method from base class 'AssertionError' is not called
https://github.com/pallets/flask/blob/main/src/flask/debughelpers.py#L19

Effect on pandas:
The following messages are no longer emitted:

super-init-not-called:
init method from base class 'BaseBlockManager' is not called
https://github.com/pandas-dev/pandas/blob/main/pandas/core/internals/managers.py#L888
super-init-not-called:
init method from base class 'BaseBlockManager' is not called
https://github.com/pandas-dev/pandas/blob/main/pandas/core/internals/managers.py#L1694
super-init-not-called:
init method from base class 'BaseArrayManager' is not called
https://github.com/pandas-dev/pandas/blob/main/pandas/core/internals/array_manager.py#L712
super-init-not-called:
init method from base class 'BaseArrayManager' is not called
https://github.com/pandas-dev/pandas/blob/main/pandas/core/internals/array_manager.py#L1201
super-init-not-called:
init method from base class 'set' is not called
https://github.com/pandas-dev/pandas/blob/main/pandas/tests/frame/methods/test_set_index.py#L676
super-init-not-called:
init method from base class 'NotImplementedError' is not called
https://github.com/pandas-dev/pandas/blob/main/pandas/errors/__init__.py#L188
super-init-not-called:
init method from base class 'dict' is not called
https://github.com/pandas-dev/pandas/blob/main/pandas/plotting/_misc.py#L558

Effect on psycopg:
The following messages are no longer emitted:

super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/psycopg/psycopg/blob/master/psycopg/psycopg/transaction.py#L40

Effect on pytest:
The following messages are no longer emitted:

super-init-not-called:
init method from base class 'LookupError' is not called
https://github.com/pytest-dev/pytest/blob/main/src/_pytest/fixtures.py#L792
super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/pytest-dev/pytest/blob/main/src/_pytest/mark/expression.py#L61
super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/pytest-dev/pytest/blob/main/src/_pytest/config/argparsing.py#L207

Effect on sentry:
The following messages are no longer emitted:

super-init-not-called:
init method from base class 'InvalidRequest' is not called
https://github.com/getsentry/sentry/blob/master/src/sentry/exceptions.py#L17
super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/getsentry/sentry/blob/master/src/sentry/coreapi.py#L24
super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/getsentry/sentry/blob/master/src/sentry/incidents/logic.py#L850
super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/getsentry/sentry/blob/master/src/sentry/api/client.py#L11
super-init-not-called:
init method from base class 'NodeStorage' is not called
https://github.com/getsentry/sentry/blob/master/src/sentry/nodestore/bigtable/backend.py#L33
super-init-not-called:
init method from base class '_local' is not called
https://github.com/getsentry/sentry/blob/master/src/sentry/metrics/base.py#L10
super-init-not-called:
init method from base class '_local' is not called
https://github.com/getsentry/sentry/blob/master/src/sentry/cache/base.py#L22
super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/getsentry/sentry/blob/master/src/sentry/tasks/symbolication.py#L43
super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/getsentry/sentry/blob/master/src/sentry/utils/sms.py#L13
super-init-not-called:
init method from base class 'Exception' is not called
https://github.com/getsentry/sentry/blob/master/src/sentry/utils/auth.py#L79
super-init-not-called:
init method from base class '_local' is not called
https://github.com/getsentry/sentry/blob/master/src/sentry/utils/services.py#L251
super-init-not-called:
init method from base class '_local' is not called
https://github.com/getsentry/sentry/blob/master/src/sentry/utils/performance/sqlquerycount.py#L12

This comment was generated for commit 759d35e

jacobtylerwalls added Bug 🪲 Work in progress labels Jul 3, 2022

jacobtylerwalls added this to the 2.15.0 milestone Jul 3, 2022

This comment has been minimized.

Sign in to view

This comment was marked as outdated.

Sign in to view

This was referenced Jul 3, 2022

Add regression test for #3651 #7117

Merged

Support an Option to Disable sys.path patching #5226

Open

jacobtylerwalls mentioned this pull request Jul 10, 2022

Refactor context manager for sys path and current working directory in tests #7155

Merged

jacobtylerwalls removed the Work in progress label Jul 17, 2022

jacobtylerwalls changed the title ~~[WIP] Fix #4444~~ Fix failure to lint packages containing identically named modules Jul 17, 2022

This comment has been minimized.

Sign in to view

jacobtylerwalls added 3 commits July 31, 2022 16:37

Fix the failure to lint modules contained under an identically named …

3ebb700

…directory

Add xfail for test_relative_beyond_top_level_two

5c1e78c

It uses epylint to run these tests, but since epylint manipulates cwd, that confounds what is under test

Remove imp-specific workaround

759d35e

jacobtylerwalls force-pushed the issue-4444 branch from 4711f42 to 759d35e Compare July 31, 2022 20:37

jacobtylerwalls changed the title ~~Fix failure to lint packages containing identically named modules~~ Fix the failure to lint modules contained under an identically named directory Jul 31, 2022

jacobtylerwalls marked this pull request as ready for review July 31, 2022 20:38

jacobtylerwalls requested a review from Pierre-Sassoulas July 31, 2022 20:38

jacobtylerwalls added the Import system label Jul 31, 2022

Pierre-Sassoulas requested changes Jul 31, 2022

View reviewed changes

jacobtylerwalls mentioned this pull request Aug 1, 2022

epylint runner confounding results of test_relative_beyond... test methods #7252

Closed

Pierre-Sassoulas approved these changes Aug 3, 2022

View reviewed changes

jacobtylerwalls merged commit 8db7b72 into pylint-dev:main Aug 3, 2022

jacobtylerwalls deleted the issue-4444 branch August 3, 2022 12:43

This was referenced Aug 13, 2022

Fix linting namespace packages under a directory on sys.path #7303

Closed

Linting fails if module contains module of the same name #4444

Open

jacobtylerwalls mentioned this pull request Dec 31, 2022

Revert "Fix the failure to lint modules contained under an identically named directory" #7304

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix the failure to lint modules contained under an identically named directory #7114

Fix the failure to lint modules contained under an identically named directory #7114

Uh oh!

jacobtylerwalls commented Jul 3, 2022 •

edited

Loading

Uh oh!

This comment has been minimized.

This comment was marked as outdated.

This comment has been minimized.

coveralls commented Jul 31, 2022

Uh oh!

Pierre-Sassoulas left a comment

Uh oh!

Pierre-Sassoulas Jul 31, 2022

Uh oh!

Pierre-Sassoulas Jul 31, 2022

Uh oh!

jacobtylerwalls Jul 31, 2022

Uh oh!

Pierre-Sassoulas Jul 31, 2022

Uh oh!

jacobtylerwalls Aug 1, 2022

Uh oh!

Pierre-Sassoulas Aug 3, 2022

Uh oh!

Pierre-Sassoulas Jul 31, 2022

Uh oh!

github-actions bot commented Jul 31, 2022

Uh oh!

Uh oh!

Uh oh!

Fix the failure to lint modules contained under an identically named directory #7114

Fix the failure to lint modules contained under an identically named directory #7114

Uh oh!

Conversation

jacobtylerwalls commented Jul 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of Changes

Description

Notes

Uh oh!

This comment has been minimized.

This comment was marked as outdated.

This comment has been minimized.

coveralls commented Jul 31, 2022

Pull Request Test Coverage Report for Build 2770829476

💛 - Coveralls

Uh oh!

Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

Uh oh!

Pierre-Sassoulas Jul 31, 2022

Choose a reason for hiding this comment

Uh oh!

Pierre-Sassoulas Jul 31, 2022

Choose a reason for hiding this comment

Uh oh!

jacobtylerwalls Jul 31, 2022

Choose a reason for hiding this comment

Uh oh!

Pierre-Sassoulas Jul 31, 2022

Choose a reason for hiding this comment

Uh oh!

jacobtylerwalls Aug 1, 2022

Choose a reason for hiding this comment

Uh oh!

Pierre-Sassoulas Aug 3, 2022

Choose a reason for hiding this comment

Uh oh!

Pierre-Sassoulas Jul 31, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 31, 2022

Uh oh!

Uh oh!

jacobtylerwalls commented Jul 3, 2022 •

edited

Loading