Add minified names to sourcemaps

This change:

* introduces a sourcemap extension to store minified names from the global and
instance namespaces.

* adds a test suite to test that this information is accurate

This is just an initial step: at this time names come from proposed names, which
include extra stuff we don't want. On a later CL I plan to change this to be
able to get the original name instead.

For example, I want to add a test for translating an instance method, right now
we would get "@method@0@" or "method$0" instead of "method".

Change-Id: I411c3bea96446e29e80581750250349799332b9a
Reviewed-on: https://dart-review.googlesource.com/65660
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>

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

@@ -24,8 +24,17 @@ class SourceMapBuilder {
   final LocationProvider locationProvider;
   final List<SourceMapEntry> entries = new List<SourceMapEntry>();
 
-  SourceMapBuilder(this.version, this.sourceMapUri, this.targetFileUri,
-      this.locationProvider);
+  /// Extension used to deobfuscate minified names in error messages.
+  final Map<String, String> minifiedGlobalNames;
+  final Map<String, String> minifiedInstanceNames;
+
+  SourceMapBuilder(
+      this.version,
+      this.sourceMapUri,
+      this.targetFileUri,
+      this.locationProvider,
+      this.minifiedGlobalNames,
+      this.minifiedInstanceNames);
 
   void addMapping(int targetOffset, SourceLocation sourceLocation) {
     entries.add(new SourceMapEntry(sourceLocation, targetOffset));
@@ -87,6 +96,9 @@ class SourceMapBuilder {
       }
     });
 
+    minifiedGlobalNames.values.forEach(nameMap.register);
+    minifiedInstanceNames.values.forEach(nameMap.register);
+
     StringBuffer mappingsBuffer = new StringBuffer();
     writeEntries(lineColumnMap, uriMap, nameMap, mappingsBuffer);
 
@@ -112,7 +124,15 @@ class SourceMapBuilder {
     buffer.write(',\n');
     buffer.write('  "mappings": "');
     buffer.write(mappingsBuffer);
-    buffer.write('"\n}\n');
+    buffer.write('",\n');
+    buffer.write('  "x_org_dartlang_dart2js": {\n');
+    buffer.write('    "minified_names": {\n');
+    buffer.write('      "global": ');
+    writeMinifiedNames(minifiedGlobalNames, nameMap, buffer);
+    buffer.write(',\n');
+    buffer.write('      "instance": ');
+    writeMinifiedNames(minifiedInstanceNames, nameMap, buffer);
+    buffer.write('\n    }\n  }\n}\n');
     return buffer.toString();
   }
 
@@ -170,6 +190,22 @@ class SourceMapBuilder {
     });
   }
 
