Revert "Fix mustCallSuper for mixins and inherited interfaces:"

This reverts commit 1ad11facec.

Reason for revert: The CL broke the flutter-analyze bot.

Original change's description:
> Fix mustCallSuper for mixins and inherited interfaces:
> 
> * If a method overrides a mixed in @mustCallSuper method, it should call super.
> * If a method overrides a method from a supertype, which itself overrides a
>   method from an interface, the first method should call super.
> 
> Fixes internal b/70374204.
> 
> Change-Id: I645de4a288a8c5ffff173e97692f8eb6fe38bdb5
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/98443
> Commit-Queue: Samuel Rawlins <srawlins@google.com>
> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

TBR=brianwilkerson@google.com,srawlins@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I9339e6a8933f6d25b3bcbdbac0b6a1d304a3693b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/98803
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Brian Wilkerson 2019-04-07 16:58:27 +00:00 committed by commit-bot@chromium.org
parent 389ccc9000
commit 86b5304ccd
7 changed files with 35 additions and 222 deletions

View file

@ -520,22 +520,19 @@ abstract class Element implements AnalysisTarget {
/// Return `true` if this element has an annotation of the form `@JS(..)`.
bool get hasJS;
/// Return `true` if this element has an annotation of the form `@literal`.
/// Return `true` if this element has an annotation of the form '@literal'.
bool get hasLiteral;
/// Return `true` if this element has an annotation of the form `@mustCallSuper`.
bool get hasMustCallSuper;
/// Return `true` if this element has an annotation of the form `@override`.
bool get hasOverride;
/// Return `true` if this element has an annotation of the form `@protected`.
bool get hasProtected;
/// Return `true` if this element has an annotation of the form `@required`.
/// Return `true` if this element has an annotation of the form '@required'.
bool get hasRequired;
/// Return `true` if this element has an annotation of the form `@sealed`.
/// Return `true` if this element has an annotation of the form '@sealed'.
bool get hasSealed;
/// Return `true` if this element has an annotation of the form

View file

@ -3388,18 +3388,6 @@ abstract class ElementImpl implements Element {
return false;
}
@override
bool get hasMustCallSuper {
var metadata = this.metadata;
for (var i = 0; i < metadata.length; i++) {
var annotation = metadata[i];
if (annotation.isMustCallSuper) {
return true;
}
}
return false;
}
@override
bool get hasOverride {
var metadata = this.metadata;
@ -7510,9 +7498,6 @@ class MultiplyDefinedElementImpl implements MultiplyDefinedElement {
@override
bool get hasLiteral => false;
@override
bool get hasMustCallSuper => false;
@override
bool get hasOverride => false;

View file

@ -394,9 +394,6 @@ abstract class ElementHandle implements Element {
@override
bool get hasLiteral => actualElement.hasLiteral;
@override
bool get hasMustCallSuper => actualElement.hasMustCallSuper;
@override
bool get hasOverride => actualElement.hasOverride;

View file

@ -424,9 +424,6 @@ abstract class Member implements Element {
@override
bool get hasLiteral => _baseElement.hasLiteral;
@override
bool get hasMustCallSuper => _baseElement.hasMustCallSuper;
@override
bool get hasOverride => _baseElement.hasOverride;

View file

@ -70,9 +70,6 @@ class WrappedCompilationUnitElement implements CompilationUnitElement {
@override
bool get hasLoadLibraryFunction => wrappedUnit.hasLoadLibraryFunction;
@override
bool get hasMustCallSuper => wrappedUnit.hasMustCallSuper;
@override
bool get hasOverride => wrappedUnit.hasOverride;
@ -263,9 +260,6 @@ class WrappedImportElement implements ImportElement {
@override
bool get hasLiteral => wrappedImport.hasLiteral;
@override
bool get hasMustCallSuper => wrappedImport.hasMustCallSuper;
@override
bool get hasOverride => wrappedImport.hasOverride;
@ -471,9 +465,6 @@ class WrappedLibraryElement implements LibraryElement {
@override
bool get hasLoadLibraryFunction => wrappedLib.hasLoadLibraryFunction;
@override
bool get hasMustCallSuper => wrappedLib.hasMustCallSuper;
@override
bool get hasOverride => wrappedLib.hasOverride;

View file

@ -4310,7 +4310,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
return;
}
MethodElement element = _findOverriddenMemberThatMustCallSuper(node);
if (element != null && _hasConcreteSuperMethod(node)) {
if (element != null) {
_InvocationCollector collector = new _InvocationCollector();
node.accept(collector);
if (!collector.superCalls.contains(element.name)) {
@ -5909,59 +5909,24 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
return result;
}
// Find a method which is overridden by [node] and which is annotated with
// `@mustCallSuper`.
//
// As per the definition of `mustCallSuper` [1], every method which overrides
// a method annotated with `@mustCallSuper` is implicitly annotated with
// `@mustCallSuper`.
//
// [1] https://pub.dartlang.org/documentation/meta/latest/meta/mustCallSuper-constant.html
MethodElement _findOverriddenMemberThatMustCallSuper(MethodDeclaration node) {
Element member = node.declaredElement;
ClassElement classElement = member.enclosingElement;
String name = member.name;
// Walk up the type hierarchy from [classElement], ignoring direct interfaces.
Queue<ClassElement> superclasses =
Queue.of(classElement.mixins.map((i) => i.element))
..addAll(classElement.superclassConstraints.map((i) => i.element))
..add(classElement.supertype?.element);
Set<ClassElement> visitedClasses = new Set<ClassElement>();
while (superclasses.isNotEmpty) {
ClassElement ancestor = superclasses.removeFirst();
if (ancestor == null || !visitedClasses.add(ancestor)) {
continue;
ExecutableElement overriddenMember =
_getOverriddenMember(node.declaredElement);
List<ExecutableElement> seen = <ExecutableElement>[];
while (
overriddenMember is MethodElement && !seen.contains(overriddenMember)) {
for (ElementAnnotation annotation in overriddenMember.metadata) {
if (annotation.isMustCallSuper) {
return overriddenMember;
}
}
ExecutableElement member = ancestor.getMethod(name) ??
ancestor.getGetter(name) ??
ancestor.getSetter(name);
if (member is MethodElement && member.hasMustCallSuper) {
return member;
}
superclasses
..addAll(ancestor.interfaces.map((i) => i.element))
..addAll(ancestor.mixins.map((i) => i.element))
..addAll(ancestor.superclassConstraints.map((i) => i.element))
..add(ancestor.supertype?.element);
seen.add(overriddenMember);
// Keep looking up the chain.
overriddenMember = _getOverriddenMember(overriddenMember);
}
return null;
}
// Returns whether [node] overrides a concrete method.
bool _hasConcreteSuperMethod(MethodDeclaration node) {
ClassElement classElement = node.declaredElement.enclosingElement;
String name = node.declaredElement.name;
Queue<ClassElement> superclasses =
Queue.of(classElement.interfaces.map((i) => i.element))
..addAll(classElement.mixins.map((i) => i.element))
..addAll(classElement.superclassConstraints.map((i) => i.element))
..add(classElement.supertype?.element);
return superclasses.any(
(parent) => parent.lookUpConcreteMethod(name, parent.library) != null);
}
/**
* Given an [expression] in a switch case whose value is expected to be an
* enum constant, return the name of the constant.
@ -6064,6 +6029,23 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
return buffer.toString();
}
ExecutableElement _getOverriddenMember(Element member) {
ClassElement classElement = member.enclosingElement;
String name = member.name;
ClassElement superclass = classElement.supertype?.element;
Set<ClassElement> visitedClasses = new Set<ClassElement>();
while (superclass != null && visitedClasses.add(superclass)) {
ExecutableElement member = superclass.getMethod(name) ??
superclass.getGetter(name) ??
superclass.getSetter(name);
if (member != null) {
return member;
}
superclass = superclass.supertype?.element;
}
return null;
}
/**
* Return the type of the first and only parameter of the given [setter].
*/

View file

@ -46,38 +46,8 @@ class A {
}
class B extends A {
@override
void a() {}
}
''', [HintCode.MUST_CALL_SUPER]);
}
test_fromExtendingClass_abstractImplementation() async {
await assertNoErrorsInCode(r'''
import 'package:meta/meta.dart';
abstract class A {
@mustCallSuper
void a();
}
class B extends A {
@override
void a() {}
}
''');
}
test_fromExtendingClass_separateImplementation() async {
await assertErrorsInCode(r'''
import 'package:meta/meta.dart';
abstract class A {
@mustCallSuper
void a();
}
class B implements A {
void a() {}
}
class C extends B {
@override
void a() {}
void a()
{}
}
''', [HintCode.MUST_CALL_SUPER]);
}
@ -96,20 +66,6 @@ class C implements A {
''');
}
test_fromMixin() async {
await assertErrorsInCode(r'''
import 'package:meta/meta.dart';
class Mixin {
@mustCallSuper
void a() {}
}
class C with Mixin {
@override
void a() {}
}
''', [HintCode.MUST_CALL_SUPER]);
}
test_indirectlyInherited() async {
await assertErrorsInCode(r'''
import 'package:meta/meta.dart';
@ -130,98 +86,6 @@ class D extends C {
''', [HintCode.MUST_CALL_SUPER]);
}
test_indirectlyInheritedFromInterface() async {
await assertErrorsInCode(r'''
import 'package:meta/meta.dart';
class A {
@mustCallSuper
void a() {}
}
class C implements A {
@override
void a() {}
}
class D extends C {
@override
void a() {}
}
''', [HintCode.MUST_CALL_SUPER]);
}
test_indirectlyInheritedMultiply_fromFirstInterface() async {
await assertErrorsInCode(r'''
import 'package:meta/meta.dart';
class A {
@mustCallSuper
void a() {}
}
class B {
void a() {}
}
// Here's the crux: D needs to call super because of C's _first_ interface.
class C implements A, B {
@override
void a() {}
}
class D extends C {
@override
void a() {}
}
''', [HintCode.MUST_CALL_SUPER]);
}
test_indirectlyInheritedMultiply_fromInterfaceAfterFirst() async {
await assertErrorsInCode(r'''
import 'package:meta/meta.dart';
class A {
@mustCallSuper
void a() {}
}
class B {
void a() {}
}
// Here's the crux: D needs to call super because of one of C's interfaces, but
// not the first.
class C implements B, A {
@override
void a() {}
}
class D extends C {
@override
void a() {}
}
''', [HintCode.MUST_CALL_SUPER]);
}
test_indirectlyInheritedFromMixin() async {
await assertErrorsInCode(r'''
import 'package:meta/meta.dart';
class Mixin {
@mustCallSuper
void b() {}
}
class C extends Object with Mixin {}
class D extends C {
@override
void b() {}
}
''', [HintCode.MUST_CALL_SUPER]);
}
test_indirectlyInheritedFromMixinConstraint() async {
await assertErrorsInCode(r'''
import 'package:meta/meta.dart';
class A {
@mustCallSuper
void a() {}
}
mixin C on A {
@override
void a() {}
}
''', [HintCode.MUST_CALL_SUPER]);
}
test_overriddenWithFuture() async {
// https://github.com/flutter/flutter/issues/11646
await assertNoErrorsInCode(r'''