Enforce @mustCallSuper rules on getters, setters

Fixes https://github.com/dart-lang/sdk/issues/37057

Change-Id: If1c87680c547c8b98ab2de163ed37dfe455013fb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197660
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Sam Rawlins 2021-05-03 18:49:19 +00:00 committed by commit-bot@chromium.org
parent 86a86ab0ed
commit 3c0f67d31c
2 changed files with 154 additions and 8 deletions

View file

@ -21,13 +21,38 @@ class MustCallSuperVerifier {
}
var element = node.declaredElement!;
var overridden = _findOverriddenMemberWithMustCallSuper(element);
if (overridden != null &&
_hasConcreteSuperMethod(element as MethodElement)) {
_SuperCallVerifier verifier = _SuperCallVerifier(overridden.name);
node.accept(verifier);
if (!verifier.superIsCalled) {
_errorReporter.reportErrorForNode(HintCode.MUST_CALL_SUPER, node.name,
[overridden.enclosingElement.name]);
if (overridden == null) {
return;
}
if (element is MethodElement && _hasConcreteSuperMethod(element)) {
_verifySuperIsCalled(
node, overridden.name, overridden.enclosingElement.name);
return;
}
var enclosingElement = element.enclosingElement as ClassElement;
if (element is PropertyAccessorElement && element.isGetter) {
var inheritedConcreteGetter = enclosingElement
.lookUpInheritedConcreteGetter(element.name, element.library);
if (inheritedConcreteGetter != null) {
_verifySuperIsCalled(
node, overridden.name, overridden.enclosingElement.name);
}
return;
}
if (element is PropertyAccessorElement && element.isSetter) {
var inheritedConcreteSetter = enclosingElement
.lookUpInheritedConcreteSetter(element.name, element.library);
if (inheritedConcreteSetter != null) {
var name = overridden.name;
// For a setter, give the name without the trailing '=' to the verifier,
// in order to check against property access.
if (name.endsWith('=')) {
name = name.substring(0, name.length - 1);
}
_verifySuperIsCalled(node, name, overridden.enclosingElement.name);
}
}
}
@ -67,6 +92,12 @@ class MustCallSuperVerifier {
if (member is MethodElement && member.hasMustCallSuper) {
return member;
}
if (member is PropertyAccessorElement && member.hasMustCallSuper) {
// TODO(srawlins): What about a field annotated with `@mustCallSuper`?
// This might seem a legitimate case, but is not called out in the
// documentation of [mustCallSuper].
return member;
}
superclasses
..addAll(ancestor.mixins.map((i) => i.element))
..addAll(ancestor.superclassConstraints.map((i) => i.element))
@ -76,7 +107,7 @@ class MustCallSuperVerifier {
}
/// Returns whether [node] overrides a concrete method.
bool _hasConcreteSuperMethod(MethodElement element) {
bool _hasConcreteSuperMethod(ExecutableElement element) {
var classElement = element.enclosingElement as ClassElement;
String name = element.name;
@ -99,6 +130,17 @@ class MustCallSuperVerifier {
return false;
}
void _verifySuperIsCalled(MethodDeclaration node, String methodName,
String? overriddenEnclosingName) {
_SuperCallVerifier verifier = _SuperCallVerifier(methodName);
node.accept(verifier);
if (!verifier.superIsCalled) {
_errorReporter.reportErrorForNode(
HintCode.MUST_CALL_SUPER, node.name, [overriddenEnclosingName]);
}
return;
}
}
/// Recursively visits an AST, looking for method invocations.
@ -109,10 +151,23 @@ class _SuperCallVerifier extends RecursiveAstVisitor<void> {
_SuperCallVerifier(this.name);
@override
void visitAssignmentExpression(AssignmentExpression node) {
var lhs = node.leftHandSide;
if (lhs is PropertyAccess) {
if (lhs.target is SuperExpression && lhs.propertyName.name == name) {
superIsCalled = true;
return;
}
}
super.visitAssignmentExpression(node);
}
@override
void visitBinaryExpression(BinaryExpression node) {
if (node.leftOperand is SuperExpression && node.operator.lexeme == name) {
superIsCalled = true;
return;
}
super.visitBinaryExpression(node);
}
@ -121,7 +176,17 @@ class _SuperCallVerifier extends RecursiveAstVisitor<void> {
void visitMethodInvocation(MethodInvocation node) {
if (node.target is SuperExpression && node.methodName.name == name) {
superIsCalled = true;
return;
}
super.visitMethodInvocation(node);
}
@override
void visitPropertyAccess(PropertyAccess node) {
if (node.target is SuperExpression && node.propertyName.name == name) {
superIsCalled = true;
return;
}
super.visitPropertyAccess(node);
}
}

View file

@ -113,6 +113,39 @@ class B extends A {
]);
}
test_fromExtendingClass_getter() async {
await assertErrorsInCode('''
import 'package:meta/meta.dart';
class A {
@mustCallSuper
int get a => 1;
}
class B extends A {
@override
int get a => 2;
}
''', [
error(HintCode.MUST_CALL_SUPER, 122, 1),
]);
}
test_fromExtendingClass_getter_containsSuperCall() async {
await assertNoErrorsInCode('''
import 'package:meta/meta.dart';
class A {
@mustCallSuper
int get a => 1;
}
class B extends A {
@override
int get a {
super.a;
return 2;
}
}
''');
}
test_fromExtendingClass_operator() async {
await assertErrorsInCode(r'''
import 'package:meta/meta.dart';
@ -143,6 +176,38 @@ class B extends A {
''');
}
test_fromExtendingClass_setter() async {
await assertErrorsInCode('''
import 'package:meta/meta.dart';
class A {
@mustCallSuper
set a(int value) {}
}
class B extends A {
@override
set a(int value) {}
}
''', [
error(HintCode.MUST_CALL_SUPER, 122, 1),
]);
}
test_fromExtendingClass_setter_containsSuperCall() async {
await assertNoErrorsInCode('''
import 'package:meta/meta.dart';
class A {
@mustCallSuper
set a(int value) {}
}
class B extends A {
@override
set a(int value) {
super.a = value;
}
}
''');
}
test_fromInterface() async {
await assertNoErrorsInCode(r'''
import 'package:meta/meta.dart';
@ -173,6 +238,22 @@ class C with Mixin {
]);
}
test_fromMixin_setter() async {
await assertErrorsInCode(r'''
import 'package:meta/meta.dart';
class Mixin {
@mustCallSuper
void set a(int value) {}
}
class C with Mixin {
@override
void set a(int value) {}
}
''', [
error(HintCode.MUST_CALL_SUPER, 137, 1),
]);
}
test_fromMixin_throughExtendingClass() async {
await assertErrorsInCode(r'''
import 'package:meta/meta.dart';