Skip to content

Commit

Permalink
[possibly-used-before-assignment] Avoid FP for typing.NoReturn & Never (
Browse files Browse the repository at this point in the history
#9714) (#9742)

Co-authored-by: Jacob Walls <[email protected]>
  • Loading branch information
Pierre-Sassoulas and jacobtylerwalls committed Jun 21, 2024
1 parent 22e4d36 commit c41c35a
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 18 deletions.
27 changes: 26 additions & 1 deletion doc/data/messages/p/possibly-used-before-assignment/details.rst
Original file line number Diff line number Diff line change
@@ -1,4 +1,29 @@
If you rely on a pattern like:
You can use ``assert_never`` to mark exhaustive choices:

.. sourcecode:: python

from typing import assert_never

def handle_date_suffix(suffix):
if suffix == "d":
...
elif suffix == "m":
...
elif suffix == "y":
...
else:
assert_never(suffix)

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:

.. sourcecode:: python

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",
)
):
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

0 comments on commit c41c35a

Please sign in to comment.