From e63aa14abed1dba4c4d5644a9f5a93c86d91632d Mon Sep 17 00:00:00 2001 From: Martin Thoma Date: Mon, 28 Mar 2022 13:41:57 +0200 Subject: [PATCH] SIM907: Use Optional[Type] instead of Union[Type, None] (#110) Closes #64 --- README.md | 17 ++++++++ flake8_simplify/__init__.py | 5 +++ flake8_simplify/rules/ast_subscript.py | 59 ++++++++++++++++++++++++++ tests/test_900_rules.py | 10 +++++ 4 files changed, 91 insertions(+) create mode 100644 flake8_simplify/rules/ast_subscript.py diff --git a/README.md b/README.md index 5703aa7..7a1ebb3 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,7 @@ Python-specific rules: * `SIM124`: Reserved for [SIM904](#sim904) once it's stable * `SIM125`: Reserved for [SIM905](#sim905) once it's stable * `SIM126`: Reserved for [SIM906](#sim906) once it's stable +* `SIM127`: Reserved for [SIM907](#sim907) once it's stable Simplifying Comparations: @@ -559,3 +560,19 @@ os.path.join(a, os.path.join(b, c)) # Good os.path.join(a, b, c) ``` + +### SIM907 + +This rule will be renamed to `SIM127` after its 6-month trial period is over. +Please report any issues you encounter with this rule! + +```python +# Bad +def foo(a: Union[int, None]) -> Union[int, None]: + return a + + +# Good +def foo(a: Optional[int]) -> Optional[int]: + return a +``` diff --git a/flake8_simplify/__init__.py b/flake8_simplify/__init__.py index f8b9558..409226f 100644 --- a/flake8_simplify/__init__.py +++ b/flake8_simplify/__init__.py @@ -37,6 +37,7 @@ get_sim401, ) from flake8_simplify.rules.ast_ifexp import get_sim210, get_sim211, get_sim212 +from flake8_simplify.rules.ast_subscript import get_sim907 from flake8_simplify.rules.ast_try import get_sim105, get_sim107 from flake8_simplify.rules.ast_unary_op import ( get_sim201, @@ -105,6 +106,10 @@ def visit_For(self, node: ast.For) -> None: self.errors += get_sim113(For(node)) self.generic_visit(node) + def visit_Subscript(self, node: ast.Subscript) -> None: + self.errors += get_sim907(node) + self.generic_visit(node) + def visit_Try(self, node: ast.Try) -> None: self.errors += get_sim105(node) self.errors += get_sim107(node) diff --git a/flake8_simplify/rules/ast_subscript.py b/flake8_simplify/rules/ast_subscript.py new file mode 100644 index 0000000..0d70941 --- /dev/null +++ b/flake8_simplify/rules/ast_subscript.py @@ -0,0 +1,59 @@ +# Core Library +import ast +from typing import List, Tuple + +# First party +from flake8_simplify.constants import BOOL_CONST_TYPES +from flake8_simplify.utils import to_source + + +def get_sim907(node: ast.Subscript) -> List[Tuple[int, int, str]]: + """ + + Subscript( + value=Name(id='Union', ctx=Load()), + slice=Tuple( + elts=[ + Name(id='int', ctx=Load()), + Name(id='str', ctx=Load()), + Constant(value=None, kind=None), + ], + ... + ) + ) + """ + errors: List[Tuple[int, int, str]] = [] + + if not (isinstance(node.value, ast.Name) and node.value.id == "Union"): + return errors + + if isinstance(node.slice, ast.Index) and isinstance( + node.slice.value, ast.Tuple # type: ignore + ): + # Python 3.8 + tuple_var = node.slice.value # type: ignore + elif isinstance(node.slice, ast.Tuple): + # Python 3.9+ + tuple_var = node.slice + else: + return errors + + has_none = False + others = [] + for elt in tuple_var.elts: # type: ignore + if isinstance(elt, BOOL_CONST_TYPES) and elt.value is None: + has_none = True + else: + others.append(elt) + + RULE = "SIM907 Use 'Optional[{type_}]' instead of '{original}'" + if len(others) == 1 and has_none: + type_ = to_source(others[0]) + errors.append( + ( + node.lineno, + node.col_offset, + RULE.format(type_=type_, original=to_source(node)), + ) + ) + return errors diff --git a/tests/test_900_rules.py b/tests/test_900_rules.py index f091ef7..7eae8c1 100644 --- a/tests/test_900_rules.py +++ b/tests/test_900_rules.py @@ -85,3 +85,13 @@ def test_sim905(): def test_sim906(s, msg): results = _results(s) assert results == {msg} + + +def test_sim907(): + results = _results( + """def foo(a: Union[int, None]) -> bool: + return a""" + ) + assert results == { + "1:11 SIM907 Use 'Optional[int]' instead of 'Union[int, None]'" + }