Check field overrides

pkg/analyzer/lib/src/task/strong/checker.dart

 // 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.
 
 // TODO(jmesserly): this was ported from package:dev_compiler, and needs to be
 // refactored to fit into analyzer.
 library analyzer.src.task.strong.checker;
 
 import 'package:analyzer/analyzer.dart';
 import 'package:analyzer/src/generated/ast.dart';
@@ skipping 33 matching lines @@
     var parent = type.superclass;
     var mixins = type.mixins;
 
     // Check overrides from applying mixins
     for (int i = 0; i < mixins.length; i++) {
       var seen = new Set<String>();
       var current = mixins[i];
       var errorLocation = node.withClause.mixinTypes[i];
       for (int j = i - 1; j >= 0; j--) {
         _checkIndividualOverridesFromType(
-            current, mixins[j], errorLocation, seen);
+            current, mixins[j], errorLocation, seen, true);
       }
-      _checkIndividualOverridesFromType(current, parent, errorLocation, seen);
+      _checkIndividualOverridesFromType(
+          current, parent, errorLocation, seen, true);
     }
   }
 
   /// Check overrides between a class and its superclasses and mixins. For
   /// example, in:
   ///
   ///      A extends B with E, F
   ///
   /// we check A against B, B super classes, E, and F.
   ///
@@ skipping 11 matching lines @@
   ///     }
   ///     class Test extends Parent {
   ///         m(B a) {} // invalid override
   ///     }
   void _checkSuperOverrides(ClassDeclaration node) {
     var seen = new Set<String>();
     var current = node.element.type;
     var visited = new Set<InterfaceType>();
     do {
       visited.add(current);
-      current.mixins.reversed
-          .forEach((m) => _checkIndividualOverridesFromClass(node, m, seen));
-      _checkIndividualOverridesFromClass(node, current.superclass, seen);
+      current.mixins.reversed.forEach(
+          (m) => _checkIndividualOverridesFromClass(node, m, seen, true));
+      _checkIndividualOverridesFromClass(node, current.superclass, seen, true);
       current = current.superclass;
     } while (!current.isObject && !visited.contains(current));
   }
 
   /// Checks that implementations correctly override all reachable interfaces.
   /// In particular, we need to check these overrides for the definitions in
   /// the class itself and each its superclasses. If a superclass is not
   /// abstract, then we can skip its transitive interfaces. For example, in:
   ///
   ///     B extends C implements G
@@ skipping 63 matching lines @@
     } else if (visited.contains(type)) {
       // Malformed type.
       return;
     } else {
       visited.add(type);
     }
 
     // Check direct overrides on [type]
     for (var interfaceType in interfaces) {
       if (node != null) {
-        _checkIndividualOverridesFromClass(node, interfaceType, seen);
+        _checkIndividualOverridesFromClass(node, interfaceType, seen, false);
       } else {
         _checkIndividualOverridesFromType(
-            type, interfaceType, errorLocation, seen);
+            type, interfaceType, errorLocation, seen, false);
       }
     }
 
     // Check overrides from its mixins
     for (int i = 0; i < type.mixins.length; i++) {
       var loc =
           errorLocation != null ? errorLocation : node.withClause.mixinTypes[i];
       for (var interfaceType in interfaces) {
         // We copy [seen] so we can report separately if more than one mixin or
         // the base class have an invalid override.
         _checkIndividualOverridesFromType(
-            type.mixins[i], interfaceType, loc, new Set.from(seen));
+            type.mixins[i], interfaceType, loc, new Set.from(seen), false);
       }
     }
 
     // Check overrides from its superclasses
     if (includeParents) {
       var parent = type.superclass;
       if (parent.isObject) return;
       var loc = errorLocation != null ? errorLocation : node.extendsClause;
       // No need to copy [seen] here because we made copies above when reporting
       // errors on mixins.
       _checkInterfacesOverrides(parent, interfaces, seen,
           visited: visited, includeParents: true, errorLocation: loc);
     }
   }
 
   /// Check that individual methods and fields in [subType] correctly override
   /// the declarations in [baseType].
   ///
   /// The [errorLocation] node indicates where errors are reported, see
   /// [_checkSingleOverride] for more details.
   ///
   /// The set [seen] is used to avoid reporting overrides more than once. It
   /// is used when invoking this function multiple times when checking several
   /// types in a class hierarchy. Errors are reported only the first time an
   /// invalid override involving a specific member is encountered.
