From 45ab768bf571e0186100f147e002d08151fe19dd Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Mon, 23 Feb 2026 07:58:34 -0800 Subject: [PATCH] DeCo. Pass LocalScope to ScopeContext.withLocalScope Change ScopeContext.withLocalScope to pass the created LocalScope to the callback, and add LocalScope helpers for bulk insertion of declared elements. Refactor resolution/scoping visitors to use the provided LocalScope rather than downcasting nameScope or relying on ad-hoc predeclaration: - Predeclare local functions, local variables, and pattern variables in the correct enclosing scope before visiting initializers/bodies, so lexical lookup binds to loop/local elements consistently (including in implicit block scopes for control-flow sub-statements). - Centralize for-loop-parts handling (declarations, identifiers, patterns) so each part is visited in the required order while installing the right loop-local bindings. - Compute and store declared pattern-variable elements directly on the relevant AST nodes, and update ForEachPartsWithPattern to carry variable elements (so flow analysis can declare them without extra indirection). Also ensure metadata on pattern variable declarations and pattern-for parts is visited in the same scope model as other declarations. Change-Id: I35d52860b6816196085d79979cda861583c850be Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/482540 Reviewed-by: Johnni Winther --- pkg/analyzer/lib/src/dart/ast/ast.dart | 2 +- pkg/analyzer/lib/src/dart/element/scope.dart | 6 + .../dart/resolver/flow_analysis_visitor.dart | 2 +- .../lib/src/dart/resolver/for_resolver.dart | 1 + .../src/dart/resolver/resolution_visitor.dart | 304 ++++++++------ .../lib/src/dart/resolver/scope_context.dart | 13 +- pkg/analyzer/lib/src/generated/resolver.dart | 210 +++++----- .../dart/resolution/for_statement_test.dart | 372 ++++++++++++++++-- .../dart/resolution/local_function_test.dart | 28 ++ .../dart/resolution/local_variable_test.dart | 17 + .../resolution/pattern_assignment_test.dart | 82 ++++ ...n_variable_declaration_statement_test.dart | 73 ++++ 12 files changed, 832 insertions(+), 278 deletions(-) diff --git a/pkg/analyzer/lib/src/dart/ast/ast.dart b/pkg/analyzer/lib/src/dart/ast/ast.dart index 8829496aef6..d76ded58d72 100644 --- a/pkg/analyzer/lib/src/dart/ast/ast.dart +++ b/pkg/analyzer/lib/src/dart/ast/ast.dart @@ -12115,7 +12115,7 @@ final class ForEachPartsWithPatternImpl extends ForEachPartsImpl DartPatternImpl _pattern; /// Variables declared in [pattern]. - late final List variables; + late final List variables; @generated ForEachPartsWithPatternImpl({ diff --git a/pkg/analyzer/lib/src/dart/element/scope.dart b/pkg/analyzer/lib/src/dart/element/scope.dart index 9bbebadd886..88f6e9789e4 100644 --- a/pkg/analyzer/lib/src/dart/element/scope.dart +++ b/pkg/analyzer/lib/src/dart/element/scope.dart @@ -539,6 +539,12 @@ class LocalScope extends EnclosedScope { _addGetter(element); } } + + void addAll(Iterable elements) { + for (var element in elements) { + add(element); + } + } } class PrefixScope implements Scope { diff --git a/pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart b/pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart index b6361926d05..5a3859edf12 100644 --- a/pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart +++ b/pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart @@ -1286,7 +1286,7 @@ class _AssignedVariablesVisitor extends RecursiveAstVisitor { assignedVariables.declare(variable); } else if (forLoopParts is ForEachPartsWithPatternImpl) { for (var variable in forLoopParts.variables) { - assignedVariables.declare(variable.element); + assignedVariables.declare(variable); } } else { throw StateError('Unrecognized for loop parts'); diff --git a/pkg/analyzer/lib/src/dart/resolver/for_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/for_resolver.dart index de6052e0680..e586d385389 100644 --- a/pkg/analyzer/lib/src/dart/resolver/for_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/for_resolver.dart @@ -76,6 +76,7 @@ class ForResolver { required ForEachPartsWithPatternImpl forLoopParts, required void Function() dispatchBody, }) { + forLoopParts.metadata.accept(_resolver); _resolver.analyzePatternForIn( node: node, hasAwait: awaitKeyword != null, diff --git a/pkg/analyzer/lib/src/dart/resolver/resolution_visitor.dart b/pkg/analyzer/lib/src/dart/resolver/resolution_visitor.dart index 9828aa6a5ea..2266b958a09 100644 --- a/pkg/analyzer/lib/src/dart/resolver/resolution_visitor.dart +++ b/pkg/analyzer/lib/src/dart/resolver/resolution_visitor.dart @@ -171,12 +171,8 @@ class ResolutionVisitor extends RecursiveAstVisitor { } @override - void visitBlock(Block node) { - _scopeContext.withLocalScope(() { - var statements = node.statements; - _buildLocalElements(statements); - statements.accept(this); - }); + void visitBlock(covariant BlockImpl node) { + _visitStatementsInScope(node.statements); } @override @@ -184,11 +180,11 @@ class ResolutionVisitor extends RecursiveAstVisitor { var exceptionTypeNode = node.exceptionType; exceptionTypeNode?.accept(this); - _scopeContext.withLocalScope(() { + _scopeContext.withLocalScope((scope) { var exceptionNode = node.exceptionParameter; if (exceptionNode != null) { var fragment = exceptionNode.declaredFragment!; - _define(fragment.element); + scope.add(fragment.element); if (exceptionTypeNode == null) { fragment.element.type = _typeProvider.objectType; @@ -200,7 +196,7 @@ class ResolutionVisitor extends RecursiveAstVisitor { var stackTraceNode = node.stackTraceParameter; if (stackTraceNode != null) { var fragment = stackTraceNode.declaredFragment!; - _define(fragment.element); + scope.add(fragment.element); fragment.element.type = _typeProvider.stackTraceType; } @@ -312,7 +308,6 @@ class ResolutionVisitor extends RecursiveAstVisitor { var element = fragment.element; _patternVariables.add(node.name.lexeme, element); - _define(element); node.type?.accept(this); @@ -343,6 +338,12 @@ class ResolutionVisitor extends RecursiveAstVisitor { } } + @override + void visitDoStatement(covariant DoStatementImpl node) { + _visitStatementInScope(node.body); + node.condition.accept(this); + } + @override void visitEnumConstantDeclaration( covariant EnumConstantDeclarationImpl node, @@ -452,36 +453,36 @@ class ResolutionVisitor extends RecursiveAstVisitor { void visitForEachPartsWithDeclaration( covariant ForEachPartsWithDeclarationImpl node, ) { - node.iterable.accept(this); - node.loopVariable.accept(this); - var fragment = node.loopVariable.declaredFragment!; - _define(fragment.element); + throw StateError('Should not be invoked'); } @override void visitForEachPartsWithPattern( covariant ForEachPartsWithPatternImpl node, ) { - _patternVariables.casePatternStart(); - super.visitForEachPartsWithPattern(node); - var variablesMap = _patternVariables.casePatternFinish(); - node.variables = variablesMap.values - .whereType() - .map((e) => e.firstFragment) - .toList(); + throw StateError('Should not be invoked'); } @override void visitForElement(covariant ForElementImpl node) { - _scopeContext.withLocalScope(() { - super.visitForElement(node); + _scopeContext.withLocalScope((scope) { + _visitForLoopParts(scope, node.forLoopParts); + _scopeContext.withLocalScope((_) { + node.body.accept(this); + }); }); } + @override + void visitForPartsWithPattern(covariant ForPartsWithPatternImpl node) { + throw StateError('Should not be invoked'); + } + @override void visitForStatement(covariant ForStatementImpl node) { - _scopeContext.withLocalScope(() { - super.visitForStatement(node); + _scopeContext.withLocalScope((scope) { + _visitForLoopParts(scope, node.forLoopParts); + _visitStatementInScope(node.body); }); } @@ -501,16 +502,6 @@ class ResolutionVisitor extends RecursiveAstVisitor { } } - @override - void visitFunctionDeclarationStatement( - covariant FunctionDeclarationStatementImpl node, - ) { - if (!_hasLocalElementsBuilt(node)) { - _defineLocalFunction(node); - } - super.visitFunctionDeclarationStatement(node); - } - @override void visitFunctionExpression(covariant FunctionExpressionImpl node) { var fragment = node.declaredFragment; @@ -617,12 +608,36 @@ class ResolutionVisitor extends RecursiveAstVisitor { @override void visitIfElement(covariant IfElementImpl node) { - _visitIf(node); + if (node.caseClause case var caseClause?) { + node.expression.accept(this); + _resolveGuardedPattern( + caseClause.guardedPattern, + then: () { + node.ifTrue.accept(this); + }, + ); + node.ifFalse?.accept(this); + } else { + node.visitChildren(this); + } } @override void visitIfStatement(covariant IfStatementImpl node) { - _visitIf(node); + if (node.caseClause case var caseClause?) { + node.expression.accept(this); + _resolveGuardedPattern( + caseClause.guardedPattern, + then: () { + _visitStatementInScope(node.ifTrue); + }, + ); + _visitStatementInScope(node.ifFalse); + } else { + node.expression.accept(this); + _visitStatementInScope(node.ifTrue); + _visitStatementInScope(node.ifFalse); + } } @override @@ -677,8 +692,8 @@ class ResolutionVisitor extends RecursiveAstVisitor { var outerScope = _labelScope; try { var unlabeled = node.unlabeled; - for (Label label in node.labels) { - SimpleIdentifier labelNameNode = label.label; + for (var label in node.labels) { + var labelNameNode = label.label; _labelScope = LabelScope( _labelScope, labelNameNode.name, @@ -793,26 +808,27 @@ class ResolutionVisitor extends RecursiveAstVisitor { } @override - void visitPatternAssignment(PatternAssignment node) { - // We need to call `casePatternStart` and `casePatternFinish` in case there - // are any declared variable patterns inside the pattern assignment (this - // could happen due to error recovery). But we don't need to keep the - // variables map that `casePatternFinish` returns. - _patternVariables.casePatternStart(); - super.visitPatternAssignment(node); - _patternVariables.casePatternFinish(); + void visitPatternAssignment(covariant PatternAssignmentImpl node) { + _scopeContext.withLocalScope((scope) { + var variables = _computeDeclaredPatternVariables(node.pattern); + scope.addAll(variables); + node.expression.accept(this); + }); } @override void visitPatternVariableDeclaration( covariant PatternVariableDeclarationImpl node, ) { - _patternVariables.casePatternStart(); - super.visitPatternVariableDeclaration(node); - var variablesMap = _patternVariables.casePatternFinish(); - node.elements = variablesMap.values - .whereType() - .toList(); + node.metadata.accept(this); + node.expression.accept(this); + } + + @override + void visitPatternVariableDeclarationStatement( + covariant PatternVariableDeclarationStatementImpl node, + ) { + node.declaration.accept(this); } @override @@ -968,11 +984,7 @@ class ResolutionVisitor extends RecursiveAstVisitor { group.variables = _patternVariables.switchStatementSharedCaseScopeFinish( group, ); - _scopeContext.withLocalScope(() { - var statements = group.statements; - _buildLocalElements(statements); - statements.accept(this); - }); + _visitStatementsInScope(group.statements); } } @@ -1001,13 +1013,6 @@ class ResolutionVisitor extends RecursiveAstVisitor { void visitVariableDeclarationList( covariant VariableDeclarationListImpl node, ) { - var parent = node.parent; - if (parent is ForPartsWithDeclarations || - parent is VariableDeclarationStatement && - !_hasLocalElementsBuilt(parent)) { - _defineLocalVariables(node); - } - node.visitChildren(this); var variables = node.variables; @@ -1028,36 +1033,57 @@ class ResolutionVisitor extends RecursiveAstVisitor { } } - void _buildLocalElements(List statements) { + @override + void visitWhileStatement(covariant WhileStatementImpl node) { + node.condition.accept(this); + _visitStatementInScope(node.body); + } + + List _computeDeclaredPatternVariables( + DartPatternImpl pattern, + ) { + var variables = _computePatternVariables(pattern); + return variables.values + .whereType() + .toList(); + } + + Map _computePatternVariables( + DartPatternImpl pattern, { + Object? sharedCaseScopeKey, + }) { + _patternVariables.casePatternStart(); + pattern.accept(this); + return _patternVariables.casePatternFinish( + sharedCaseScopeKey: sharedCaseScopeKey, + ); + } + + void _defineLocalElements(LocalScope scope, List statements) { for (var statement in statements) { - if (statement is FunctionDeclarationStatementImpl) { - _defineLocalFunction(statement); - } else if (statement is VariableDeclarationStatement) { - _defineLocalVariables(statement.variables); + statement = statement.unlabeled; + switch (statement) { + case FunctionDeclarationStatementImpl(): + var declaration = statement.functionDeclaration; + var element = declaration.declaredFragment!.element; + scope.add(element); + case PatternVariableDeclarationStatementImpl(): + var declaration = statement.declaration; + _definePatternVariableDeclarationElements(scope, declaration); + case VariableDeclarationStatementImpl(): + scope.addAll(statement.variables.declaredElements); } } } - void _define(Element element) { - if (nameScope case LocalScope nameScope) { - nameScope.add(element); - } - } - - void _defineLocalFunction(FunctionDeclarationStatementImpl statement) { - var fragment = statement.functionDeclaration.declaredFragment; - if (fragment != null && !_isWildCardVariable(fragment.name)) { - _define(fragment.element); - } - } - - void _defineLocalVariables(VariableDeclarationList variables) { - for (var variable in variables.variables) { - var fragment = variable.declaredFragment; - if (fragment != null) { - _define(fragment.element); - } - } + void _definePatternVariableDeclarationElements( + LocalScope scope, + PatternVariableDeclarationImpl declaration, + ) { + var pattern = declaration.pattern; + var variables = _computeDeclaredPatternVariables(pattern); + declaration.elements = variables; + scope.addAll(variables); } NullabilitySuffix _getNullability(bool hasQuestion) { @@ -1068,25 +1094,18 @@ class ResolutionVisitor extends RecursiveAstVisitor { } } - bool _isWildCardVariable(String? name) => - name == '_' && - _libraryElement.featureSet.isEnabled(Feature.wildcard_variables); - void _resolveGuardedPattern( GuardedPatternImpl guardedPattern, { Object? sharedCaseScopeKey, void Function()? then, }) { - _patternVariables.casePatternStart(); - guardedPattern.pattern.accept(this); - var variables = _patternVariables.casePatternFinish( + var variables = _computePatternVariables( + guardedPattern.pattern, sharedCaseScopeKey: sharedCaseScopeKey, ); // Matched variables are available in `whenClause`. - _scopeContext.withLocalScope(() { - for (var variable in variables.values) { - _define(variable); - } + _scopeContext.withLocalScope((scope) { + scope.addAll(variables.values); guardedPattern.variables = variables; guardedPattern.whenClause?.accept(this); if (then != null) { @@ -1283,22 +1302,66 @@ class ResolutionVisitor extends RecursiveAstVisitor { ); } - void _visitIf(IfElementOrStatementImpl node) { - var caseClause = node.caseClause; - if (caseClause != null) { - node.expression.accept(this); - _resolveGuardedPattern( - caseClause.guardedPattern, - then: () { - node.ifTrue.accept(this); - }, - ); - node.ifFalse?.accept(this); - } else { - node.visitChildren(this); + void _visitForLoopParts(LocalScope scope, ForLoopPartsImpl node) { + switch (node) { + case ForEachPartsWithDeclarationImpl(): + node.iterable.accept(this); + scope.add(node.loopVariable.declaredFragment!.element); + node.loopVariable.accept(this); + case ForEachPartsWithIdentifierImpl(): + node.iterable.accept(this); + node.identifier.accept(this); + case ForEachPartsWithPatternImpl(): + node.iterable.accept(this); + var variables = _computeDeclaredPatternVariables(node.pattern); + node.variables = variables; + scope.addAll(variables); + node.metadata.accept(this); + case ForPartsWithDeclarationsImpl(): + scope.addAll(node.variables.declaredElements); + node.variables.accept(this); + node.condition?.accept(this); + node.updaters.accept(this); + case ForPartsWithExpressionImpl(): + node.initialization?.accept(this); + node.condition?.accept(this); + node.updaters.accept(this); + case ForPartsWithPatternImpl(): + _definePatternVariableDeclarationElements(scope, node.variables); + node.variables.accept(this); + node.condition?.accept(this); + node.updaters.accept(this); } } + /// Visits [statement], ensuring that if it is a block it is visited as such, + /// and if it is not, it is wrapped in an implicit block scope. + /// + /// This implements the requirement from the specification that sub-statements + /// of control flow statements (like `if`, `while`, `do`, `for`) introduce + /// a new scope, even if they are not explicitly blocks. + void _visitStatementInScope(StatementImpl? statement) { + if (statement != null) { + if (statement is BlockImpl) { + visitBlock(statement); + } else { + _scopeContext.withLocalScope((scope) { + _defineLocalElements(scope, [statement]); + statement.accept(this); + }); + } + } + } + + void _visitStatementsInScope(List statements) { + _scopeContext.withLocalScope((scope) { + _defineLocalElements(scope, statements); + for (var statement in statements) { + statement.accept(this); + } + }); + } + void _withEnclosingInstanceElement( InstanceElement element, void Function() f, @@ -1315,10 +1378,6 @@ class ResolutionVisitor extends RecursiveAstVisitor { /// We always build local elements for [VariableDeclarationStatement]s and /// [FunctionDeclarationStatement]s in blocks, because invalid code might try /// to use forward references. - static bool _hasLocalElementsBuilt(Statement node) { - var parent = node.parent; - return parent is Block || parent is SwitchMember; - } /// Associate each of the annotation [nodes] with the corresponding /// [ElementAnnotation] in [annotations]. @@ -1429,3 +1488,12 @@ class _VariableBinderErrors ); } } + +extension _VariableDeclarationList on VariableDeclarationList { + List get declaredElements { + return variables + .map((v) => v.declaredFragment!.element) + .cast() + .toList(); + } +} diff --git a/pkg/analyzer/lib/src/dart/resolver/scope_context.dart b/pkg/analyzer/lib/src/dart/resolver/scope_context.dart index 940d3a8e746..707b84e1bac 100644 --- a/pkg/analyzer/lib/src/dart/resolver/scope_context.dart +++ b/pkg/analyzer/lib/src/dart/resolver/scope_context.dart @@ -97,8 +97,9 @@ class ScopeContext { } /// Run [operation] with a new [LocalScope]. - void withLocalScope(void Function() operation) { - withScope(LocalScope(nameScope), operation); + void withLocalScope(void Function(LocalScope scope) operation) { + var scope = LocalScope(nameScope); + withScope(scope, () => operation(scope)); } void withPrimaryParameterScope( @@ -156,3 +157,11 @@ extension on T { } } } + +extension LocalScopeExtension on LocalScope { + void addFormalParameterList(FormalParameterList node) { + for (var formalParameter in node.parameters) { + add(formalParameter.declaredFragment!.element); + } + } +} diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart index d341b9fe1c2..8ba55dc0ebc 100644 --- a/pkg/analyzer/lib/src/generated/resolver.dart +++ b/pkg/analyzer/lib/src/generated/resolver.dart @@ -88,7 +88,6 @@ import 'package:analyzer/src/generated/static_type_analyzer.dart'; import 'package:analyzer/src/generated/utilities_dart.dart'; import 'package:analyzer/src/generated/variable_type_provider.dart'; import 'package:analyzer/src/util/ast_data_extractor.dart'; -import 'package:analyzer/src/utilities/extensions/element.dart'; import 'package:analyzer/src/utilities/extensions/object.dart'; /// Function determining which source files should have inference logging @@ -3755,6 +3754,7 @@ class ResolverVisitor extends ThrowingAstVisitor void visitPatternVariableDeclaration( covariant PatternVariableDeclarationImpl node, ) { + node.metadata.accept(this); var patternSchema = analyzePatternVariableDeclaration( node, node.pattern, @@ -5068,12 +5068,10 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { void visitAnonymousMethodInvocation(AnonymousMethodInvocation node) { node.target?.accept(this); - _scopeContext.withLocalScope(() { + _scopeContext.withLocalScope((scope) { var parameters = node.parameters; if (parameters != null) { - for (var parameter in parameters.parameters) { - _define(parameter.declaredFragment!.element); - } + scope.addFormalParameterList(parameters); } node.parameters?.accept(this); node.body.accept(this); @@ -5090,7 +5088,7 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { @override void visitBlock(covariant BlockImpl node) { - _withDeclaredLocals(node, node.statements, () { + _withDeclaredLocals(node, node.statements, (_) { super.visitBlock(node); }); } @@ -5115,11 +5113,11 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { void visitCatchClause(CatchClause node) { var exception = node.exceptionParameter; if (exception != null) { - _scopeContext.withLocalScope(() { - _define(exception.declaredFragment!.element); + _scopeContext.withLocalScope((scope) { + scope.add(exception.declaredFragment!.element); var stackTrace = node.stackTraceParameter; if (stackTrace != null) { - _define(stackTrace.declaredFragment!.element); + scope.add(stackTrace.declaredFragment!.element); } super.visitCatchClause(node); }); @@ -5200,13 +5198,7 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { } @override - void visitDeclaredIdentifier(DeclaredIdentifier node) { - _define(node.declaredFragment!.element); - super.visitDeclaredIdentifier(node); - } - - @override - void visitDoStatement(DoStatement node) { + void visitDoStatement(covariant DoStatementImpl node) { ImplicitLabelScope outerImplicitScope = _implicitLabelScope; try { _implicitLabelScope = _implicitLabelScope.nest(node); @@ -5328,33 +5320,27 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { // We visit the iterator before the pattern because the pattern variables // cannot be in scope while visiting the iterator. node.iterable.accept(this); - - for (var variable in node.variables) { - _define(variable.asElement2); - } - + node.metadata.accept(this); node.pattern.accept(this); } @override void visitForElement(covariant ForElementImpl node) { - _scopeContext.withLocalScope(() { + _scopeContext.withLocalScope((scope) { node.nameScope = nameScope; - _predeclareForPartsVariables(node.forLoopParts); - node.forLoopParts.accept(this); + _visitForLoopParts(scope, node.forLoopParts); node.body.accept(this); }); } @override void visitForStatement(covariant ForStatementImpl node) { - _scopeContext.withLocalScope(() { + _scopeContext.withLocalScope((scope) { var outerImplicitScope = _implicitLabelScope; _implicitLabelScope = _implicitLabelScope.nest(node); try { node.nameScope = nameScope; - _predeclareForPartsVariables(node.forLoopParts); - node.forLoopParts.accept(this); + _visitForLoopParts(scope, node.forLoopParts); _visitStatementInScope(node.body); } finally { _implicitLabelScope = outerImplicitScope; @@ -5424,8 +5410,8 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { node.parameters.accept(this); // Visiting the parameters added them to the scope as a side effect. So it // is safe to visit the documentation comment now. - _scopeContext.withLocalScope(() { - (nameScope as LocalScope).addFormalParameters(node.parameters); + _scopeContext.withLocalScope((scope) { + scope.addFormalParameterList(node.parameters); _visitDocumentationComment(node.documentationComment); }); }); @@ -5469,10 +5455,8 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { _scopeContext.withTypeParameterList( functionTypeNode.typeParameters, () { - _scopeContext.withLocalScope(() { - (nameScope as LocalScope).addFormalParameters( - functionTypeNode.parameters, - ); + _scopeContext.withLocalScope((scope) { + scope.addFormalParameterList(functionTypeNode.parameters); _visitDocumentationComment(node.documentationComment); }); }, @@ -5486,10 +5470,6 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { @override void visitGuardedPattern(covariant GuardedPatternImpl node) { var patternVariables = node.variables.values.toList(); - for (var variable in patternVariables) { - _define(variable); - } - node.pattern.accept(this); for (var variable in patternVariables) { @@ -5593,17 +5573,6 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { node.typeArguments?.accept(this); } - @override - void visitPatternVariableDeclaration( - covariant PatternVariableDeclarationImpl node, - ) { - for (var variable in node.elements) { - _define(variable); - } - - super.visitPatternVariableDeclaration(node); - } - @override void visitPrefixedIdentifier(PrefixedIdentifier node) { // Do not visit the identifier after the `.`, since it is not meant to be @@ -5712,13 +5681,11 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { node.expression.accept(this); for (var case_ in node.cases) { - _withNameScope(() { + _scopeContext.withLocalScope((scope) { case_.nameScope = nameScope; var guardedPattern = case_.guardedPattern; var variables = guardedPattern.variables; - for (var variable in variables.values) { - _define(variable); - } + scope.addAll(variables.values); case_.accept(this); }); } @@ -5748,7 +5715,8 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { if (member is SwitchCaseImpl) { member.expression.accept(this); } else if (member is SwitchPatternCaseImpl) { - _withNameScope(() { + _scopeContext.withLocalScope((scope) { + scope.addAll(member.guardedPattern.variables.values); member.guardedPattern.accept(this); }); } @@ -5757,10 +5725,8 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { return; } var lastMember = group.members.last; - _withDeclaredLocals(lastMember, lastMember.statements, () { - for (var variable in group.variables.values) { - _define(variable); - } + _withDeclaredLocals(lastMember, lastMember.statements, (scope) { + scope.addAll(group.variables.values); lastMember.statements.accept(this); }); } @@ -5780,16 +5746,7 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { } @override - void visitVariableDeclaration(VariableDeclaration node) { - super.visitVariableDeclaration(node); - - if (node.parent!.parent is ForParts) { - _define(node.declaredFragment!.element); - } - } - - @override - void visitWhileStatement(WhileStatement node) { + void visitWhileStatement(covariant WhileStatementImpl node) { node.condition.accept(this); ImplicitLabelScope outerImplicitScope = _implicitLabelScope; try { @@ -5814,10 +5771,6 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { return outerScope; } - void _define(Element element) { - (nameScope as LocalScope).add(element); - } - /// Return the target of a break or continue statement, and update the static /// element of its label (if any). The [parentNode] is the AST node of the /// break or continue statement. The [labelNode] is the label contained in @@ -5874,20 +5827,6 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { } } - /// Predeclare `for`-parts variables so lexical lookup during traversal of - /// `forLoopParts` (initializer, condition, updaters) binds to loop-local - /// elements. - /// - /// This is only about binding. Reads that occur before the declaration point - /// are still reported later by error verification. - void _predeclareForPartsVariables(ForLoopParts forLoopParts) { - if (forLoopParts is ForPartsWithDeclarations) { - for (var variable in forLoopParts.variables.variables) { - _define(variable.declaredFragment!.element); - } - } - } - /// Visits a documentation comment with a [DocumentationCommentScope] that /// encloses the current [nameScope]. void _visitDocumentationComment(CommentImpl? node) { @@ -5898,25 +5837,71 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { }); } + void _visitForLoopParts(LocalScope scope, ForLoopPartsImpl parts) { + switch (parts) { + case ForEachPartsWithDeclarationImpl(): + parts.iterable.accept(this); + scope.add(parts.loopVariable.declaredFragment!.element); + parts.loopVariable.accept(this); + case ForEachPartsWithIdentifierImpl(): + parts.iterable.accept(this); + parts.identifier.accept(this); + case ForEachPartsWithPatternImpl(): + parts.iterable.accept(this); + scope.addAll(parts.variables); + parts.metadata.accept(this); + parts.pattern.accept(this); + case ForPartsWithDeclarationsImpl(): + scope.addAll( + parts.variables.variables.map( + (variable) => variable.declaredFragment!.element, + ), + ); + parts.variables.accept(this); + parts.condition?.accept(this); + parts.updaters.accept(this); + case ForPartsWithExpressionImpl(): + parts.initialization?.accept(this); + parts.condition?.accept(this); + parts.updaters.accept(this); + case ForPartsWithPatternImpl(): + scope.addAll(parts.variables.elements); + parts.variables.accept(this); + parts.condition?.accept(this); + parts.updaters.accept(this); + } + } + void _visitIf(IfElementOrStatementImpl node) { node.expression.accept(this); var caseClause = node.caseClause; if (caseClause != null) { var guardedPattern = caseClause.guardedPattern; - _withNameScope(() { + _scopeContext.withLocalScope((scope) { caseClause.nameScope = nameScope; var variables = guardedPattern.variables; - for (var variable in variables.values) { - _define(variable); - } + scope.addAll(variables.values); guardedPattern.accept(this); - node.ifTrue.accept(this); + if (node is IfStatementImpl) { + _visitStatementInScope(node.ifTrue); + } else { + node.ifTrue.accept(this); + } }); - node.ifFalse?.accept(this); + if (node is IfStatementImpl) { + _visitStatementInScope(node.ifFalse); + } else { + node.ifFalse?.accept(this); + } } else { - node.ifTrue.accept(this); - node.ifFalse?.accept(this); + if (node is IfStatementImpl) { + _visitStatementInScope(node.ifTrue); + _visitStatementInScope(node.ifFalse); + } else { + node.ifTrue.accept(this); + node.ifFalse?.accept(this); + } } } @@ -5924,25 +5909,25 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { /// /// This is used by [ResolverVisitor] to correctly visit the 'then' and 'else' /// statements of an 'if' statement. - void _visitStatementInScope(Statement? node) { - if (node is BlockImpl) { - // Don't create a scope around a block because the block will create it's - // own scope. - visitBlock(node); - } else if (node != null) { - _scopeContext.withLocalScope(() { - node.accept(this); - }); + void _visitStatementInScope(StatementImpl? node) { + if (node != null) { + if (node is BlockImpl) { + visitBlock(node); + } else { + _scopeContext.withLocalScope((scope) { + scope.addAll(BlockScope.elementsInStatements([node])); + node.accept(this); + }); + } } } void _withDeclaredLocals( AstNodeWithNameScopeMixin node, List statements, - void Function() f, + void Function(LocalScope scope) f, ) { - _scopeContext.withLocalScope(() { - var enclosedScope = nameScope as LocalScope; + _scopeContext.withLocalScope((enclosedScope) { for (var statement in BlockScope.elementsInStatements(statements)) { if (!statement.isWildcardFunction) { enclosedScope.add(statement); @@ -5951,15 +5936,10 @@ class ScopeResolverVisitor extends UnifyingAstVisitor { node.nameScope = enclosedScope; - f(); + f(enclosedScope); }); } - /// Run [f] with the new name scope. - void _withNameScope(void Function() f) { - _scopeContext.withLocalScope(f); - } - /// Return the [Scope] to use while resolving inside the [node]. /// /// Not every node has the scope set, for example we set the scopes for @@ -6271,11 +6251,3 @@ extension on Element { name == '_' && library.hasWildcardVariablesFeatureEnabled; } - -extension on LocalScope { - void addFormalParameters(FormalParameterList formalParameterList) { - for (var formalParameter in formalParameterList.parameters) { - add(formalParameter.declaredFragment!.element); - } - } -} diff --git a/pkg/analyzer/test/src/dart/resolution/for_statement_test.dart b/pkg/analyzer/test/src/dart/resolution/for_statement_test.dart index 7e4d6f34b1c..6e16158af0a 100644 --- a/pkg/analyzer/test/src/dart/resolution/for_statement_test.dart +++ b/pkg/analyzer/test/src/dart/resolution/for_statement_test.dart @@ -1627,6 +1627,118 @@ ForStatement '''); } + test_metadata() async { + await assertNoErrorsInCode(r''' +void f(List x) { + for (@foo var (a) in x) { + a; + } +} + +const foo = 0; +'''); + + var node = findNode.singleForStatement; + assertResolvedNodeText(node, r''' +ForStatement + forKeyword: for + leftParenthesis: ( + forLoopParts: ForEachPartsWithPattern + metadata + Annotation + atSign: @ + name: SimpleIdentifier + token: foo + element: ::@getter::foo + staticType: null + element: ::@getter::foo + keyword: var + pattern: ParenthesizedPattern + leftParenthesis: ( + pattern: DeclaredVariablePattern + name: a + declaredFragment: isPublic a@39 + element: hasImplicitType isPublic + type: int + matchedValueType: int + rightParenthesis: ) + matchedValueType: int + inKeyword: in + iterable: SimpleIdentifier + token: x + element: ::@function::f::@formalParameter::x + staticType: List + rightParenthesis: ) + body: Block + leftBracket: { + statements + ExpressionStatement + expression: SimpleIdentifier + token: a + element: a@39 + staticType: int + semicolon: ; + rightBracket: } +'''); + } + + test_metadata_shadowing() async { + await assertErrorsInCode( + r''' +const a = 42; +void f(List x) { + for (@a var (a) in x) { + a; + } +} +''', + [error(diag.invalidAnnotation, 43, 2)], + ); + + var node = findNode.singleForStatement; + assertResolvedNodeText(node, r''' +ForStatement + forKeyword: for + leftParenthesis: ( + forLoopParts: ForEachPartsWithPattern + metadata + Annotation + atSign: @ + name: SimpleIdentifier + token: a + element: a@51 + staticType: null + element: a@51 + keyword: var + pattern: ParenthesizedPattern + leftParenthesis: ( + pattern: DeclaredVariablePattern + name: a + declaredFragment: isPublic a@51 + element: hasImplicitType isPublic + type: int + matchedValueType: int + rightParenthesis: ) + matchedValueType: int + inKeyword: in + iterable: SimpleIdentifier + token: x + element: ::@function::f::@formalParameter::x + staticType: List + rightParenthesis: ) + body: Block + leftBracket: { + statements + ExpressionStatement + expression: SimpleIdentifier + token: a + element: a@51 + staticType: int + semicolon: ; + rightBracket: } +'''); + } + test_sync_iterable_contextType_patternVariable_typed() async { await assertErrorsInCode( r''' @@ -2808,6 +2920,182 @@ ForStatement @reflectiveTest class ForStatementResolutionTest_ForPartsWithPattern extends PubPackageResolutionTest { + test_metadata() async { + await assertNoErrorsInCode(r''' +void f() { + for (@deprecated var (a) = (0); a <= 2; a++) { + a; + } +} +'''); + + var node = findNode.singleForStatement; + assertResolvedNodeText(node, r''' +ForStatement + forKeyword: for + leftParenthesis: ( + forLoopParts: ForPartsWithPattern + variables: PatternVariableDeclaration + metadata + Annotation + atSign: @ + name: SimpleIdentifier + token: deprecated + element: dart:core::@getter::deprecated + staticType: null + element: dart:core::@getter::deprecated + keyword: var + pattern: ParenthesizedPattern + leftParenthesis: ( + pattern: DeclaredVariablePattern + name: a + declaredFragment: isPublic a@35 + element: hasImplicitType isPublic + type: int + matchedValueType: int + rightParenthesis: ) + matchedValueType: int + equals: = + expression: ParenthesizedExpression + leftParenthesis: ( + expression: IntegerLiteral + literal: 0 + staticType: int + rightParenthesis: ) + staticType: int + patternTypeSchema: _ + leftSeparator: ; + condition: BinaryExpression + leftOperand: SimpleIdentifier + token: a + element: a@35 + staticType: int + operator: <= + rightOperand: IntegerLiteral + literal: 2 + correspondingParameter: dart:core::@class::num::@method::<=::@formalParameter::other + staticType: int + element: dart:core::@class::num::@method::<= + staticInvokeType: bool Function(num) + staticType: bool + rightSeparator: ; + updaters + PostfixExpression + operand: SimpleIdentifier + token: a + element: a@35 + staticType: null + operator: ++ + readElement: a@35 + readType: int + writeElement: a@35 + writeType: int + element: dart:core::@class::num::@method::+ + staticType: int + rightParenthesis: ) + body: Block + leftBracket: { + statements + ExpressionStatement + expression: SimpleIdentifier + token: a + element: a@35 + staticType: int + semicolon: ; + rightBracket: } +'''); + } + + test_metadata_shadowing() async { + await assertErrorsInCode( + r''' +const a = 42; +void f() { + for (@a var (a) = (0); a <= 2; a++) { + a; + } +} +''', + [error(diag.invalidAnnotation, 32, 2)], + ); + + var node = findNode.singleForStatement; + assertResolvedNodeText(node, r''' +ForStatement + forKeyword: for + leftParenthesis: ( + forLoopParts: ForPartsWithPattern + variables: PatternVariableDeclaration + metadata + Annotation + atSign: @ + name: SimpleIdentifier + token: a + element: a@40 + staticType: null + element: a@40 + keyword: var + pattern: ParenthesizedPattern + leftParenthesis: ( + pattern: DeclaredVariablePattern + name: a + declaredFragment: isPublic a@40 + element: hasImplicitType isPublic + type: int + matchedValueType: int + rightParenthesis: ) + matchedValueType: int + equals: = + expression: ParenthesizedExpression + leftParenthesis: ( + expression: IntegerLiteral + literal: 0 + staticType: int + rightParenthesis: ) + staticType: int + patternTypeSchema: _ + leftSeparator: ; + condition: BinaryExpression + leftOperand: SimpleIdentifier + token: a + element: a@40 + staticType: int + operator: <= + rightOperand: IntegerLiteral + literal: 2 + correspondingParameter: dart:core::@class::num::@method::<=::@formalParameter::other + staticType: int + element: dart:core::@class::num::@method::<= + staticInvokeType: bool Function(num) + staticType: bool + rightSeparator: ; + updaters + PostfixExpression + operand: SimpleIdentifier + token: a + element: a@40 + staticType: null + operator: ++ + readElement: a@40 + readType: int + writeElement: a@40 + writeType: int + element: dart:core::@class::num::@method::+ + staticType: int + rightParenthesis: ) + body: Block + leftBracket: { + statements + ExpressionStatement + expression: SimpleIdentifier + token: a + element: a@40 + staticType: int + semicolon: ; + rightBracket: } +'''); + } + test_scope_afterLoop_uses_outer_despite_patternVariable() async { await assertNoErrorsInCode(r''' void f((int, bool) x, int a) { @@ -3141,6 +3429,7 @@ ForStatement } test_scope_patternVariables_shadows_outer_in_expression() async { + // TODO(scheglov): should report an error await assertNoErrorsInCode(r''' void f((int, bool) a) { for (var (a, b) = a; b; a--) {} @@ -3208,13 +3497,14 @@ ForStatement '''); } - test_scope_variables_uses_outer() async { + test_scope_patternVariables_visibleIn_condition_updaters() async { await assertNoErrorsInCode(r''' -void f((int, bool) a) { - for (var (a2, b) = a; b; a2--) {} +void f() { + for (var (a) = (0); a <= 2; a++) { + a; + } } '''); - var node = findNode.singleForStatement; assertResolvedNodeText(node, r''' ForStatement @@ -3223,55 +3513,63 @@ ForStatement forLoopParts: ForPartsWithPattern variables: PatternVariableDeclaration keyword: var - pattern: RecordPattern + pattern: ParenthesizedPattern leftParenthesis: ( - fields - PatternField - pattern: DeclaredVariablePattern - name: a2 - declaredFragment: isPublic a2@36 - element: hasImplicitType isPublic - type: int - matchedValueType: int - element: - PatternField - pattern: DeclaredVariablePattern - name: b - declaredFragment: isPublic b@40 - element: hasImplicitType isPublic - type: bool - matchedValueType: bool - element: + pattern: DeclaredVariablePattern + name: a + declaredFragment: isPublic a@23 + element: hasImplicitType isPublic + type: int + matchedValueType: int rightParenthesis: ) - matchedValueType: (int, bool) + matchedValueType: int equals: = - expression: SimpleIdentifier - token: a - element: ::@function::f::@formalParameter::a - staticType: (int, bool) - patternTypeSchema: (_, _) + expression: ParenthesizedExpression + leftParenthesis: ( + expression: IntegerLiteral + literal: 0 + staticType: int + rightParenthesis: ) + staticType: int + patternTypeSchema: _ leftSeparator: ; - condition: SimpleIdentifier - token: b - element: b@40 + condition: BinaryExpression + leftOperand: SimpleIdentifier + token: a + element: a@23 + staticType: int + operator: <= + rightOperand: IntegerLiteral + literal: 2 + correspondingParameter: dart:core::@class::num::@method::<=::@formalParameter::other + staticType: int + element: dart:core::@class::num::@method::<= + staticInvokeType: bool Function(num) staticType: bool rightSeparator: ; updaters PostfixExpression operand: SimpleIdentifier - token: a2 - element: a2@36 + token: a + element: a@23 staticType: null - operator: -- - readElement: a2@36 + operator: ++ + readElement: a@23 readType: int - writeElement: a2@36 + writeElement: a@23 writeType: int - element: dart:core::@class::num::@method::- + element: dart:core::@class::num::@method::+ staticType: int rightParenthesis: ) body: Block leftBracket: { + statements + ExpressionStatement + expression: SimpleIdentifier + token: a + element: a@23 + staticType: int + semicolon: ; rightBracket: } '''); } diff --git a/pkg/analyzer/test/src/dart/resolution/local_function_test.dart b/pkg/analyzer/test/src/dart/resolution/local_function_test.dart index a0f6de6f69e..7b2a3ca5bc3 100644 --- a/pkg/analyzer/test/src/dart/resolution/local_function_test.dart +++ b/pkg/analyzer/test/src/dart/resolution/local_function_test.dart @@ -127,6 +127,34 @@ MethodInvocation rightParenthesis: ) staticInvokeType: Null Function() staticType: Null +'''); + } + + test_recursiveReference_ifStatement_nonBlock() async { + await assertErrorsInCode( + r''' +f(bool c) { + if (c) + g() { + g(); // ref + } +} +''', + [error(diag.unusedElement, 25, 1)], + ); + + var node = findNode.singleMethodInvocation; + assertResolvedNodeText(node, r''' +MethodInvocation + methodName: SimpleIdentifier + token: g + element: g@25 + staticType: dynamic Function() + argumentList: ArgumentList + leftParenthesis: ( + rightParenthesis: ) + staticInvokeType: dynamic Function() + staticType: dynamic '''); } } diff --git a/pkg/analyzer/test/src/dart/resolution/local_variable_test.dart b/pkg/analyzer/test/src/dart/resolution/local_variable_test.dart index 8e5c919cd94..b3c40e48367 100644 --- a/pkg/analyzer/test/src/dart/resolution/local_variable_test.dart +++ b/pkg/analyzer/test/src/dart/resolution/local_variable_test.dart @@ -139,6 +139,23 @@ void f() { expect(x.isStatic, isFalse); } + test_initializerReference_ifStatement_nonBlock() async { + await assertNoErrorsInCode(r''' +void f(bool c) { + if (c) + // ignore: unused_local_variable + var a = 0, b = a; // ref +} +'''); + + assertResolvedNodeText(findNode.simple('a; // ref'), r''' +SimpleIdentifier + token: a + element: a@71 + staticType: int +'''); + } + test_localVariable_wildcardFunction() async { await assertErrorsInCode( ''' diff --git a/pkg/analyzer/test/src/dart/resolution/pattern_assignment_test.dart b/pkg/analyzer/test/src/dart/resolution/pattern_assignment_test.dart index 13fbf1c2c46..c328b63a012 100644 --- a/pkg/analyzer/test/src/dart/resolution/pattern_assignment_test.dart +++ b/pkg/analyzer/test/src/dart/resolution/pattern_assignment_test.dart @@ -338,6 +338,88 @@ PatternAssignment '''); } + test_context_arrowBody_formalParameter() async { + await assertNoErrorsInCode(r''' +int f(int x) => (x) = 0; +'''); + + var node = findNode.singlePatternAssignment; + assertResolvedNodeText(node, r''' +PatternAssignment + pattern: ParenthesizedPattern + leftParenthesis: ( + pattern: AssignedVariablePattern + name: x + element: ::@function::f::@formalParameter::x + matchedValueType: int + rightParenthesis: ) + matchedValueType: int + equals: = + expression: IntegerLiteral + literal: 0 + staticType: int + patternTypeSchema: int + staticType: int +'''); + } + + test_context_returnExpression_formalParameter() async { + await assertNoErrorsInCode(r''' +int f(int x) { + return (x) = 0; +} +'''); + + var node = findNode.singlePatternAssignment; + assertResolvedNodeText(node, r''' +PatternAssignment + pattern: ParenthesizedPattern + leftParenthesis: ( + pattern: AssignedVariablePattern + name: x + element: ::@function::f::@formalParameter::x + matchedValueType: int + rightParenthesis: ) + matchedValueType: int + equals: = + expression: IntegerLiteral + literal: 0 + staticType: int + patternTypeSchema: int + staticType: int +'''); + } + + test_context_variableInitializer_localVariable() async { + await assertNoErrorsInCode(r''' +void f() { + int x = 1; + var y = (x) = 0; + x; + y; +} +'''); + + var node = findNode.singlePatternAssignment; + assertResolvedNodeText(node, r''' +PatternAssignment + pattern: ParenthesizedPattern + leftParenthesis: ( + pattern: AssignedVariablePattern + name: x + element: x@17 + matchedValueType: int + rightParenthesis: ) + matchedValueType: int + equals: = + expression: IntegerLiteral + literal: 0 + staticType: int + patternTypeSchema: int + staticType: int +'''); + } + test_declaredVariable_inPatternAssignment_referenced() async { // Note: the error is reporting during parsing but we test it here to make // sure that error recovery produces an AST that can be analyzed without diff --git a/pkg/analyzer/test/src/dart/resolution/pattern_variable_declaration_statement_test.dart b/pkg/analyzer/test/src/dart/resolution/pattern_variable_declaration_statement_test.dart index 5a1ed62675c..3e7da61e89d 100644 --- a/pkg/analyzer/test/src/dart/resolution/pattern_variable_declaration_statement_test.dart +++ b/pkg/analyzer/test/src/dart/resolution/pattern_variable_declaration_statement_test.dart @@ -125,6 +125,79 @@ PatternVariableDeclarationStatement '''); } + test_scope_shadows_beforeDeclaration() async { + await assertErrorsInCode( + r''' +int a = 0; +void f() { + a; + var (a) = 1; +} +''', + [ + error( + diag.referencedBeforeDeclaration, + 24, + 1, + contextMessages: [message(testFile, 34, 1)], + ), + ], + ); + + var node = findNode.simple('a;'); + assertResolvedNodeText(node, r''' +SimpleIdentifier + token: a + element: a@34 + staticType: InvalidType +'''); + } + + test_scope_shadows_class() async { + await assertErrorsInCode( + r''' +class A {} + +void f() { + var (A) = []; +} +''', + [error(diag.nonTypeAsTypeArgument, 36, 1)], + ); + + var node = findNode.singlePatternVariableDeclarationStatement; + assertResolvedNodeText(node, r''' +PatternVariableDeclarationStatement + declaration: PatternVariableDeclaration + keyword: var + pattern: ParenthesizedPattern + leftParenthesis: ( + pattern: DeclaredVariablePattern + name: A + declaredFragment: isPublic A@30 + element: hasImplicitType isPublic + type: List + matchedValueType: List + rightParenthesis: ) + matchedValueType: List + equals: = + expression: ListLiteral + typeArguments: TypeArgumentList + leftBracket: < + arguments + NamedType + name: A + element: A@30 + type: InvalidType + rightBracket: > + leftBracket: [ + rightBracket: ] + staticType: List + patternTypeSchema: _ + semicolon: ; +'''); + } + test_var_typed() async { await assertNoErrorsInCode(r''' void f() {