Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ flake8-pyi uses Calendar Versioning (CalVer).
### New Error Codes

* Y068: Don't use `@override` in stub files
* Y069: Detect identical definitions in different `sys.version_info` or
`sys.platform` branches

### Other changes

Expand Down
1 change: 1 addition & 0 deletions ERRORCODES.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ The following warnings are currently emitted by default:
| <a id="Y066" href="#Y066">Y066</a> | When using if/else with `sys.version_info`, put the code for new Python versions first. | Style
| <a id="Y067" href="#Y067">Y067</a> | Don't use `Incomplete \| None = None` in argument annotations. Instead, just use `=None`. | Style
| <a id="Y068" href="#Y068">Y068</a> | Don't use `@override` in stub files. Problems with a function signature deviating from its superclass are inherited from the implementation, and other tools such as stubtest are better placed to recognize deviations between stubs and the implementation. | Understanding stubs
| <a id="Y069" href="#Y069">Y069</a> | Definitions inside `sys.version_info` or `sys.platform` branches should not be identical across branches. Identical definitions can be moved outside the branch. | Redundant code

## Warnings disabled by default

Expand Down
4 changes: 4 additions & 0 deletions flake8_pyi/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ class Error(NamedTuple):
)
Y067 = 'Y067 Use "=None" instead of "Incomplete | None = None"'
Y068 = 'Y068 Do not use "@override" in stub files.'
Y069 = (
'Y069 Definition "{name}" is identical in multiple sys.version_info/sys.platform '
"branches"
)

