Skip to content

Commit

Permalink
Add support for stub files. (#150)
Browse files Browse the repository at this point in the history
  • Loading branch information
hadialqattan committed Jul 4, 2022
1 parent 542147d commit 896421a
Show file tree
Hide file tree
Showing 10 changed files with 185 additions and 30 deletions.
14 changes: 11 additions & 3 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
35 changes: 28 additions & 7 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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

Expand Down Expand Up @@ -343,7 +347,7 @@ pycln:

#### Default

> `.*\.py$`
> `.*\.pyi?$`

#### Behaviour

Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions pycln/utils/pathu.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,16 @@ 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.
:param exclude: regex pattern to be excluded.
: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()
Expand Down
32 changes: 28 additions & 4 deletions pycln/utils/refactor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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

Expand Down
19 changes: 15 additions & 4 deletions pycln/utils/regexu.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)/"
Expand Down Expand Up @@ -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.
Expand Down
Empty file added tests/data/paths/b.pyi
Empty file.
Empty file added tests/data/paths/dir/a.pyi
Empty file.
26 changes: 18 additions & 8 deletions tests/test_pathu.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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", []),
Expand All @@ -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", []),
Expand All @@ -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", []),
Expand All @@ -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"]),
Expand All @@ -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", []),
Expand Down
53 changes: 53 additions & 0 deletions tests/test_refactor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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",
[
Expand Down

0 comments on commit 896421a

Please sign in to comment.