Implement correct scoping rules for variables.

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

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of resolution;
 
 abstract class TreeElements {
   Element get currentElement;
   Set<Node> get superUses;
 
@@ skipping 1344 matching lines @@
     cancel(node,
            'internal error: Unhandled node: ${node.getObjectDescription()}');
   }
 
   R visitEmptyStatement(Node node) => null;
 
   /** Convenience method for visiting nodes that may be null. */
   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);
   }
 
   void dualError(Node node, DualKind kind, [Map arguments = const {}]) {
     error(node, kind.error, arguments);
   }
 
   void warning(Node node, MessageKind kind, [Map arguments = const {}]) {
     ResolutionWarning message =
         new ResolutionWarning(kind, arguments, compiler.terseDiagnostics);
@@ skipping 531 matching lines @@
   ErroneousElement warnAndCreateErroneousElement(Node node,
                                                  SourceString name,
                                                  DualKind kind,
                                                  [Map arguments = const {}]) {
     ResolutionWarning warning = new ResolutionWarning(
         kind.warning, arguments, compiler.terseDiagnostics);
     compiler.reportWarning(node, warning);
     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) {
         error(node, MessageKind.NO_INSTANCE_AVAILABLE, {'name': node});
       }
       return null;
     } else if (node.isSuper()) {
       if (!inInstanceContext) error(node, MessageKind.NO_SUPER_IN_STATIC);
       if ((ElementCategory.SUPER & allowedCategory) == 0) {
         error(node, MessageKind.INVALID_USE_OF_SUPER);
       }
       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;
       }
       element = reportLookupErrorIfAny(element, node, node.source);
       if (element == null) {
         if (!inInstanceContext) {
           element = warnAndCreateErroneousElement(
               node, node.source, MessageKind.CANNOT_RESOLVE,
               {'name': node});
           compiler.backend.registerThrowNoSuchMethod(mapping);
@@ skipping 50 matching lines @@
     if (!initializers.isEmpty &&
         Initializers.isConstructorRedirect(initializers.head)) {
       Selector selector =
           getRedirectingThisOrSuperConstructorSelector(initializers.head);
       final ClassElement classElement = constructor.getEnclosingClass();
       return classElement.lookupConstructor(selector);
     }
     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() &&
         enclosingElement.kind != ElementKind.CLASS) {
       compiler.reportError(node, MessageKind.ILLEGAL_STATIC);
     }
 
     scope = new MethodScope(scope, function);
     // Put the parameters in scope.
     FunctionSignature functionParameters =
@@ skipping 30 matching lines @@
   }
 
   visitCascadeReceiver(CascadeReceiver node) {
     visit(node.expression);
   }
 
   Element visitClassNode(ClassNode node) {
     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);