+  void writeMinifiedNames(Map<String, String> minifiedNames,
+      IndexMap<String> nameMap, StringBuffer buffer) {
+    bool first = true;
+    buffer.write('{');
+    minifiedNames.forEach((String minifiedName, String name) {
+      if (!first) buffer.write(',');
+      buffer.write('"');
+      writeJsonEscapedCharsOn(minifiedName, buffer);
+      buffer.write('"');
+      buffer.write(':');
+      buffer.write(nameMap[name]);
+      first = false;
+    });
+    buffer.write('}');
+  }
+
   /// Returns the source map tag to put at the end a .js file in [fileUri] to
   /// make it point to the source map file in [sourceMapUri].
   static String generateSourceMapTag(Uri sourceMapUri, Uri fileUri) {
@@ -191,6 +227,8 @@ class SourceMapBuilder {
   static void outputSourceMap(
       SourceLocationsProvider sourceLocationsProvider,
       LocationProvider locationProvider,
+      Map<String, String> minifiedGlobalNames,
+      Map<String, String> minifiedInstanceNames,
       String name,
       Uri sourceMapUri,
       Uri fileUri,
@@ -201,7 +239,12 @@ class SourceMapBuilder {
     sourceLocationsProvider.sourceLocations
         .forEach((SourceLocations sourceLocations) {
       SourceMapBuilder sourceMapBuilder = new SourceMapBuilder(
-          sourceLocations.name, sourceMapUri, fileUri, locationProvider);
+          sourceLocations.name,
+          sourceMapUri,
+          fileUri,
+          locationProvider,
+          minifiedGlobalNames,
+          minifiedInstanceNames);
       sourceLocations.forEachSourceLocation(sourceMapBuilder.addMapping);
       String sourceMap = sourceMapBuilder.build();
       String extension = 'js.map';

pkg/compiler/lib/src/js_backend/namer.dart

@@ -521,6 +521,21 @@ class Namer {
   final Map<String, jsAst.Name> internalGlobals =
       new HashMap<String, jsAst.Name>();
 
+  Map<String, String> createMinifiedGlobalNameMap() {
+    var map = <String, String>{};
+    userGlobals.forEach((entity, jsName) {
+      // Non-finalized names are not present in the output program
+      if (jsName is TokenName && !jsName.isFinalized) return;
+      map[jsName.name] = entity.name;
+    });
+    internalGlobals.forEach((name, jsName) {
+      // Non-finalized names are not present in the output program
+      if (jsName is TokenName && !jsName.isFinalized) return;
+      map[jsName.name] = name;
+    });
+    return map;
+  }
+
   /// Used disambiguated names in the instance namespace, issued by
   /// [_disambiguateMember], [_disambiguateInternalMember],
   /// [_disambiguateOperator], and [reservePublicMemberName].
@@ -532,6 +547,29 @@ class Namer {
   final Map<String, jsAst.Name> userInstanceOperators =
       new HashMap<String, jsAst.Name>();
 
+  Map<String, String> createMinifiedInstanceNameMap() {
+    var map = <String, String>{};
+    internalInstanceMembers.forEach((entity, jsName) {
+      // Non-finalized names are not present in the output program
+      if (jsName is TokenName && !jsName.isFinalized) return;
+      map[jsName.name] = entity.name;
+    });
+    userInstanceMembers.forEach((name, jsName) {
+      // Non-finalized names are not present in the output program
+      if (jsName is TokenName && !jsName.isFinalized) return;
+      map[jsName.name] = name;
+    });
+
+    // TODO(sigmund): reverse the operator names back to the original Dart
+    // names.
+    userInstanceOperators.forEach((name, jsName) {
+      // Non-finalized names are not present in the output program
+      if (jsName is TokenName && !jsName.isFinalized) return;
+      map[jsName.name] = name;
+    });
+    return map;
+  }
+
   /// Used to disambiguate names for constants in [constantName].
   final NamingScope constantScope = new NamingScope();
 

pkg/compiler/lib/src/js_emitter/full_emitter/emitter.dart

@@ -1304,6 +1304,8 @@ class Emitter extends js_emitter.EmitterBase {
       SourceMapBuilder.outputSourceMap(
           mainOutput,
           locationCollector,
+          namer.createMinifiedGlobalNameMap(),
+          namer.createMinifiedInstanceNameMap(),
           '',
           compiler.options.sourceMapUri,
           compiler.options.outputUri,
@@ -1677,8 +1679,15 @@ class Emitter extends js_emitter.EmitterBase {
 
         output.add(SourceMapBuilder.generateSourceMapTag(mapUri, partUri));
         output.close();
-        SourceMapBuilder.outputSourceMap(output, locationCollector, partName,
-            mapUri, partUri, compiler.outputProvider);
+        SourceMapBuilder.outputSourceMap(
+            output,
+            locationCollector,
+            namer.createMinifiedGlobalNameMap(),
+            namer.createMinifiedInstanceNameMap(),
+            partName,
+            mapUri,
+            partUri,
+            compiler.outputProvider);
       } else {
         output.close();
       }

pkg/compiler/lib/src/js_emitter/startup_emitter/model_emitter.dart

@@ -296,6 +296,8 @@ class ModelEmitter {
       SourceMapBuilder.outputSourceMap(
           mainOutput,
           locationCollector,
+          namer.createMinifiedGlobalNameMap(),
+          namer.createMinifiedInstanceNameMap(),
           '',
           compiler.options.sourceMapUri,
           compiler.options.outputUri,
@@ -380,8 +382,15 @@ class ModelEmitter {
 
       output.add(SourceMapBuilder.generateSourceMapTag(mapUri, partUri));
       output.close();
-      SourceMapBuilder.outputSourceMap(output, locationCollector, partName,
-          mapUri, partUri, compiler.outputProvider);
+      SourceMapBuilder.outputSourceMap(
+          output,
+          locationCollector,
+          namer.createMinifiedGlobalNameMap(),
+          namer.createMinifiedInstanceNameMap(),
+          partName,
+          mapUri,
+          partUri,
+          compiler.outputProvider);
     } else {
       output.close();
     }

tests/compiler/dart2js/generic_methods/generic_method_test.dart

@@ -174,12 +174,13 @@ noSuchMethod: Class2.method6<int>
 
 main(List<String> args) {
   asyncTest(() async {
-    Compiler compiler = await runWithD8(memorySourceFiles: {
+    D8Result result = await runWithD8(memorySourceFiles: {
       'main.dart': SOURCE
     }, options: [
       Flags.strongMode,
       Flags.disableRtiOptimization,
     ], expectedOutput: OUTPUT, printJs: args.contains('-v'));
+    Compiler compiler = result.compilationResult.compiler;
     JClosedWorld closedWorld = compiler.backendClosedWorldForTesting;
     ElementEnvironment elementEnvironment = closedWorld.elementEnvironment;
 

tests/compiler/dart2js/helpers/d8_helper.dart

@@ -10,7 +10,6 @@ import 'dart:async';
 import 'dart:io';
 
 import 'package:compiler/compiler_new.dart';
-import 'package:compiler/src/compiler.dart';
 import 'package:compiler/src/dart2js.dart' as dart2js;
 import 'package:compiler/src/filenames.dart';
 import 'package:expect/expect.dart';
@@ -32,7 +31,7 @@ Future createTemp(Uri entryPoint, Map<String, String> memorySourceFiles,
   return entryPoint;
 }
 
-Future<Compiler> runWithD8(
+Future<D8Result> runWithD8(
     {Uri entryPoint,
     Map<String, String> memorySourceFiles: const <String, String>{},
     List<String> options: const <String>[],
@@ -70,5 +69,13 @@ Future<Compiler> runWithD8(
     Expect.stringEquals(expectedOutput.trim(),
         runResult.stdout.replaceAll('\r\n', '\n').trim());
   }
-  return result.compiler;
+  return new D8Result(result, runResult, output);
+}
+
+class D8Result {
+  final CompilationResult compilationResult;
+  final ProcessResult runResult;
+  final String outputPath;
+
+  D8Result(this.compilationResult, this.runResult, this.outputPath);
 }

tests/compiler/dart2js/sourcemaps/minified/instance.dart

@@ -0,0 +1,13 @@
+// Copyright (c) 2018, 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.
+
+// Error pattern: Instance of '([^']*)'
+// Kind of minified name: global
+// Expected deobfuscated name: MyClass
+
+main() {
+  throw new MyClass();
+}
+
+class MyClass {}

tests/compiler/dart2js/sourcemaps/minified_names_test.dart

@@ -0,0 +1,136 @@
+// Copyright (c) 2018, 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.
+
+import 'dart:async';
+import 'dart:io';
+import 'dart:convert';
+
+import 'package:args/args.dart';
+import 'package:async_helper/async_helper.dart';
+import 'package:compiler/src/commandline_options.dart';
+
+import '../helpers/d8_helper.dart';
+import 'package:expect/expect.dart';
+
+void main(List<String> args) {
+  ArgParser argParser = new ArgParser(allowTrailingOptions: true);
+  argParser.addFlag('continued', abbr: 'c', defaultsTo: false);
+  ArgResults argResults = argParser.parse(args);
+  Directory dataDir =
+      new Directory.fromUri(Platform.script.resolve('minified'));
+  asyncTest(() async {
+    bool continuing = false;
+    await for (FileSystemEntity entity in dataDir.list()) {
+      String name = entity.uri.pathSegments.last;
+      if (!name.endsWith('.dart')) continue;
+      if (argResults.rest.isNotEmpty &&
+          !argResults.rest.contains(name) &&
+          !continuing) {
+        continue;
+      }
+      print('----------------------------------------------------------------');
+      print('Checking ${entity.uri}');
+      print('----------------------------------------------------------------');
+      await runTest(await new File.fromUri(entity.uri).readAsString());
+      if (argResults['continued']) continuing = true;
+    }
+  });
+}
+
+// Object to hold the expectations of a individual minified-name test.
+class MinifiedNameTest {
+  /// Pattern used to find a minified name in the error message.
+  final RegExp pattern;
+
+  /// The kind of minified name, it can be global, instance, or other
+  /// (the first two correspond to the two namespaces that contain extra data in
+  /// the source-map file).
+  final String _kind;
+
+  /// Whether the minified name is from the global namespace.
+  bool get isGlobal => _kind == 'global';
+
+  /// Whether the minified name is from the instance namespace.
+  bool get isInstance => _kind == 'instance';
+
+  /// The deobfuscated name we expect to find.
+  final String expectedName;
+
+  /// The actual test code.
+  final String code;
+
+  MinifiedNameTest(this.pattern, this._kind, this.expectedName, this.code);
+}
+
+RegExp _patternMatcher = new RegExp("// Error pattern: (.*)\n");
+RegExp _kindMatcher = new RegExp("// Kind of minified name: (.*)\n");
+RegExp _nameMatcher = new RegExp("// Expected deobfuscated name: (.*)\n");
+
+Future runTest(String code) async {
+  var patternMatch = _patternMatcher.firstMatch(code);
+  Expect.isNotNull(patternMatch, "Could not find the error pattern.");
+  var pattern = new RegExp(patternMatch.group(1));
+  var kindMatch = _kindMatcher.firstMatch(code);
+  Expect.isNotNull(kindMatch, "Could not find the expected minified kind.");
+  var kind = kindMatch.group(1);
+
+  // TODO(sigmund): add support for "other" when we encode symbol information
+  // directly for each field and local variable.
+  const validKinds = const ['global', 'instance'];
+  Expect.isTrue(validKinds.contains(kind),
+      "Invalid kind: $kind, please use one of $validKinds");
+
+  var nameMatch = _nameMatcher.firstMatch(code);
+  Expect.isNotNull(nameMatch, "Could not find the expected deobfuscated name.");
+  var expectedName = nameMatch.group(1);
+  var test = new MinifiedNameTest(pattern, kind, expectedName, code);
+  print(test.code);
+  await checkExpectation(test, false);
+  await checkExpectation(test, true);
+}
+
+checkExpectation(MinifiedNameTest test, bool minified) async {
+  print('-- ${minified ? 'minified' : 'not-minified'}  '
+      '-----------------------------------------------');
+  D8Result result = await runWithD8(
+      memorySourceFiles: {'main.dart': test.code},
+      options: minified ? [Flags.minify] : []);
+  String stdout = result.runResult.stdout;
+  String error = _extractError(stdout);
+  Expect.isNotNull(error, 'Couldn\'t find the error message in $stdout');
+
+  var match = test.pattern.firstMatch(error);
+  Expect.isNotNull(
+      match,
+      'Error didn\'t match the test pattern'
+      '\nerror: $error\npattern:${test.pattern}');
+  var name = match.group(1);
+  Expect.isNotNull(name, 'Error didn\'t contain a name\nerror: $error');
+
+  var sourceMap = '${result.outputPath}.map';
+  var json = jsonDecode(await new File(sourceMap).readAsString());
+
+  var extensions = json['x_org_dartlang_dart2js'];
+  Expect.isNotNull(extensions, "Source-map doesn't contain dart2js extensions");
+  var minifiedNames = extensions['minified_names'];
+  Expect.isNotNull(minifiedNames, "Source-map doesn't contain minified-names");
+
+  if (test.isGlobal) {
+    var actualName = json['names'][minifiedNames['global'][name]];
+    Expect.equals(test.expectedName, actualName);
+  } else if (test.isInstance) {
+    var actualName = json['names'][minifiedNames['instance'][name]];
+    Expect.equals(test.expectedName, actualName);
+  } else {
+    Expect.fail('unexpected');
+  }
+}
+
+String _extractError(String stdout) {
+  var firstStackFrame = stdout.indexOf('\n    at');
+  if (firstStackFrame == -1) return null;
+  var prevLine = stdout.lastIndexOf('\n', firstStackFrame - 1);
+  if (prevLine == -1) return null;
+  return stdout.substring(prevLine + 1, firstStackFrame);
+}