Skip to content

Commit

Permalink
Add a check for sys.version_info version order (#481)
Browse files Browse the repository at this point in the history
  • Loading branch information
tomasr8 committed Apr 18, 2024
1 parent 7117446 commit fa7bab6
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 0 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Change Log

## Unreleased

New error codes:
* Y066: When using if/else with `sys.version_info`,
put the code for new Python versions first.

## 24.4.0

Bugfixes:
Expand Down
1 change: 1 addition & 0 deletions ERRORCODES.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ The following warnings are currently emitted by default:
| Y063 | Use [PEP 570 syntax](https://peps.python.org/pep-0570/) (e.g. `def foo(x: int, /) -> None: ...`) to denote positional-only arguments, rather than [the older Python 3.7-compatible syntax described in PEP 484](https://peps.python.org/pep-0484/#positional-only-arguments) (`def foo(__x: int) -> None: ...`, etc.). | Style
| Y064 | Use simpler syntax to define final literal types. For example, use `x: Final = 42` instead of `x: Final[Literal[42]]`. | Style
| Y065 | Don't use bare `Incomplete` in argument and return annotations. Instead, leave them unannotated. Omitting an annotation entirely from a function will cause some type checkers to view the parameter or return type as "untyped"; this may result in stricter type-checking on code that makes use of the stubbed function. | Style
| Y066 | When using if/else with `sys.version_info`, put the code for new Python versions first. | Style

## Warnings disabled by default

Expand Down
34 changes: 34 additions & 0 deletions pyi.py
Original file line number Diff line number Diff line change
Expand Up @@ -1586,6 +1586,8 @@ def _visit_slice_tuple(self, node: ast.Tuple, parent: str | None) -> None:
self.visit(node)

def visit_If(self, node: ast.If) -> None:
self._check_for_Y066_violations(node)

test = node.test
# No types can appear in if conditions, so avoid confusing additional errors.
with self.string_literals_allowed.enabled():
Expand Down Expand Up @@ -1621,6 +1623,34 @@ def _check_if_expression(self, node: ast.expr) -> None:
else:
self.error(node, Y002)

def _check_for_Y066_violations(self, node: ast.If) -> None:
def is_version_info(attr: ast.expr) -> bool:
return (
isinstance(attr, ast.Attribute)
and _is_name(attr.value, "sys")
and attr.attr == "version_info"
)

def if_chain_ends_with_else(if_chain: ast.If) -> bool:
orelse = if_chain.orelse
if len(orelse) == 1 and isinstance(orelse[0], ast.If):
return if_chain_ends_with_else(orelse[0])
return bool(orelse)

test = node.test
if not isinstance(test, ast.Compare):
return

left = test.left
op = test.ops[0]
if (
is_version_info(left)
and isinstance(op, ast.Lt) # sys.version_info < ...
and if_chain_ends_with_else(node)
):
new_syntax = "if " + unparse(test).replace("<", ">=", 1)
self.error(node, Y066.format(new_syntax=new_syntax))

def _check_subscript_version_check(self, node: ast.Compare) -> None:
# unless this is on, comparisons against a single integer aren't allowed
must_be_single = False
Expand Down Expand Up @@ -2415,6 +2445,10 @@ def parse_options(options: argparse.Namespace) -> None:
Y063 = "Y063 Use PEP-570 syntax to indicate positional-only arguments"
Y064 = 'Y064 Use "{suggestion}" instead of "{original}"'
Y065 = 'Y065 Leave {what} unannotated rather than using "Incomplete"'
Y066 = (
"Y066 When using if/else with sys.version_info, "
'put the code for new Python versions first, e.g. "{new_syntax}"'
)
Y090 = (
'Y090 "{original}" means '
'"a tuple of length 1, in which the sole element is of type {typ!r}". '
Expand Down
26 changes: 26 additions & 0 deletions tests/sysversioninfo.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,29 @@ if sys.version_info <= (3, 0): ... # Y006 Use only < and >= for version compari
if sys.version_info < (3, 5): ...
if sys.version_info >= (3, 5): ...
if (2, 7) <= sys.version_info < (3, 5): ... # Y002 If test must be a simple comparison against sys.platform or sys.version_info

if sys.version_info >= (3, 10):
def foo1(x, *, bar=True, baz=False): ...
elif sys.version_info >= (3, 9):
def foo1(x, *, bar=True): ...
else:
def foo1(x): ...

if sys.version_info < (3, 9):
def foo2(x): ...
elif sys.version_info < (3, 10):
def foo2(x, *, bar=True): ...

if sys.version_info < (3, 10): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 10)"
def foo3(x): ...
else:
def foo3(x, *, bar=True): ...

if sys.version_info < (3, 8): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 8)"
def foo4(x): ...
elif sys.version_info < (3, 9): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 9)"
def foo4(x, *, bar=True): ...
elif sys.version_info < (3, 10): # Y066 When using if/else with sys.version_info, put the code for new Python versions first, e.g. "if sys.version_info >= (3, 10)"
def foo4(x, *, bar=True, baz=False): ...
else:
def foo4(x, *, bar=True, baz=False, qux=1): ...

0 comments on commit fa7bab6

Please sign in to comment.