From db02242b142285e615a664a8d3324470bb711306 Mon Sep 17 00:00:00 2001 From: dosisod <39638017+dosisod@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:30:05 -0700 Subject: [PATCH] Type deduce object subclasses: Closes #329. This commit adds the ability to detect the subclass of a given class, allowing Refurb to safely detect types derived from ABC classes like `Sequence`, `Mapping`, and so on. This commit will give more warnings in user code, and even gave a warning for a line in Refurb. There are still many improvements to the type system left to implement, but this was the big one on my list. --- refurb/checks/builtin/no_ignored_enumerate.py | 5 +- refurb/checks/builtin/writelines.py | 5 +- refurb/checks/common.py | 69 ++++++++++--------- refurb/checks/iterable/implicit_readlines.py | 4 +- test/test_checks.py | 2 +- 5 files changed, 42 insertions(+), 43 deletions(-) diff --git a/refurb/checks/builtin/no_ignored_enumerate.py b/refurb/checks/builtin/no_ignored_enumerate.py index 404407d..35cbf62 100644 --- a/refurb/checks/builtin/no_ignored_enumerate.py +++ b/refurb/checks/builtin/no_ignored_enumerate.py @@ -15,7 +15,7 @@ check_for_loop_like, get_mypy_type, is_name_unused_in_contexts, - is_same_type, + is_subclass, stringify, ) from refurb.error import Error @@ -74,8 +74,7 @@ def check_enumerate_call( callee=NameExpr(fullname="builtins.enumerate"), args=[enumerate_arg], ), - ) if is_same_type(get_mypy_type(enumerate_arg), list, tuple): - # TODO: support more sequence types + ) if is_subclass(get_mypy_type(enumerate_arg), "typing.Sequence"): check_unused_index_or_value(index, value, contexts, errors, enumerate_arg) diff --git a/refurb/checks/builtin/writelines.py b/refurb/checks/builtin/writelines.py index 21ee38b..e99843d 100644 --- a/refurb/checks/builtin/writelines.py +++ b/refurb/checks/builtin/writelines.py @@ -11,7 +11,7 @@ WithStmt, ) -from refurb.checks.common import get_mypy_type, is_equivalent, is_same_type, stringify +from refurb.checks.common import get_mypy_type, is_equivalent, is_subclass, stringify from refurb.error import Error @@ -47,8 +47,7 @@ class ErrorInfo(Error): def is_file_object(f: Expression) -> bool: - # TODO: support more file-like types - return is_same_type(get_mypy_type(f), "io.TextIOWrapper", "io.BufferedWriter") + return is_subclass(get_mypy_type(f), "io.IOBase") def check(node: WithStmt, errors: list[Error]) -> None: diff --git a/refurb/checks/common.py b/refurb/checks/common.py index 144e7b4..2c97627 100644 --- a/refurb/checks/common.py +++ b/refurb/checks/common.py @@ -728,55 +728,56 @@ def mypy_type_to_python_type(ty: Type | SymbolNode | None) -> type | None: return None # pragma: no cover -MAPPING_TYPES = ( - dict, - "collections.ChainMap", - "collections.Counter", - "collections.OrderedDict", - "collections.UserDict", - "collections.abc.Mapping", - "collections.abc.MutableMapping", - "collections.defaultdict", - "os._Environ", - "typing.Mapping", - "typing.MutableMapping", -) - - -# TODO: support any Mapping subclass def is_mapping(expr: Expression) -> bool: return is_mapping_type(get_mypy_type(expr)) def is_mapping_type(ty: Type | SymbolNode | None) -> bool: - return is_same_type(ty, *MAPPING_TYPES) + return is_subclass(ty, "typing.Mapping") + + +def is_bool_literal(node: Node) -> TypeGuard[NameExpr]: + return is_true_literal(node) or is_false_literal(node) + + +def is_true_literal(node: Node) -> TypeGuard[NameExpr]: + return isinstance(node, NameExpr) and node.fullname == "builtins.True" + + +def is_false_literal(node: Node) -> TypeGuard[NameExpr]: + return isinstance(node, NameExpr) and node.fullname == "builtins.False" def is_sized(node: Expression) -> bool: return is_sized_type(get_mypy_type(node)) -# TODO: support any Sized subclass def is_sized_type(ty: Type | SymbolNode | None) -> bool: - return is_mapping_type(ty) or is_same_type( - ty, - frozenset, - list, - set, - str, - tuple, - "_collections_abc.dict_keys", - "_collections_abc.dict_values", - ) + # Certain object MROs (like dict) doesn't reference Sized directly, only Collection. We might + # need to add more derived Sized types if Mypy doesn't fully resolve the MRO. + return is_subclass(ty, "typing.Sized", "typing.Collection") -def is_bool_literal(node: Node) -> TypeGuard[NameExpr]: - return is_true_literal(node) or is_false_literal(node) +def is_subclass(ty: Any, *expected: TypeLike) -> bool: # type: ignore[misc] + if type_info := extract_typeinfo(ty): + return any(is_same_type(x, *expected) for x in type_info.mro) -def is_true_literal(node: Node) -> TypeGuard[NameExpr]: - return isinstance(node, NameExpr) and node.fullname == "builtins.True" + return False # pragma: no cover -def is_false_literal(node: Node) -> TypeGuard[NameExpr]: - return isinstance(node, NameExpr) and node.fullname == "builtins.False" +def extract_typeinfo(ty: Type | SymbolNode | None) -> TypeInfo | None: + match ty: + case TypeInfo(): + return ty # pragma: no cover + + case Instance(): + return ty.type + + case TupleType(): + tmp = _get_builtin_mypy_type("tuple") + assert tmp + + return tmp.type + + return None # pragma: no cover diff --git a/refurb/checks/iterable/implicit_readlines.py b/refurb/checks/iterable/implicit_readlines.py index ee5dbd2..bd1fb62 100644 --- a/refurb/checks/iterable/implicit_readlines.py +++ b/refurb/checks/iterable/implicit_readlines.py @@ -2,7 +2,7 @@ from mypy.nodes import CallExpr, Expression, ForStmt, GeneratorExpr, MemberExpr -from refurb.checks.common import get_mypy_type, is_same_type, stringify +from refurb.checks.common import get_mypy_type, is_subclass, stringify from refurb.error import Error @@ -40,7 +40,7 @@ def check_readline_expr(expr: Expression, errors: list[Error]) -> None: case CallExpr( callee=MemberExpr(expr=f, name="readlines"), args=[], - ) if is_same_type(get_mypy_type(f), "io.TextIOWrapper", "io.BufferedReader"): + ) if is_subclass(get_mypy_type(f), "io.IOBase"): tmp = stringify(f) msg = f"Replace `{tmp}.readlines()` with `{tmp}`" diff --git a/test/test_checks.py b/test/test_checks.py index 3238652..e5f024f 100644 --- a/test/test_checks.py +++ b/test/test_checks.py @@ -38,7 +38,7 @@ def test_ignore_check_is_respected() -> None: errors = run_refurb(Settings(files=[test_file], ignore={ErrorCode(100), ErrorCode(123)})) - assert len(errors) == 0 + assert not errors def test_ignore_custom_check_is_respected() -> None: