Emit diagnostics for bad NSM implementations

pkg/compiler/lib/src/js_backend/no_such_method_registry.dart

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of js_backend;
 
 /**
  * Categorizes `noSuchMethod` implementations.
  *
  * If user code includes `noSuchMethod` implementations, type inference is
@@ skipping 24 matching lines @@
  * are in the same category as the superclass implementation. This covers a
  * common case, where users implement `noSuchMethod` with these dummy
  * implementations to avoid warnings.
  */
 class NoSuchMethodRegistry {
   /// The implementations that fall into category A, described above.
   final Set<FunctionElement> defaultImpls = new Set<FunctionElement>();
   /// The implementations that fall into category B, described above.
   final Set<FunctionElement> throwingImpls = new Set<FunctionElement>();
   /// The implementations that fall into category C, described above.
-  final Set<Element> otherImpls = new Set<Element>();
+  final Set<FunctionElement> otherImpls = new Set<FunctionElement>();
+
+  /// The implementations that fall into category C1
+  final Set<FunctionElement> complexNoReturnImpls = new Set<FunctionElement>();
+  /// The implementations that fall into category C2
+  final Set<FunctionElement> complexReturningImpls = new Set<FunctionElement>();
 
   /// The implementations that have not yet been categorized.
-  final Set<Element> _uncategorizedImpls = new Set<Element>();
+  final Set<FunctionElement> _uncategorizedImpls = new Set<FunctionElement>();
 
   final JavaScriptBackend _backend;
   final Compiler _compiler;
 
   NoSuchMethodRegistry(JavaScriptBackend backend)
       : this._backend = backend,
         this._compiler = backend.compiler;
 
   bool get hasThrowingNoSuchMethod => throwingImpls.isNotEmpty;
   bool get hasComplexNoSuchMethod => otherImpls.isNotEmpty;
 
-  void registerNoSuchMethod(Element noSuchMethodElement) {
+  void registerNoSuchMethod(FunctionElement noSuchMethodElement) {
     _uncategorizedImpls.add(noSuchMethodElement);
   }
 
   void onQueueEmpty() {
     _uncategorizedImpls.forEach(_categorizeImpl);
     _uncategorizedImpls.clear();
   }
 
+  /// Now that type inference is complete, split category C into two
+  /// subcategories: C1, those that have no return type, and C2, those
+  /// that have a return type.
+  void onInferenceComplete() {
+    otherImpls.forEach(_subcategorizeOther);
+  }
+
+  /// Emits a diagnostic
+  void emitDiagnostic() {
+    throwingImpls.forEach((e) {
+        if (!_hasForwardingSyntax(e)) {
+          _compiler.reportHint(e,
+                               MessageKind.DIRECTLY_THROWING_NSM);
+        }
+      });
+    complexNoReturnImpls.forEach((e) {
+        if (!_hasForwardingSyntax(e)) {
+          _compiler.reportHint(e,
+                               MessageKind.COMPLEX_THROWING_NSM);
+        }
+      });
+    complexReturningImpls.forEach((e) {
+        if (!_hasForwardingSyntax(e)) {
+          _compiler.reportHint(e,
+                               MessageKind.COMPLEX_RETURNING_NSM);
+        }
+      });
+  }
+
   /// Returns [true] if the given element is a complex [noSuchMethod]
   /// implementation. An implementation is complex if it falls into
   /// category C, as described above.
   bool isComplex(FunctionElement element) {
     assert(element.name == Compiler.NO_SUCH_METHOD);
     return otherImpls.contains(element);
   }
 
-  NsmCategory _categorizeImpl(Element noSuchMethodElement) {
-    assert(noSuchMethodElement.name == Compiler.NO_SUCH_METHOD);
-    if (defaultImpls.contains(noSuchMethodElement)) {
+  _subcategorizeOther(FunctionElement element) {
+    TypeMask returnType =
+        _compiler.typesTask.getGuaranteedReturnTypeOfElement(element);
+    if (returnType == const TypeMask.nonNullEmpty()) {
+      complexNoReturnImpls.add(element);
+    } else {
+      complexReturningImpls.add(element);
+    }
+  }
+
+  NsmCategory _categorizeImpl(FunctionElement element) {
+    assert(element.name == Compiler.NO_SUCH_METHOD);
+    if (defaultImpls.contains(element)) {
       return NsmCategory.DEFAULT;
     }
-    if (throwingImpls.contains(noSuchMethodElement)) {
+    if (throwingImpls.contains(element)) {
       return NsmCategory.THROWING;
     }
-    if (otherImpls.contains(noSuchMethodElement)) {
+    if (otherImpls.contains(element)) {
       return NsmCategory.OTHER;
     }
-    if (noSuchMethodElement is! FunctionElement ||
-        !_compiler.noSuchMethodSelector.signatureApplies(noSuchMethodElement)) {
-      otherImpls.add(noSuchMethodElement);
+    if (!_compiler.noSuchMethodSelector.signatureApplies(element)) {
+      otherImpls.add(element);
       return NsmCategory.OTHER;
     }
-    FunctionElement noSuchMethodFunc = noSuchMethodElement as FunctionElement;
-    if (_isDefaultNoSuchMethodImplementation(noSuchMethodFunc)) {
-      defaultImpls.add(noSuchMethodFunc);
+    if (_isDefaultNoSuchMethodImplementation(element)) {
+      defaultImpls.add(element);
       return NsmCategory.DEFAULT;
-    } else if (_hasForwardingSyntax(noSuchMethodFunc)) {
+    } else if (_hasForwardingSyntax(element)) {
       // If the implementation is 'noSuchMethod(x) => super.noSuchMethod(x);'
       // then it is in the same category as the super call.
-      Element superCall = noSuchMethodFunc.enclosingClass
+      Element superCall = element.enclosingClass
           .lookupSuperSelector(_compiler.noSuchMethodSelector);
       NsmCategory category = _categorizeImpl(superCall);
       switch(category) {
         case NsmCategory.DEFAULT:
-          defaultImpls.add(noSuchMethodFunc);
+          defaultImpls.add(element);
           break;
         case NsmCategory.THROWING:
-          throwingImpls.add(noSuchMethodFunc);
+          throwingImpls.add(element);
           break;
         case NsmCategory.OTHER:
-          otherImpls.add(noSuchMethodFunc);
+          otherImpls.add(element);
           break;
       }
       return category;
-    } else if (_hasThrowingSyntax(noSuchMethodFunc)) {
-      throwingImpls.add(noSuchMethodFunc);
+    } else if (_hasThrowingSyntax(element)) {
+      throwingImpls.add(element);
       return NsmCategory.THROWING;
     } else {
-      otherImpls.add(noSuchMethodFunc);
+      otherImpls.add(element);
       return NsmCategory.OTHER;
     }
   }
 
-  bool _isDefaultNoSuchMethodImplementation(Element element) {
+  bool _isDefaultNoSuchMethodImplementation(FunctionElement element) {
     ClassElement classElement = element.enclosingClass;
     return classElement == _compiler.objectClass
         || classElement == _backend.jsInterceptorClass
         || classElement == _backend.jsNullClass;
   }
 
   bool _hasForwardingSyntax(FunctionElement element) {
     // At this point we know that this is signature-compatible with
     // Object.noSuchMethod, but it may have more than one argument as long as
     // it only has one required argument.
@@ skipping 39 matching lines @@
       if (body.statements.nodes.head is ExpressionStatement) {
         ExpressionStatement stmt = body.statements.nodes.head;
         return stmt.expression is Throw;
       }
     }
     return false;
   }
 }
 
 enum NsmCategory { DEFAULT, THROWING, OTHER }