Revert "Use a symbol for static length/name (and other Function properties) for Closure, to avoid ES5->ES6 lowering bug (google/closure-compiler#1460)."

This reverts commit a637e1b.

Author: Olivier Chafik

lib/src/codegen/js_codegen.dart

@@ -86,7 +86,6 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
   final _moduleItems = <JS.Statement>[];
   final _temps = new HashMap<Element, JS.TemporaryId>();
   final _qualifiedIds = new List<Tuple2<Element, JS.MaybeQualifiedId>>();
-  final _topLevelExtensionNames = new Set<String>();
 
   /// The name for the library's exports inside itself.
   /// `exports` was chosen as the most similar to ES module patterns.
@@ -169,11 +168,6 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
 
     // Flush any unwritten fields/properties.
     _flushLibraryProperties(_moduleItems);
-    // TODO(ochafik): Refactor to merge this with the dartxNames codepath.
-    if (_topLevelExtensionNames.isNotEmpty) {
-      _moduleItems.add(_emitExtensionNamesDeclaration(
-          _topLevelExtensionNames.map(js.string).toList()));
-    }
 
     // Mark all qualified names as qualified or not, depending on if they need
     // to be loaded lazily or not.
@@ -681,13 +675,6 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
     }
   }
 
-  JS.Statement _emitExtensionNamesDeclaration(List<JS.Expression> names) =>
-      js.statement('dart.defineExtensionNames(#)',
-          [new JS.ArrayInitializer(names.toList(), multiline: true)]);
-
-  JS.Expression _emitExtensionName(String name) =>
-      js.call('dartx.#', _propertyName(name));
-
   _isQualifiedPath(JS.Expression node) =>
       node is JS.Identifier ||
       node is JS.PropertyAccess &&
@@ -732,7 +719,8 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
         }
       }
       if (dartxNames.isNotEmpty) {
-        body.add(_emitExtensionNamesDeclaration(dartxNames));
+        body.add(js.statement('dart.defineExtensionNames(#)',
+            [new JS.ArrayInitializer(dartxNames, multiline: true)]));
       }
     }
 
@@ -2271,7 +2259,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
         isLoaded && (field.isConst || _constField.isFieldInitConstant(field));
 
     var fieldName = field.name.name;
