Skip to content

Commit

Permalink
Handle more edge cases with if and try block imports.
Browse files Browse the repository at this point in the history
* Test mypy against Python 3.8 not 3.6
  • Loading branch information
domdfcoding committed Apr 2, 2024
1 parent b2ca004 commit 25a70db
Show file tree
Hide file tree
Showing 11 changed files with 151 additions and 22 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/flake8.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
if: steps.changes.outputs.code == 'true'
uses: "actions/setup-python@v5"
with:
python-version: "3.6"
python-version: "3.8"

- name: Install dependencies 🔧
if: steps.changes.outputs.code == 'true'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/mypy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ jobs:
if: steps.changes.outputs.code == 'true'
uses: "actions/setup-python@v5"
with:
python-version: "3.6"
python-version: "3.8"

- name: Install dependencies 🔧
run: |
Expand Down
66 changes: 54 additions & 12 deletions flake8_dunder_all/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
# stdlib
import ast
import sys
from typing import Any, Generator, List, Set, Tuple, Type, Union
from typing import Any, Generator, Iterator, List, Set, Tuple, Type, Union

# 3rd party
from consolekit.terminal_colours import Fore
Expand Down Expand Up @@ -159,11 +159,10 @@ def handle_import(self, node: Union[ast.Import, ast.ImportFrom]) -> None:
"""

if self.use_endlineno:
if node.end_lineno > self.last_import: # type: ignore[union-attr]
self.last_import = node.end_lineno # type: ignore[union-attr]
if node.end_lineno is not None:
self.last_import = max(self.last_import, node.end_lineno)
else:
if node.lineno > self.last_import:
self.last_import = node.lineno
self.last_import = max(self.last_import, node.lineno)

def visit_Import(self, node: ast.Import) -> None:
"""
Expand Down Expand Up @@ -191,16 +190,59 @@ def visit_If(self, node: ast.If) -> None:
:param node: The node being visited.
"""
# Check if the condition is checking for TYPE_CHECKING
if isinstance(node.test, ast.Name) and node.test.id == "TYPE_CHECKING":
# Process the body of the if statement for any import statements
for body_node in node.body:
if isinstance(body_node, (ast.Import, ast.ImportFrom)):
self.handle_import(body_node)
# Continue visiting other nodes

if _is_type_checking(node.test):
if self.use_endlineno and node.end_lineno is not None:
self.last_import = max(self.last_import, node.end_lineno)
else:
self.last_import = max(self.last_import, max(_descend_node(node)))

self.generic_visit(node)

def visit_Try(self, node: ast.Try) -> None:
"""
Visit a Try statement.
:param node: The node being visited.
"""

if any(isinstance(n, (ast.Import, ast.ImportFrom)) for n in node.body):
if self.use_endlineno and node.end_lineno is not None and sys.implementation.name != "pypy": # pragma: no cover (pypy)
self.last_import = max(self.last_import, node.end_lineno)
else: # pragma: no cover (!pypy)
end_lineno = max(
*_descend_node(node),
*_descend_node(node, "handlers"),
*_descend_node(node, "orelse"),
*_descend_node(node, "finalbody"),
)
self.last_import = max(self.last_import, end_lineno)

self.generic_visit(node)


def _descend_node(node, attr: str = "body") -> Iterator[int]:
for child in getattr(node, attr, []):
yield child.lineno
yield from _descend_node(child)


_nameconstant = ast.Constant if sys.version_info >= (3, 8) else ast.NameConstant


def _is_type_checking(node: ast.AST):
"""

Check warning on line 234 in flake8_dunder_all/__init__.py

View workflow job for this annotation

GitHub Actions / Flake8

D400: First line should end with a period
Does the given ``if`` node indicate a `TYPE_CHECKING` block?
"""

if isinstance(node, ast.Name) and node.id == "TYPE_CHECKING":
return True
elif isinstance(node, _nameconstant) and node.value is False:
return True
elif isinstance(node, ast.BoolOp):
return any(_is_type_checking(value) for value in node.values)


class Plugin:
"""
A Flake8 plugin which checks to ensure modules have defined ``__all__``.
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ autodoc_exclude_members = [
]

[tool.mypy]
python_version = "3.6"
python_version = "3.8"
namespace_packages = true
check_untyped_defs = true
warn_unused_ignores = true
Expand Down
1 change: 0 additions & 1 deletion repo_helper.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ primary_conda_channel: "domdfcoding"
conda_channels:
- conda-forge

