support @virtual fields, fix #27384

[email protected]

Review URL: https://codereview.chromium.org/2352433002 .

CHANGELOG.md

@@ -73,6 +73,23 @@
     }
     ```
 
+* New feature - use `@virtual` to allow field overrides in strong mode
+    (SDK issue [27384](https://github.com/dart-lang/sdk/issues/27384)).
+
+    ```dart
+    import 'package:meta/meta.dart' show virtual;
+    class Base {
+      @virtual int x;
+    }
+    class Derived extends Base {
+      int x;
+
+      // Expose the hidden storage slot:
+      int get superX => super.x;
+      set superX(int v) { super.x = v; }
+    }
+    ```
+
 * Breaking change - infer list and map literals from the context type as well as
     their values, consistent with generic methods and instance creation
     (SDK issue [27151](https://github.com/dart-lang/sdk/issues/27151)).

pkg/analyzer/lib/dart/element/element.dart

@@ -1166,6 +1166,11 @@ abstract class FieldElement
    */
   bool get isEnumConstant;
 
+  /**
+   * Returns `true` if this field can be overridden in strong mode.
+   */
+  bool get isVirtual;
+
   @override
   AstNode computeNode();
 }

pkg/analyzer/lib/src/dart/element/element.dart

@@ -2513,6 +2513,12 @@ class ElementAnnotationImpl implements ElementAnnotation {
    */
   static String _REQUIRED_VARIABLE_NAME = "required";
 
+  /**
+   * The name of the top-level variable used to mark a member as intended to be
+   * overridden.
+   */
+  static String _VIRTUAL_VARIABLE_NAME = "virtual";
+
   /**
    * The element representing the field, variable, or constructor being used as
    * an annotation.
@@ -2615,6 +2621,18 @@ class ElementAnnotationImpl implements ElementAnnotation {
           element.name == _REQUIRED_VARIABLE_NAME &&
           element.library?.name == _META_LIB_NAME;
 
+  /**
+   * Return `true` if this annotation marks the associated member as supporting
+   * overrides.
+   *
+   * This is currently used by fields in Strong Mode, as other members are
+   * already virtual-by-default.
+   */
+  bool get isVirtual =>
+      element is PropertyAccessorElement &&
+      element.name == _VIRTUAL_VARIABLE_NAME &&
+      element.library?.name == _META_LIB_NAME;
+
   /**
    * Get the library containing this annotation.
    */
@@ -4186,6 +4204,16 @@ class FieldElementImpl extends PropertyInducingElementImpl
     return hasModifier(Modifier.STATIC);
   }
 
+  @override
+  bool get isVirtual {
+    for (ElementAnnotationImpl annotation in metadata) {
+      if (annotation.isVirtual) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   @override
   ElementKind get kind => ElementKind.FIELD;
 

pkg/analyzer/lib/src/dart/element/handle.dart

@@ -584,6 +584,9 @@ class FieldElementHandle extends PropertyInducingElementHandle
   @override
   bool get isEnumConstant => actualElement.isEnumConstant;
 
+  @override
+  bool get isVirtual => actualElement.isVirtual;
+
   @override
   ElementKind get kind => ElementKind.FIELD;
 

pkg/analyzer/lib/src/dart/element/member.dart

@@ -275,6 +275,9 @@ class FieldMember extends VariableMember implements FieldElement {
   @override
   bool get isEnumConstant => baseElement.isEnumConstant;
 
+  @override
+  bool get isVirtual => baseElement.isVirtual;
+
   @override
   DartType get propagatedType => substituteFor(baseElement.propagatedType);
 

pkg/analyzer/lib/src/summary/link.dart

@@ -4001,6 +4001,9 @@ class ParameterElementForLink_VariableSetter implements ParameterElementImpl {
 
   ParameterElementForLink_VariableSetter(this.enclosingElement);
 
+  @override
+  bool inheritsCovariant = false;
+
   @override
   bool get isCovariant => false;
 

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

@@ -13,6 +13,7 @@ import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
+import 'package:analyzer/src/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/error/codes.dart' show StrongModeCode;
 import 'package:analyzer/src/generated/engine.dart' show AnalysisOptionsImpl;
@@ -74,10 +75,10 @@ Element _getKnownElement(Expression expression) {
 
 /// Return the field on type corresponding to member, or null if none
 /// exists or the "field" is actually a getter/setter.
-PropertyInducingElement _getMemberField(
+FieldElement _getMemberField(
     InterfaceType type, PropertyAccessorElement member) {
   String memberName = member.name;
-  PropertyInducingElement field;
+  FieldElement field;
   if (member.isGetter) {
     // The subclass member is an explicit getter or a field
     // - lookup the getter on the superclass.
@@ -1388,9 +1389,10 @@ class _OverrideChecker {
 
     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) {
+      // as a field.  We effectively treat fields as final / non-virtual,
+      // unless they are explicitly marked as @virtual
+      var field = _getMemberField(type, element);
+      if (field != null && !field.isVirtual) {
         _checker._recordMessage(
             errorLocation, StrongModeCode.INVALID_FIELD_OVERRIDE, [
           element.enclosingElement.name,

pkg/analyzer/lib/src/task/strong_mode.dart

@@ -306,11 +306,7 @@ class InstanceMemberInferrer {
     for (int i = 0; i < length; ++i) {
       ParameterElement parameter = parameters[i];
       if (parameter is ParameterElementImpl) {
-        // If a parameter is covariant, any parameters that override it are too.
-        parameter.inheritsCovariant = overriddenTypes.any((f) {
-          var param = _getCorrespondingParameter(parameter, i, f.parameters);
-          return param != null && param.isCovariant;
-        });
+        _inferParameterCovariance(parameter, i, overriddenTypes);
 
         if (parameter.hasImplicitType) {
           parameter.type = _computeParameterType(parameter, i, overriddenTypes);
@@ -327,9 +323,19 @@ class InstanceMemberInferrer {
    * which no type was provided, infer the type of the field.
    */
   void _inferField(FieldElement fieldElement) {
-    if (!fieldElement.isSynthetic &&
-        !fieldElement.isStatic &&
-        fieldElement.hasImplicitType) {
+    if (fieldElement.isSynthetic || fieldElement.isStatic) {
+      return;
+    }
+    List<ExecutableElement> overriddenSetters =
+        inheritanceManager.lookupOverrides(
+            fieldElement.enclosingElement, fieldElement.name + '=');
+    var setter = fieldElement.setter;
+    if (setter != null && overriddenSetters.isNotEmpty) {
+      _inferParameterCovariance(
+          setter.parameters[0], 0, overriddenSetters.map((s) => s.type));
+    }
+
+    if (fieldElement.hasImplicitType) {
       //
       // First look for overridden getters with the same name as the field.
       //
@@ -339,9 +345,7 @@ class InstanceMemberInferrer {
       if (overriddenGetters.isNotEmpty && _onlyGetters(overriddenGetters)) {
         newType =
             _computeReturnType(overriddenGetters.map((e) => e.returnType));
-        List<ExecutableElement> overriddenSetters =
-            inheritanceManager.lookupOverrides(
-                fieldElement.enclosingElement, fieldElement.name + '=');
+
         if (!_isCompatible(newType, overriddenSetters)) {
           newType = null;
         }
@@ -375,6 +379,17 @@ class InstanceMemberInferrer {
     }
   }
 
+  /**
+   * If a parameter is covariant, any parameters that override it are too.
+   */
+  void _inferParameterCovariance(ParameterElementImpl parameter, int index,
+      Iterable<FunctionType> overriddenTypes) {
+    parameter.inheritsCovariant = overriddenTypes.any((f) {
+      var param = _getCorrespondingParameter(parameter, index, f.parameters);
+      return param != null && param.isCovariant;
+    });
+  }
+
   /**
    * Infer type information for all of the instance members in the given
    * interface [type].

pkg/analyzer/test/src/task/strong/checker_test.dart

@@ -540,8 +540,18 @@ class A {
   get foo => '';
   set foo(_) {}
 }
+
 class B extends A {
-  @checked int foo;
+  @checked num foo;
+}
+class C extends A {
+  @checked @virtual num foo;
+}
+class D extends C {
+  @virtual int foo;
+}
+class E extends D {
+  @virtual /*error:INVALID_METHOD_OVERRIDE*/num foo;
 }
     ''');
   }
@@ -752,6 +762,36 @@ class H implements F {
  ''');
   }
 
+  void test_fieldOverride_virtual() {
+    _addMetaLibrary();
+    checkFile(r'''
+import 'meta.dart';
+class C {
+  @virtual int x;
+}
+class OverrideGetter extends C {
+  int get x => 42;
+}
+class OverrideSetter extends C {
+  set x(int v) {}
+}
+class OverrideBoth extends C {
+  int get x => 42;
+  set x(int v) {}
+}
+class OverrideWithField extends C {
+  int x;
+
+  // expose the hidden storage slot
+  int get superX => super.x;
+  set superX(int v) { super.x = v; }
+}
+class VirtualNotInherited extends OverrideWithField {
+  /*error:INVALID_FIELD_OVERRIDE*/int x;
+}
+    ''');
+  }
+
   void test_fieldSetterOverride() {
     checkFile('''
 class A {}
@@ -3876,5 +3916,8 @@ void _addMetaLibrary() {
 library meta;
 class _Checked { const _Checked(); }
 const Object checked = const _Checked();
+
+class _Virtual { const _Virtual(); }
+const Object virtual = const _Virtual();
     ''', name: '/meta.dart');
 }

pkg/meta/CHANGELOG.md

@@ -1,3 +1,21 @@
+## 1.0.4
+* Introduce `@virtual` to allow field overrides in strong mode
+    (SDK issue [27384](https://github.com/dart-lang/sdk/issues/27384)).
+
+    ```dart
+    import 'package:meta/meta.dart' show virtual;
+    class Base {
+      @virtual int x;
+    }
+    class Derived extends Base {
+      int x;
+
+      // Expose the hidden storage slot:
+      int get superX => super.x;
+      set superX(int v) { super.x = v; }
+    }
+    ```
+
 ## 1.0.3
 * Introduce `@checked` to override a method and tighten a parameter
     type (SDK issue [25578](https://github.com/dart-lang/sdk/issues/25578)).

pkg/meta/lib/meta.dart

@@ -128,7 +128,6 @@ class Required {
   const Required([this.reason]);
 }
 
-
 /// Used to annotate a parameter of an instance method that overrides another
 /// method.
 ///
@@ -137,6 +136,9 @@ class Required {
 /// is a subtype of the overridden parameter type.
 const _Checked checked = const _Checked();
 
+/// Used to annotate a field is allowed to be overridden in Strong Mode.
+const _Virtual virtual = const _Virtual();
+
 class _Checked {
   const _Checked();
 }
@@ -161,6 +163,10 @@ class _Protected {
   const _Protected();
 }
 
+class _Virtual {
+  const _Virtual();
+}
+
 class _VisibleForTesting {
   const _VisibleForTesting();
 }

pkg/meta/pubspec.yaml

@@ -1,5 +1,5 @@
 name: meta
-version: 1.0.3
+version: 1.0.4
 author: Dart Team <[email protected]>
 homepage: http://www.dartlang.org
 description: >