-  _checkIndividualOverridesFromType(InterfaceType subType,
-      InterfaceType baseType, AstNode errorLocation, Set<String> seen) {
+  _checkIndividualOverridesFromType(
+      InterfaceType subType,
+      InterfaceType baseType,
+      AstNode errorLocation,
+      Set<String> seen,
+      bool isSubclass) {
     void checkHelper(ExecutableElement e) {
       if (e.isStatic) return;
       if (seen.contains(e.name)) return;
-      if (_checkSingleOverride(e, baseType, null, errorLocation)) {
+      if (_checkSingleOverride(e, baseType, null, errorLocation, isSubclass)) {
         seen.add(e.name);
       }
     }
     subType.methods.forEach(checkHelper);
     subType.accessors.forEach(checkHelper);
   }
 
   /// Check that individual methods and fields in [subType] correctly override
   /// the declarations in [baseType].
   ///
   /// The [errorLocation] node indicates where errors are reported, see
   /// [_checkSingleOverride] for more details.
-  _checkIndividualOverridesFromClass(
-      ClassDeclaration node, InterfaceType baseType, Set<String> seen) {
+  _checkIndividualOverridesFromClass(ClassDeclaration node,
+      InterfaceType baseType, Set<String> seen, bool isSubclass) {
     for (var member in node.members) {
       if (member is ConstructorDeclaration) continue;
       if (member is FieldDeclaration) {
         if (member.isStatic) continue;
         for (var variable in member.fields.variables) {
           var element = variable.element as PropertyInducingElement;
           var name = element.name;
           if (seen.contains(name)) continue;
           var getter = element.getter;
           var setter = element.setter;
-          bool found = _checkSingleOverride(getter, baseType, variable, member);
+          bool found = _checkSingleOverride(
+              getter, baseType, variable, member, isSubclass);
           if (!variable.isFinal &&
               !variable.isConst &&
-              _checkSingleOverride(setter, baseType, variable, member)) {
+              _checkSingleOverride(
+                  setter, baseType, variable, member, isSubclass)) {
             found = true;
           }
           if (found) seen.add(name);
         }
       } else {
         if ((member as MethodDeclaration).isStatic) continue;
         var method = (member as MethodDeclaration).element;
         if (seen.contains(method.name)) continue;
-        if (_checkSingleOverride(method, baseType, member, member)) {
+        if (_checkSingleOverride(
+            method, baseType, member, member, isSubclass)) {
           seen.add(method.name);
         }
       }
     }
   }
 
   /// Checks that [element] correctly overrides its corresponding member in
   /// [type]. Returns `true` if an override was found, that is, if [element] has
   /// a corresponding member in [type] that it overrides.
   ///
@@ skipping 13 matching lines @@
   ///
   ///     error: mixin introduces an invalid override. The type of C.foo is not
   ///     a subtype of E.foo:
   ///       class A extends B with C implements E { ... }
   ///                              ^
   ///
   /// When checking for overrides from a type and it's super types, [node] is
   /// the AST node that defines [element]. This is used to determine whether the
   /// type of the element could be inferred from the types in the super classes.
   bool _checkSingleOverride(ExecutableElement element, InterfaceType type,
-      AstNode node, AstNode errorLocation) {
+      AstNode node, AstNode errorLocation, bool isSubclass) {
     assert(!element.isStatic);
 
     FunctionType subType = _rules.elementType(element);
     // TODO(vsm): Test for generic
     FunctionType baseType = _getMemberType(type, element);
+    if (baseType == null) return false;
 
-    if (baseType == null) return false;
+    if (isSubclass && element is PropertyAccessorElement) {
+      // Disallow any overriding if the base class defines this member
+      // as a field.  We effectively treat fields as final / non-virtual.
+      PropertyInducingElement field = _getMemberField(type, element);
+      if (field != null) {
+        _recordMessage(new InvalidFieldOverride(
+            errorLocation, element, type, subType, baseType));
+      }
+    }
     if (!_rules.isAssignable(subType, baseType)) {
       // See whether non-assignable cases fit one of our common patterns:
       //
       // Common pattern 1: Inferable return type (on getters and methods)
       //   class A {
       //     int get foo => ...;
       //     String toString() { ... }
       //   }
       //   class B extends A {
       //     get foo => e; // no type specified.
@@ skipping 632 matching lines @@
       // TODO(jmesserly): if we're run again on the same AST, we'll produce the
       // same annotations. This should be harmless. This might go away once
       // CodeChecker is integrated better with analyzer, as it will know that
       // checking has already been performed.
       // assert(CoercionInfo.get(info.node) == null);
       CoercionInfo.set(info.node, info);
     }
   }
 }
 
+// Return the field on type corresponding to member, or null if none
+// exists or the "field" is actually a getter/setter.
+PropertyInducingElement _getMemberField(
+    InterfaceType type, PropertyAccessorElement member) {
+  String memberName = member.name;
+  PropertyInducingElement field;
+  if (member.isGetter) {
+    // The subclass member is an explicit getter or a field
+    // - lookup the getter on the superclass.
+    var getter = type.getGetter(memberName);
+    if (getter == null || getter.isStatic) return null;
+    field = getter.variable;
+  } else if (!member.isSynthetic) {
+    // The subclass member is an explicit setter
+    // - lookup the setter on the superclass.
+    // Note: an implicit (synthetic) setter would have already been flagged on
+    // the getter above.
+    var setter = type.getSetter(memberName);
+    if (setter == null || setter.isStatic) return null;
+    field = setter.variable;
+  } else {
+    return null;
+  }
+  if (field.isSynthetic) return null;
+  return field;
+}
+
 /// Looks up the declaration that matches [member] in [type] and returns it's
 /// declared type.
 FunctionType _getMemberType(InterfaceType type, ExecutableElement member) =>
     _memberTypeGetter(member)(type);
 
 typedef FunctionType _MemberTypeGetter(InterfaceType type);
 
 _MemberTypeGetter _memberTypeGetter(ExecutableElement member) {
   String memberName = member.name;
   final isGetter = member is PropertyAccessorElement && member.isGetter;
   final isSetter = member is PropertyAccessorElement && member.isSetter;
 
   FunctionType f(InterfaceType type) {
     ExecutableElement baseMethod;
+
+    if (member.isPrivate) {
+      var subtypeLibrary = member.library;
+      var baseLibrary = type.element.library;
+      if (baseLibrary != subtypeLibrary) {
+        return null;
+      }
+    }
+
     try {
       if (isGetter) {
         assert(!isSetter);
         // Look for getter or field.
         baseMethod = type.getGetter(memberName);
       } else if (isSetter) {
         baseMethod = type.getSetter(memberName);
       } else {
         baseMethod = type.getMethod(memberName);
       }
     } catch (e) {
       // TODO(sigmund): remove this try-catch block (see issue #48).
     }
     if (baseMethod == null || baseMethod.isStatic) return null;
     return baseMethod.type;
   }
   ;
   return f;
 }