Skip to content

Commit

Permalink
Fix #3299 false positives with names in string literal type annotatio…
Browse files Browse the repository at this point in the history
…ns (#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
  • Loading branch information
lggruspe committed Sep 4, 2022
1 parent 8973766 commit 2f766a1
Show file tree
Hide file tree
Showing 12 changed files with 167 additions and 1 deletion.
3 changes: 3 additions & 0 deletions doc/whatsnew/fragments/3299.false_positive
Original file line number Diff line number Diff line change
@@ -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
22 changes: 22 additions & 0 deletions pylint/checkers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
41 changes: 40 additions & 1 deletion pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
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
Expand Down Expand Up @@ -2341,6 +2341,10 @@ def _check_is_unused(
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:
Expand Down Expand Up @@ -2904,6 +2908,41 @@ def _check_potential_index_error(
)
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))
26 changes: 26 additions & 0 deletions tests/checkers/unittest_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -489,3 +489,29 @@ def visit_assname(self, node: nodes.NodeNG) -> 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])
Original file line number Diff line number Diff line change
@@ -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"
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[testoptions]
min_pyver=3.10
Original file line number Diff line number Diff line change
@@ -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["", " ", "?"] = "?"
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[testoptions]
min_pyver=3.8
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[testoptions]
min_pyver=3.9

0 comments on commit 2f766a1

Please sign in to comment.