From e4f1b8b6c85895ff980917f9857f780cd24357af Mon Sep 17 00:00:00 2001 From: Levi Gruspe Date: Mon, 5 Sep 2022 23:43:58 +0800 Subject: [PATCH 1/5] Disambiguate between str and enum member args to typing.Literal --- pylint/checkers/variables.py | 24 ++++++++++--------- ..._in_string_literal_type_annotation_py38.py | 9 ++++++- ...in_string_literal_type_annotation_py38.txt | 5 ++-- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 0644b4d07d..317569d306 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -2919,16 +2919,6 @@ def visit_const(self, node: nodes.Const) -> None: 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 @@ -2940,7 +2930,19 @@ def visit_const(self, node: nodes.Const) -> None: if origin is not None and utils.is_typing_literal(origin): return - self._type_annotation_names.append(node.value) + if node.value.isidentifier(): + self._type_annotation_names.append(node.value) + return + + try: + annotation = extract_node(node.value) + self._store_type_annotation_node(annotation) + except ValueError: + # e.g. node.value is white space + pass + except astroid.AstroidSyntaxError: + # e.g. "?" or ":" in typing.Literal["?", ":"] + pass def register(linter: PyLinter) -> None: 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 index 497f64937b..edf715f85e 100644 --- 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 @@ -2,8 +2,10 @@ from argparse import ArgumentParser # [unused-import] from argparse import Namespace # [unused-import] -from typing import Literal as Lit +import http #[unused-import] +from http import HTTPStatus import typing as t +from typing import Literal as Lit # str inside Literal shouldn't be treated as names example1: t.Literal["ArgumentParser", Lit["Namespace", "ArgumentParser"]] @@ -18,3 +20,8 @@ def unused_variable_example(): # pylint shouldn't crash with the following strings in a type annotation context example3: Lit["", " ", "?"] = "?" + + +# See https://peps.python.org/pep-0586/#literals-enums-and-forward-references +example4: t.Literal["http.HTTPStatus.OK", "http.HTTPStatus.NOT_FOUND"] +example5: "t.Literal[HTTPStatus.OK, HTTPStatus.NOT_FOUND]" 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 index 082595bfff..6f8c709bf2 100644 --- 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 @@ -1,4 +1,5 @@ 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 +unused-import:5:0:5:11::Unused import http:UNDEFINED +unused-variable:15:4:15:9:unused_variable_example:Unused variable 'hello':UNDEFINED +unused-variable:16:4:16:9:unused_variable_example:Unused variable 'world':UNDEFINED From b6a0577295c2662fc758f48c5e6097691f3bf899 Mon Sep 17 00:00:00 2001 From: Levi Gruspe Date: Tue, 6 Sep 2022 10:10:37 +0800 Subject: [PATCH 2/5] Increase test coverage --- .../unused/unused_name_in_string_literal_type_annotation.py | 5 +++++ 1 file changed, 5 insertions(+) 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 index 389647657b..400e7725e0 100644 --- 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 @@ -21,6 +21,11 @@ def example3(_: "os.PathLike[str]") -> None: def example4(_: "PathLike[str]") -> None: """unused-import shouldn't be emitted for PathLike.""" +# pylint shouldn't crash with the following strings in a type annotation context +example5: Set[""] +example6: Set[" "] +example7: Set["?"] + class Class: """unused-import shouldn't be emitted for Namespace""" cls: "Namespace" From c857038d0b58763153459d0ddacaac6fadad301c Mon Sep 17 00:00:00 2001 From: Levi Gruspe Date: Wed, 7 Sep 2022 20:03:56 +0800 Subject: [PATCH 3/5] Update tests/functional/u/unused/unused_name_in_string_literal_type_annotation_py38.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com> --- .../unused_name_in_string_literal_type_annotation_py38.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index edf715f85e..96658ae369 100644 --- 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 @@ -2,7 +2,7 @@ from argparse import ArgumentParser # [unused-import] from argparse import Namespace # [unused-import] -import http #[unused-import] +import http # [unused-import] from http import HTTPStatus import typing as t from typing import Literal as Lit From c95baf35267e35fb79422e8cbd59c389d502cd8e Mon Sep 17 00:00:00 2001 From: Levi Gruspe Date: Wed, 7 Sep 2022 20:23:45 +0800 Subject: [PATCH 4/5] Remove redundant branch --- pylint/checkers/variables.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 317569d306..9476642e99 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -2924,16 +2924,11 @@ def visit_const(self, node: nodes.Const) -> None: 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 - if node.value.isidentifier(): - self._type_annotation_names.append(node.value) - return - try: annotation = extract_node(node.value) self._store_type_annotation_node(annotation) From 24fc87f05af38d295f25d0cd7ca21cd57a3cc1cd Mon Sep 17 00:00:00 2001 From: Levi Gruspe Date: Sat, 10 Sep 2022 18:37:36 +0800 Subject: [PATCH 5/5] Create news fragment --- doc/whatsnew/fragments/3299.bugfix | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 doc/whatsnew/fragments/3299.bugfix diff --git a/doc/whatsnew/fragments/3299.bugfix b/doc/whatsnew/fragments/3299.bugfix new file mode 100644 index 0000000000..dd45d19789 --- /dev/null +++ b/doc/whatsnew/fragments/3299.bugfix @@ -0,0 +1,4 @@ +Fix bug in scanning of names inside arguments to `typing.Literal`. +See https://peps.python.org/pep-0586/#literals-enums-and-forward-references for details. + +Refs #3299