diff options
author | Levi Gruspe <mail.levig@gmail.com> | 2022-09-04 19:10:41 +0800 |
---|---|---|
committer | Pierre Sassoulas <pierre.sassoulas@gmail.com> | 2022-09-06 23:33:59 +0200 |
commit | 40a53aa4954e643baa60006f0c6341df15fdb9c4 (patch) | |
tree | 136e581668061a6a8ee0e7463a238d9bde22526a | |
parent | d476a8bd2557ec1a075e4a8ac630469700ba753a (diff) | |
download | pylint-git-40a53aa4954e643baa60006f0c6341df15fdb9c4.tar.gz |
Fix #3299 false positives with names in string literal type annotations (#7400)
Don't emit 'unused-variable' or 'unused-import' on names in string literal type annotations (#3299)
Don't treat strings inside typing.Literal as names
12 files changed, 167 insertions, 1 deletions
diff --git a/doc/whatsnew/fragments/3299.false_positive b/doc/whatsnew/fragments/3299.false_positive new file mode 100644 index 000000000..b1e61c931 --- /dev/null +++ b/doc/whatsnew/fragments/3299.false_positive @@ -0,0 +1,3 @@ +Fix false positive for ``unused-variable`` and ``unused-import`` when a name is only used in a string literal type annotation. + +Closes #3299 diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index 08676c2e8..68452db0e 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -1891,6 +1891,28 @@ def in_type_checking_block(node: nodes.NodeNG) -> bool: return False +def is_typing_literal(node: nodes.NodeNG) -> bool: + """Check if a node refers to typing.Literal.""" + if isinstance(node, nodes.Name): + try: + import_from = node.lookup(node.name)[1][0] + except IndexError: + return False + if isinstance(import_from, nodes.ImportFrom): + return ( + import_from.modname == "typing" + and import_from.real_name(node.name) == "Literal" + ) + elif isinstance(node, nodes.Attribute): + inferred_module = safe_infer(node.expr) + return ( + isinstance(inferred_module, nodes.Module) + and inferred_module.name == "typing" + and node.attrname == "Literal" + ) + return False + + @lru_cache() def in_for_else_branch(parent: nodes.NodeNG, stmt: nodes.Statement) -> bool: """Returns True if stmt is inside the else branch for a parent For stmt.""" diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index ce6a38420..74b8c4c70 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -19,7 +19,7 @@ from functools import lru_cache from typing import TYPE_CHECKING, Any, NamedTuple import astroid -from astroid import nodes +from astroid import extract_node, nodes from astroid.typing import InferenceResult from pylint.checkers import BaseChecker, utils @@ -2341,6 +2341,10 @@ class VariablesChecker(BaseChecker): if name in comprehension_target_names: return + # Ignore names in string literal type annotation. + if name in self._type_annotation_names: + return + argnames = node.argnames() # Care about functions with unknown argument (builtins) if name in argnames: @@ -2904,6 +2908,41 @@ class VariablesChecker(BaseChecker): ) return + @utils.only_required_for_messages( + "unused-import", + "unused-variable", + ) + def visit_const(self, node: nodes.Const) -> None: + """Take note of names that appear inside string literal type annotations + unless the string is a parameter to typing.Literal. + """ + if node.pytype() != "builtins.str": + return + if not utils.is_node_in_type_annotation_context(node): + return + if not node.value.isidentifier(): + try: + annotation = extract_node(node.value) + self._store_type_annotation_node(annotation) + except ValueError: + # e.g. node.value is white space + return + except astroid.AstroidSyntaxError: + # e.g. "?" or ":" in typing.Literal["?", ":"] + return + + # Check if parent's or grandparent's first child is typing.Literal + parent = node.parent + if isinstance(parent, nodes.Tuple): + parent = parent.parent + + if isinstance(parent, nodes.Subscript): + origin = next(parent.get_children(), None) + if origin is not None and utils.is_typing_literal(origin): + return + + self._type_annotation_names.append(node.value) + def register(linter: PyLinter) -> None: linter.register_checker(VariablesChecker(linter)) diff --git a/tests/checkers/unittest_utils.py b/tests/checkers/unittest_utils.py index 8b9189892..f68a48dbb 100644 --- a/tests/checkers/unittest_utils.py +++ b/tests/checkers/unittest_utils.py @@ -489,3 +489,29 @@ def test_deprecation_check_messages() -> None: records[0].message.args[0] == "utils.check_messages will be removed in favour of calling utils.only_required_for_messages in pylint 3.0" ) + + +def test_is_typing_literal() -> None: + code = astroid.extract_node( + """ + from typing import Literal as Lit, Set as Literal + import typing as t + + Literal #@ + Lit #@ + t.Literal #@ + """ + ) + + assert not utils.is_typing_literal(code[0]) + assert utils.is_typing_literal(code[1]) + assert utils.is_typing_literal(code[2]) + + code = astroid.extract_node( + """ + Literal #@ + typing.Literal #@ + """ + ) + assert not utils.is_typing_literal(code[0]) + assert not utils.is_typing_literal(code[1]) diff --git a/tests/functional/u/unused/unused_name_in_string_literal_type_annotation.py b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation.py new file mode 100644 index 000000000..389647657 --- /dev/null +++ b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation.py @@ -0,0 +1,26 @@ +"""Test if pylint sees names inside string literal type annotations. #3299""" +# pylint: disable=too-few-public-methods + +from argparse import ArgumentParser, Namespace +import os +from os import PathLike +from pathlib import Path +from typing import NoReturn, Set + +# unused-import shouldn't be emitted for Path +example1: Set["Path"] = set() + +def example2(_: "ArgumentParser") -> "NoReturn": + """unused-import shouldn't be emitted for ArgumentParser or NoReturn.""" + while True: + pass + +def example3(_: "os.PathLike[str]") -> None: + """unused-import shouldn't be emitted for os.""" + +def example4(_: "PathLike[str]") -> None: + """unused-import shouldn't be emitted for PathLike.""" + +class Class: + """unused-import shouldn't be emitted for Namespace""" + cls: "Namespace" diff --git a/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py310.py b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py310.py new file mode 100644 index 000000000..00bf5799f --- /dev/null +++ b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py310.py @@ -0,0 +1,9 @@ +# pylint: disable=missing-docstring + +from typing import TypeAlias + +def unused_variable_should_not_be_emitted(): + """unused-variable shouldn't be emitted for Example.""" + Example: TypeAlias = int + result: set["Example"] = set() + return result diff --git a/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py310.rc b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py310.rc new file mode 100644 index 000000000..68a8c8ef1 --- /dev/null +++ b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py310.rc @@ -0,0 +1,2 @@ +[testoptions] +min_pyver=3.10 diff --git a/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py38.py b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py38.py new file mode 100644 index 000000000..497f64937 --- /dev/null +++ b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py38.py @@ -0,0 +1,20 @@ +# pylint: disable=missing-docstring + +from argparse import ArgumentParser # [unused-import] +from argparse import Namespace # [unused-import] +from typing import Literal as Lit +import typing as t + +# str inside Literal shouldn't be treated as names +example1: t.Literal["ArgumentParser", Lit["Namespace", "ArgumentParser"]] + + +def unused_variable_example(): + hello = "hello" # [unused-variable] + world = "world" # [unused-variable] + example2: Lit["hello", "world"] = "hello" + return example2 + + +# pylint shouldn't crash with the following strings in a type annotation context +example3: Lit["", " ", "?"] = "?" diff --git a/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py38.rc b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py38.rc new file mode 100644 index 000000000..85fc502b3 --- /dev/null +++ b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py38.rc @@ -0,0 +1,2 @@ +[testoptions] +min_pyver=3.8 diff --git a/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py38.txt b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py38.txt new file mode 100644 index 000000000..082595bff --- /dev/null +++ b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py38.txt @@ -0,0 +1,4 @@ +unused-import:3:0:3:35::Unused ArgumentParser imported from argparse:UNDEFINED +unused-import:4:0:4:30::Unused Namespace imported from argparse:UNDEFINED +unused-variable:13:4:13:9:unused_variable_example:Unused variable 'hello':UNDEFINED +unused-variable:14:4:14:9:unused_variable_example:Unused variable 'world':UNDEFINED diff --git a/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py39.py b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py39.py new file mode 100644 index 000000000..1258844cd --- /dev/null +++ b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py39.py @@ -0,0 +1,11 @@ +# pylint: disable=missing-docstring + +import graphlib +from graphlib import TopologicalSorter + +def example( + sorter1: "graphlib.TopologicalSorter[int]", + sorter2: "TopologicalSorter[str]", +) -> None: + """unused-import shouldn't be emitted for graphlib or TopologicalSorter.""" + print(sorter1, sorter2) diff --git a/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py39.rc b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py39.rc new file mode 100644 index 000000000..16b75eea7 --- /dev/null +++ b/tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py39.rc @@ -0,0 +1,2 @@ +[testoptions] +min_pyver=3.9 |