diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 6ea11b5..417443e 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,17 +10,25 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- [Pycln skips redundant alias imports in compliance with PEP 484 in stub files (`.pyi`) by @hadialqattan](https://github.com/hadialqattan/pycln/pull/150) + +### Changed + +- [Now Pycln treats `.pyi` files as regular `.py` files in the pathfinding functionality by @hadialqattan](https://github.com/hadialqattan/pycln/pull/150) + ### Fixed -- [outputting code diff when both `--check` and `--diff` options are used by @hadialqattan](https://github.com/hadialqattan/pycln/pull/147) +- [Outputting code diff when both `--check` and `--diff` options are used by @hadialqattan](https://github.com/hadialqattan/pycln/pull/147) ## [1.3.5] - 2022-06-23 ### Fixed -- [in the modules' path finding functionality non-directory paths are treated as directories causing `Permission denied` errors crashing Pycln by @hadialqattan](https://github.com/hadialqattan/pycln/pull/145) +- [In the modules' path finding functionality non-directory paths are treated as directories causing `Permission denied` errors crashing Pycln by @hadialqattan](https://github.com/hadialqattan/pycln/pull/145) -- [useful `pass` statements are removed from `orelse` parent nodes when both `finallybody` and `orelse` exist causing syntax errors by @hadialqattan](https://github.com/hadialqattan/pycln/pull/144) +- [Useful `pass` statements are removed from `orelse` parent nodes when both `finallybody` and `orelse` exist causing syntax errors by @hadialqattan](https://github.com/hadialqattan/pycln/pull/144) ## [1.3.4] - 2022-06-22 diff --git a/docs/README.md b/docs/README.md index e8538bb..d4f0d35 100644 --- a/docs/README.md +++ b/docs/README.md @@ -155,6 +155,9 @@ Please see [--skip-imports](?id=-skip-imports-option) option. > Directories' paths and/or files' paths and/or reading from stdin. +NOTE: Pycln treats `.pyi` files as regular `.py` files in the pathfinding functionality, +so anything true for `.py` files is true for `.pyi` files as well. + #### Usage - Specify a directory to handle all its subdirs/files (recursively): @@ -170,17 +173,18 @@ Please see [--skip-imports](?id=-skip-imports-option) option. $ pycln dir1/ dir2/ main.py cli.py ``` - Reading from `STDIN` (`-` as a file path): + ```bash $ cat file.py | pycln - # please read the notes below which clarifies the necessity of using `-s/--silence` flag. ``` -Notes about reading from `STDIN`: + Notes about reading from `STDIN`: -- For the time being, both the final report and the formatted code will be sent to - `STDOUT`, therefore, it's necessary to use [`-s/--silence`](?id=-s-silence-flag) flag - in order to receive only the formatted code via `STDOUT`. -- You can read from `STDIN` and provide normal paths at the same time (the order doesn't - matter). + - For the time being, both the final report and the formatted code will be sent to + `STDOUT`, therefore, it's necessary to use [`-s/--silence`](?id=-s-silence-flag) + flag in order to receive only the formatted code via `STDOUT`. + - You can read from `STDIN` and provide normal paths at the same time (the order + doesn't matter). ## CLI Options @@ -343,7 +347,7 @@ pycln: #### Default -> `.*\.py$` +> `.*\.pyi?$` #### Behaviour @@ -1085,6 +1089,23 @@ Now let us review the same two cases but with an `__all__` dunder: You may notice that using an `__all__` dunder makes the two cases distinguishable for both the developers and QA tools. +### Stub files (`.pyi`) redundant aliases + +> Pycln skips redundant alias imports in compliance with +> [PEP 484](https://peps.python.org/pep-0484/#stub-files) for the purposes of exporting +> modules and symbols for static type checking. + +- case a: + + ```python + import X as X # marked as used. + ``` + +- case b: + ```python + from X import Y as Y # marked as used. + ``` + # Unsupported Cases ## Specific diff --git a/pycln/utils/pathu.py b/pycln/utils/pathu.py index 87cce80..0162f56 100644 --- a/pycln/utils/pathu.py +++ b/pycln/utils/pathu.py @@ -52,8 +52,8 @@ def yield_sources( gitignore: PathSpec, reporter: Report, ) -> Generator[Path, None, None]: - """Yields `.py` paths to handle. Walk throw path sub-directories/files - recursively. + """Yields `.py` and `.pyi` paths to handle. Walk throw path sub- + directories/files recursively. :param path: A path to start searching from. :param include: regex pattern to be included. @@ -61,7 +61,7 @@ def yield_sources( :param extend_exclude: regex pattern to be excluded in addition to `exclude`. :param gitignore: gitignore PathSpec object. :param reporter: a `report.Report` object. - :returns: generator of `.py` files paths. + :returns: generator of `.py` and `.pyi` files paths. """ dirs: Set[Path] = set() diff --git a/pycln/utils/refactor.py b/pycln/utils/refactor.py index e868565..4739e72 100644 --- a/pycln/utils/refactor.py +++ b/pycln/utils/refactor.py @@ -2,9 +2,10 @@ import ast import os from importlib import import_module -from pathlib import Path +from pathlib import Path, _posix_flavour, _windows_flavour # type: ignore from typing import Iterable, List, Optional, Set, Tuple, Union, cast +from .. import ISWIN from . import iou, pathu, regexu, scan from ._exceptions import ( ReadPermissionError, @@ -25,6 +26,21 @@ PYCLN_UTILS = "pycln.utils" +class PyPath(Path): + + """Path subclass that has `is_stub` property.""" + + _flavour = _windows_flavour if ISWIN else _posix_flavour + + def __init__(self, *args) -> None: # pylint: disable=unused-argument + super().__init__() # Path.__init__ does not take any args. + self._is_stub = regexu.is_stub_file(self) + + @property + def is_stub(self) -> bool: + return self._is_stub + + class LazyLibCSTLoader: """`transform.py` takes about '0.3s' to be loaded because of LibCST, @@ -67,13 +83,13 @@ def __init__(self, configs: Config, reporter: Report): # Resetables. self._import_stats = scan.ImportStats(set(), set()) self._source_stats = scan.SourceStats(set(), set(), set()) - self._path = Path("") + self._path = PyPath("") self._is_init_without_all = False def _reset(self) -> None: self._import_stats = scan.ImportStats(set(), set()) self._source_stats = scan.SourceStats(set(), set(), set()) - self._path = Path("") + self._path = PyPath("") self._is_init_without_all = False @staticmethod @@ -139,7 +155,7 @@ def session(self, path: Path) -> None: :param path: `.py` file to refactor. """ - self._path = path + self._path = PyPath(path) try: if path == iou.STDIN_FILE: content, encoding, newline = iou.read_stdin() @@ -417,6 +433,14 @@ def _should_remove( real_name = node.module if isinstance(node, ImportFrom) else alias.name used_name = alias.asname if alias.asname else alias.name + #: (for `.pyi`) PEP 484 - Redundant Module/Symbol Aliases rule: + #: + #: >>> import X as X # exported (should be treated as used) + #: + #: More info: https://peps.python.org/pep-0484/#stub-files + if self._path.is_stub and alias.name == alias.asname: + return False + if real_name and real_name.split(".")[0] in self.configs.skip_imports: return False diff --git a/pycln/utils/regexu.py b/pycln/utils/regexu.py index 1366dea..e7f515f 100644 --- a/pycln/utils/regexu.py +++ b/pycln/utils/regexu.py @@ -17,9 +17,10 @@ GITIGNORE = ".gitignore" SKIP_FILE_REGEX = r"# *(nopycln *: *file).*" SKIP_IMPORT_REGEX = r"# *((noqa *:*)|(nopycln *: *import)).*" -INIT_FILE_REGEX = r"^__init__.py$" +INIT_FILE_REGEX = r"^__init__.pyi?$" +STUB_FILE_REGEX = r".*\.pyi$" EMPTY_REGEX = r"^$" -INCLUDE_REGEX = r".*\.py$" +INCLUDE_REGEX = r".*\.pyi?$" EXCLUDE_REGEX = ( r"(\.eggs|\.git|\.hg|\.mypy_cache|__pycache__|\.nox|" + r"\.tox|\.venv|\.svn|buck-out|build|dist)/" @@ -61,15 +62,25 @@ def strpath(path: Path) -> str: def is_init_file(path: Path) -> bool: - """Check if the file name is `__init__.py`. + """Check if the file name is `__init__.py(i)`. :param path: file-system path to check. - :returns: True if the file is `__init__.py` else False. + :returns: True if the file is `__init__.py(i)` else False. """ regex: Pattern[str] = safe_compile(INIT_FILE_REGEX, INCLUDE) return bool(regex.match(path.name)) +def is_stub_file(path: Path) -> bool: + """Check if the file extension is `.pyi`. + + :param path: file-system path to check. + :returns: True if the file extension is `.pyi` else False. + """ + regex: Pattern[str] = safe_compile(STUB_FILE_REGEX, INCLUDE) + return bool(regex.search(path.name)) + + def is_included(path: Path, regex: Pattern[str]) -> bool: """Check if the file/directory name match include pattern. diff --git a/tests/data/paths/b.pyi b/tests/data/paths/b.pyi new file mode 100644 index 0000000..e69de29 diff --git a/tests/data/paths/dir/a.pyi b/tests/data/paths/dir/a.pyi new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_pathu.py b/tests/test_pathu.py index 215522a..dabbcf1 100644 --- a/tests/test_pathu.py +++ b/tests/test_pathu.py @@ -52,11 +52,12 @@ def clear_all_lru_cache(self): [ pytest.param( Path(DATA_DIR / "paths" / "dir"), - re.compile(r".*\.py$"), + re.compile(r".*\.pyi?$"), re.compile(r"(.*s\.py|git/)$"), re.compile(regexu.EMPTY_REGEX), PathSpec.from_lines("gitwildmatch", ["*u.py", "utils/"]), { + "a.pyi", "x.py", "y.py", "z.py", @@ -65,7 +66,7 @@ def clear_all_lru_cache(self): ), pytest.param( Path(DATA_DIR / "paths"), - re.compile(r".*\.py$"), + re.compile(r".*\.pyi?$"), re.compile(r"paths/"), re.compile(regexu.EMPTY_REGEX), PathSpec.from_lines("gitwildmatch", []), @@ -74,7 +75,7 @@ def clear_all_lru_cache(self): ), pytest.param( Path(DATA_DIR / "paths"), - re.compile(r".*\.py$"), + re.compile(r".*\.pyi?$"), re.compile(regexu.EMPTY_REGEX), re.compile(r"paths/"), PathSpec.from_lines("gitwildmatch", []), @@ -83,7 +84,7 @@ def clear_all_lru_cache(self): ), pytest.param( Path(DATA_DIR / "paths" / "dir"), - re.compile(r".*\.py$"), + re.compile(r".*\.pyi?$"), re.compile(r"paths/"), re.compile(regexu.EMPTY_REGEX), PathSpec.from_lines("gitwildmatch", []), @@ -92,16 +93,25 @@ def clear_all_lru_cache(self): ), pytest.param( Path(DATA_DIR / "paths" / "a.py"), - re.compile(r".*\.py$"), + re.compile(r".*\.pyi?$"), re.compile(regexu.EMPTY_REGEX), re.compile(regexu.EMPTY_REGEX), PathSpec.from_lines("gitwildmatch", []), {"a.py"}, - id="path: file", + id="path: file .py", + ), + pytest.param( + Path(DATA_DIR / "paths" / "b.pyi"), + re.compile(r".*\.pyi?$"), + re.compile(regexu.EMPTY_REGEX), + re.compile(regexu.EMPTY_REGEX), + PathSpec.from_lines("gitwildmatch", []), + {"b.pyi"}, + id="path: file .pyi", ), pytest.param( Path(DATA_DIR / "paths" / "a.py"), - re.compile(r".*\.py$"), + re.compile(r".*\.pyi?$"), re.compile(regexu.EMPTY_REGEX), re.compile(regexu.EMPTY_REGEX), PathSpec.from_lines("gitwildmatch", ["a.py"]), @@ -110,7 +120,7 @@ def clear_all_lru_cache(self): ), pytest.param( Path(DATA_DIR / "paths" / "b.c"), - re.compile(r".*\.py$"), + re.compile(r".*\.pyi?$"), re.compile(regexu.EMPTY_REGEX), re.compile(regexu.EMPTY_REGEX), PathSpec.from_lines("gitwildmatch", []), diff --git a/tests/test_refactor.py b/tests/test_refactor.py index 5d427d0..f963a97 100644 --- a/tests/test_refactor.py +++ b/tests/test_refactor.py @@ -24,6 +24,22 @@ MOCK = "pycln.utils.refactor.%s" +class TestPyPath: + + """`PyPath` class test case.""" + + def test_is_subclass_of_pathlib_path(self): + assert issubclass(refactor.PyPath, Path) + + @pytest.mark.parametrize( + "path, expected_is_stub", + [pytest.param("a.py", False), pytest.param("a.pyi", True)], + ) + def test_is_stub_property(self, path, expected_is_stub): + pypath = refactor.PyPath(path) + assert pypath.is_stub == expected_is_stub + + class TestRefactor: """`Refactor` methods test case.""" @@ -935,6 +951,43 @@ def test_should_remove( val = self.session_maker._should_remove(node, alias, False) assert val == expec_val + @pytest.mark.parametrize( + "node, expec_should_remove", + [ + pytest.param( + Import(NodeLocation((1, 0), 1), [ast.alias(name="x", asname="x")]), + False, + id="x as x", + ), + pytest.param( + Import(NodeLocation((1, 0), 1), [ast.alias(name="x", asname="y")]), + True, + id="x as y", + ), + pytest.param( + Import(NodeLocation((1, 0), 1), [ast.alias(name="x", asname=None)]), + True, + id="no as", + ), + ], + ) + @mock.patch(MOCK % "Refactor._has_side_effects") + @mock.patch(MOCK % "Refactor._has_used") + @mock.patch(MOCK % "PyPath.is_stub") + def test_should_remove_skip_pep484( + self, is_stub, _has_used, _has_side_effects, node, expec_should_remove + ): + #: Test PEP 484 - Redundant Module/Symbol Aliases rule for stub files: + #: + #: >>> import X as X # exported (should be treated as used) + #: + #: More info: https://peps.python.org/pep-0484/#stub-files + is_stub.return_value = True + _has_used.return_value = False + self.session_maker.configs.all_ = True + val = self.session_maker._should_remove(node, node.names[0], False) + assert val == expec_should_remove + @pytest.mark.parametrize( "node, should_assert", [ diff --git a/tests/test_regexu.py b/tests/test_regexu.py index ca712bf..f3b1382 100644 --- a/tests/test_regexu.py +++ b/tests/test_regexu.py @@ -94,7 +94,12 @@ def test_strpath(self, path, expec_strpath): pytest.param( Path("path/to/__init__.py"), True, - id="exact - true", + id=".py - true", + ), + pytest.param( + Path("path/to/__init__.pyi"), + True, + id=".pyi - true", ), pytest.param( Path("path/to/not__init__.py"), @@ -111,6 +116,29 @@ def test_strpath(self, path, expec_strpath): def test_is_init_file(self, path, expec): assert regexu.is_init_file(path) == expec + @pytest.mark.parametrize( + "path, expec", + [ + pytest.param( + Path("path/to/a.pyi"), + True, + id="ext:pyi", + ), + pytest.param( + Path("path/to/a.py"), + False, + id="ext:py", + ), + pytest.param( + Path("path/to/a.so"), + False, + id="ext:so", + ), + ], + ) + def test_is_stub_file(self, path, expec): + assert regexu.is_stub_file(path) == expec + @pytest.mark.parametrize( "path, regex, expec", [