Y090 = (
'Y090 "{original}" means '
Expand Down
115 changes: 114 additions & 1 deletion flake8_pyi/visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,24 @@ def active(self) -> bool:
return bool(self.nesting)


def _definition_name(node: ast.stmt) -> str | None:
match node:
case (
ast.FunctionDef(name=name)
| ast.AsyncFunctionDef(name=name)
| ast.ClassDef(name=name)
):
return name
case ast.AnnAssign(target=ast.Name(id=name)):
return name
case ast.Assign(targets=[ast.Name(id=name)]):
return name
case _:
if sys.version_info >= (3, 12) and isinstance(node, ast.TypeAlias):
return node.name.id
return None


class PyiVisitor(ast.NodeVisitor):
filename: str
errors: list[Error]
Expand Down Expand Up @@ -875,6 +893,7 @@ class PyiVisitor(ast.NodeVisitor):
in_function: NestingCounter
visiting_arg: NestingCounter
Y061_suppressed: NestingCounter
duplicate_branch_check_suppressed: NestingCounter

# This is only relevant for visiting classes
enclosing_class_ctx: EnclosingClassContext | None = None
Expand All @@ -893,6 +912,7 @@ def __init__(self, filename: str) -> None:
self.in_function = NestingCounter()
self.visiting_arg = NestingCounter()
self.Y061_suppressed = NestingCounter()
self.duplicate_branch_check_suppressed = NestingCounter()

def __repr__(self) -> str:
return f"{self.__class__.__name__}(filename={self.filename!r})"
Expand Down Expand Up @@ -1429,6 +1449,8 @@ def _visit_slice_tuple(self, node: ast.Tuple, parent: str | None) -> None:

def visit_If(self, node: ast.If) -> None:
self._check_for_Y066_violations(node)
if not self.duplicate_branch_check_suppressed.active:
self._check_for_duplicate_branch_definitions(node)

test = node.test
# No types can appear in if conditions, so avoid confusing additional errors.
Expand All @@ -1439,8 +1461,99 @@ def visit_If(self, node: ast.If) -> None:
self._check_if_expression(expression)
else:
self._check_if_expression(test)
for line in chain(node.body, node.orelse):
for line in node.body:
self.visit(line)
if (
len(node.orelse) == 1
and isinstance(node.orelse[0], ast.If)
and node.orelse[0].col_offset == node.col_offset
):
with self.duplicate_branch_check_suppressed.enabled():
self.visit(node.orelse[0])
else:
for line in node.orelse:
self.visit(line)

def _check_for_duplicate_branch_definitions(self, node: ast.If) -> None:
if not self._is_sys_branch_test(node.test):
return

branches = list(self._iter_if_chain_bodies(node))
suppress_nonconsecutive = self._is_sys_version_branch_chain(node)

seen_defs: dict[str, int] = {}
reported_defs: set[str] = set()
for branch_index, branch in enumerate(branches):
branch_defs: set[str] = set()
for statement in branch:
definition_name = _definition_name(statement)
if definition_name is None:
continue
definition_dump = ast.dump(statement, include_attributes=False)
previous_branch_index = seen_defs.get(definition_dump)
if (
previous_branch_index is not None
and definition_dump not in reported_defs
and (
not suppress_nonconsecutive
or branch_index == previous_branch_index + 1
)
):
self.error(statement, errors.Y069.format(name=definition_name))
reported_defs.add(definition_dump)
else:
branch_defs.add(definition_dump)
for definition_dump in branch_defs:
seen_defs[definition_dump] = branch_index

def _iter_if_chain_bodies(self, node: ast.If) -> Iterator[list[ast.stmt]]:
current = node
while True:
yield current.body
match current.orelse:
case [ast.If() as next_if] if (
next_if.col_offset == current.col_offset
and self._is_sys_branch_test(next_if.test)
):
current = next_if
case orelse:
if orelse:
yield orelse
return

def _is_sys_branch_test(
self, node: ast.expr, *, version_only: bool = False
) -> bool:
match node:
case ast.BoolOp(values=values):
return all(
self._is_sys_branch_test(value, version_only=version_only)
for value in values
)
case ast.Compare(comparators=[_], left=left):
match left:
case ast.Attribute(value=ast.Name("sys"), attr="platform"):
return not version_only
case ast.Attribute(value=ast.Name("sys"), attr="version_info"):
return True
case ast.Subscript(
value=ast.Attribute(value=ast.Name("sys"), attr="version_info")
):
return True
case _:
pass
return False

def _is_sys_version_branch_chain(self, node: ast.If) -> bool:
current = node
while True:
if not self._is_sys_branch_test(current.test, version_only=True):
return False
match current.orelse:
case [ast.If() as next_if] if next_if.col_offset == current.col_offset:
current = next_if
case _:
return True

def _check_if_expression(self, node: ast.expr) -> None:
if not isinstance(node, ast.Compare):
Expand Down
7 changes: 7 additions & 0 deletions tests/sysplatform.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,10 @@ if sys.platform == 10.12: ... # Y007 Unrecognized sys.platform check
if sys.platform == 'linus': ... # Y008 Unrecognized platform "linus"
if sys.platform != 'linux': ...
if sys.platform == 'win32': ...

if sys.platform == 'win32':
platform_specific: str
platform_same: float
else:
platform_specific: int
platform_same: float # Y069 Definition "platform_same" is identical in multiple sys.version_info/sys.platform branches
52 changes: 52 additions & 0 deletions tests/sysversioninfo.pyi
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import sys
from typing import overload

if sys.version_info[0] == 2: ...
if sys.version_info[0] == True: ... # Y003 Unrecognized sys.version_info check # E712 comparison to True should be 'if cond is True:' or 'if cond:'
Expand Down Expand Up @@ -55,3 +56,54 @@ elif sys.version_info < (3, 10): # Y066 When using if/else with sys.version_inf
def foo4(x, *, bar=True, baz=False): ...
else:
def foo4(x, *, bar=True, baz=False, qux=1): ...

if sys.version_info >= (3, 15):
different: str
same: float
else:
different: int
same: float # Y069 Definition "same" is identical in multiple sys.version_info/sys.platform branches

if sys.version_info >= (3, 15):
def same_function(x: str) -> None: ...
elif sys.version_info >= (3, 14):
def same_function(x: int) -> None: ...
else:
def same_function(x: str) -> None: ...

if sys.version_info >= (3, 15):
class SameClass:
attr: int
elif sys.version_info >= (3, 14):
class SameClass: # Y069 Definition "SameClass" is identical in multiple sys.version_info/sys.platform branches
attr: int

if sys.version_info >= (3, 15):
same_assignment = ...
else:
same_assignment = ... # Y069 Definition "same_assignment" is identical in multiple sys.version_info/sys.platform branches

if sys.version_info >= (3, 15):
outer: int
else:
if sys.platform == 'win32':
nested_same: str
else:
nested_same: str # Y069 Definition "nested_same" is identical in multiple sys.version_info/sys.platform branches

if sys.version_info >= (3, 15):
@overload
def overloaded_same(x: int) -> int: ...
@overload
def overloaded_same(x: str) -> str: ...
else:
@overload
def overloaded_same(x: int) -> int: ... # Y069 Definition "overloaded_same" is identical in multiple sys.version_info/sys.platform branches
@overload
def overloaded_same(x: bytes) -> bytes: ...

if sys.version_info >= (3, 15):
@overload
def overloaded_different(x: int) -> int: ...
else:
def overloaded_different(x: int) -> int: ...
Loading