Skip to content

Commit

Permalink
Fix false-positive in pylint plugin (#74244)
Browse files Browse the repository at this point in the history
  • Loading branch information
epenet authored Jul 9, 2022
1 parent d80d16a commit 40ee7ba
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 67 deletions.
67 changes: 38 additions & 29 deletions pylint/plugins/hass_enforce_type_hints.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,18 @@ class ClassTypeHintMatch:
_TYPE_HINT_MATCHERS: dict[str, re.Pattern[str]] = {
# a_or_b matches items such as "DiscoveryInfoType | None"
"a_or_b": re.compile(r"^(\w+) \| (\w+)$"),
# x_of_y matches items such as "Awaitable[None]"
"x_of_y": re.compile(r"^(\w+)\[(.*?]*)\]$"),
# x_of_y_comma_z matches items such as "Callable[..., Awaitable[None]]"
"x_of_y_comma_z": re.compile(r"^(\w+)\[(.*?]*), (.*?]*)\]$"),
# x_of_y_of_z_comma_a matches items such as "list[dict[str, Any]]"
"x_of_y_of_z_comma_a": re.compile(r"^(\w+)\[(\w+)\[(.*?]*), (.*?]*)\]\]$"),
}
_INNER_MATCH = r"((?:\w+)|(?:\.{3})|(?:\w+\[.+\]))"
_INNER_MATCH_POSSIBILITIES = [i + 1 for i in range(5)]
_TYPE_HINT_MATCHERS.update(
{
f"x_of_y_{i}": re.compile(
rf"^(\w+)\[{_INNER_MATCH}" + f", {_INNER_MATCH}" * (i - 1) + r"\]$"
)
for i in _INNER_MATCH_POSSIBILITIES
}
)


_MODULE_REGEX: re.Pattern[str] = re.compile(r"^homeassistant\.components\.\w+(\.\w+)?$")

Expand Down Expand Up @@ -1438,42 +1443,46 @@ def _is_valid_type(
and _is_valid_type(match.group(2), node.right)
)

# Special case for xxx[yyy[zzz, aaa]]`
if match := _TYPE_HINT_MATCHERS["x_of_y_of_z_comma_a"].match(expected_type):
return (
isinstance(node, nodes.Subscript)
and _is_valid_type(match.group(1), node.value)
and isinstance(subnode := node.slice, nodes.Subscript)
and _is_valid_type(match.group(2), subnode.value)
and isinstance(subnode.slice, nodes.Tuple)
and _is_valid_type(match.group(3), subnode.slice.elts[0])
and _is_valid_type(match.group(4), subnode.slice.elts[1])
# Special case for `xxx[aaa, bbb, ccc, ...]
if (
isinstance(node, nodes.Subscript)
and isinstance(node.slice, nodes.Tuple)
and (
match := _TYPE_HINT_MATCHERS[f"x_of_y_{len(node.slice.elts)}"].match(
expected_type
)
)

# Special case for xxx[yyy, zzz]`
if match := _TYPE_HINT_MATCHERS["x_of_y_comma_z"].match(expected_type):
# Handle special case of Mapping[xxx, Any]
if in_return and match.group(1) == "Mapping" and match.group(3) == "Any":
):
# This special case is separate because we want Mapping[str, Any]
# to also match dict[str, int] and similar
if (
len(node.slice.elts) == 2
and in_return
and match.group(1) == "Mapping"
and match.group(3) == "Any"
):
return (
isinstance(node, nodes.Subscript)
and isinstance(node.value, nodes.Name)
isinstance(node.value, nodes.Name)
# We accept dict when Mapping is needed
and node.value.name in ("Mapping", "dict")
and isinstance(node.slice, nodes.Tuple)
and _is_valid_type(match.group(2), node.slice.elts[0])
# Ignore second item
# and _is_valid_type(match.group(3), node.slice.elts[1])
)

# This is the default case
return (
isinstance(node, nodes.Subscript)
and _is_valid_type(match.group(1), node.value)
_is_valid_type(match.group(1), node.value)
and isinstance(node.slice, nodes.Tuple)
and _is_valid_type(match.group(2), node.slice.elts[0])
and _is_valid_type(match.group(3), node.slice.elts[1])
and all(
_is_valid_type(match.group(n + 2), node.slice.elts[n])
for n in range(len(node.slice.elts))
)
)

# Special case for xxx[yyy]`
if match := _TYPE_HINT_MATCHERS["x_of_y"].match(expected_type):
# Special case for xxx[yyy]
if match := _TYPE_HINT_MATCHERS["x_of_y_1"].match(expected_type):
return (
isinstance(node, nodes.Subscript)
and _is_valid_type(match.group(1), node.value)
Expand Down
159 changes: 121 additions & 38 deletions tests/pylint/test_enforce_type_hints.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,53 +40,34 @@ def test_regex_get_module_platform(


@pytest.mark.parametrize(
("string", "expected_x", "expected_y", "expected_z", "expected_a"),
("string", "expected_count", "expected_items"),
[
("list[dict[str, str]]", "list", "dict", "str", "str"),
("list[dict[str, Any]]", "list", "dict", "str", "Any"),
],
)
def test_regex_x_of_y_of_z_comma_a(
hass_enforce_type_hints: ModuleType,
string: str,
expected_x: str,
expected_y: str,
expected_z: str,
expected_a: str,
) -> None:
"""Test x_of_y_of_z_comma_a regexes."""
matchers: dict[str, re.Pattern] = hass_enforce_type_hints._TYPE_HINT_MATCHERS

assert (match := matchers["x_of_y_of_z_comma_a"].match(string))
assert match.group(0) == string
assert match.group(1) == expected_x
assert match.group(2) == expected_y
assert match.group(3) == expected_z
assert match.group(4) == expected_a


@pytest.mark.parametrize(
("string", "expected_x", "expected_y", "expected_z"),
[
("Callable[..., None]", "Callable", "...", "None"),
("Callable[..., Awaitable[None]]", "Callable", "...", "Awaitable[None]"),
("Callable[..., None]", 2, ("Callable", "...", "None")),
("Callable[..., Awaitable[None]]", 2, ("Callable", "...", "Awaitable[None]")),
("tuple[int, int, int, int]", 4, ("tuple", "int", "int", "int", "int")),
(
"tuple[int, int, int, int, int]",
5,
("tuple", "int", "int", "int", "int", "int"),
),
("Awaitable[None]", 1, ("Awaitable", "None")),
("list[dict[str, str]]", 1, ("list", "dict[str, str]")),
("list[dict[str, Any]]", 1, ("list", "dict[str, Any]")),
],
)
def test_regex_x_of_y_comma_z(
def test_regex_x_of_y_i(
hass_enforce_type_hints: ModuleType,
string: str,
expected_x: str,
expected_y: str,
expected_z: str,
expected_count: int,
expected_items: tuple[str, ...],
) -> None:
"""Test x_of_y_comma_z regexes."""
"""Test x_of_y_i regexes."""
matchers: dict[str, re.Pattern] = hass_enforce_type_hints._TYPE_HINT_MATCHERS

assert (match := matchers["x_of_y_comma_z"].match(string))
assert (match := matchers[f"x_of_y_{expected_count}"].match(string))
assert match.group(0) == string
assert match.group(1) == expected_x
assert match.group(2) == expected_y
assert match.group(3) == expected_z
for index in range(expected_count):
assert match.group(index + 1) == expected_items[index]


@pytest.mark.parametrize(
Expand Down Expand Up @@ -743,3 +724,105 @@ def capability_attributes(

with assert_no_messages(linter):
type_hint_checker.visit_classdef(class_node)


def test_valid_long_tuple(
linter: UnittestLinter, type_hint_checker: BaseChecker
) -> None:
"""Check invalid entity properties are ignored by default."""
# Set ignore option
type_hint_checker.config.ignore_missing_annotations = False

class_node, _, _ = astroid.extract_node(
"""
class Entity():
pass
class ToggleEntity(Entity):
pass
class LightEntity(ToggleEntity):
pass
class TestLight( #@
LightEntity
):
@property
def rgbw_color( #@
self
) -> tuple[int, int, int, int]:
pass
@property
def rgbww_color( #@
self
) -> tuple[int, int, int, int, int]:
pass
""",
"homeassistant.components.pylint_test.light",
)
type_hint_checker.visit_module(class_node.parent)

with assert_no_messages(linter):
type_hint_checker.visit_classdef(class_node)


def test_invalid_long_tuple(
linter: UnittestLinter, type_hint_checker: BaseChecker
) -> None:
"""Check invalid entity properties are ignored by default."""
# Set ignore option
type_hint_checker.config.ignore_missing_annotations = False

class_node, rgbw_node, rgbww_node = astroid.extract_node(
"""
class Entity():
pass
class ToggleEntity(Entity):
pass
class LightEntity(ToggleEntity):
pass
class TestLight( #@
LightEntity
):
@property
def rgbw_color( #@
self
) -> tuple[int, int, int, int, int]:
pass
@property
def rgbww_color( #@
self
) -> tuple[int, int, int, int, float]:
pass
""",
"homeassistant.components.pylint_test.light",
)
type_hint_checker.visit_module(class_node.parent)

with assert_adds_messages(
linter,
pylint.testutils.MessageTest(
msg_id="hass-return-type",
node=rgbw_node,
args=["tuple[int, int, int, int]", None],
line=15,
col_offset=4,
end_line=15,
end_col_offset=18,
),
pylint.testutils.MessageTest(
msg_id="hass-return-type",
node=rgbww_node,
args=["tuple[int, int, int, int, int]", None],
line=21,
col_offset=4,
end_line=21,
end_col_offset=19,
),
):
type_hint_checker.visit_classdef(class_node)

0 comments on commit 40ee7ba

Please sign in to comment.