From 865e808f15035462e4647ee8be1e9a5161f67fd5 Mon Sep 17 00:00:00 2001 From: John Messerly Date: Mon, 19 Sep 2016 09:45:24 -0700 Subject: [PATCH] support `@virtual` fields, fix #27384 R=leafp@google.com Review URL: https://codereview.chromium.org/2352433002 . --- CHANGELOG.md | 17 +++++++ pkg/analyzer/lib/dart/element/element.dart | 5 +++ .../lib/src/dart/element/element.dart | 28 ++++++++++++ pkg/analyzer/lib/src/dart/element/handle.dart | 3 ++ pkg/analyzer/lib/src/dart/element/member.dart | 3 ++ pkg/analyzer/lib/src/summary/link.dart | 3 ++ pkg/analyzer/lib/src/task/strong/checker.dart | 12 ++--- pkg/analyzer/lib/src/task/strong_mode.dart | 37 ++++++++++----- .../test/src/task/strong/checker_test.dart | 45 ++++++++++++++++++- pkg/meta/CHANGELOG.md | 18 ++++++++ pkg/meta/lib/meta.dart | 8 +++- pkg/meta/pubspec.yaml | 2 +- 12 files changed, 162 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9054ac85ef6..bf984aba0ce 100644 --- a/CHANGELOG.md +++ b/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)). diff --git a/pkg/analyzer/lib/dart/element/element.dart b/pkg/analyzer/lib/dart/element/element.dart index 018cb771212..df7db661d53 100644 --- a/pkg/analyzer/lib/dart/element/element.dart +++ b/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(); } diff --git a/pkg/analyzer/lib/src/dart/element/element.dart b/pkg/analyzer/lib/src/dart/element/element.dart index bca9a6e75f6..62b7d75f18b 100644 --- a/pkg/analyzer/lib/src/dart/element/element.dart +++ b/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; diff --git a/pkg/analyzer/lib/src/dart/element/handle.dart b/pkg/analyzer/lib/src/dart/element/handle.dart index ecde0b53884..93edecad415 100644 --- a/pkg/analyzer/lib/src/dart/element/handle.dart +++ b/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; diff --git a/pkg/analyzer/lib/src/dart/element/member.dart b/pkg/analyzer/lib/src/dart/element/member.dart index ec1c874dff1..0ee331a54d3 100644 --- a/pkg/analyzer/lib/src/dart/element/member.dart +++ b/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); diff --git a/pkg/analyzer/lib/src/summary/link.dart b/pkg/analyzer/lib/src/summary/link.dart index 92d5cd9382a..d46d119e37b 100644 --- a/pkg/analyzer/lib/src/summary/link.dart +++ b/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; diff --git a/pkg/analyzer/lib/src/task/strong/checker.dart b/pkg/analyzer/lib/src/task/strong/checker.dart index 0fd6f265907..1183ce39b40 100644 --- a/pkg/analyzer/lib/src/task/strong/checker.dart +++ b/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, diff --git a/pkg/analyzer/lib/src/task/strong_mode.dart b/pkg/analyzer/lib/src/task/strong_mode.dart index 93de3ddc149..37c91cf1432 100644 --- a/pkg/analyzer/lib/src/task/strong_mode.dart +++ b/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 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 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 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]. diff --git a/pkg/analyzer/test/src/task/strong/checker_test.dart b/pkg/analyzer/test/src/task/strong/checker_test.dart index e647d641107..ae8ba3a2928 100644 --- a/pkg/analyzer/test/src/task/strong/checker_test.dart +++ b/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'); } diff --git a/pkg/meta/CHANGELOG.md b/pkg/meta/CHANGELOG.md index 11e1a69c81c..b91da1c7795 100644 --- a/pkg/meta/CHANGELOG.md +++ b/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)). diff --git a/pkg/meta/lib/meta.dart b/pkg/meta/lib/meta.dart index 633a4d362fb..984287546f6 100644 --- a/pkg/meta/lib/meta.dart +++ b/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(); } diff --git a/pkg/meta/pubspec.yaml b/pkg/meta/pubspec.yaml index ace2bff88c4..421a0ba7454 100644 --- a/pkg/meta/pubspec.yaml +++ b/pkg/meta/pubspec.yaml @@ -1,5 +1,5 @@ name: meta -version: 1.0.3 +version: 1.0.4 author: Dart Team homepage: http://www.dartlang.org description: >