[dartdevc] fix dart-lang#33621, name collision due to inheriting JS s…

…tatics

Change-Id: I6e3f0bb741e919d8303422ac2df853142649e7ff
Reviewed-on: https://dart-review.googlesource.com/c/83563
Commit-Queue: Jenny Messerly <jmesserly@google.com>
Auto-Submit: Jenny Messerly <jmesserly@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>

pkg/dev_compiler/lib/src/analyzer/code_generator.dart

@@ -65,7 +65,7 @@ import 'type_utilities.dart';
 // expressions (which result in JS.Expression) and statements
 // (which result in (JS.Statement).
 class CodeGenerator extends Object
-    with NullableTypeInference, SharedCompiler<LibraryElement>
+    with NullableTypeInference, SharedCompiler<LibraryElement, ClassElement>
     implements AstVisitor<JS.Node> {
   final SummaryDataStore summaryData;
 
@@ -1318,8 +1318,8 @@ class CodeGenerator extends Object
           ctorBody
               .add(_emitSuperConstructorCall(className, ctor.name, jsParams));
         }
-        body.add(_addConstructorToClass(
-            className, ctor.name, JS.Fun(jsParams, JS.Block(ctorBody))));
+        body.add(_addConstructorToClass(classElem, className, ctor.name,
+            JS.Fun(jsParams, JS.Block(ctorBody))));
       }
     }
 
@@ -1879,7 +1879,7 @@ class CodeGenerator extends Object
     }
 
     addConstructor(String name, JS.Expression jsCtor) {
-      body.add(_addConstructorToClass(className, name, jsCtor));
+      body.add(_addConstructorToClass(classElem, className, name, jsCtor));
     }
 
     if (classElem.isEnum) {
@@ -1943,12 +1943,31 @@ class CodeGenerator extends Object
             c.isSynthetic && c.name != '' || c.isFactory || c.isExternal);
   }
 
