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

[possibly-used-before-assignment] Avoid FP for typing.NoReturn #9714

Merged
merged 1 commit into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ You can use ``assert_never`` to mark exhaustive choices:
if suffix in "dmy":
handle_date_suffix(suffix)

Or, instead of `assert_never()`, you can call a function with a return
annotation of `Never` or `NoReturn`. Unlike in the general case, where
by design pylint ignores type annotations and does its own static analysis,
here, pylint treats these special annotations like a disable comment.

Pylint currently allows repeating the same test like this, even though this
lets some error cases through, as pylint does not assess the intervening code:

Expand Down
5 changes: 5 additions & 0 deletions doc/whatsnew/fragments/9674.false_positive
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Prevent emitting ``possibly-used-before-assignment`` when relying on names
only potentially not defined in conditional blocks guarded by functions
annotated with ``typing.Never`` or ``typing.NoReturn``.

Closes #9674
25 changes: 24 additions & 1 deletion pylint/checkers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
from astroid.nodes._base_nodes import ImportNode, Statement
from astroid.typing import InferenceResult, SuccessfulInferenceResult

from pylint.constants import TYPING_NEVER, TYPING_NORETURN

if TYPE_CHECKING:
from functools import _lru_cache_wrapper

Expand Down Expand Up @@ -2150,7 +2152,9 @@ def is_singleton_const(node: nodes.NodeNG) -> bool:


def is_terminating_func(node: nodes.Call) -> bool:
"""Detect call to exit(), quit(), os._exit(), or sys.exit()."""
"""Detect call to exit(), quit(), os._exit(), sys.exit(), or
functions annotated with `typing.NoReturn` or `typing.Never`.
"""
if (
not isinstance(node.func, nodes.Attribute)
and not (isinstance(node.func, nodes.Name))
Expand All @@ -2165,6 +2169,25 @@ def is_terminating_func(node: nodes.Call) -> bool:
and inferred.qname() in TERMINATING_FUNCS_QNAMES
):
return True
# Unwrap to get the actual function node object
if isinstance(inferred, astroid.BoundMethod) and isinstance(
inferred._proxied, astroid.UnboundMethod
):
inferred = inferred._proxied._proxied
if (
isinstance(inferred, nodes.FunctionDef)
and isinstance(inferred.returns, nodes.Name)
and (inferred_func := safe_infer(inferred.returns))
and hasattr(inferred_func, "qname")
and inferred_func.qname()
in (
*TYPING_NEVER,
*TYPING_NORETURN,
# In Python 3.7 - 3.8, NoReturn is alias of '_SpecialForm'
"typing._SpecialForm",
Copy link
Member Author

Choose a reason for hiding this comment

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

We'll be dropping support for 3.8 in the next minor release, so this inelegance doesn't bother me ATM.

)
):
return True
except (StopIteration, astroid.InferenceError):
pass

Expand Down
2 changes: 1 addition & 1 deletion pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -2630,7 +2630,7 @@ def _loopvar_name(self, node: astroid.Name) -> None:
else_stmt, (nodes.Return, nodes.Raise, nodes.Break, nodes.Continue)
):
return
# TODO: 4.0: Consider using RefactoringChecker._is_function_def_never_returning
# TODO: 4.0: Consider using utils.is_terminating_func
if isinstance(else_stmt, nodes.Expr) and isinstance(
else_stmt.value, nodes.Call
):
Expand Down
17 changes: 17 additions & 0 deletions tests/functional/u/used/used_before_assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# pylint: disable=consider-using-f-string, missing-function-docstring
import datetime
import sys
from typing import NoReturn

MSG = "hello %s" % MSG # [used-before-assignment]

Expand Down Expand Up @@ -205,3 +206,19 @@ def inner_if_continues_outer_if_has_no_other_statements():
else:
order = None
print(order)


class PlatformChecks:
"""https://github.com/pylint-dev/pylint/issues/9674"""
def skip(self, msg) -> NoReturn:
raise Exception(msg) # pylint: disable=broad-exception-raised

def print_platform_specific_command(self):
if sys.platform == "linux":
cmd = "ls"
elif sys.platform == "win32":
cmd = "dir"
else:
self.skip("only runs on Linux/Windows")

print(cmd)
30 changes: 15 additions & 15 deletions tests/functional/u/used/used_before_assignment.txt
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
used-before-assignment:6:19:6:22::Using variable 'MSG' before assignment:HIGH
used-before-assignment:8:20:8:24::Using variable 'MSG2' before assignment:HIGH
used-before-assignment:11:4:11:9:outer:Using variable 'inner' before assignment:HIGH
used-before-assignment:20:20:20:40:ClassWithProperty:Using variable 'redefine_time_import' before assignment:HIGH
used-before-assignment:24:0:24:9::Using variable 'calculate' before assignment:HIGH
used-before-assignment:32:10:32:14:redefine_time_import:Using variable 'time' before assignment:HIGH
used-before-assignment:46:3:46:7::Using variable 'VAR2' before assignment:INFERENCE
possibly-used-before-assignment:64:3:64:7::Possibly using variable 'VAR4' before assignment:INFERENCE
possibly-used-before-assignment:74:3:74:7::Possibly using variable 'VAR5' before assignment:INFERENCE
used-before-assignment:79:3:79:7::Using variable 'VAR6' before assignment:INFERENCE
used-before-assignment:114:6:114:11::Using variable 'VAR10' before assignment:INFERENCE
possibly-used-before-assignment:120:6:120:11::Possibly using variable 'VAR12' before assignment:CONTROL_FLOW
used-before-assignment:151:10:151:14::Using variable 'SALE' before assignment:INFERENCE
used-before-assignment:183:10:183:18::Using variable 'ALL_DONE' before assignment:INFERENCE
used-before-assignment:194:6:194:24::Using variable 'NOT_ALWAYS_DEFINED' before assignment:INFERENCE
used-before-assignment:7:19:7:22::Using variable 'MSG' before assignment:HIGH
used-before-assignment:9:20:9:24::Using variable 'MSG2' before assignment:HIGH
used-before-assignment:12:4:12:9:outer:Using variable 'inner' before assignment:HIGH
used-before-assignment:21:20:21:40:ClassWithProperty:Using variable 'redefine_time_import' before assignment:HIGH
used-before-assignment:25:0:25:9::Using variable 'calculate' before assignment:HIGH
used-before-assignment:33:10:33:14:redefine_time_import:Using variable 'time' before assignment:HIGH
used-before-assignment:47:3:47:7::Using variable 'VAR2' before assignment:INFERENCE
possibly-used-before-assignment:65:3:65:7::Possibly using variable 'VAR4' before assignment:INFERENCE
possibly-used-before-assignment:75:3:75:7::Possibly using variable 'VAR5' before assignment:INFERENCE
used-before-assignment:80:3:80:7::Using variable 'VAR6' before assignment:INFERENCE
used-before-assignment:115:6:115:11::Using variable 'VAR10' before assignment:INFERENCE
possibly-used-before-assignment:121:6:121:11::Possibly using variable 'VAR12' before assignment:CONTROL_FLOW
used-before-assignment:152:10:152:14::Using variable 'SALE' before assignment:INFERENCE
used-before-assignment:184:10:184:18::Using variable 'ALL_DONE' before assignment:INFERENCE
used-before-assignment:195:6:195:24::Using variable 'NOT_ALWAYS_DEFINED' before assignment:INFERENCE
Loading