Forbid lazy imports - #3758
Conversation
|
CI falling precisely because of code coverage :) |
sobolevn
left a comment
sobolevn
left a comment
There was a problem hiding this comment.
Please test lazy from, add a changelog entry.
| ) | ||
|
|
||
|
|
||
| def test_lazy_import(): |
|
|
||
| lazy import json | ||
|
|
||
| def some(): |
|
|
||
| def some(): | ||
| import json | ||
| ... |
| Is only emitted on ``python3.15+``. | ||
|
|
||
| Reasoning: | ||
| ... |
There was a problem hiding this comment.
It is an overly complicated feature that is needed for just a couple use-cases. Retuning an object from a function does exactly the same thing: it imports something and returns it, when needed.
| self._check_lazy_import(node) | ||
|
|
||
| def _check_lazy_import(self, node: ast.Import | ast.ImportFrom) -> None: | ||
| if PY315 and imports.is_lazy_import(node): |
There was a problem hiding this comment.
if PY315 should be a part of the logic, not visitor.
| class _ImportLazy(_BaseImportValidator): | ||
| """Validator of ``ast.Import`` and ``ast.ImportFrom`` nodes.""" | ||
|
|
||
| def validate(self, node: ast.Import | ast.ImportFrom) -> None: |
There was a problem hiding this comment.
Do we have an alias for this? AnyImport?
sobolevn
left a comment
sobolevn
left a comment
There was a problem hiding this comment.
Please, add 3.15rc1 into the test matrix and fix the CI :)
| def validate(self, node: AnyImport) -> None: | ||
| self._check_lazy_import(node) | ||
|
|
||
| def _check_lazy_import(self, node: AnyImport) -> None: |
There was a problem hiding this comment.
I think we can squash these two methods into one
| return blacklisted or with_from_or_to or too_short | ||
|
|
||
|
|
||
| def is_lazy_import(node: AnyImport) -> bool: |
There was a problem hiding this comment.
Please, add conditional coverage everywhere.
| >>> is_lazy_import(ast.Import(names=[ast.alias(name='a')])) | ||
| False | ||
|
|
||
| >>> is_lazy_import(ast.Import(names=[ast.alias(name='a')], is_lazy=1)) | ||
| True | ||
|
|
There was a problem hiding this comment.
| >>> is_lazy_import(ast.Import(names=[ast.alias(name='a')])) | |
| False | |
| >>> is_lazy_import(ast.Import(names=[ast.alias(name='a')], is_lazy=1)) | |
| True |
function is simple enough, we can remove this example / test.
| if not PY315: | ||
| return False | ||
| return bool(getattr(node, 'is_lazy', False)) |
There was a problem hiding this comment.
| if not PY315: | |
| return False | |
| return bool(getattr(node, 'is_lazy', False)) | |
| return bool(getattr(node, 'is_lazy', False)) |
It would return False on other python versions by default
There was a problem hiding this comment.
Of course, but I thought that it would be useful for docs :)
| from wemake_python_styleguide.visitors.ast.imports import WrongImportVisitor | ||
|
|
||
| if not PY315: # pragma: >=3.15 no cover | ||
| pytest.skip( # pragma: no cover |
There was a problem hiding this comment.
can we use skipif with module-level?
| """ | ||
|
|
||
| error_template = ( | ||
| 'Found a lazy import, use a regular import inside a function instead' |
There was a problem hiding this comment.
| 'Found a lazy import, use a regular import inside a function instead' | |
| 'Found a lazy import' |
| ### Features | ||
|
|
||
| - Adds `WPS482`: forbid lazy imports, #3639 | ||
|
|
There was a problem hiding this comment.
Please test lazy from, add a changelog entry.
🤔
There was a problem hiding this comment.
we use 2 empty lines between versions :)
2 participants
I have made things!
Refs #3639
We need to wait 3.15