Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #3299 false positives with names in string literal type annotations #7400

Merged
merged 12 commits into from
Sep 4, 2022
Merged
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
19 changes: 19 additions & 0 deletions pylint/checkers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1891,6 +1891,25 @@ 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):
import_from = node.lookup(node.name)[1][0]
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
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
42 changes: 41 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,42 @@ 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.
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
"""
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 whitespace
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))
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
"""Test if pylint sees names inside string literal type annotations. #3299"""
from argparse import ArgumentParser, Namespace
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
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."""

# pylint: disable=too-few-public-methods
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
class Class:
"""unused-import shouldn't be emitted for Namespace"""
cls: "Namespace"
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# pylint: disable=missing-docstring
from typing import TypeAlias
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved

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,19 @@
# pylint: disable=missing-docstring
from argparse import ArgumentParser # [unused-import]
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
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:2:0:2:35::Unused ArgumentParser imported from argparse:UNDEFINED
unused-import:3:0:3:30::Unused Namespace imported from argparse:UNDEFINED
unused-variable:12:4:12:9:unused_variable_example:Unused variable 'hello':UNDEFINED
unused-variable:13:4:13:9:unused_variable_example:Unused variable 'world':UNDEFINED
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# pylint: disable=missing-docstring
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved
import graphlib
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
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