Skip to content

Commit

Permalink
Fix invalid type false positive (#8206) (#8386)
Browse files Browse the repository at this point in the history
(cherry picked from commit e64f043)

Co-authored-by: Nick Drozd <nicholasdrozd@gmail.com>
  • Loading branch information
cdce8p and nickdrozd committed Mar 6, 2023
1 parent 08bac36 commit fb4512c
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 8 deletions.
3 changes: 3 additions & 0 deletions doc/whatsnew/fragments/8205.false_positive
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix false positive for isinstance-second-argument-not-valid-type with union types.

Closes #8205
15 changes: 14 additions & 1 deletion pylint/checkers/typecheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
supports_membership_test,
supports_setitem,
)
from pylint.constants import PY310_PLUS
from pylint.interfaces import HIGH, INFERENCE
from pylint.typing import MessageDefinitionTuple

Expand Down Expand Up @@ -796,6 +797,10 @@ def _is_c_extension(module_node: InferenceResult) -> bool:

def _is_invalid_isinstance_type(arg: nodes.NodeNG) -> bool:
# Return True if we are sure that arg is not a type
if PY310_PLUS and isinstance(arg, nodes.BinOp) and arg.op == "|":
return _is_invalid_isinstance_type(arg.left) or _is_invalid_isinstance_type(
arg.right
)
inferred = utils.safe_infer(arg)
if not inferred:
# Cannot infer it so skip it.
Expand All @@ -806,6 +811,10 @@ def _is_invalid_isinstance_type(arg: nodes.NodeNG) -> bool:
return False
if isinstance(inferred, astroid.Instance) and inferred.qname() == BUILTIN_TUPLE:
return False
if PY310_PLUS and isinstance(inferred, bases.UnionType):
return _is_invalid_isinstance_type(
inferred.left
) or _is_invalid_isinstance_type(inferred.right)
return True


Expand Down Expand Up @@ -1398,7 +1407,11 @@ def _check_isinstance_args(self, node: nodes.Call) -> None:

second_arg = node.args[1]
if _is_invalid_isinstance_type(second_arg):
self.add_message("isinstance-second-argument-not-valid-type", node=node)
self.add_message(
"isinstance-second-argument-not-valid-type",
node=node,
confidence=INFERENCE,
)

# pylint: disable = too-many-branches, too-many-locals, too-many-statements
def visit_call(self, node: nodes.Call) -> None:
Expand Down
1 change: 1 addition & 0 deletions pylint/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

PY38_PLUS = sys.version_info[:2] >= (3, 8)
PY39_PLUS = sys.version_info[:2] >= (3, 9)
PY310_PLUS = sys.version_info[:2] >= (3, 10)

IS_PYPY = platform.python_implementation() == "PyPy"

Expand Down
2 changes: 1 addition & 1 deletion tests/functional/d/dataclass/dataclass_typecheck.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ unsupported-delete-operation:72:4:72:13::'obj.attr1' does not support item delet
not-context-manager:97:0:98:8::Context manager 'str' doesn't implement __enter__ and __exit__.:UNDEFINED
invalid-metaclass:105:0:105:11:Test2:Invalid metaclass 'Instance of builtins.int' used:UNDEFINED
unhashable-member:111:0:111:2::'obj.attr5' is unhashable and can't be used as a key in a dict:INFERENCE
isinstance-second-argument-not-valid-type:121:6:121:30::Second argument of isinstance is not a type:UNDEFINED
isinstance-second-argument-not-valid-type:121:6:121:30::Second argument of isinstance is not a type:INFERENCE
2 changes: 1 addition & 1 deletion tests/functional/ext/typing/redundant_typehint_argument.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
""""Checks for redundant Union typehints in assignments"""
"""Checks for redundant Union typehints in assignments"""
# pylint: disable=deprecated-typing-alias,consider-alternative-union-syntax,consider-using-alias

from __future__ import annotations
Expand Down
10 changes: 5 additions & 5 deletions tests/functional/i/isinstance_second_argument.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
isinstance-second-argument-not-valid-type:27:0:27:23::Second argument of isinstance is not a type:UNDEFINED
isinstance-second-argument-not-valid-type:28:0:28:19::Second argument of isinstance is not a type:UNDEFINED
isinstance-second-argument-not-valid-type:29:0:29:34::Second argument of isinstance is not a type:UNDEFINED
isinstance-second-argument-not-valid-type:30:0:30:54::Second argument of isinstance is not a type:UNDEFINED
isinstance-second-argument-not-valid-type:31:0:31:18::Second argument of isinstance is not a type:UNDEFINED
isinstance-second-argument-not-valid-type:27:0:27:23::Second argument of isinstance is not a type:INFERENCE
isinstance-second-argument-not-valid-type:28:0:28:19::Second argument of isinstance is not a type:INFERENCE
isinstance-second-argument-not-valid-type:29:0:29:34::Second argument of isinstance is not a type:INFERENCE
isinstance-second-argument-not-valid-type:30:0:30:54::Second argument of isinstance is not a type:INFERENCE
isinstance-second-argument-not-valid-type:31:0:31:18::Second argument of isinstance is not a type:INFERENCE
27 changes: 27 additions & 0 deletions tests/functional/i/isinstance_second_argument_py310.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'''Tests for invalid isinstance with compound types'''

# True negatives
isinstance(0, int | str)
isinstance(0, int | int | int)
isinstance(0, int | str | list | float)
isinstance(0, (int | str) | (list | float))

IntOrStr = int | str
isinstance(0, IntOrStr)
ListOrDict = list | dict
isinstance(0, (float | ListOrDict) | IntOrStr)

# True positives
isinstance(0, int | 5) # [isinstance-second-argument-not-valid-type]
isinstance(0, str | 5 | int) # [isinstance-second-argument-not-valid-type]
INT = 5
isinstance(0, INT | int) # [isinstance-second-argument-not-valid-type]


# FALSE NEGATIVES

# Parameterized generics will raise type errors at runtime.
# Warnings should be raised, but aren't (yet).
isinstance(0, list[int])
ListOfInts = list[int]
isinstance(0, ListOfInts)
2 changes: 2 additions & 0 deletions tests/functional/i/isinstance_second_argument_py310.rc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[testoptions]
min_pyver=3.10
3 changes: 3 additions & 0 deletions tests/functional/i/isinstance_second_argument_py310.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
isinstance-second-argument-not-valid-type:15:0:15:22::Second argument of isinstance is not a type:INFERENCE
isinstance-second-argument-not-valid-type:16:0:16:28::Second argument of isinstance is not a type:INFERENCE
isinstance-second-argument-not-valid-type:18:0:18:24::Second argument of isinstance is not a type:INFERENCE

0 comments on commit fb4512c

Please sign in to comment.