+    });
   }
 
   /**
    * Introduces new default targets for break and continue
    * before visiting the body of the loop
    */
   visitLoopBodyIn(Node loop, Node body, Scope bodyScope) {
     TargetElement element = getOrCreateTargetElement(loop);
     statementScope.enterLoop(element);
     visitIn(body, bodyScope);
     statementScope.exitLoop();
     if (!element.isTarget) {
       mapping.remove(loop);
     }
   }
 
   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);
   }
 
   visitEmptyStatement(EmptyStatement node) { }
 
   visitExpressionStatement(ExpressionStatement node) {
     ExpressionStatement oldExpressionStatement = currentExpressionStatement;
     currentExpressionStatement = node;
     visit(node.expression);
     currentExpressionStatement = oldExpressionStatement;
   }
 
   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);
   }
 
   visitFunctionDeclaration(FunctionDeclaration node) {
     assert(node.function.name != null);
     visit(node.function);
     FunctionElement functionElement = mapping[node.function];
     // TODO(floitsch): this might lead to two errors complaining about
     // shadowing.
     defineElement(node, functionElement);
@@ skipping 24 matching lines @@
 
     scope = oldScope;
     enclosingElement = previousEnclosingElement;
 
     world.registerClosure(function, mapping);
     world.registerInstantiatedClass(compiler.functionClass, mapping);
   }
 
   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) {
     String str = op.source.stringValue;
     return (identical(str, '&&') || str == '||' || str == '!');
   }
 
   Element resolveSend(Send node) {
     Selector selector = resolveSelector(node, null);
     if (node.isSuperCall) mapping.superUses.add(node);
@@ skipping 562 matching lines @@
 
   visitThrow(Throw node) {
     compiler.backend.registerThrowExpression(mapping);
     visit(node.expression);
   }
 
   visitVariableDefinitions(VariableDefinitions node) {
     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;
       for (var nodes = modifiers.nodes; !nodes.isEmpty; nodes = nodes.tail) {
         if (modifier == nodes.head.asIdentifier().source.stringValue) {
           modifierNode = nodes.head;
           break;
         }
       }
       assert(modifierNode != null);
       compiler.reportError(modifierNode, MessageKind.EXTRANEOUS_MODIFIER,
           {'modifier': modifier});
     }
     if (modifiers.isFinal() && (modifiers.isConst() || modifiers.isVar())) {
       reportExtraModifier('final');
     }
     if (modifiers.isVar() && (modifiers.isConst() || node.type != null)) {
       reportExtraModifier('var');
     }
 
     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) {
     bool oldSendIsMemberAccess = sendIsMemberAccess;
     sendIsMemberAccess = false;
     visit(node.expression);
     sendIsMemberAccess = oldSendIsMemberAccess;
   }
 
   visitNewExpression(NewExpression node) {
@@ skipping 229 matching lines @@
   visitForIn(ForIn node) {
     LibraryElement library = enclosingElement.getLibrary();
     mapping.setIteratorSelector(node, compiler.iteratorSelector);
     world.registerDynamicGetter(compiler.iteratorSelector);
     mapping.setCurrentSelector(node, compiler.currentSelector);
     world.registerDynamicGetter(compiler.currentSelector);
     mapping.setMoveNextSelector(node, compiler.moveNextSelector);
     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();
     VariableDefinitions variableDefinitions =
         declaration.asVariableDefinitions();
     Element loopVariable;
     Selector loopVariableSelector;
     if (send != null) {
       loopVariable = mapping[send];
       Identifier identifier = send.selector.asIdentifier();
       if (identifier == null) {
         compiler.reportError(send.selector, MessageKind.INVALID_FOR_IN);
       } else {
         loopVariableSelector = new Selector.setter(identifier.source, library);
       }
       if (send.receiver != null) {
         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);
       }
       Node first = nodes.head;
       Identifier identifier = first.asIdentifier();
       if (identifier == null) {
         compiler.reportError(first, MessageKind.INVALID_FOR_IN);
       } else {
         loopVariableSelector = new Selector.setter(identifier.source, library);
@@ skipping 160 matching lines @@
         mapping.remove(label.label);
       }
     });
     // TODO(ngeoffray): We should check here instead of the SSA backend if
     // there might be an error.
     compiler.backend.registerFallThroughError(mapping);
   }
 
   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) {
     visit(node.expression);
   }
 
   visitTryStatement(TryStatement node) {
     visit(node.tryBlock);
     if (node.catchBlocks.isEmpty && node.finallyBlock == null) {
       // TODO(ngeoffray): The precise location is
@@ skipping 46 matching lines @@
           }
           TypeAnnotation type = declaration.type;
           if (type != null) {
             error(type, MessageKind.PARAMETER_WITH_TYPE_IN_CATCH);
           }
         }
       }
     }
 
     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);
       Node exceptionVariable = exceptionDefinition.definitions.nodes.head;
       VariableElementX exceptionElement = mapping[exceptionVariable];
       exceptionElement.variables.type = exceptionType;
     }
     if (stackTraceDefinition != null) {
       Node stackTraceVariable = stackTraceDefinition.definitions.nodes.head;
       VariableElementX stackTraceElement = mapping[stackTraceVariable];
@@ skipping 642 matching lines @@
       return;
     }
     loadSupertype(e, node);
   }
 }
 
 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;
   }
 
   SourceString visitIdentifier(Identifier node) {
     // The variable is initialized to null.
     resolver.world.registerInstantiatedClass(compiler.nullClass,
                                              resolver.mapping);
     if (definitions.modifiers.isConst()) {
       compiler.reportError(node, MessageKind.CONST_WITHOUT_INITIALIZER);
     }
     return node.source;
   }
 
   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);
     }
   }
 }
 
 /**
  * [SignatureResolver] resolves function signatures.
  */
 class SignatureResolver extends CommonResolverVisitor<Element> {
   final Element enclosingElement;
   final bool defaultValuesAllowed;
@@ skipping 356 matching lines @@
       }
     } else {
       internalError(node.receiver, 'unexpected element $e');
     }
     return e;
   }
 
   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,
                                           MessageKind.CANNOT_RESOLVE,
                                           {'name': name});
     } else if (e.isErroneous()) {
       return e;
     } else if (identical(e.kind, ElementKind.TYPEDEF)) {
       error(node, MessageKind.CANNOT_INSTANTIATE_TYPEDEF,
             {'typedefName': name});
     } else if (identical(e.kind, ElementKind.TYPE_VARIABLE)) {
       error(node, MessageKind.CANNOT_INSTANTIATE_TYPE_VARIABLE,
             {'typeVariableName': name});
     } else if (!identical(e.kind, ElementKind.CLASS)
         && !identical(e.kind, ElementKind.PREFIX)) {
       error(node, MessageKind.NOT_A_TYPE.error, {'node': name});
     }
     return e;
   }
 
   /// Assumed to be called by [resolveRedirectingFactory].
   Element visitReturn(Return node) {
     Node expression = node.expression;
     return finishConstructorReference(visit(expression),
                                       expression, expression);
   }
 }