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 undefined-loop-variable with NoReturn and Never #7476

Merged
merged 11 commits into from Sep 19, 2022
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/7311.false_positive
@@ -0,0 +1,4 @@
Fix false positive for ``undefined-loop-variable`` in ``for-else`` loops that use a function
having a return type annotation of ``NoReturn`` or ``Never``.

Closes #7311
43 changes: 35 additions & 8 deletions pylint/checkers/variables.py
Expand Up @@ -19,7 +19,7 @@
from typing import TYPE_CHECKING, Any, NamedTuple

import astroid
from astroid import extract_node, nodes
from astroid import bases, extract_node, nodes
from astroid.nodes import _base_nodes
from astroid.typing import InferenceResult

Expand All @@ -28,7 +28,12 @@
in_type_checking_block,
is_postponed_evaluation_enabled,
)
from pylint.constants import PY39_PLUS, TYPING_TYPE_CHECKS_GUARDS
from pylint.constants import (
PY39_PLUS,
TYPING_NEVER,
TYPING_NORETURN,
TYPING_TYPE_CHECKS_GUARDS,
)
from pylint.interfaces import CONTROL_FLOW, HIGH, INFERENCE, INFERENCE_FAILURE
from pylint.typing import MessageDefinitionTuple

Expand Down Expand Up @@ -2254,13 +2259,35 @@ def _loopvar_name(self, node: astroid.Name) -> None:
if not isinstance(assign, nodes.For):
self.add_message("undefined-loop-variable", args=node.name, node=node)
return
if any(
isinstance(
for else_stmt in assign.orelse:
if isinstance(
else_stmt, (nodes.Return, nodes.Raise, nodes.Break, nodes.Continue)
)
for else_stmt in assign.orelse
):
return
):
return
# TODO: 2.16: Consider using RefactoringChecker._is_function_def_never_returning
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function doesn't actually infer but uses the attrname, which we also match not_typing.we_do_return.NoReturn. That isn't really safe..

if isinstance(else_stmt, nodes.Expr) and isinstance(
else_stmt.value, nodes.Call
):
inferred_func = utils.safe_infer(else_stmt.value.func)
if (
isinstance(inferred_func, nodes.FunctionDef)
and inferred_func.returns
):
inferred_return = utils.safe_infer(inferred_func.returns)
if isinstance(
inferred_return, nodes.FunctionDef
) and inferred_return.qname() in {
*TYPING_NORETURN,
*TYPING_NEVER,
"typing._SpecialForm",
}:
return
# typing_extensions.NoReturn returns a _SpecialForm
if (
isinstance(inferred_return, bases.Instance)
and inferred_return.qname() == "typing._SpecialForm"
):
return

maybe_walrus = utils.get_node_first_ancestor_of_type(node, nodes.NamedExpr)
if maybe_walrus:
Expand Down
13 changes: 13 additions & 0 deletions pylint/constants.py
Expand Up @@ -155,3 +155,16 @@ def _get_pylint_home() -> str:


PYLINT_HOME = _get_pylint_home()

TYPING_NORETURN = frozenset(
(
"typing.NoReturn",
"typing_extensions.NoReturn",
)
)
TYPING_NEVER = frozenset(
(
"typing.Never",
"typing_extensions.Never",
)
)
7 changes: 1 addition & 6 deletions pylint/extensions/typing.py
Expand Up @@ -17,6 +17,7 @@
only_required_for_messages,
safe_infer,
)
from pylint.constants import TYPING_NORETURN
from pylint.interfaces import INFERENCE

if TYPE_CHECKING:
Expand Down Expand Up @@ -75,12 +76,6 @@ class TypingAlias(NamedTuple):

ALIAS_NAMES = frozenset(key.split(".")[1] for key in DEPRECATED_TYPING_ALIASES)
UNION_NAMES = ("Optional", "Union")
TYPING_NORETURN = frozenset(
(
"typing.NoReturn",
"typing_extensions.NoReturn",
)
)


class DeprecatedTypingAliasMsg(NamedTuple):
Expand Down
20 changes: 20 additions & 0 deletions tests/functional/u/undefined/undefined_loop_variable.py
@@ -1,5 +1,13 @@
# pylint: disable=missing-docstring,redefined-builtin, consider-using-f-string, unnecessary-direct-lambda-call

import sys

if sys.version_info >= (3, 8):
from typing import NoReturn
else:
from typing_extensions import NoReturn


def do_stuff(some_random_list):
for var in some_random_list:
pass
Expand Down Expand Up @@ -125,6 +133,18 @@ def for_else_continue(iterable):
print(thing)


def for_else_no_return(iterable):
def fail() -> NoReturn:
...

while True:
for thing in iterable:
break
else:
fail()
print(thing)


lst = []
lst2 = [1, 2, 3]

Expand Down
8 changes: 4 additions & 4 deletions tests/functional/u/undefined/undefined_loop_variable.txt
@@ -1,4 +1,4 @@
undefined-loop-variable:6:11:6:14:do_stuff:Using possibly undefined loop variable 'var':UNDEFINED
undefined-loop-variable:25:7:25:11::Using possibly undefined loop variable 'var1':UNDEFINED
undefined-loop-variable:75:11:75:14:do_stuff_with_redefined_range:Using possibly undefined loop variable 'var':UNDEFINED
undefined-loop-variable:181:11:181:20:find_even_number:Using possibly undefined loop variable 'something':UNDEFINED
undefined-loop-variable:14:11:14:14:do_stuff:Using possibly undefined loop variable 'var':UNDEFINED
undefined-loop-variable:33:7:33:11::Using possibly undefined loop variable 'var1':UNDEFINED
undefined-loop-variable:83:11:83:14:do_stuff_with_redefined_range:Using possibly undefined loop variable 'var':UNDEFINED
undefined-loop-variable:201:11:201:20:find_even_number:Using possibly undefined loop variable 'something':UNDEFINED
17 changes: 17 additions & 0 deletions tests/functional/u/undefined/undefined_loop_variable_py311.py
@@ -0,0 +1,17 @@
"""Tests for undefined-loop-variable using Python 3.11 syntax."""

from typing import Never


def for_else_never(iterable):
"""Test for-else with Never type."""

def idontreturn() -> Never:
"""This function never returns."""

while True:
for thing in iterable:
break
else:
idontreturn()
print(thing)
2 changes: 2 additions & 0 deletions tests/functional/u/undefined/undefined_loop_variable_py311.rc
@@ -0,0 +1,2 @@
[testoptions]
min_pyver=3.11