From c41c35a8e0dd71ba96944613e6a2ced830407670 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 21 Jun 2024 16:42:44 +0200 Subject: [PATCH] [possibly-used-before-assignment] Avoid FP for typing.NoReturn & Never (#9714) (#9742) Co-authored-by: Jacob Walls --- .../details.rst | 27 ++++++++++++++++- doc/whatsnew/fragments/9674.false_positive | 5 ++++ pylint/checkers/utils.py | 25 +++++++++++++++- pylint/checkers/variables.py | 2 +- .../u/used/used_before_assignment.py | 17 +++++++++++ .../u/used/used_before_assignment.txt | 30 +++++++++---------- 6 files changed, 88 insertions(+), 18 deletions(-) create mode 100644 doc/whatsnew/fragments/9674.false_positive diff --git a/doc/data/messages/p/possibly-used-before-assignment/details.rst b/doc/data/messages/p/possibly-used-before-assignment/details.rst index 4737d26685..5f87081a5c 100644 --- a/doc/data/messages/p/possibly-used-before-assignment/details.rst +++ b/doc/data/messages/p/possibly-used-before-assignment/details.rst @@ -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 diff --git a/doc/whatsnew/fragments/9674.false_positive b/doc/whatsnew/fragments/9674.false_positive new file mode 100644 index 0000000000..d1e3fb6814 --- /dev/null +++ b/doc/whatsnew/fragments/9674.false_positive @@ -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 diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py index a3e6496519..7f7df0eb12 100644 --- a/pylint/checkers/utils.py +++ b/pylint/checkers/utils.py @@ -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 @@ -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)) @@ -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 diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 8e8b0e0bfb..bfc80697c4 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -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 ): diff --git a/tests/functional/u/used/used_before_assignment.py b/tests/functional/u/used/used_before_assignment.py index c706af043f..37da03d7e5 100644 --- a/tests/functional/u/used/used_before_assignment.py +++ b/tests/functional/u/used/used_before_assignment.py @@ -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] @@ -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) diff --git a/tests/functional/u/used/used_before_assignment.txt b/tests/functional/u/used/used_before_assignment.txt index 6f97f4324c..eacf082399 100644 --- a/tests/functional/u/used/used_before_assignment.txt +++ b/tests/functional/u/used/used_before_assignment.txt @@ -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