fix #29233, final fields can be settable in a mock

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

 // Copyright (c) 2016, 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:collection' show HashMap, HashSet, Queue;
 
-import 'package:analyzer/dart/ast/ast.dart' show Identifier;
 import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/dart/element/type.dart' show InterfaceType;
 import 'package:analyzer/src/dart/element/element.dart' show FieldElementImpl;
 
 import '../js_ast/js_ast.dart' as JS;
 import 'element_helpers.dart';
+import 'extension_types.dart';
 import 'js_names.dart' as JS;
 
 /// Dart allows all fields to be overridden.
 ///
 /// To prevent a performance/code size penalty for allowing this, we analyze
 /// private classes within each library that is being compiled to determine
 /// if those fields should be virtual or not. In effect, we devirtualize fields
 /// when possible by analyzing the class hierarchy and using knowledge of
 /// which members are private and thus, could not be overridden outside of the
 /// current library.
@@ skipping 154 matching lines @@
   /// The set of inherited getters, used because JS getters/setters are paired,
   /// so if we're generating a setter we may need to emit a getter that calls
   /// super.
   final inheritedGetters = new HashSet<String>();
 
   /// The set of inherited setters, used because JS getters/setters are paired,
   /// so if we're generating a getter we may need to emit a setter that calls
   /// super.
   final inheritedSetters = new HashSet<String>();
 
