diff --git a/CHANGELOG.md b/CHANGELOG.md index e48e564..19f2aec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/ERRORCODES.md b/ERRORCODES.md index 0c8f984..5e864c7 100644 --- a/ERRORCODES.md +++ b/ERRORCODES.md @@ -82,6 +82,7 @@ The following warnings are currently emitted by default: | Y066 | When using if/else with `sys.version_info`, put the code for new Python versions first. | Style | Y067 | Don't use `Incomplete \| None = None` in argument annotations. Instead, just use `=None`. | Style | Y068 | 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 +| Y069 | 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 diff --git a/flake8_pyi/errors.py b/flake8_pyi/errors.py index 756b6a2..486a2a8 100644 --- a/flake8_pyi/errors.py +++ b/flake8_pyi/errors.py @@ -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 ' diff --git a/flake8_pyi/visitor.py b/flake8_pyi/visitor.py index 5e1fca7..acda59d 100644 --- a/flake8_pyi/visitor.py +++ b/flake8_pyi/visitor.py @@ -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] @@ -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 @@ -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})" @@ -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. @@ -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): diff --git a/tests/sysplatform.pyi b/tests/sysplatform.pyi index c319974..5562240 100644 --- a/tests/sysplatform.pyi +++ b/tests/sysplatform.pyi @@ -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 diff --git a/tests/sysversioninfo.pyi b/tests/sysversioninfo.pyi index 190b8df..3e2533f 100644 --- a/tests/sysversioninfo.pyi +++ b/tests/sysversioninfo.pyi @@ -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:' @@ -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: ...