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.

Sign up for GitHub

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

Jump to bottom

pylint crashes when importing submodule from a module of same name as the current file #7488

Closed
skshetry opened this issue Sep 19, 2022 · 2 comments · Fixed by pylint-dev/astroid#1802
Closed

pylint crashes when importing submodule from a module of same name as the current file #7488

skshetry opened this issue Sep 19, 2022 · 2 comments · Fixed by pylint-dev/astroid#1802
Labels
Astroid Related to astroid Cannot reproduce 🤷 Crash 💥 A bug that makes pylint crash Downstream Bug 🪲 The problem happens in a lib depending on pylint, not pylint

Comments

@skshetry
Copy link
Contributor

skshetry commented Sep 19, 2022 •
edited
Loading

Bug description

Let's say you have the following directory structure:

$ tree package 
package
├── celery.py
└── __init__.py

and celery.py has a content import celery.result.

You can generate the above result using:

mkdir package
touch package/__init__.py
echo "import celery.result" > package/celery.py

This crashes pylint. See below for the traceback. Note that astroid==2.12.9 works, it's probably a regression in 2.12.10

Configuration

No response

Command used

`pylint package`

Pylint output

Content:
When parsing the following file:

import celery.result

pylint crashed with a AstroidError and with the following stacktrace:

Traceback (most recent call last):
File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/pylint/lint/pylinter.py", line 782, in  _lint_file
   check_astroid_module(module)
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/pylint/lint/pylinter.py", line 1049, in check_astroid_module
   retval = self._check_astroid_module(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/pylint/lint/pylinter.py", line 1099, in _check_astroid_module
   walker.walk(node)
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/pylint/utils/ast_walker.py", line 93, in walk
   self.walk(child)
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/pylint/utils/ast_walker.py", line 90, in walk
   callback(astroid)
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/pylint/checkers/imports.py", line 497, in visit_import
   self._add_imported_module(node, imported_module.name)
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/pylint/checkers/imports.py", line 833, in _add_imported_module
   importedmodname = astroid.modutils.get_module_part(
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/astroid/modutils.py", line 438, in get_module_part
   file_from_modpath(
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/astroid/modutils.py", line 334, in file_from_modpath
   return file_info_from_modpath(modpath, path, context_file).location
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/astroid/modutils.py", line 384, in file_info_from_modpath
   return _spec_from_modpath(modpath, path, context)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/astroid/modutils.py", line 590, in _spec_from_modpath
   found_spec = spec.find_spec(modpath, [context])
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/astroid/interpreter/_import/spec.py", line 392, in find_spec
   finder, spec = _find_spec_with_path(
                  ^^^^^^^^^^^^^^^^^^^^^
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/astroid/interpreter/_import/spec.py", line 354, in _find_spec_with_path
   spec = finder_instance.find_module(
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/astroid/interpreter/_import/spec.py", line 204, in find_module
   submodule_path = sys.modules[modname].__path__
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: module 'celery.result' has no attribute '__path__'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/pylint/lint/pylinter.py", line 747, in _lint_files
   self._lint_file(fileitem, module, check_astroid_module)
 File "/home/saugat/projects/iterative/dvc/venv/lib/python3.11/site-packages/pylint/lint/pylinter.py", line 784, in _lint_file
   raise astroid.AstroidError from e
astroid.exceptions.AstroidError

Expected behavior

Pylint does not crashes.

Pylint version

pylint 2.15.2
astroid 2.12.10
Python 3.11.0rc1+ (heads/3.11:2ba877258a, Aug 29 2022, 17:05:58) [GCC 12.2.0]

OS / Environment

No response

Additional dependencies

No response
@skshetry skshetry added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Sep 19, 2022
skshetry added a commit to iterative/dvc that referenced this issue Sep 19, 2022
@skshetry
lint: pin astroid version
7931528 
See pylint-dev/pylint#7488. Pylint is failing on our codebase.
@skshetry skshetry mentioned this issue Sep 19, 2022
Merged
skshetry added a commit to iterative/dvc that referenced this issue Sep 19, 2022
@skshetry
lint: pin astroid version (#8323)
7e93fc8 
See pylint-dev/pylint#7488. Pylint is failing on our codebase.
@Pierre-Sassoulas Pierre-Sassoulas added Astroid Related to astroid Crash 💥 A bug that makes pylint crash Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Sep 19, 2022
@mbyrnepr2 mbyrnepr2 added Needs reproduction 🔍 Need a way to reproduce it locally on a maintainer's machine Cannot reproduce 🤷 labels Sep 19, 2022
@mbyrnepr2
Copy link
Member

I've tried reproducing with the reported Pylint/astroid/Python versions but no luck.

@skshetry
Copy link
Contributor Author

Looking closely, it seems like this is happening when pylint_pytest plugin is used. Sorry, the traceback made me think that it was pylint.

[tool.pylint.master]
load-plugins = ["pylint_pytest"]

@Pierre-Sassoulas Pierre-Sassoulas added Downstream Bug 🪲 The problem happens in a lib depending on pylint, not pylint and removed Needs reproduction 🔍 Need a way to reproduce it locally on a maintainer's machine Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning labels Sep 20, 2022
@Pierre-Sassoulas Pierre-Sassoulas closed this as not planned Won't fix, can't repro, duplicate, stale Sep 20, 2022
skshetry added a commit to skshetry/astroid that referenced this issue Sep 21, 2022
@skshetry
improve is_namespace check
46be0f7 
See https://stackoverflow.com/a/42962529.

Let's take the following contents as an example:
```python
import celery.result

```

From pylint-dev#1777, astroid started to use `processed_components` for
namespace check. In the above case, the `modname` is `celery.result`,
it first checks for `celery` and then `celery.result`.
Before that PR, it'd always check for `celery.result`.

But if you have imported it as `celery.result`,
`sys.modules["celery"].__spec__` is going to be `None`, and hence
the function will return True, and but below where we try to load
get `submodule_path`/`__path__` for `celery.result` will fail as
it is not a package.

See https://github.com/PyCQA/astroid/blob/056d8e5fab7a167f73115d524ab92170b3ed5f9f/astroid/interpreter/_import/spec.py#L205-L207

---

The `celery.result` gets imported for me when pylint-pytest plugin tries
to load fixtures, but this could happen anytime if any plugin imports
packages. In that case, `find_spec("celery")` will raise ValueError
since it's already in `sys.modules` and does not have a spec.

I still think there's a bug for the
`ExplicitNamespacePackageFinder.find_module` for namespace packages,
since `modname` might be `namespace.package.module` but since
`is_namespace` package goes through components, the successive
`sys.modules[modname].__path__` may raise error, since modname
could be not a package in a namespaced package. But I am not quite sure
about that.

Fixes pylint-dev/pylint#7488.
@skshetry skshetry mentioned this issue Sep 21, 2022
Merged
2 tasks
skshetry added a commit to skshetry/astroid that referenced this issue Sep 21, 2022
@skshetry
improve is_namespace check
61ee4f0 
See https://stackoverflow.com/a/42962529.

Let's take the following contents as an example:
```python
import celery.result

```

From pylint-dev#1777, astroid started to use `processed_components` for
namespace check. In the above case, the `modname` is `celery.result`,
it first checks for `celery` and then `celery.result`.
Before that PR, it'd always check for `celery.result`.

But if you have imported it as `celery.result`,
`sys.modules["celery"].__spec__` is going to be `None`, and hence
the function will return True, and but below where we try to load
get `submodule_path`/`__path__` for `celery.result` will fail as
it is not a package.

See https://github.com/PyCQA/astroid/blob/056d8e5fab7a167f73115d524ab92170b3ed5f9f/astroid/interpreter/_import/spec.py#L205-L207

---

The `celery.result` gets imported for me when pylint-pytest plugin tries
to load fixtures, but this could happen anytime if any plugin imports
packages. In that case, `find_spec("celery")` will raise ValueError
since it's already in `sys.modules` and does not have a spec.

Fixes pylint-dev/pylint#7488.
skshetry added a commit to skshetry/astroid that referenced this issue Sep 21, 2022
@skshetry
improve is_namespace check
0d9e05c 
See https://stackoverflow.com/a/42962529.

Let's take the following contents as an example:
```python
import celery.result

```

From pylint-dev#1777, astroid started to use `processed_components` for
namespace check. In the above case, the `modname` is `celery.result`,
it first checks for `celery` and then `celery.result`.
Before that PR, it'd always check for `celery.result`.

`celery` is recreating module to make it lazily load.
See
https://github.com/celery/celery/blob/34533ab44d2a6492004bc3df44dc04ad5c6611e7/celery/__init__.py#L150.

This module does not have `__spec__` set.

Reading through Python's docs, it seems that `__spec__` can be set
to None, so it seems like it's not a thing that we can depend upon
for namespace checks.

See https://docs.python.org/3/reference/import.html#spec__.

---

The `celery.result` gets imported for me when pylint-pytest plugin tries
to load fixtures, but this could happen anytime if any plugin imports
packages. In that case, `importlib.util._find_spec_from_path("celery")` will raise ValueError
since it's already in `sys.modules` and does not have a spec.

Fixes pylint-dev/pylint#7488.
@skshetry skshetry mentioned this issue Sep 22, 2022
Merged
DanielNoord pushed a commit to pylint-dev/astroid that referenced this issue Sep 23, 2022
@skshetry @DanielNoord
improve is_namespace check
8559936 
See https://stackoverflow.com/a/42962529.

Let's take the following contents as an example:
```python
import celery.result

```

From #1777, astroid started to use `processed_components` for
namespace check. In the above case, the `modname` is `celery.result`,
it first checks for `celery` and then `celery.result`.
Before that PR, it'd always check for `celery.result`.

`celery` is recreating module to make it lazily load.
See
https://github.com/celery/celery/blob/34533ab44d2a6492004bc3df44dc04ad5c6611e7/celery/__init__.py#L150.

This module does not have `__spec__` set.

Reading through Python's docs, it seems that `__spec__` can be set
to None, so it seems like it's not a thing that we can depend upon
for namespace checks.

See https://docs.python.org/3/reference/import.html#spec__.

---

The `celery.result` gets imported for me when pylint-pytest plugin tries
to load fixtures, but this could happen anytime if any plugin imports
packages. In that case, `importlib.util._find_spec_from_path("celery")` will raise ValueError
since it's already in `sys.modules` and does not have a spec.

Fixes pylint-dev/pylint#7488.
Pierre-Sassoulas pushed a commit to pylint-dev/astroid that referenced this issue Oct 10, 2022
@skshetry @Pierre-Sassoulas
improve is_namespace check
2e8b339 
See https://stackoverflow.com/a/42962529.

Let's take the following contents as an example:
```python
import celery.result

```

From #1777, astroid started to use `processed_components` for
namespace check. In the above case, the `modname` is `celery.result`,
it first checks for `celery` and then `celery.result`.
Before that PR, it'd always check for `celery.result`.

`celery` is recreating module to make it lazily load.
See
https://github.com/celery/celery/blob/34533ab44d2a6492004bc3df44dc04ad5c6611e7/celery/__init__.py#L150.

This module does not have `__spec__` set.

Reading through Python's docs, it seems that `__spec__` can be set
to None, so it seems like it's not a thing that we can depend upon
for namespace checks.

See https://docs.python.org/3/reference/import.html#spec__.

---

The `celery.result` gets imported for me when pylint-pytest plugin tries
to load fixtures, but this could happen anytime if any plugin imports
packages. In that case, `importlib.util._find_spec_from_path("celery")` will raise ValueError
since it's already in `sys.modules` and does not have a spec.

Fixes pylint-dev/pylint#7488.
Pierre-Sassoulas pushed a commit to pylint-dev/astroid that referenced this issue Oct 10, 2022
@skshetry @Pierre-Sassoulas
improve is_namespace check
b05a51f 
See https://stackoverflow.com/a/42962529.

Let's take the following contents as an example:
```python
import celery.result

```

From #1777, astroid started to use `processed_components` for
namespace check. In the above case, the `modname` is `celery.result`,
it first checks for `celery` and then `celery.result`.
Before that PR, it'd always check for `celery.result`.

`celery` is recreating module to make it lazily load.
See
https://github.com/celery/celery/blob/34533ab44d2a6492004bc3df44dc04ad5c6611e7/celery/__init__.py#L150.

This module does not have `__spec__` set.

Reading through Python's docs, it seems that `__spec__` can be set
to None, so it seems like it's not a thing that we can depend upon
for namespace checks.

See https://docs.python.org/3/reference/import.html#spec__.

---

The `celery.result` gets imported for me when pylint-pytest plugin tries
to load fixtures, but this could happen anytime if any plugin imports
packages. In that case, `importlib.util._find_spec_from_path("celery")` will raise ValueError
since it's already in `sys.modules` and does not have a spec.

Fixes pylint-dev/pylint#7488.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Astroid Related to astroid Cannot reproduce 🤷 Crash 💥 A bug that makes pylint crash Downstream Bug 🪲 The problem happens in a lib depending on pylint, not pylint
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Improve is_namespace() check for modules where __spec__ is None skshetry/astroid
3 participants
@Pierre-Sassoulas @skshetry @mbyrnepr2