diff options
Diffstat (limited to 'pylint/checkers/variables.py')
| -rw-r--r-- | pylint/checkers/variables.py | 380 |
1 files changed, 219 insertions, 161 deletions
diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py index 8cd1ed83c..2504c0c84 100644 --- a/pylint/checkers/variables.py +++ b/pylint/checkers/variables.py @@ -1,6 +1,6 @@ # Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html -# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE -# Copyright (c) https://github.com/PyCQA/pylint/blob/main/CONTRIBUTORS.txt +# For details: https://github.com/pylint-dev/pylint/blob/main/LICENSE +# Copyright (c) https://github.com/pylint-dev/pylint/blob/main/CONTRIBUTORS.txt """Variables checkers for Python code.""" @@ -11,11 +11,10 @@ import copy import itertools import os import re -import sys from collections import defaultdict from collections.abc import Generator, Iterable, Iterator from enum import Enum -from functools import lru_cache +from functools import cached_property from typing import TYPE_CHECKING, Any, NamedTuple import astroid @@ -26,18 +25,15 @@ from astroid.typing import InferenceResult from pylint.checkers import BaseChecker, utils from pylint.checkers.utils import ( in_type_checking_block, + is_module_ignored, is_postponed_evaluation_enabled, is_sys_guard, + overridden_method, ) from pylint.constants import PY39_PLUS, TYPING_NEVER, TYPING_NORETURN from pylint.interfaces import CONTROL_FLOW, HIGH, INFERENCE, INFERENCE_FAILURE from pylint.typing import MessageDefinitionTuple -if sys.version_info >= (3, 8): - from functools import cached_property -else: - from astroid.decorators import cachedproperty as cached_property - if TYPE_CHECKING: from pylint.lint import PyLinter @@ -151,26 +147,6 @@ def _is_from_future_import(stmt: nodes.ImportFrom, name: str) -> bool | None: return None -@lru_cache(maxsize=1000) -def overridden_method( - klass: nodes.LocalsDictNodeNG, name: str | None -) -> nodes.FunctionDef | None: - """Get overridden method if any.""" - try: - parent = next(klass.local_attr_ancestors(name)) - except (StopIteration, KeyError): - return None - try: - meth_node = parent[name] - except KeyError: - # We have found an ancestor defining <name> but it's not in the local - # dictionary. This may happen with astroid built from living objects. - return None - if isinstance(meth_node, nodes.FunctionDef): - return meth_node - return None - - def _get_unpacking_extra_info(node: nodes.Assign, inferred: InferenceResult) -> str: """Return extra information to add to the message for unpacking-non-sequence and unbalanced-tuple/dict-unpacking errors. @@ -242,17 +218,11 @@ def _detect_global_scope( return node.lineno < defframe.lineno # type: ignore[no-any-return] if not isinstance(node.parent, (nodes.FunctionDef, nodes.Arguments)): return False - elif any( - not isinstance(f, (nodes.ClassDef, nodes.Module)) for f in (frame, defframe) - ): - # Not interested in other frames, since they are already - # not in a global scope. - return False break_scopes = [] - for current_scope in (scope, def_scope): + for current_scope in (scope or frame, def_scope): # Look for parent scopes. If there is anything different - # than a module or a class scope, then they frames don't + # than a module or a class scope, then the frames don't # share a global scope. parent_scope = current_scope while parent_scope: @@ -263,7 +233,7 @@ def _detect_global_scope( parent_scope = parent_scope.parent.scope() else: break - if break_scopes and len(set(break_scopes)) != 1: + if len(set(break_scopes)) > 1: # Store different scopes than expected. # If the stored scopes are, in fact, the very same, then it means # that the two frames (frame and defframe) share the same scope, @@ -564,7 +534,6 @@ class NamesConsumer: copy.copy(node.locals), {}, collections.defaultdict(list), scope_type ) self.node = node - self._if_nodes_deemed_uncertain: set[nodes.If] = set() def __repr__(self) -> str: _to_consumes = [f"{k}->{v}" for k, v in self._atomic.to_consume.items()] @@ -715,26 +684,28 @@ scope_type : {self._atomic.scope_type} return found_nodes @staticmethod - def _exhaustively_define_name_raise_or_return( - name: str, node: nodes.NodeNG - ) -> bool: - """Return True if there is a collectively exhaustive set of paths under - this `if_node` that define `name`, raise, or return. + def _inferred_to_define_name_raise_or_return(name: str, node: nodes.NodeNG) -> bool: + """Return True if there is a path under this `if_node` + that is inferred to define `name`, raise, or return. """ # Handle try and with if isinstance(node, (nodes.TryExcept, nodes.TryFinally)): # Allow either a path through try/else/finally OR a path through ALL except handlers - return ( - NamesConsumer._defines_name_raises_or_returns_recursive(name, node) - or isinstance(node, nodes.TryExcept) - and all( - NamesConsumer._defines_name_raises_or_returns_recursive( - name, handler - ) - for handler in node.handlers + try_except_node = node + if isinstance(node, nodes.TryFinally): + try_except_node = next( + (child for child in node.nodes_of_class(nodes.TryExcept)), + None, ) + handlers = try_except_node.handlers if try_except_node else [] + return NamesConsumer._defines_name_raises_or_returns_recursive( + name, node + ) or all( + NamesConsumer._defines_name_raises_or_returns_recursive(name, handler) + for handler in handlers ) - if isinstance(node, nodes.With): + + if isinstance(node, (nodes.With, nodes.For, nodes.While)): return NamesConsumer._defines_name_raises_or_returns_recursive(name, node) if not isinstance(node, nodes.If): @@ -748,13 +719,29 @@ scope_type : {self._atomic.scope_type} if NamesConsumer._defines_name_raises_or_returns(name, node): return True - # If there is no else, then there is no collectively exhaustive set of paths - if not node.orelse: - return False + test = node.test.value if isinstance(node.test, nodes.NamedExpr) else node.test + all_inferred = utils.infer_all(test) + only_search_if = False + only_search_else = True + for inferred in all_inferred: + if not isinstance(inferred, nodes.Const): + only_search_else = False + continue + val = inferred.value + only_search_if = only_search_if or (val != NotImplemented and val) + only_search_else = only_search_else and not val + + # Only search else branch when test condition is inferred to be false + if all_inferred and only_search_else: + return NamesConsumer._branch_handles_name(name, node.orelse) + # Only search if branch when test condition is inferred to be true + if all_inferred and only_search_if: + return NamesConsumer._branch_handles_name(name, node.body) + # Search both if and else branches return NamesConsumer._branch_handles_name( name, node.body - ) and NamesConsumer._branch_handles_name(name, node.orelse) + ) or NamesConsumer._branch_handles_name(name, node.orelse) @staticmethod def _branch_handles_name(name: str, body: Iterable[nodes.NodeNG]) -> bool: @@ -762,9 +749,16 @@ scope_type : {self._atomic.scope_type} NamesConsumer._defines_name_raises_or_returns(name, if_body_stmt) or isinstance( if_body_stmt, - (nodes.If, nodes.TryExcept, nodes.TryFinally, nodes.With), + ( + nodes.If, + nodes.TryExcept, + nodes.TryFinally, + nodes.With, + nodes.For, + nodes.While, + ), ) - and NamesConsumer._exhaustively_define_name_raise_or_return( + and NamesConsumer._inferred_to_define_name_raise_or_return( name, if_body_stmt ) for if_body_stmt in body @@ -776,57 +770,80 @@ scope_type : {self._atomic.scope_type} """Identify nodes of uncertain execution because they are defined under tests that evaluate false. - Don't identify a node if there is a collectively exhaustive set of paths - that define the name, raise, or return (e.g. every if/else branch). + Don't identify a node if there is a path that is inferred to + define the name, raise, or return (e.g. any executed if/elif/else branch). """ uncertain_nodes = [] for other_node in found_nodes: - if in_type_checking_block(other_node): - continue - - if not isinstance(other_node, nodes.AssignName): + if isinstance(other_node, nodes.AssignName): + name = other_node.name + elif isinstance(other_node, (nodes.Import, nodes.ImportFrom)): + name = node.name + else: continue - closest_if = utils.get_node_first_ancestor_of_type(other_node, nodes.If) - if closest_if is None: - continue - if node.frame() is not closest_if.frame(): - continue - if closest_if is not None and closest_if.parent_of(node): + all_if = [ + n + for n in other_node.node_ancestors() + if isinstance(n, nodes.If) and not n.parent_of(node) + ] + if not all_if: continue - # Name defined in every if/else branch - if NamesConsumer._exhaustively_define_name_raise_or_return( - other_node.name, closest_if + closest_if = all_if[0] + if ( + isinstance(node, nodes.AssignName) + and node.frame() is not closest_if.frame() ): continue + if closest_if.parent_of(node): + continue - # Higher-level if already determined to be always false - if any( - if_node.parent_of(closest_if) - for if_node in self._if_nodes_deemed_uncertain - ): - uncertain_nodes.append(other_node) + outer_if = all_if[-1] + if NamesConsumer._node_guarded_by_same_test(node, outer_if): continue - # All inferred values must test false - if isinstance(closest_if.test, nodes.NamedExpr): - test = closest_if.test.value - else: - test = closest_if.test - all_inferred = utils.infer_all(test) - if not all_inferred or not all( - isinstance(inferred, nodes.Const) and not inferred.value - for inferred in all_inferred - ): + # Name defined in the if/else control flow + if NamesConsumer._inferred_to_define_name_raise_or_return(name, outer_if): continue uncertain_nodes.append(other_node) - self._if_nodes_deemed_uncertain.add(closest_if) return uncertain_nodes @staticmethod + def _node_guarded_by_same_test(node: nodes.NodeNG, other_if: nodes.If) -> bool: + """Identify if `node` is guarded by an equivalent test as `other_if`. + + Two tests are equivalent if their string representations are identical + or if their inferred values consist only of constants and those constants + are identical, and the if test guarding `node` is not a Name. + """ + other_if_test_as_string = other_if.test.as_string() + other_if_test_all_inferred = utils.infer_all(other_if.test) + for ancestor in node.node_ancestors(): + if not isinstance(ancestor, nodes.If): + continue + if ancestor.test.as_string() == other_if_test_as_string: + return True + if isinstance(ancestor.test, nodes.Name): + continue + all_inferred = utils.infer_all(ancestor.test) + if len(all_inferred) == len(other_if_test_all_inferred): + if any( + not isinstance(test, nodes.Const) + for test in (*all_inferred, *other_if_test_all_inferred) + ): + continue + if {test.value for test in all_inferred} != { + test.value for test in other_if_test_all_inferred + }: + continue + return True + + return False + + @staticmethod def _uncertain_nodes_in_except_blocks( found_nodes: list[nodes.NodeNG], node: nodes.NodeNG, @@ -921,6 +938,18 @@ scope_type : {self._atomic.scope_type} for child_named_expr in node.nodes_of_class(nodes.NamedExpr) ): return True + if isinstance(node, (nodes.Import, nodes.ImportFrom)) and any( + (node_name[1] and node_name[1] == name) or (node_name[0] == name) + for node_name in node.names + ): + return True + if isinstance(node, nodes.With) and any( + isinstance(item[1], nodes.AssignName) and item[1].name == name + for item in node.items + ): + return True + if isinstance(node, (nodes.ClassDef, nodes.FunctionDef)) and node.name == name: + return True return False @staticmethod @@ -939,18 +968,23 @@ scope_type : {self._atomic.scope_type} for nested_stmt in stmt.get_children() ): return True + if isinstance( + stmt, nodes.TryExcept + ) and NamesConsumer._defines_name_raises_or_returns_recursive(name, stmt): + return True return False @staticmethod def _check_loop_finishes_via_except( node: nodes.NodeNG, other_node_try_except: nodes.TryExcept ) -> bool: - """Check for a case described in https://github.com/PyCQA/pylint/issues/5683. + """Check for a specific control flow scenario. - It consists of a specific control flow scenario where the only - non-break exit from a loop consists of the very except handler we are - examining, such that code in the `else` branch of the loop can depend on it - being assigned. + Described in https://github.com/pylint-dev/pylint/issues/5683. + + A scenario where the only non-break exit from a loop consists of the very + except handler we are examining, such that code in the `else` branch of + the loop can depend on it being assigned. Example: @@ -1247,6 +1281,9 @@ class VariablesChecker(BaseChecker): tuple[nodes.ExceptHandler, nodes.AssignName] ] = [] """This is a queue, last in first out.""" + self._evaluated_type_checking_scopes: dict[ + str, list[nodes.LocalsDictNodeNG] + ] = {} self._postponed_evaluation_enabled = False @utils.only_required_for_messages( @@ -1707,21 +1744,16 @@ class VariablesChecker(BaseChecker): if found_nodes is None: return (VariableVisitConsumerAction.CONTINUE, None) if not found_nodes: - if node.name in current_consumer.consumed_uncertain: - confidence = CONTROL_FLOW - else: - confidence = HIGH - self.add_message( - "used-before-assignment", - args=node.name, - node=node, - confidence=confidence, - ) + self._report_unfound_name_definition(node, current_consumer) # Mark for consumption any nodes added to consumed_uncertain by # get_next_to_consume() because they might not have executed. + nodes_to_consume = current_consumer.consumed_uncertain[node.name] + nodes_to_consume = self._filter_type_checking_import_from_consumption( + node, nodes_to_consume + ) return ( VariableVisitConsumerAction.RETURN, - current_consumer.consumed_uncertain[node.name], + nodes_to_consume, ) self._check_late_binding_closure(node) @@ -1858,7 +1890,9 @@ class VariablesChecker(BaseChecker): confidence=HIGH, ) - elif self._is_only_type_assignment(node, defstmt): + elif not self._is_builtin(node.name) and self._is_only_type_assignment( + node, defstmt + ): if node.scope().locals.get(node.name): self.add_message( "used-before-assignment", args=node.name, node=node, confidence=HIGH @@ -1885,6 +1919,61 @@ class VariablesChecker(BaseChecker): return (VariableVisitConsumerAction.RETURN, found_nodes) + def _report_unfound_name_definition( + self, node: nodes.NodeNG, current_consumer: NamesConsumer + ) -> None: + """Reports used-before-assignment when all name definition nodes + get filtered out by NamesConsumer. + """ + if ( + self._postponed_evaluation_enabled + and utils.is_node_in_type_annotation_context(node) + ): + return + if self._is_builtin(node.name): + return + if self._is_variable_annotation_in_function(node): + return + if ( + node.name in self._evaluated_type_checking_scopes + and node.scope() in self._evaluated_type_checking_scopes[node.name] + ): + return + + confidence = ( + CONTROL_FLOW if node.name in current_consumer.consumed_uncertain else HIGH + ) + self.add_message( + "used-before-assignment", + args=node.name, + node=node, + confidence=confidence, + ) + + def _filter_type_checking_import_from_consumption( + self, node: nodes.NodeNG, nodes_to_consume: list[nodes.NodeNG] + ) -> list[nodes.NodeNG]: + """Do not consume type-checking import node as used-before-assignment + may invoke in different scopes. + """ + type_checking_import = next( + ( + n + for n in nodes_to_consume + if isinstance(n, (nodes.Import, nodes.ImportFrom)) + and in_type_checking_block(n) + ), + None, + ) + # If used-before-assignment reported for usage of type checking import + # keep track of its scope + if type_checking_import and not self._is_variable_annotation_in_function(node): + self._evaluated_type_checking_scopes.setdefault(node.name, []).append( + node.scope() + ) + nodes_to_consume = [n for n in nodes_to_consume if n != type_checking_import] + return nodes_to_consume + @utils.only_required_for_messages("no-name-in-module") def visit_import(self, node: nodes.Import) -> None: """Check modules attribute accesses.""" @@ -2049,7 +2138,6 @@ class VariablesChecker(BaseChecker): parent = parent.parent return False - # pylint: disable = too-many-branches @staticmethod def _is_variable_violation( node: nodes.Name, @@ -2203,42 +2291,6 @@ class VariablesChecker(BaseChecker): anc is defnode.value for anc in node.node_ancestors() ) - # Look for type checking definitions inside a type checking guard. - # Relevant for function annotations only, not variable annotations (AnnAssign) - if ( - isinstance(defstmt, (nodes.Import, nodes.ImportFrom)) - and isinstance(defstmt.parent, nodes.If) - and in_type_checking_block(defstmt) - and not in_type_checking_block(node) - ): - defstmt_parent = defstmt.parent - - maybe_annotation = utils.get_node_first_ancestor_of_type( - node, nodes.AnnAssign - ) - if not ( - maybe_annotation - and utils.get_node_first_ancestor_of_type( - maybe_annotation, nodes.FunctionDef - ) - ): - # Exempt those definitions that are used inside the type checking - # guard or that are defined in any elif/else type checking guard branches. - used_in_branch = defstmt_parent.parent_of(node) - if not used_in_branch: - if defstmt_parent.has_elif_block(): - defined_in_or_else = utils.is_defined( - node.name, defstmt_parent.orelse[0] - ) - else: - defined_in_or_else = any( - utils.is_defined(node.name, content) - for content in defstmt_parent.orelse - ) - - if not defined_in_or_else: - maybe_before_assign = True - return maybe_before_assign, annotation_return, use_outer_definition @staticmethod @@ -2254,7 +2306,7 @@ class VariablesChecker(BaseChecker): return any( VariablesChecker._maybe_used_and_assigned_at_once(elt) for elt in defstmt.value.elts - if isinstance(elt, NODES_WITH_VALUE_ATTR + (nodes.IfExp, nodes.Match)) + if isinstance(elt, (*NODES_WITH_VALUE_ATTR, nodes.IfExp, nodes.Match)) ) value = defstmt.value if isinstance(value, nodes.IfExp): @@ -2278,18 +2330,15 @@ class VariablesChecker(BaseChecker): for call in value.nodes_of_class(klass=nodes.Call) ) - def _is_only_type_assignment( - self, node: nodes.Name, defstmt: nodes.Statement - ) -> bool: + def _is_builtin(self, name: str) -> bool: + return name in self.linter.config.additional_builtins or utils.is_builtin(name) + + @staticmethod + def _is_only_type_assignment(node: nodes.Name, defstmt: nodes.Statement) -> bool: """Check if variable only gets assigned a type and never a value.""" if not isinstance(defstmt, nodes.AnnAssign) or defstmt.value: return False - if node.name in self.linter.config.additional_builtins or utils.is_builtin( - node.name - ): - return False - defstmt_frame = defstmt.frame(future=True) node_frame = node.frame(future=True) @@ -2378,6 +2427,16 @@ class VariablesChecker(BaseChecker): return True return False + @staticmethod + def _is_variable_annotation_in_function(node: nodes.NodeNG) -> bool: + is_annotation = utils.get_node_first_ancestor_of_type(node, nodes.AnnAssign) + return ( + is_annotation + and utils.get_node_first_ancestor_of_type( # type: ignore[return-value] + is_annotation, nodes.FunctionDef + ) + ) + def _ignore_class_scope(self, node: nodes.NodeNG) -> bool: """Return True if the node is in a local class scope, as an assignment. @@ -2427,10 +2486,7 @@ class VariablesChecker(BaseChecker): # the usage is safe because the function will not be defined either if # the variable is not defined. scope = node.scope() - # FunctionDef subclasses Lambda due to a curious ontology. Check both. - # See https://github.com/PyCQA/astroid/issues/291 - # TODO: Revisit when astroid 3.0 includes the change - if isinstance(scope, nodes.Lambda) and any( + if isinstance(scope, (nodes.Lambda, nodes.FunctionDef)) and any( asmt.scope().parent_of(scope) for asmt in astmts ): return @@ -2481,7 +2537,7 @@ class VariablesChecker(BaseChecker): else_stmt, (nodes.Return, nodes.Raise, nodes.Break, nodes.Continue) ): return - # TODO: 2.16: Consider using RefactoringChecker._is_function_def_never_returning + # TODO: 3.0: Consider using RefactoringChecker._is_function_def_never_returning if isinstance(else_stmt, nodes.Expr) and isinstance( else_stmt.value, nodes.Call ): @@ -2906,7 +2962,7 @@ class VariablesChecker(BaseChecker): @staticmethod def _nodes_to_unpack(node: nodes.NodeNG) -> list[nodes.NodeNG] | None: """Return the list of values of the `Assign` node.""" - if isinstance(node, (nodes.Tuple, nodes.List) + DICT_TYPES): + if isinstance(node, (nodes.Tuple, nodes.List, *DICT_TYPES)): return node.itered() # type: ignore[no-any-return] if isinstance(node, astroid.Instance) and any( ancestor.qname() == "typing.NamedTuple" for ancestor in node.ancestors() @@ -2961,7 +3017,9 @@ class VariablesChecker(BaseChecker): if not isinstance(module, nodes.Module): return None except astroid.NotFoundError: - if module.name in self._ignored_modules: + # Unable to import `name` from `module`. Since `name` may itself be a + # module, we first check if it matches the ignored modules. + if is_module_ignored(f"{module.qname()}.{name}", self._ignored_modules): return None self.add_message( "no-name-in-module", args=(name, module.name), node=node |