-    if (eagerInit && !JS.invalidStaticFieldName(fieldName, options)) {
+    if (eagerInit && !JS.invalidStaticFieldName(fieldName)) {
       return annotateVariable(
           js.statement('#.# = #;', [
             classElem.name,
@@ -2331,7 +2319,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
           field.element);
     }
 
-    if (eagerInit && !JS.invalidStaticFieldName(fieldName, options)) {
+    if (eagerInit && !JS.invalidStaticFieldName(fieldName)) {
       return annotateVariable(
           js.statement('# = #;', [_visit(field.name), jsInit]), field.element);
     }
@@ -3405,17 +3393,8 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
       bool unary: false,
       bool isStatic: false,
       bool allowExtensions: true}) {
-    // Static members skip the rename steps, except for Function properties.
-    if (isStatic) {
-      // Avoid colliding with names on Function that are disallowed in ES6,
-      // or where we need to work around transpilers.
-      if (invalidStaticFieldName(name, options)) {
-        _topLevelExtensionNames.add(name);
-        return _emitExtensionName(name);
-      } else {
-        return _propertyName(name);
-      }
-    }
+    // Static members skip the rename steps.
+    if (isStatic) return _propertyName(name);
 
     if (name.startsWith('_')) {
       return _privateNames.putIfAbsent(
@@ -3442,7 +3421,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
     if (allowExtensions &&
         _extensionTypes.contains(baseType.element) &&
         !_isObjectProperty(name)) {
-      return _emitExtensionName(name);
+      return js.call('dartx.#', _propertyName(name));
     }
 
     return _propertyName(name);

lib/src/codegen/js_names.dart

@@ -5,7 +5,6 @@
 import 'dart:collection';
 
 import '../js/js_ast.dart';
-import '../options.dart';
 
 /// Unique instance for temporary variables. Will be renamed consistently
 /// across the entire file. Different instances will be named differently
@@ -284,20 +283,14 @@ bool invalidVariableName(String keyword, {bool strictMode: true}) {
   return false;
 }
 
-/// Returns true for invalid static field names in strict mode or for some
-/// transpilers (e.g. when doing ES6->ES5 lowering with the Closure Compiler).
+/// Returns true for invalid static field names in strict mode.
 /// In particular, "caller" "callee" and "arguments" cannot be used.
-bool invalidStaticFieldName(String name, CodegenOptions options) {
+bool invalidStaticFieldName(String name) {
   switch (name) {
     case "arguments":
     case "caller":
     case "callee":
       return true;
-    // Workarounds for Closure:
-    // (see https://github.com/google/closure-compiler/issues/1460)
-    case "name":
-    case "length":
-      return options.closure;
   }
   return false;
 }

test/codegen/closure.dart

@@ -1,6 +1,7 @@
 library test;
 import 'dart:js';
 
+
 typedef void Callback({int i});
 
 class Foo<T> {
@@ -14,7 +15,7 @@ class Foo<T> {
   factory Foo.build() => new Foo(1, null);
 
   untyped_method(a, b) {}
-
+  
   T pass(T t) => t;
 
   String typed_method(
@@ -28,15 +29,6 @@ class Foo<T> {
 
   static named_params(a, {b, c}) {}
 
-  // Avoid colliding with Function.name & Function.length, as Closure fails to
-  // lower these to ES6 (https://github.com/google/closure-compiler/issues/1460)
-  static name() => 'Foo.name()';
-  static length() => 'Foo.length()';
-
-  static arguments() => 'Foo.arguments()';
-  static caller() => 'Foo.caller()';
-  static callee() => 'Foo.callee()';
-
   nullary_method() {}
 
   function_params(int f(x, [y]), g(x, {String y, z}), Callback cb) {
@@ -60,14 +52,8 @@ class Baz extends Foo<int> with Bar {
   Baz(int i) : super(i, 123);
 }
 
-void main(args) {
-  print(Foo.name());
-  print(Foo.length());
-  print(Foo.arguments());
-  print(Foo.caller());
-  print(Foo.callee());
-}
+void main(args) {}
 
 const String some_top_level_constant = "abc";
 final String some_top_level_final = "abc";
-String some_top_level_var = "abc";
+String some_top_level_var = "abc";

test/codegen/expect/closure.js

@@ -67,21 +67,6 @@ dart_library.library('closure', null, /* Imports */[
         let b = opts && 'b' in opts ? opts.b : null;
         let c = opts && 'c' in opts ? opts.c : null;
       }
-      static [dartx.name]() {
-        return 'Foo.name()';
-      }
-      static [dartx.length]() {
-        return 'Foo.length()';
-      }
-      static [dartx.arguments]() {
-        return 'Foo.arguments()';
-      }
-      static [dartx.caller]() {
-        return 'Foo.caller()';
-      }
-      static [dartx.callee]() {
-        return 'Foo.callee()';
-      }
       nullary_method() {}
       /**
        * @param {function(?, ?=):?number} f
@@ -119,15 +104,8 @@ dart_library.library('closure', null, /* Imports */[
         nullary_method: [dart.dynamic, []],
         function_params: [dart.dynamic, [dart.functionType(core.int, [dart.dynamic], [dart.dynamic]), dart.functionType(dart.dynamic, [dart.dynamic], {y: core.String, z: dart.dynamic}), Callback]]
       }),
-      statics: () => ({
-        named_params: [dart.dynamic, [dart.dynamic], {b: dart.dynamic, c: dart.dynamic}],
-        [dartx.name]: [dart.dynamic, []],
-        [dartx.length]: [dart.dynamic, []],
-        [dartx.arguments]: [dart.dynamic, []],
-        [dartx.caller]: [dart.dynamic, []],
-        [dartx.callee]: [dart.dynamic, []]
-      }),
-      names: ['named_params', dartx.name, dartx.length, dartx.arguments, dartx.caller, dartx.callee]
+      statics: () => ({named_params: [dart.dynamic, [dart.dynamic], {b: dart.dynamic, c: dart.dynamic}]}),
+      names: ['named_params']
     });
     /** @final {string} */
     Foo.some_static_constant = "abc";
@@ -151,11 +129,6 @@ dart_library.library('closure', null, /* Imports */[
   });
   /** @param {?} args */
   function main(args) {
-    core.print(Foo[dartx.name]());
-    core.print(Foo[dartx.length]());
-    core.print(Foo[dartx.arguments]());
-    core.print(Foo[dartx.caller]());
-    core.print(Foo[dartx.callee]());
   }
   dart.fn(main, dart.void, [dart.dynamic]);
   /** @final {string} */
@@ -164,13 +137,6 @@ dart_library.library('closure', null, /* Imports */[
   exports.some_top_level_final = "abc";
   /** @type {string} */
   exports.some_top_level_var = "abc";
-  dart.defineExtensionNames([
-    "name",
-    "length",
-    "arguments",
-    "caller",
-    "callee"
-  ]);
   // Exports:
   exports.Callback = Callback;
   exports.Foo$ = Foo$;

test/codegen/expect/names.js

@@ -20,45 +20,40 @@ dart_library.library('names', null, /* Imports */[
   }
   dart.fn(_foo);
   class Frame extends core.Object {
-    [dartx.caller](arguments$) {
+    caller(arguments$) {
       this.arguments = arguments$;
     }
-    static [dartx.callee]() {
+    static callee() {
       return null;
     }
   }
-  dart.defineNamedConstructor(Frame, dartx.caller);
+  dart.defineNamedConstructor(Frame, 'caller');
   dart.setSignature(Frame, {
-    constructors: () => ({[dartx.caller]: [Frame, [core.List]]}),
-    statics: () => ({[dartx.callee]: [dart.dynamic, []]}),
-    names: [dartx.callee]
+    constructors: () => ({caller: [Frame, [core.List]]}),
+    statics: () => ({callee: [dart.dynamic, []]}),
+    names: ['callee']
   });
   class Frame2 extends core.Object {}
   dart.defineLazyProperties(Frame2, {
-    get [dartx.caller]() {
+    get caller() {
       return 100;
     },
-    set [dartx.caller](_) {},
-    get [dartx.arguments]() {
+    set caller(_) {},
+    get arguments() {
       return 200;
     },
-    set [dartx.arguments](_) {}
+    set arguments(_) {}
   });
   function main() {
     core.print(exports.exports);
     core.print(new Foo()[_foo$]());
     core.print(_foo());
-    core.print(new Frame[dartx.caller]([1, 2, 3]));
-    let eval$ = Frame[dartx.callee];
+    core.print(new Frame.caller([1, 2, 3]));
+    let eval$ = Frame.callee;
     core.print(eval$);
-    core.print(dart.notNull(Frame2[dartx.caller]) + dart.notNull(Frame2[dartx.arguments]));
+    core.print(dart.notNull(Frame2.caller) + dart.notNull(Frame2.arguments));
   }
   dart.fn(main);
-  dart.defineExtensionNames([
-    "caller",
-    "callee",
-    "arguments"
-  ]);
   // Exports:
   exports.Foo = Foo;
   exports.Frame = Frame;