python_deploy_version: 3.6
use_whey: True
sphinx_html_theme: furo
min_coverage: 100
Expand Down
36 changes: 36 additions & 0 deletions tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,39 @@ def a_function():
def a_function(): ...
"""

if_type_checking_else_source = """
if False or sys.version_info < (3, 7):
import foo
else:
import bar
def a_function(): ...
"""

if_type_checking_try_source = """
from typing import TYPE_CHECKING
if TYPE_CHECKING:
try:
from x import y
except ImportError:
pass
def a_function(): ...
"""

if_type_checking_try_finally_source = """
from typing import TYPE_CHECKING
if TYPE_CHECKING:
try:
from x import y
except ImportError:
pass
finally:
pass
def a_function(): ...
"""
14 changes: 14 additions & 0 deletions tests/test_flake8_dunder_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
import pytest
from coincidence.regressions import AdvancedFileRegressionFixture
from common import (
if_type_checking_else_source,
if_type_checking_source,
if_type_checking_try_finally_source,
if_type_checking_try_source,
mangled_source,
results,
testing_source_a,
Expand Down Expand Up @@ -125,6 +128,14 @@ def test_visitor(source: str, members: List[str], found_all: bool, last_import:
pytest.param(testing_source_j, ["a_function"], False, 14, id="multiline import"),
pytest.param(testing_source_m, ["a_function"], False, 7, id="if False"),
pytest.param(if_type_checking_source, ["a_function"], False, 5, id="if TYPE_CHECKING:"),
pytest.param(if_type_checking_else_source, ["a_function"], False, 5, id="if TYPE_CHECKING else"),
pytest.param(if_type_checking_try_source, ["a_function"], False, 8, id="if TYPE_CHECKING try"),
pytest.param(
if_type_checking_try_finally_source, ["a_function"],
False,
10,
id="if TYPE_CHECKING try finally"
),
]
)
def test_visitor_endlineno(source: str, members: List[str], found_all: bool, last_import: int):
Expand Down Expand Up @@ -160,6 +171,9 @@ def test_visitor_endlineno(source: str, members: List[str], found_all: bool, las
pytest.param(testing_source_l, [], 0, id="typing.overload"),
pytest.param(testing_source_m, [], 1, id="if False"),
pytest.param(testing_source_n, [], 1, id="if TYPE_CHECKING"),
pytest.param(if_type_checking_else_source, [], 1, id="if TYPE_CHECKING else"),
pytest.param(if_type_checking_try_source, [], 1, id="if TYPE_CHECKING try"),
pytest.param(if_type_checking_try_finally_source, [], 1, id="if TYPE_CHECKING try finally"),
]
)
def test_check_and_add_all(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

if False or sys.version_info < (3, 7):
import foo
else:
import bar

__all__ = ["a_function"]


def a_function(): ...
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

from typing import TYPE_CHECKING

if TYPE_CHECKING:
try:
from x import y
except ImportError:
pass

__all__ = ["a_function"]


def a_function(): ...
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

from typing import TYPE_CHECKING

if TYPE_CHECKING:
try:
from x import y
except ImportError:
pass
finally:
pass

__all__ = ["a_function"]


def a_function(): ...
12 changes: 6 additions & 6 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ test =
pypy38-flake8{4,5,6,7}
pypy39-flake8{4,5,6,7}
qa = mypy, lint
cov = py36-flake84, coverage
cov = py38-flake84, coverage

[testenv]
setenv =
Expand Down Expand Up @@ -117,7 +117,7 @@ commands =
check-wheel-contents dist/

[testenv:lint]
basepython = python3.6
basepython = python3.8
changedir = {toxinidir}
ignore_errors = True
skip_install = False
Expand Down Expand Up @@ -147,15 +147,15 @@ deps =
commands = python3 -m flake8_rst_docstrings_sphinx flake8_dunder_all tests --allow-toolbox {posargs}

[testenv:perflint]
basepython = python3.6
basepython = python3.8
changedir = {toxinidir}
ignore_errors = True
skip_install = True
deps = perflint
commands = python3 -m perflint flake8_dunder_all {posargs}

[testenv:mypy]
basepython = python3.6
basepython = python3.8
ignore_errors = True
changedir = {toxinidir}
deps =
Expand All @@ -165,15 +165,15 @@ deps =
commands = mypy flake8_dunder_all tests {posargs}

[testenv:pyup]
basepython = python3.6
basepython = python3.8
skip_install = True
ignore_errors = True
changedir = {toxinidir}
deps = pyupgrade-directories
commands = pyup_dirs flake8_dunder_all tests --py36-plus --recursive

[testenv:coverage]
basepython = python3.6
basepython = python3.8
skip_install = True
ignore_errors = True
whitelist_externals = /bin/bash
Expand Down

0 comments on commit 25a70db

Please sign in to comment.