Compute better names for source maps.

BUG=http://dartbug.com/23336
R=floitsch@google.com

Review URL: https://codereview.chromium.org//1139693002

pkg/compiler/lib/src/elements/elements.dart

@@ -973,7 +973,8 @@ abstract class MemberElement extends Element implements ExecutableElement {
 }
 
 /// A function, variable or parameter defined in an executable context.
-abstract class LocalElement extends Element implements TypedElement, Local {
+abstract class LocalElement extends Element
+    implements AstElement, TypedElement, Local {
 }
 
 /// A top level, static or instance field, a formal parameter or local variable.

pkg/compiler/lib/src/io/source_information.dart

@@ -5,7 +5,9 @@
 library dart2js.source_information;
 
 import '../dart2jslib.dart' show SourceSpan, MessageKind;
-import '../elements/elements.dart' show AstElement;
+import '../elements/elements.dart' show
+    AstElement,
+    LocalElement;
 import '../scanner/scannerlib.dart' show Token;
 import '../tree/tree.dart' show Node;
 import '../js/js.dart' show JavaScriptNodeSourceInformation;
@@ -103,7 +105,7 @@ class StartEndSourceInformation extends SourceInformation {
 
     AstElement implementation = element.implementation;
     SourceFile sourceFile = implementation.compilationUnit.script.file;
-    String name = element.name;
+    String name = computeElementNameForSourceMaps(element);
     Node node = implementation.node;
     Token beginToken;
     Token endToken;
@@ -156,7 +158,7 @@ class StartEndSourceInformationBuilder extends SourceInformationBuilder {
 
   StartEndSourceInformationBuilder(AstElement element)
       : sourceFile = element.compilationUnit.script.file,
-        name = element.name;
+        name = computeElementNameForSourceMaps(element);
 
   SourceInformation buildDeclaration(AstElement element) {
     return StartEndSourceInformation.computeSourceInformation(element);
@@ -339,7 +341,7 @@ class PositionSourceInformationBuilder implements SourceInformationBuilder {
 
   PositionSourceInformationBuilder(AstElement element)
       : sourceFile = element.implementation.compilationUnit.script.file,
-        name = element.name;
+        name = computeElementNameForSourceMaps(element);
 
   SourceInformation buildDeclaration(AstElement element) {
     if (element.isSynthesized) {
@@ -379,3 +381,32 @@ class PositionSourceInformationBuilder implements SourceInformationBuilder {
     return new PositionSourceInformationBuilder(element);
   }
 }
+
+/// Compute the source map name for [element].
+String computeElementNameForSourceMaps(AstElement element) {
+  if (element.isClosure) {
+    return computeElementNameForSourceMaps(element.enclosingElement);
+  } else if (element.isClass) {
+    return element.name;
+  } else if (element.isConstructor || element.isGenerativeConstructorBody) {
+    String className = element.enclosingClass.name;
+    if (element.name == '') {
+      return className;
+    }
+    return '$className.${element.name}';
+  } else if (element.isLocal) {
+    LocalElement local = element;
+    String name = local.name;
+    if (name == '') {
+      name = '<anonymous function>';
+    }
+    return '${computeElementNameForSourceMaps(local.executableContext)}.$name';
+  } else if (element.enclosingClass != null) {
+    if (element.enclosingClass.isClosure) {
+      return computeElementNameForSourceMaps(element.enclosingClass);
+    }
+    return '${element.enclosingClass.name}.${element.name}';
+  } else {
+    return element.name;
+  }
+}

tests/compiler/dart2js/source_map_name_test.dart

@@ -0,0 +1,165 @@
+// Copyright (c) 2015, 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.
+
+library source_map_name_test;
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+import 'package:compiler/src/dart2jslib.dart';
+import 'package:compiler/src/elements/elements.dart';
+import 'package:compiler/src/io/source_information.dart';
+import 'memory_compiler.dart';
+
+const String SOURCE = '''
+
+var toplevelField;
+void toplevelMethod() {}
+void toplevelAnonymous() {
+  var foo = () {};
+}
+void toplevelLocal() {
+  void localMethod() {}
+}
+
+class Class {
+  Class() {
+    var foo = () {};
+  }
+  Class.named() {
+    void localMethod() {}
+  }
+  static var staticField;
+  static staticMethod() {}
+  static void staticAnonymous() {
+    var foo = () {};
+  }
+  static void staticLocal() {
+    void localMethod() {}
+  }
+  var instanceField;
+  instanceMethod() {}
+  void instanceAnonymous() {
+    var foo = () {};
+  }
+  void instanceLocal() {
+    void localMethod() {}
+  }
+  void instanceNestedLocal() {
+    void localMethod() {
+      var foo = () {};
+      void nestedLocalMethod() {}
+    }
+  }
+}
+
+main() {
+  toplevelField = toplevelMethod();
+  toplevelAnonymous();
+  toplevelLocal();
+
+  Class.staticField = Class.staticMethod;
+  Class.staticAnonymous();
+  Class.staticLocal();
+
+  var c = new Class();
+  c = new Class.named();
+  c.instanceField = c.instanceMethod();
+  c.instanceAnonymous();
+  c.instanceLocal();
+  c.instanceNestedLocal();
+}
+''';
+
+check(Element element, String expectedName) {
+  String name = computeElementNameForSourceMaps(element);
+  Expect.equals(
+      expectedName,
+      name,
+      "Unexpected name '$name' for $element, expected '$expectedName'.");
+}
+
+main() {
+  asyncTest(() async {
+    Compiler compiler = compilerFor({'main.dart': SOURCE});
+    await compiler.run(Uri.parse('memory:main.dart'));
+
+    Element lookup(String name) {
+      Element element;
+      int dotPosition = name.indexOf('.');
+      if (dotPosition != -1) {
+        String clsName = name.substring(0, dotPosition);
+        ClassElement cls = compiler.mainApp.find(clsName);
+        Expect.isNotNull(cls, "Class '$clsName' not found.");
+        element = cls.localLookup(name.substring(dotPosition + 1));
+      } else {
+        element = compiler.mainApp.find(name);
+      }
+      Expect.isNotNull(element, "Element '$name' not found.");
+      return element;
+    }
+
+    void checkName(String expectedName,
+                   [List<String> expectedClosureNames,
+                    String lookupName]) {
+      if (lookupName == null) {
+        lookupName = expectedName;
+      }
+      var element = lookup(lookupName);
+      check(element, expectedName);
+      if (element.isConstructor) {
+        var constructorBody =
+            element.enclosingClass.lookupBackendMember(element.name);
+        Expect.isNotNull(element,
+                         "Constructor body '${element.name}' not found.");
+        check(constructorBody, expectedName);
+      }
+
+      if (expectedClosureNames != null) {
+        int index = 0;
+        for (var closure in element.nestedClosures) {
+          String expectedName = expectedClosureNames[index];
+          check(closure, expectedName);
+          check(closure.expression, expectedName);
+          check(closure.enclosingClass, expectedName);
+          index++;
+        }
+      }
+    }
+
+    checkName('toplevelField');
+    checkName('toplevelMethod');
+    checkName('toplevelAnonymous',
+              ['toplevelAnonymous.<anonymous function>']);
+    checkName('toplevelLocal',
+              ['toplevelLocal.localMethod']);
+    checkName('Class');
+    checkName('main');
+
+    checkName('Class.staticField');
+    checkName('Class.staticMethod');
+    checkName('Class.staticAnonymous',
+              ['Class.staticAnonymous.<anonymous function>']);
+    checkName('Class.staticLocal',
+              ['Class.staticLocal.localMethod']);
+
+    checkName('Class',
+              ['Class.<anonymous function>'],
+              'Class.');
+    checkName('Class.named',
+              ['Class.named.localMethod']);
+
+    checkName('Class.instanceField');
+    checkName('Class.instanceMethod');
+    checkName('Class.instanceAnonymous',
+              ['Class.instanceAnonymous.<anonymous function>']);
+    checkName('Class.instanceLocal',
+              ['Class.instanceLocal.localMethod']);
+    checkName('Class.instanceNestedLocal',
+              ['Class.instanceNestedLocal.localMethod',
+               'Class.instanceNestedLocal.localMethod.<anonymous function>',
+               'Class.instanceNestedLocal.localMethod.nestedLocalMethod']);
+
+
+  });
+}

tests/compiler/dart2js/source_map_validator_helper.dart

@@ -11,11 +11,16 @@ import 'package:expect/expect.dart';
 import 'package:source_maps/source_maps.dart' hide SourceFile;
 import 'package:compiler/src/apiimpl.dart';
 import 'package:compiler/src/elements/elements.dart'
-    show LibraryElement,
-         CompilationUnitElement,
+    show AstElement,
          ClassElement,
-         AstElement;
+         CompilationUnitElement,
+         Element,
+         FunctionElement,
+         LibraryElement,
+         MemberElement;
 import 'package:compiler/src/io/source_file.dart' show SourceFile;
+import 'package:compiler/src/io/source_information.dart'
+    show computeElementNameForSourceMaps;
 
 validateSourceMap(Uri targetUri,
                   {Uri mainUri,
@@ -143,16 +148,58 @@ checkNames(Uri targetUri, Uri mapUri,
                               positionFromOffset(end));
         }
 
+        AstElement findInnermost(AstElement element) {
+          bool isInsideElement(FunctionElement closure) {
+            Element enclosing = closure;
+            while (enclosing != null) {
+              if (enclosing == element) return true;
+              enclosing = enclosing.enclosingElement;
+            }
+            return false;
+          }
+
+          if (element is MemberElement) {
+            MemberElement member = element;
+            member.nestedClosures.forEach((closure) {
+              var localFunction = closure.expression;
+              Interval interval = intervalFromElement(localFunction);
+              if (interval != null &&
+                  interval.contains(sourcePosition) &&
+                  isInsideElement(localFunction)) {
+                element = localFunction;
+              }
+            });
+          }
+          return element;
+        }
+
         void match(AstElement element) {
           Interval interval = intervalFromElement(element);
           if (interval != null && interval.contains(sourcePosition)) {
-            if (name != 'call') {
-              // TODO(johnniwinther): Check closures.
-              Expect.equals(element.name, name);
-            } else if (name != element.name) {
-              print("${targetUri}$targetPosition:\n"
-                    "Name '$name' does not match element $element in "
-                    "${sourceFile.filename}$sourcePosition.");
+            AstElement innerElement = findInnermost(element);
+            String expectedName =
+                computeElementNameForSourceMaps(innerElement);
+            if (name != expectedName) {
+              // For the code
+              //    (){}();
+              //    ^
+              // the indicated position is within the scope of the local
+              // function but it is also the position for the invocation of it.
+              // Allow name to be either from the local or from its calling
+              // context.
+              if (innerElement.isLocal && innerElement.isFunction) {
+                var enclosingElement = innerElement.enclosingElement;
+                String expectedName2 =
+                    computeElementNameForSourceMaps(enclosingElement);
+                Expect.isTrue(name == expectedName2,
+                    "Unexpected name '${name}', "
+                    "expected '${expectedName}' for $innerElement "
+                    "or '${expectedName2}' for $enclosingElement.");
+              } else {
+                Expect.equals(expectedName, name,
+                    "Unexpected name '${name}', "
+                    "expected '${expectedName}' or for $innerElement.");
+              }
             }
           }
         }
@@ -266,7 +313,7 @@ class Position {
            line == other.line && column <= other.column;
   }
 
-  String toString() => '[$line,$column]';
+  String toString() => '[${line + 1},${column + 1}]';
 }
 
 class Interval {