-  JS.Statement _addConstructorToClass(
-      JS.Expression className, String name, JS.Expression jsCtor) {
-    jsCtor = defineValueOnClass(className, _constructorName(name), jsCtor);
+  JS.Statement _addConstructorToClass(ClassElement c, JS.Expression className,
+      String name, JS.Expression jsCtor) {
+    jsCtor = defineValueOnClass(c, className, _constructorName(name), jsCtor);
     return js.statement('#.prototype = #.prototype;', [jsCtor, className]);
   }
 
+  @override
+  bool superclassHasStatic(ClassElement c, String name) {
+    // Note: because we're only considering statics, we can ignore mixins.
+    // We're only trying to find conflicts due to JS inheriting statics.
+    var library = c.library;
+    while (true) {
+      var supertype = c.supertype;
+      if (supertype == null) return false;
+      c = supertype.element;
+      for (var members in [c.methods, c.accessors]) {
+        for (var m in members) {
+          if (m.isStatic && m.name == name && m.isAccessibleIn(library)) {
+            return true;
+          }
+        }
+      }
+    }
+  }
+
   /// Emits static fields for a class, and initialize them eagerly if possible,
   /// otherwise define them as lazy properties.
   void _emitStaticFields(ClassElement classElem,
@@ -1957,7 +1976,7 @@ class CodeGenerator extends Object
       // Emit enum static fields
       var type = classElem.type;
       void addField(FieldElement e, JS.Expression value) {
-        body.add(defineValueOnClass(_emitStaticClassName(classElem),
+        body.add(defineValueOnClass(classElem, _emitStaticClassName(classElem),
                 _declareMemberName(e.getter), value)
             .toStatement());
       }
@@ -2252,8 +2271,8 @@ class CodeGenerator extends Object
     var parameters = element.parameters
         .map((p) => ParameterElementImpl.synthetic(
             p.name,
-            // ignore: deprecated_member_use
             _isCovariant(p) ? objectClass.type : p.type,
+            // ignore: deprecated_member_use
             p.parameterKind))
         .toList();
 

pkg/dev_compiler/lib/src/compiler/shared_compiler.dart

@@ -13,7 +13,7 @@ import '../js_ast/js_ast.dart' show js;
 ///
 /// This class should only implement functionality that depends purely on JS
 /// classes, rather than on Analyzer/Kernel types.
-abstract class SharedCompiler<Library> {
+abstract class SharedCompiler<Library, Class> {
   /// When inside a `[]=` operator, this will be a non-null value that should be
   /// returned by any `return;` statement.
   ///
@@ -141,21 +141,26 @@ abstract class SharedCompiler<Library> {
     });
   }
 
-  /// Emits an expression to set the property [name] on the class [className],
+  /// Emits an expression to set the property [nameExpr] on the class [className],
   /// with [value].
   ///
   /// This will use `className.name = value` if possible, otherwise it will use
   /// `dart.defineValue(className, name, value)`. This is required when
   /// `Function.prototype` already defins a getters with the same name.
-  JS.Expression defineValueOnClass(
-      JS.Expression className, JS.Expression name, JS.Expression value) {
-    var args = [className, name, value];
-    if (name is JS.LiteralString &&
-        JS.isFunctionPrototypeGetter(name.valueWithoutQuotes)) {
-      return runtimeCall('defineValue(#, #, #)', args);
+  JS.Expression defineValueOnClass(Class c, JS.Expression className,
+      JS.Expression nameExpr, JS.Expression value) {
+    var args = [className, nameExpr, value];
+    if (nameExpr is JS.LiteralString) {
+      var name = nameExpr.valueWithoutQuotes;
+      if (JS.isFunctionPrototypeGetter(name) || superclassHasStatic(c, name)) {
+        return runtimeCall('defineValue(#, #, #)', args);
+      }
     }
     return js.call('#.# = #', args);
   }
+
+  /// Whether any superclass of [c] defines a static [name].
+  bool superclassHasStatic(Class c, String name);
 }
 
 /// Whether a variable with [name] is referenced in the [node].

pkg/dev_compiler/lib/src/js_ast/builder.dart

@@ -663,7 +663,7 @@ class MiniJsParser {
 
   void getToken() {
     skippedNewline = false;
-    for (;;) {
+    while (true) {
       if (position >= src.length) break;
       int code = src.codeUnitAt(position);
       //  Skip '//' and '/*' style comments.
@@ -990,7 +990,7 @@ class MiniJsParser {
 
     expectCategory(LPAREN);
     if (!acceptCategory(RPAREN)) {
-      for (;;) {
+      while (true) {
         if (acceptCategory(ELLIPSIS)) {
           params.add(RestParameter(parseParameter()));
           expectCategory(RPAREN);
@@ -1039,7 +1039,7 @@ class MiniJsParser {
 
   Expression parseObjectInitializer() {
     List<Property> properties = <Property>[];
-    for (;;) {
+    while (true) {
       if (acceptCategory(RBRACE)) break;
       // Limited subset of ES6 object initializers.
       //

pkg/dev_compiler/lib/src/kernel/compiler.dart

@@ -32,7 +32,7 @@ import 'property_model.dart';
 import 'type_table.dart';
 
 class ProgramCompiler extends Object
-    with SharedCompiler<Library>
+    with SharedCompiler<Library, Class>
     implements
         StatementVisitor<JS.Statement>,
         ExpressionVisitor<JS.Expression>,
@@ -773,7 +773,7 @@ class ProgramCompiler extends Object
           ctorBody.add(_emitSuperConstructorCall(className, name, jsParams));
         }
         body.add(_addConstructorToClass(
-            className, name, JS.Fun(jsParams, JS.Block(ctorBody))));
+            c, className, name, JS.Fun(jsParams, JS.Block(ctorBody))));
       }
     }
 
@@ -877,7 +877,7 @@ class ProgramCompiler extends Object
     }
 
     addConstructor(String name, JS.Expression jsCtor) {
-      body.add(_addConstructorToClass(className, name, jsCtor));
+      body.add(_addConstructorToClass(c, className, name, jsCtor));
     }
 
     var fields = c.fields;
@@ -1141,6 +1141,7 @@ class ProgramCompiler extends Object
       for (var f in fields) {
         assert(f.isConst);
         body.add(defineValueOnClass(
+                c,
                 classRef,
                 _emitStaticMemberName(f.name.name),
                 _visitInitializer(f.initializer, f.annotations))
@@ -1600,11 +1601,28 @@ class ProgramCompiler extends Object
   }
 
   JS.Statement _addConstructorToClass(
-      JS.Expression className, String name, JS.Expression jsCtor) {
-    jsCtor = defineValueOnClass(className, _constructorName(name), jsCtor);
+      Class c, JS.Expression className, String name, JS.Expression jsCtor) {
+    jsCtor = defineValueOnClass(c, className, _constructorName(name), jsCtor);
     return js.statement('#.prototype = #.prototype;', [jsCtor, className]);
   }
 
+  @override
+  bool superclassHasStatic(Class c, String memberName) {
+    // Note: because we're only considering statics, we can ignore mixins.
+    // We're only trying to find conflicts due to JS inheriting statics.
+    var name = Name(memberName, c.enclosingLibrary);
+    while (true) {
+      c = c.superclass;
+      if (c == null) return false;
+      for (var m in c.members) {
+        if (m.name == name &&
+            (m is Procedure && m.isStatic || m is Field && m.isStatic)) {
+          return true;
+        }
+      }
+    }
+  }
+
   List<JS.Method> _emitClassMethods(Class c) {
     var virtualFields = _classProperties.virtualFields;
 

tests/language_2/static_const_field_reserved_name_test.dart

@@ -3,7 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 // Dart test for testing redefinition of reserved names as static const fields.
-// Bug #587.
+// Issue https://github.com/dart-archive/dev_compiler/issues/587
 
 import "package:expect/expect.dart";
 
@@ -17,6 +17,19 @@ class StaticConstFieldReservedNameTest {
   }
 }
 
+class Foo {
+  int get foo => 42;
+  static final baz = new Foo();
+}
+
+class Bar extends Foo {
+  get foo => 123;
+  Bar.baz();
+}
+
 void main() {
   StaticConstFieldReservedNameTest.testMain();
+
+  // Regression test for https://github.com/dart-lang/sdk/issues/33621
+  Expect.equals(Bar.baz().foo, 123);
 }