Perform override and inheritance checks.

sdk/lib/_internal/compiler/implementation/resolution/class_members.dart

Index: sdk/lib/_internal/compiler/implementation/resolution/class_members.dart
diff --git a/sdk/lib/_internal/compiler/implementation/resolution/class_members.dart b/sdk/lib/_internal/compiler/implementation/resolution/class_members.dart
index a34160b5f151b2c1886aaa7c8b5de7f6c03bae1e..1964bd5f2ede692b8c98c027a9becaf18f498cf4 100644
--- a/sdk/lib/_internal/compiler/implementation/resolution/class_members.dart
+++ b/sdk/lib/_internal/compiler/implementation/resolution/class_members.dart
@@ -39,13 +39,33 @@ class MembersCreator {
   Map<Name, Member> classMembers = new Map<Name, Member>();
   Map<Name, Signature> interfaceMembers = new Map<Name, Signature>();
 
+  Map<Object, Set<MessageKind>> reportedMessages =

[karlklose, 2014/01/28 12:27:45]

Please add a comment about the possible types of keys (perhaps inline like:
'Map<Object /* a | b */, Set<MessageKind>>').

Would it make sense to use dynamic here?

[Johnni Winther, 2014/01/31 12:29:30]

Done.

+      new Map<Object, Set<MessageKind>>();
+
   MembersCreator(Compiler this.compiler, ClassElement this.cls);
 
+  void reportMessage(var marker, MessageKind kind, report()) {
+    Set<MessageKind> messages =
+        reportedMessages.putIfAbsent(marker,
+            () => new Set<MessageKind>());
+    if (messages.add(kind)) {
+      report();
+    }
+  }
+
   void computeMembers() {
     Map<Name, Set<Member>> inheritedInterfaceMembers =
         _computeSuperMembers();
     Map<Name, Member> declaredMembers = _computeClassMembers();
     _computeInterfaceMembers(inheritedInterfaceMembers, declaredMembers);
+
+    if (!cls.modifiers.isAbstract() &&
+        !cls.isProxy &&
+        !declaredMembers.containsKey(const PublicName('noSuchMethod'))) {
+      // Check for unimplemented member on concrete classes that neither have

[karlklose, 2014/01/28 12:27:45]

'member' -> 'members'?

[Johnni Winther, 2014/01/31 12:29:30]

Done.

+      // a @proxy annotation nor declare a `noSuchMethod` method.

[karlklose, 2014/01/28 12:27:45]

'@proxy' -> '`@proxy`'.

[Johnni Winther, 2014/01/31 12:29:30]

Done.

+      checkInterfaceImplementation();
+    }
   }
 
   Map<Name, Set<Member>> _computeSuperMembers() {
@@ -94,6 +114,7 @@ class MembersCreator {
     void overrideMember(DeclaredMember declared) {
       DeclaredMember inherited = classMembers[declared.name];
       classMembers[declared.name] = declared;
+      checkValidOverride(declared, inherited);
     }
 
     if (cls.isMixinApplication) {
@@ -175,6 +196,10 @@ class MembersCreator {
         (Name name, Set<Member> inheritedMembers) {
       Member declared = declaredMembers[name];
       if (declared != null) {
+        // Check that [declaredMember] is a valid override
+        for (Member inherited in inheritedMembers) {
+          checkValidOverride(declared, inherited);
+        }
         if (!declared.isStatic) {
           interfaceMembers[name] = declared;
         }
@@ -201,6 +226,26 @@ class MembersCreator {
               () => new Set<Member>()).add(inherited);
         }
         if (someAreGetters && !allAreGetters) {
+          compiler.reportWarningCode(cls,
+                                     MessageKind.INHERIT_GETTER_AND_METHOD,
+                                     {'class': thisType, 'name': name.text });
+          for (Member inherited in inheritedMembers) {
+            MessageKind kind;
+            if (inherited.isMethod) {
+              kind = MessageKind.INHERITED_METHOD;
+            } else {
+              assert(invariant(cls, inherited.isGetter,
+                  message: 'Conflicting member is neither a method nor a '
+                           'getter.'));
+              if (inherited.isDeclaredByField) {
+                kind = MessageKind.INHERITED_IMPLICIT_GETTER;
+              } else {
+                kind = MessageKind.INHERITED_EXPLICIT_GETTER;
+              }
+            }
+            compiler.reportInfo(inherited.element, kind,
+                {'class': inherited.declarer, 'name': name.text });
+          }
           interfaceMembers[name] = new ErroneousMember(inheritedMembers);
         } else if (subtypesOfAllInherited.length == 1) {
           // All signatures have the same type.
@@ -290,4 +335,214 @@ class MembersCreator {
       }
     });
   }
+
+  void checkInterfaceImplementation() {

[karlklose, 2014/01/28 12:27:45]

Please add a comment on what this method checks.

[Johnni Winther, 2014/01/31 12:29:30]

Done.

+    LibraryElement library = cls.getLibrary();
+
+    interfaceMembers.forEach((Name name, Signature interfaceMember) {
+      if (!name.isAccessibleFrom(library)) return;
+      Member classMember = classMembers[name];
+      /*if (compiler.inUserCode(cls)) {

[karlklose, 2014/01/28 12:27:45]

Remove debug code.

[Johnni Winther, 2014/01/31 12:29:30]

Done.

+        print('cls:$cls, sig:$interfaceMember, member=$classMember (${classMember == null || classMember.isAbstract})');
+      }*/
+      if (classMember != null) return;
+      if (interfaceMember is DeclaredMember &&
+          interfaceMember.declarer.element == cls) {
+        // Abstract method declared in [cls].
+        MessageKind kind = MessageKind.ABSTRACT_METHOD;
+        if (interfaceMember.isSetter) {
+          kind = MessageKind.ABSTRACT_SETTER;
+        } else if (interfaceMember.isGetter) {
+          kind = MessageKind.ABSTRACT_GETTER;
+        }
+        reportMessage(
+            interfaceMember.element, MessageKind.ABSTRACT_METHOD, () {
+          compiler.reportWarningCode(
+              interfaceMember.element, kind,
+              {'class': cls.name, 'name': name.text});
+        });
+      } else {
+         reportWarning(MessageKind singleKind,
+                       MessageKind multipleKind,
+                       MessageKind explicitlyDeclaredKind,
+                       [MessageKind implicitlyDeclaredKind]) {
+          Member inherited = interfaceMember.declarations.first;
+          reportMessage(
+              interfaceMember, MessageKind.UNIMPLEMENTED_METHOD, () {
+            compiler.reportWarningCode(cls,
+                interfaceMember.declarations.length == 1
+                    ? singleKind : multipleKind,
+                {'class': cls.name,
+                 'name': name.text,
+                 'method': interfaceMember,
+                 'declarer': inherited.declarer});
+            for (Member inherited in interfaceMember.declarations) {
+              compiler.reportInfo(inherited.element,
+                  inherited.isDeclaredByField ?
+                      implicitlyDeclaredKind : explicitlyDeclaredKind,
+                  {'class': inherited.declarer.name,
+                   'name': name.text});
+            }
+          });
+        }
+        if (interfaceMember.isSetter) {
+          reportWarning(MessageKind.UNIMPLEMENTED_SETTER_ONE,
+                        MessageKind.UNIMPLEMENTED_SETTER,
+                        MessageKind.UNIMPLEMENTED_EXPLICIT_SETTER,
+                        MessageKind.UNIMPLEMENTED_IMPLICIT_SETTER);
+        } else if (interfaceMember.isGetter) {
+          reportWarning(MessageKind.UNIMPLEMENTED_GETTER_ONE,
+                        MessageKind.UNIMPLEMENTED_GETTER,
+                        MessageKind.UNIMPLEMENTED_EXPLICIT_GETTER,
+                        MessageKind.UNIMPLEMENTED_IMPLICIT_GETTER);
+        } else if (interfaceMember.isMethod) {
+          reportWarning(MessageKind.UNIMPLEMENTED_METHOD_ONE,
+                        MessageKind.UNIMPLEMENTED_METHOD,
+                        MessageKind.UNIMPLEMENTED_METHOD_CONT);
+        }
+      }
+      // TODO(johnniwinther): If [cls] is not abstract, check that for all
+      // interface members, there is a class member whose type is a subtype of
+      // the interface member.
+    });
+  }
+
+  void checkValidOverride(Member declared, Signature superMember) {
+    if (superMember != null) {

[karlklose, 2014/01/28 12:27:45]

Perhaps 'assert(declared.name == superMember.name)'.

[Johnni Winther, 2014/01/31 12:29:30]

Done.

+      if (declared.isStatic) {
+        for (Member inherited in superMember.declarations) {
+          reportMessage(
+              inherited.element, MessageKind.NO_STATIC_OVERRIDE, () {
+            reportErrorWithContext(
+                declared.element, MessageKind.NO_STATIC_OVERRIDE,
+                inherited.element, MessageKind.NO_STATIC_OVERRIDE_CONT);
+          });
+        }
+      }
+
+      DartType declaredType = declared.functionType;
+      for (Member inherited in superMember.declarations) {
+
+        void reportError(MessageKind errorKind, MessageKind infoKind) {
+          reportMessage(
+              inherited.element, MessageKind.INVALID_OVERRIDE_METHOD, () {
+            compiler.reportError(declared.element, errorKind,
+                {'name': declared.name.text,
+                 'class': cls.thisType,
+                 'inheritedClass': inherited.declarer});
+            compiler.reportInfo(inherited.element, infoKind,
+                {'name': declared.name.text,
+                 'class': inherited.declarer});
+          });
+        }
+
+        if (declared.isDeclaredByField && inherited.isMethod) {
+          reportError(MessageKind.CANNOT_OVERRIDE_METHOD_WITH_FIELD,
+              MessageKind.CANNOT_OVERRIDE_METHOD_WITH_FIELD_CONT);
+        } else if (declared.isMethod && inherited.isDeclaredByField) {
+          reportError(MessageKind.CANNOT_OVERRIDE_FIELD_WITH_METHOD,
+              MessageKind.CANNOT_OVERRIDE_FIELD_WITH_METHOD_CONT);
+        } else if (declared.isGetter && inherited.isMethod) {
+          reportError(MessageKind.CANNOT_OVERRIDE_METHOD_WITH_GETTER,
+                      MessageKind.CANNOT_OVERRIDE_METHOD_WITH_GETTER_CONT);
+        } else if (declared.isMethod && inherited.isGetter) {
+          reportError(MessageKind.CANNOT_OVERRIDE_GETTER_WITH_METHOD,
+                      MessageKind.CANNOT_OVERRIDE_GETTER_WITH_METHOD_CONT);
+        } else {
+          DartType inheritedType = inherited.functionType;
+          if (!compiler.types.isSubtype(declaredType, inheritedType)) {
+            void reportWarning(var marker,
+                               MessageKind warningKind,
+                               MessageKind infoKind) {
+              reportMessage(marker, MessageKind.INVALID_OVERRIDE_METHOD, () {
+                compiler.reportWarningCode(declared.element, warningKind,
+                    {'declaredType': declared.type,
+                     'name': declared.name.text,
+                     'class': cls.thisType,
+                     'inheritedType': inherited.type,
+                     'inheritedClass': inherited.declarer});
+                compiler.reportInfo(inherited.element, infoKind,
+                    {'name': declared.name.text,
+                     'class': inherited.declarer});
+              });
+            }
+            if (declared.isDeclaredByField) {
+              if (inherited.isDeclaredByField) {
+                reportWarning(inherited.element,
+                              MessageKind.INVALID_OVERRIDE_FIELD,
+                              MessageKind.INVALID_OVERRIDDEN_FIELD);
+              } else if (inherited.isGetter) {
+                reportWarning(inherited,
+                              MessageKind.INVALID_OVERRIDE_GETTER_WITH_FIELD,
+                              MessageKind.INVALID_OVERRIDDEN_GETTER);
+              } else if (inherited.isSetter) {
+                reportWarning(inherited,
+                              MessageKind.INVALID_OVERRIDE_SETTER_WITH_FIELD,
+                              MessageKind.INVALID_OVERRIDDEN_SETTER);
+              }
+            } else if (declared.isGetter) {
+              if (inherited.isDeclaredByField) {
+                reportWarning(inherited,
+                              MessageKind.INVALID_OVERRIDE_FIELD_WITH_GETTER,
+                              MessageKind.INVALID_OVERRIDDEN_FIELD);
+              } else {
+                reportWarning(inherited,
+                              MessageKind.INVALID_OVERRIDE_GETTER,
+                              MessageKind.INVALID_OVERRIDDEN_GETTER);
+              }
+            } else if (declared.isSetter) {
+              if (inherited.isDeclaredByField) {
+                reportWarning(inherited,
+                              MessageKind.INVALID_OVERRIDE_FIELD_WITH_SETTER,
+                              MessageKind.INVALID_OVERRIDDEN_FIELD);
+              } else {
+                reportWarning(inherited,
+                              MessageKind.INVALID_OVERRIDE_SETTER,
+                              MessageKind.INVALID_OVERRIDDEN_SETTER);
+              }
+            } else {
+              reportWarning(inherited,
+                            MessageKind.INVALID_OVERRIDE_METHOD,
+                            MessageKind.INVALID_OVERRIDDEN_METHOD);
+            }
+          }
+        }
+      }
+    } else {

[karlklose, 2014/01/28 12:27:45]

Consider changing the else-branch to be the then-branch; it is shorter than the
other branch and in this order is is easier to skip it.

[Johnni Winther, 2014/01/31 12:29:30]

Done.

+      if (!declared.isStatic) {
+        ClassElement superclass = cls.superclass;
+        while (superclass != null) {
+          //print('checkValidOverride($declared,$superclass');

[karlklose, 2014/01/28 12:27:45]

Remove debug code.

[Johnni Winther, 2014/01/31 12:29:30]

Done.

+          Member superMember =
+              superclass.lookupClassMember(declared.name);
+          if (superMember != null && superMember.isStatic) {
+            reportMessage(superMember, MessageKind.INSTANCE_STATIC_SAME_NAME,
+                () {
+              compiler.reportWarningCode(
+                  declared.element,
+                  MessageKind.INSTANCE_STATIC_SAME_NAME,
+                  {'memberName': declared.name,
+                    'className': superclass.name});
+              compiler.reportInfo(superMember.element,
+                  MessageKind.INSTANCE_STATIC_SAME_NAME_CONT);
+            });
+            break;
+          }
+          superclass = superclass.superclass;
+        }
+      }
+    }
+  }
+
+  void reportErrorWithContext(Element errorneousElement,
+                         MessageKind errorMessage,
+                         Element contextElement,
+                         MessageKind contextMessage) {
+    compiler.reportError(
+        errorneousElement,
+        errorMessage,
+        {'memberName': contextElement.name,
+         'className': contextElement.getEnclosingClass().name});
+    compiler.reportInfo(contextElement, contextMessage);
+  }
 }