Implement correct scoping rules for variables.

sdk/lib/_internal/compiler/implementation/resolution/members.dart

Index: sdk/lib/_internal/compiler/implementation/resolution/members.dart
diff --git a/sdk/lib/_internal/compiler/implementation/resolution/members.dart b/sdk/lib/_internal/compiler/implementation/resolution/members.dart
index 1fd0b735fd95470f9e080d6097ebe1db3d687f45..4c8a018d119533196bf19d89da28adec5ddbba52 100644
--- a/sdk/lib/_internal/compiler/implementation/resolution/members.dart
+++ b/sdk/lib/_internal/compiler/implementation/resolution/members.dart
@@ -1362,6 +1362,8 @@ class CommonResolverVisitor<R> extends Visitor<R> {
   R visit(Node node) => (node == null) ? null : node.accept(this);
 
   void error(Node node, MessageKind kind, [Map arguments = const {}]) {
+    // TODO(karlklose): change this to use [compiler.reportError] and
+    // explicitly report fatal errors where necessary.
     compiler.reportFatalError(node, kind, arguments);
   }
 
@@ -1913,6 +1915,23 @@ class ResolverVisitor extends MappingVisitor<Element> {
     return new ErroneousElementX(kind.error, arguments, name, enclosingElement);
   }
 
+  Element resolveIdentifier(Identifier node) {
+      Element result = scope.lookup(node.source);
+      if (result == null) return null;
+
+      if (result is VariableElement &&
+          Elements.isLocal(result)) {

[ngeoffray, 2013/09/26 07:48:38]

One line? Also, what is this check? That result is a local but not a parameter?
How about function  statements/expression? Aren't their runtime type
FunctionElement?

Can't you do Elements.isLocal(result) && !result.isParameter() &&
!result.isFunction()? It would look clearer to me.

+        VariableElement variable = result;
+        Node definition = variable.parseNode(compiler);
+        if (definition.getEndToken().charOffset >=
+            node.getBeginToken().charOffset) {
+          compiler.reportError(node, MessageKind.ACCESS_BEFORE_INITIALIZATION,
+                {'variableName': node});
+        }
+      }
+      return result;
+  }
+
   Element visitIdentifier(Identifier node) {
     if (node.isThis()) {
       if (!inInstanceContext) {
@@ -1926,8 +1945,8 @@ class ResolverVisitor extends MappingVisitor<Element> {
       }
       return null;
     } else {
+      Element element = resolveIdentifier(node);
       SourceString name = node.source;
-      Element element = scope.lookup(name);
       if (Elements.isUnresolved(element) && name.slowToString() == 'dynamic') {
         element = compiler.dynamicClass;
       }
@@ -1998,6 +2017,27 @@ class ResolverVisitor extends MappingVisitor<Element> {
     return null;
   }
 
+  void declareVariables(VariableDefinitions declaration) {
+    VariableListElement variables = new VariableListElementX.node(
+        declaration, ElementKind.VARIABLE_LIST, enclosingElement);
+    if (declaration.type != null) {
+      variables.type = resolveTypeAnnotation(declaration.type);
+    } else {
+      variables.type = compiler.types.dynamicType;
+    }
+
+    for (Node node in declaration.definitions) {
+      Identifier identifier = node.asIdentifier();
+      if (identifier == null) {
+        identifier = node.asSend().selector.asIdentifier();
+      }
+      SourceString name = identifier.source;
+      VariableElement element =
+          new VariableElementX(name, variables, ElementKind.VARIABLE, node);
+      defineElement(node, element);
+    }
+  }
+
   void setupFunction(FunctionExpression node, FunctionElement function) {
     Element enclosingElement = function.enclosingElement;
     if (node.modifiers.isStatic() &&
@@ -2048,12 +2088,18 @@ class ResolverVisitor extends MappingVisitor<Element> {
     cancel(node, "shouldn't be called");
   }
 
-  visitIn(Node node, Scope nestedScope) {
+  inScope(Scope nestedScope, f()) {
     Scope oldScope = scope;
     scope = nestedScope;
-    Element element = visit(node);
+    var result = f();
     scope = oldScope;
-    return element;
+    return result;
+  }
+
+  visitIn(Node node, Scope nestedScope) {
+    return inScope(nestedScope, () {
+      return visit(node);
+    });
   }
 
   /**
@@ -2071,11 +2117,14 @@ class ResolverVisitor extends MappingVisitor<Element> {
   }
 
   visitBlock(Block node) {
-    visitIn(node.statements, new BlockScope(scope));
+    inScope(new BlockScope(scope), () {
+      node.declarations.forEach(declareVariables);
+      visit(node.statements);
+    });
   }
 
   visitDoWhile(DoWhile node) {
-    visitLoopBodyIn(node, node.body, new BlockScope(scope));
+    visitLoopBodyIn(node, node.body, createDeclarationScope(node.body));
     visit(node.condition);
   }
 
@@ -2089,10 +2138,15 @@ class ResolverVisitor extends MappingVisitor<Element> {
   }
 
   visitFor(For node) {
-    Scope blockScope = new BlockScope(scope);
-    visitIn(node.initializer, blockScope);
-    visitIn(node.condition, blockScope);
-    visitIn(node.update, blockScope);
+    BlockScope blockScope = createDeclarationScope(node.body);
+    inScope(blockScope, () {
+      if (node.initializer is VariableDefinitions) {
+        declareVariables(node.initializer);
+      }
+      visit(node.initializer);
+      visit(node.condition);
+      visit(node.update);
+    });
     visitLoopBodyIn(node, node.body, blockScope);
   }
 
@@ -2137,8 +2191,8 @@ class ResolverVisitor extends MappingVisitor<Element> {
 
   visitIf(If node) {
     visit(node.condition);
-    visitIn(node.thenPart, new BlockScope(scope));
-    visitIn(node.elsePart, new BlockScope(scope));
+    visitIn(node.thenPart, createDeclarationScope(node.thenPart));
+    visitIn(node.elsePart, createDeclarationScope(node.elsePart));
   }
 
   static bool isLogicalOperator(Identifier op) {
@@ -2721,16 +2775,6 @@ class ResolverVisitor extends MappingVisitor<Element> {
     VariableDefinitionsVisitor visitor =
         new VariableDefinitionsVisitor(compiler, node, this,
                                        ElementKind.VARIABLE);
-    // Ensure that we set the type of the [VariableListElement] since it depends
-    // on the current scope. If the current scope is a [MethodScope] or
-    // [BlockScope] it will not be available for the
-    // [VariableListElement.computeType] method.
-    if (node.type != null) {
-      visitor.variables.type = resolveTypeAnnotation(node.type);
-    } else {
-      visitor.variables.type = compiler.types.dynamicType;
-    }
-
     Modifiers modifiers = node.modifiers;
     void reportExtraModifier(String modifier) {
       Node modifierNode;
@@ -2754,9 +2798,19 @@ class ResolverVisitor extends MappingVisitor<Element> {
     visitor.visit(node.definitions);
   }
 
+  BlockScope createDeclarationScope(Node body) {
+    Scope blockScope = new BlockScope(scope);
+    if (body != null && body.asVariableDefinitions() != null) {
+      inScope(blockScope, () {
+        declareVariables(body);
+      });
+    }
+    return blockScope;
+  }
+
   visitWhile(While node) {
     visit(node.condition);
-    visitLoopBodyIn(node, node.body, new BlockScope(scope));
+    visitLoopBodyIn(node, node.body, createDeclarationScope(node.body));
   }
 
   visitParenthesizedExpression(ParenthesizedExpression node) {
@@ -3006,8 +3060,8 @@ class ResolverVisitor extends MappingVisitor<Element> {
     world.registerDynamicInvocation(compiler.moveNextSelector);
 
     visit(node.expression);
-    Scope blockScope = new BlockScope(scope);
     Node declaration = node.declaredIdentifier;
+    BlockScope blockScope = createDeclarationScope(node.body);
     visitIn(declaration, blockScope);
 
     Send send = declaration.asSend();
@@ -3027,6 +3081,9 @@ class ResolverVisitor extends MappingVisitor<Element> {
         compiler.reportError(send.receiver, MessageKind.INVALID_FOR_IN);
       }
     } else if (variableDefinitions != null) {
+      inScope(blockScope, () {
+        declareVariables(variableDefinitions);
+      });
       Link<Node> nodes = variableDefinitions.definitions.nodes;
       if (!nodes.tail.isEmpty) {
         compiler.reportError(nodes.tail.head, MessageKind.INVALID_FOR_IN);
@@ -3207,7 +3264,10 @@ class ResolverVisitor extends MappingVisitor<Element> {
 
   visitSwitchCase(SwitchCase node) {
     node.labelsAndCases.accept(this);
-    visitIn(node.statements, new BlockScope(scope));
+    inScope(new BlockScope(scope), () {
+      node.declarations.forEach(declareVariables);
+      visit(node.statements);
+    });
   }
 
   visitCaseMatch(CaseMatch node) {
@@ -3274,12 +3334,23 @@ class ResolverVisitor extends MappingVisitor<Element> {
     }
 
     Scope blockScope = new BlockScope(scope);
-    doInCheckContext(() => visitIn(node.type, blockScope));
-    visitIn(node.formals, blockScope);
-    var oldInCatchBlock = inCatchBlock;
-    inCatchBlock = true;
-    visitIn(node.block, blockScope);
-    inCatchBlock = oldInCatchBlock;
+    List<VariableDefinitions> declarations = <VariableDefinitions>[];
+    if (exceptionDefinition != null) {
+      declarations.add(exceptionDefinition);
+    }
+    if (stackTraceDefinition != null) {
+      declarations.add(stackTraceDefinition);
+    }
+
+    inScope(blockScope, () {
+      declarations.forEach(declareVariables);
+      doInCheckContext(() => visitIn(node.type, blockScope));
+      visitIn(node.formals, blockScope);
+      var oldInCatchBlock = inCatchBlock;
+      inCatchBlock = true;
+      visit(node.block);
+      inCatchBlock = oldInCatchBlock;
+    });
 
     if (node.type != null && exceptionDefinition != null) {
       DartType exceptionType = mapping.getType(node.type);
@@ -3942,26 +4013,17 @@ class VariableDefinitionsVisitor extends CommonResolverVisitor<SourceString> {
   VariableDefinitions definitions;
   ResolverVisitor resolver;
   ElementKind kind;
-  VariableListElement variables;
 
   VariableDefinitionsVisitor(Compiler compiler,
                              this.definitions, this.resolver, this.kind)
       : super(compiler) {
-    variables = new VariableListElementX.node(
-        definitions, ElementKind.VARIABLE_LIST, resolver.enclosingElement);
   }
 
   SourceString visitSendSet(SendSet node) {
     assert(node.arguments.tail.isEmpty); // Sanity check
     Identifier identifier = node.selector;
     SourceString name = identifier.source;
-    VariableDefinitionScope scope =
-        new VariableDefinitionScope(resolver.scope, name);
-    resolver.visitIn(node.arguments.head, scope);
-    if (scope.variableReferencedInInitializer) {
-      resolver.error(identifier, MessageKind.REFERENCE_IN_INITIALIZATION,
-                     {'variableName': name.toString()});
-    }
+    resolver.visit(node.arguments.head);
     return name;
   }
 
@@ -3978,9 +4040,6 @@ class VariableDefinitionsVisitor extends CommonResolverVisitor<SourceString> {
   visitNodeList(NodeList node) {
     for (Link<Node> link = node.nodes; !link.isEmpty; link = link.tail) {
       SourceString name = visit(link.head);
-      VariableElement element =
-          new VariableElementX(name, variables, kind, link.head);
-      resolver.defineElement(link.head, element);
     }
   }
 }
@@ -4357,7 +4416,7 @@ class ConstructorResolver extends CommonResolverVisitor<Element> {
   Element visitIdentifier(Identifier node) {
     SourceString name = node.source;
     Element e = resolver.reportLookupErrorIfAny(
-        resolver.scope.lookup(name), node, name);
+        resolver.resolveIdentifier(node), node, name);
     // TODO(johnniwinther): Change errors to warnings, cf. 11.11.1.
     if (e == null) {
       return failOrReturnErroneousElement(resolver.enclosingElement, node, name,