Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(51)

Unified Diff: pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart

Issue 2755983002: Implement override checks for methods. (Closed)
Patch Set: Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « pkg/front_end/lib/src/fasta/colors.dart ('k') | pkg/front_end/lib/src/fasta/kernel/kernel_target.dart » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart
diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart
index 347b08488602aa4c7a6d0b70cbe950d7d90611d9..81bdf1c94aa45453550cbb2c1b2f2de06cf124da 100644
--- a/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/kernel_class_builder.dart
@@ -7,23 +7,32 @@ library fasta.kernel_class_builder;
import 'package:kernel/ast.dart'
show
Class,
+ Constructor,
DartType,
Expression,
ExpressionStatement,
Field,
+ FunctionNode,
InterfaceType,
ListLiteral,
Member,
Name,
+ Procedure,
+ ProcedureKind,
StaticGet,
StringLiteral,
Supertype,
- Throw;
+ Throw,
+ VariableDeclaration;
+
+import 'package:kernel/class_hierarchy.dart' show ClassHierarchy;
import '../errors.dart' show internalError;
import '../messages.dart' show warning;
+import '../dill/dill_member_builder.dart' show DillMemberBuilder;
+
import 'kernel_builder.dart'
show
Builder,
@@ -38,8 +47,6 @@ import 'kernel_builder.dart'
TypeVariableBuilder,
computeDefaultTypeArguments;
-import '../dill/dill_member_builder.dart' show DillMemberBuilder;
-
import 'redirecting_factory_body.dart' show RedirectingFactoryBody;
abstract class KernelClassBuilder
@@ -166,4 +173,109 @@ abstract class KernelClassBuilder
literal.expressions
.add(new StaticGet(constructor.target)..parent = literal);
}
+
+ void checkOverrides(ClassHierarchy hierarchy) {
+ hierarchy.forEachOverridePair(cls, checkOverride);
+ }
+
+ void checkOverride(
+ Member declaredMember, Member interfaceMember, bool isSetter) {
+ if (declaredMember is Constructor || interfaceMember is Constructor) {
+ internalError(
+ "Constructor in override check.", fileUri, declaredMember.fileOffset);
+ }
+ if (declaredMember is Procedure && interfaceMember is Procedure) {
+ if (declaredMember.kind == ProcedureKind.Method &&
+ interfaceMember.kind == ProcedureKind.Method) {
+ checkMethodOverride(declaredMember, interfaceMember);
+ return;
+ }
Johnni Winther 2017/03/17 11:46:48 TODO: check getters/setters/operators
ahe 2017/03/17 13:24:37 Done.
+ }
+ }
+
+ void checkMethodOverride(
+ Procedure declaredMember, Procedure interfaceMember) {
+ assert(declaredMember.kind == ProcedureKind.Method);
+ assert(interfaceMember.kind == ProcedureKind.Method);
+ FunctionNode declaredFunction = declaredMember.function;
+ FunctionNode interfaceFunction = interfaceMember.function;
+ if (declaredFunction.typeParameters?.length !=
+ interfaceFunction.typeParameters?.length) {
+ if ("patched_sdk/lib/typed_data/typed_data.dart" !=
+ library.relativeFileUri) {
+ // TODO(ahe): https://github.com/dart-lang/sdk/issues/29100
+ addWarning(
+ declaredMember.fileOffset,
+ "Declared type variables of '$name::${declaredMember.name.name}' "
+ "doesn't match those on overridden method "
+ "'${interfaceMember.enclosingClass.name}::"
+ "${interfaceMember.name.name}'.");
+ }
+ }
+ if (declaredFunction.positionalParameters.length <
+ interfaceFunction.requiredParameterCount ||
+ declaredFunction.positionalParameters.length <
+ interfaceFunction.positionalParameters.length) {
+ addWarning(
+ declaredMember.fileOffset,
+ "The method '$name::${declaredMember.name.name}' has fewer "
+ "positional arguments than those of overridden method "
+ "'${interfaceMember.enclosingClass.name}::"
+ "${interfaceMember.name.name}'.");
+ }
+ if (interfaceFunction.requiredParameterCount <
+ declaredFunction.requiredParameterCount) {
+ addWarning(
+ declaredMember.fileOffset,
+ "The method '$name::${declaredMember.name.name}' has more "
+ "positional arguments than those of overridden method "
Johnni Winther 2017/03/17 11:46:47 'positional' -> 'required'
ahe 2017/03/17 13:24:37 That was intentional. I consider this case similar
Johnni Winther 2017/03/17 13:36:19 Yes, but from the users perspective it doesn't mak
ahe 2017/03/17 14:02:51 Good point. I'll send an update.
+ "'${interfaceMember.enclosingClass.name}::"
+ "${interfaceMember.name.name}'.");
+ }
+ if (declaredFunction.namedParameters.isEmpty &&
+ interfaceFunction.namedParameters.isEmpty) {
+ return;
+ }
+ if (declaredFunction.namedParameters.length <
+ interfaceFunction.namedParameters.length) {
+ addWarning(
+ declaredMember.fileOffset,
+ "The method '$name::${declaredMember.name.name}' has fewer named "
+ "arguments than those of overridden method "
+ "'${interfaceMember.enclosingClass.name}::"
+ "${interfaceMember.name.name}'.");
+ }
+ Iterator<VariableDeclaration> declaredNamedParameters =
+ declaredFunction.namedParameters.iterator;
+ Iterator<VariableDeclaration> interfaceNamedParameters =
+ interfaceFunction.namedParameters.iterator;
+ outer:
+ while (declaredNamedParameters.moveNext() &&
+ interfaceNamedParameters.moveNext()) {
+ while (declaredNamedParameters.current.name !=
+ interfaceNamedParameters.current.name) {
+ if (!declaredNamedParameters.moveNext()) {
+ addWarning(
+ declaredMember.fileOffset,
+ "The method '$name::${declaredMember.name.name}' doesn't have "
+ "the named parameter '${interfaceNamedParameters.current.name}' "
+ "of override method '${interfaceMember.enclosingClass.name}::"
Johnni Winther 2017/03/17 11:46:47 override -> overridden
ahe 2017/03/17 13:24:37 Done.
+ "${interfaceMember.name.name}'.");
+ break outer;
+ }
+ }
+ }
+ }
+
+ void addCompileTimeError(int charOffset, String message) {
+ library.addCompileTimeError(charOffset, message, fileUri: fileUri);
+ }
+
+ void addWarning(int charOffset, String message) {
+ library.addWarning(charOffset, message, fileUri: fileUri);
+ }
+
+ void addNit(int charOffset, String message) {
+ library.addNit(charOffset, message, fileUri: fileUri);
+ }
}
« no previous file with comments | « pkg/front_end/lib/src/fasta/colors.dart ('k') | pkg/front_end/lib/src/fasta/kernel/kernel_target.dart » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698