-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add future=True to all NodeNG.statement() calls
#5310
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
Changes from all commits
0bd10b0
d0bef5a
3ca5398
673a74a
6e17580
fe4b5ab
0d18355
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,6 +72,7 @@ | |
| from typing import Any, Callable, Iterator, List, Optional, Pattern, Tuple | ||
|
|
||
| import astroid | ||
| import astroid.exceptions | ||
| from astroid import bases, nodes | ||
|
|
||
| from pylint.checkers import BaseChecker, utils | ||
|
|
@@ -615,7 +616,7 @@ def _has_parent_of_type(node, node_type, statement): | |
|
|
||
|
|
||
| def _no_context_variadic_keywords(node, scope): | ||
| statement = node.statement() | ||
| statement = node.statement(future=True) | ||
| variadics = () | ||
|
|
||
| if isinstance(scope, nodes.Lambda) and not isinstance(scope, nodes.FunctionDef): | ||
|
|
@@ -649,7 +650,7 @@ def _no_context_variadic(node, variadic_name, variadic_type, variadics): | |
| is_in_lambda_scope = not isinstance(scope, nodes.FunctionDef) and isinstance( | ||
| scope, nodes.Lambda | ||
| ) | ||
| statement = node.statement() | ||
| statement = node.statement(future=True) | ||
| for name in statement.nodes_of_class(nodes.Name): | ||
| if name.name != variadic_name: | ||
| continue | ||
|
|
@@ -667,7 +668,7 @@ def _no_context_variadic(node, variadic_name, variadic_type, variadics): | |
| # so we need to go the lambda instead | ||
| inferred_statement = inferred.parent.parent | ||
| else: | ||
| inferred_statement = inferred.statement() | ||
| inferred_statement = inferred.statement(future=True) | ||
|
|
||
| if not length and isinstance(inferred_statement, nodes.Lambda): | ||
| is_in_starred_context = _has_parent_of_type(node, variadic_type, statement) | ||
|
|
@@ -1029,10 +1030,12 @@ def visit_attribute(self, node: nodes.Attribute) -> None: | |
| if not [ | ||
| n | ||
| for n in owner.getattr(node.attrname) | ||
| if not isinstance(n.statement(), nodes.AugAssign) | ||
| if not isinstance(n.statement(future=True), nodes.AugAssign) | ||
| ]: | ||
| missingattr.add((owner, name)) | ||
| continue | ||
| except astroid.exceptions.StatementMissing: | ||
| continue | ||
| except AttributeError: | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is also catching another |
||
| continue | ||
| except astroid.DuplicateBasesError: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1110,7 +1110,7 @@ def visit_name(self, node: nodes.Name) -> None: | |
| It's important that all 'Name' nodes are visited, otherwise the | ||
| 'NamesConsumers' won't be correct. | ||
| """ | ||
| stmt = node.statement() | ||
| stmt = node.statement(future=True) | ||
| if stmt.fromlineno is None: | ||
| # name node from an astroid built from live code, skip | ||
| assert not stmt.root().file.endswith(".py") | ||
|
|
@@ -1261,7 +1261,7 @@ def _check_consumer( | |
| return (VariableVisitConsumerAction.CONSUME, found_nodes) | ||
|
|
||
| defnode = utils.assign_parent(found_nodes[0]) | ||
| defstmt = defnode.statement() | ||
| defstmt = defnode.statement(future=True) | ||
| defframe = defstmt.frame() | ||
|
|
||
| # The class reuses itself in the class scope. | ||
|
|
@@ -1515,7 +1515,10 @@ def _allow_global_unused_variables(self): | |
| @staticmethod | ||
| def _defined_in_function_definition(node, frame): | ||
| in_annotation_or_default_or_decorator = False | ||
| if isinstance(frame, nodes.FunctionDef) and node.statement() is frame: | ||
| if ( | ||
| isinstance(frame, nodes.FunctionDef) | ||
| and node.statement(future=True) is frame | ||
| ): | ||
| in_annotation_or_default_or_decorator = ( | ||
| ( | ||
| node in frame.args.annotations | ||
|
|
@@ -1563,8 +1566,8 @@ def _in_lambda_or_comprehension_body( | |
| def _is_variable_violation( | ||
|
cdce8p marked this conversation as resolved.
|
||
| node: nodes.Name, | ||
| defnode, | ||
| stmt, | ||
| defstmt, | ||
| stmt: nodes.Statement, | ||
| defstmt: nodes.Statement, | ||
| frame, # scope of statement of node | ||
| defframe, | ||
| base_scope_type, | ||
|
|
@@ -1753,12 +1756,8 @@ def _is_variable_violation( | |
|
|
||
| return maybe_before_assign, annotation_return, use_outer_definition | ||
|
|
||
| # pylint: disable-next=fixme | ||
| # TODO: The typing of `NodeNG.statement()` in astroid is non-specific | ||
| # After this has been updated the typing of `defstmt` should reflect this | ||
| # See: https://github.com/PyCQA/astroid/pull/1217 | ||
| @staticmethod | ||
| def _is_only_type_assignment(node: nodes.Name, defstmt: nodes.NodeNG) -> bool: | ||
| 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 | ||
|
|
@@ -1866,7 +1865,7 @@ def _ignore_class_scope(self, node): | |
| # ... | ||
|
|
||
| name = node.name | ||
| frame = node.statement().scope() | ||
| frame = node.statement(future=True).scope() | ||
| in_annotation_or_default_or_decorator = self._defined_in_function_definition( | ||
| node, frame | ||
| ) | ||
|
|
@@ -1890,29 +1889,36 @@ def _loopvar_name(self, node: astroid.Name) -> None: | |
| # the variable is not defined. | ||
| scope = node.scope() | ||
| if isinstance(scope, nodes.FunctionDef) and any( | ||
| asmt.statement().parent_of(scope) for asmt in astmts | ||
| asmt.scope().parent_of(scope) for asmt in astmts | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We want to check if the scope of |
||
| ): | ||
| return | ||
|
|
||
| # filter variables according their respective scope test is_statement | ||
| # and parent to avoid #74747. This is not a total fix, which would | ||
| # Filter variables according to their respective scope. Test parent | ||
| # and statement to avoid #74747. This is not a total fix, which would | ||
| # introduce a mechanism similar to special attribute lookup in | ||
| # modules. Also, in order to get correct inference in this case, the | ||
| # scope lookup rules would need to be changed to return the initial | ||
| # assignment (which does not exist in code per se) as well as any later | ||
| # modifications. | ||
| # pylint: disable-next=too-many-boolean-expressions | ||
|
Pierre-Sassoulas marked this conversation as resolved.
|
||
| if ( | ||
| not astmts | ||
| or (astmts[0].is_statement or astmts[0].parent) | ||
| and astmts[0].statement().parent_of(node) | ||
| or ( | ||
| astmts[0].parent == astmts[0].root() | ||
| and astmts[0].parent.parent_of(node) | ||
| ) | ||
| or ( | ||
| astmts[0].is_statement | ||
| or not isinstance(astmts[0].parent, nodes.Module) | ||
| and astmts[0].statement(future=True).parent_of(node) | ||
| ) | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was particularly trigger to get right, but this is testing what we want. If
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tbh I think you know more about this particular method than I do. It looks like it could work 😅 |
||
| ): | ||
| _astmts = [] | ||
| else: | ||
| _astmts = astmts[:1] | ||
| for i, stmt in enumerate(astmts[1:]): | ||
| if astmts[i].statement().parent_of(stmt) and not in_for_else_branch( | ||
| astmts[i].statement(), stmt | ||
| ): | ||
| if astmts[i].statement(future=True).parent_of( | ||
| stmt | ||
| ) and not in_for_else_branch(astmts[i].statement(future=True), stmt): | ||
| continue | ||
| _astmts.append(stmt) | ||
| astmts = _astmts | ||
|
|
@@ -1922,7 +1928,7 @@ def _loopvar_name(self, node: astroid.Name) -> None: | |
| assign = astmts[0].assign_type() | ||
| if not ( | ||
| isinstance(assign, (nodes.For, nodes.Comprehension, nodes.GeneratorExp)) | ||
| and assign.statement() is not node.statement() | ||
| and assign.statement(future=True) is not node.statement(future=True) | ||
| ): | ||
| return | ||
|
|
||
|
|
@@ -2136,7 +2142,8 @@ def _check_late_binding_closure(self, node: nodes.Name) -> None: | |
| maybe_for | ||
| and maybe_for.parent_of(node_scope) | ||
| and not utils.is_being_called(node_scope) | ||
| and not isinstance(node_scope.statement(), nodes.Return) | ||
| and node_scope.parent | ||
| and not isinstance(node_scope.statement(future=True), nodes.Return) | ||
| ): | ||
| self.add_message("cell-var-from-loop", node=node, args=node.name) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| """Test to see we don't crash on this code in pandas. | ||
| See: https://github.com/pandas-dev/pandas/blob/master/pandas/core/arrays/sparse/array.py | ||
| Code written by Guido van Rossum here: https://github.com/python/typing/issues/684""" | ||
| # pylint: disable=no-member, redefined-builtin, invalid-name, missing-class-docstring | ||
|
|
||
| from typing import TYPE_CHECKING | ||
|
|
||
| if TYPE_CHECKING: | ||
| from enum import Enum | ||
|
|
||
| class ellipsis(Enum): | ||
| Ellipsis = "..." | ||
|
|
||
| Ellipsis = ellipsis.Ellipsis | ||
|
|
||
|
|
||
| else: | ||
| ellipsis = type(Ellipsis) | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a regression test for code I found while working on this. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same effect as doing
node.parent.fromlinenobut avoids theStatementMissingexception from callingstatement.