-  ClassPropertyModel.build(VirtualFieldModel fieldModel, ClassElement classElem,
-      Iterable<ExecutableElement> extensionMembers) {
+  final mockMembers = <String, ExecutableElement>{};
+
+  final extensionMembers = new Set<ExecutableElement>();
+  final mixinExtensionMembers = new Set<ExecutableElement>();
+
+  ClassPropertyModel.build(ExtensionTypeSet extensionTypes,
+      VirtualFieldModel fieldModel, ClassElement classElem) {
     // Visit superclasses to collect information about their fields/accessors.
     // This is expensive so we try to collect everything in one pass.
     for (var base in getSuperclasses(classElem)) {
       for (var accessor in base.accessors) {
         // For getter/setter pairs only process them once.
         if (accessor.correspondingGetter != null) continue;
 
         var field = accessor.variable;
-        var name = field.name;
         // Ignore private names from other libraries.
-        if (Identifier.isPrivateName(name) &&
-            accessor.library != classElem.library) {
+        if (field.isPrivate && accessor.library != classElem.library) {
           continue;
         }
 
-        if (field.getter?.isAbstract == false) inheritedGetters.add(name);
-        if (field.setter?.isAbstract == false) inheritedSetters.add(name);
+        if (field.getter?.isAbstract == false) inheritedGetters.add(field.name);
+        if (field.setter?.isAbstract == false) inheritedSetters.add(field.name);
       }
     }
 
-    var extensionNames =
-        new HashSet<String>.from(extensionMembers.map((e) => e.name));
+    _collectMockMembers(classElem.type);
+    _collectExtensionMembers(extensionTypes, classElem);
+
+    var virtualAccessorNames = new HashSet<String>()
+      ..addAll(inheritedGetters)
+      ..addAll(inheritedSetters)
+      ..addAll(extensionMembers
+          .map((m) => m is PropertyAccessorElement ? m.variable.name : m.name))
+      ..addAll(mockMembers.values
+          .map((m) => m is PropertyAccessorElement ? m.variable.name : m.name));
 
     // Visit accessors in the current class, and see if they need to be
     // generated differently based on the inherited fields/accessors.
     for (var accessor in classElem.accessors) {
       // For getter/setter pairs only process them once.
       if (accessor.correspondingGetter != null) continue;
       // Also ignore abstract fields.
       if (accessor.isAbstract) continue;
 
       var field = accessor.variable;
       var name = field.name;
       // Is it a field?
       if (!field.isSynthetic && field is FieldElementImpl) {
-        if (inheritedGetters.contains(name) ||
-            inheritedSetters.contains(name) ||
-            extensionNames.contains(name) ||
+        if (virtualAccessorNames.contains(name) ||
             fieldModel.isVirtual(field)) {
           if (field.isStatic) {
             staticFieldOverrides.add(field);
           } else {
             virtualFields[field] = new JS.TemporaryId(name);
           }
         }
       }
     }
   }
+
+  void _collectMockMembers(InterfaceType type) {
+    // TODO(jmesserly): every type with nSM will generate new stubs for all
+    // abstract members. For example:
+    //
+    //     class C { m(); noSuchMethod(...) { ... } }
+    //     class D extends C { m(); noSuchMethod(...) { ... } }
+    //
+    // We'll generate D.m even though it is not necessary.
+    //
+    // Doing better is a bit tricky, as our current codegen strategy for the
+    // mock methods encodes information about the number of arguments (and type
+    // arguments) that D expects.
+    var element = type.element;
+    if (!hasNoSuchMethod(element)) return;
+
+    // Collect all unimplemented members.
+    //
+    // Initially, we track abstract and concrete members separately, then
+    // remove concrete from the abstract set. This is done because abstract
+    // members are allowed to "override" concrete ones in Dart.

[vsm, 2017/04/07 20:32:02]

Unrelated to this CL since the code just moved, but does this cause a problem if
the abstract override has a looser signature?  

[Jennifer Messerly, 2017/04/07 20:50:22]

Hmmm quite possibly! I'll see if I can reproduce that and if so, file a bug
about it. Good eye -- you may have found a soundness bug! :)

+    // (In that case, it will still be treated as a concrete member and can be
+    // called at runtime.)
+    var concreteMembers = new HashSet<String>();
+
+    void visit(InterfaceType type, bool isAbstract) {
+      if (type == null) return;
+      visit(type.superclass, isAbstract);
+      for (var m in type.mixins) visit(m, isAbstract);
+      for (var i in type.interfaces) visit(i, true);
+
+      for (var m in [type.methods, type.accessors].expand((m) => m)) {
+        if (isAbstract || m.isAbstract) {
+          mockMembers[m.name] = m;
+        } else if (!m.isStatic) {
+          concreteMembers.add(m.name);
+        }
+      }
+    }
+
+    visit(type, false);
+
+    concreteMembers.forEach(mockMembers.remove);
+  }
+
+  void _collectExtensionMembers(
+      ExtensionTypeSet extensionTypes, ClassElement element) {
+    if (extensionTypes.isNativeClass(element)) return;
+
+    // Collect all extension types we implement.
+    var type = element.type;
+    var types = extensionTypes.collectNativeInterfaces(element);
+    if (types.isEmpty) return;
+
+    // Collect all possible extension method names.
+    var possibleExtensions = new HashSet<String>();
+    for (var t in types) {
+      for (var m in [t.methods, t.accessors].expand((m) => m)) {
+        if (!m.isStatic && m.isPublic) possibleExtensions.add(m.name);
+      }
+    }
+
+    // Collect all of extension methods this type and its mixins implement.
+
+    void visitType(InterfaceType type, bool isMixin) {
+      for (var mixin in type.mixins) {
+        // If the mixin isn't native, make sure to visit it too, because those
+        // methods haven't been accounted for yet.
+        if (!extensionTypes.hasNativeSubtype(mixin)) visitType(mixin, true);
+      }
+      for (var m in [type.methods, type.accessors].expand((m) => m)) {
+        if (!m.isAbstract && possibleExtensions.contains(m.name)) {
+          (isMixin ? mixinExtensionMembers : extensionMembers).add(m);
+        }
+      }
+    }
+
+    visitType(type, false);
+
+    for (var m in mockMembers.values) {
+      if (possibleExtensions.contains(m.name)) extensionMembers.add(m);
+    }
+  }
 }