Report lint finding for missing type annotations on override methods and fields.

PiperOrigin-RevId: 345088203

Author: rishipal

src/com/google/javascript/jscomp/lint/CheckJSDocStyle.java

@@ -24,6 +24,7 @@
 import com.google.javascript.jscomp.DiagnosticGroup;
 import com.google.javascript.jscomp.DiagnosticType;
 import com.google.javascript.jscomp.ExportTestFunctions;
+import com.google.javascript.jscomp.JSError;
 import com.google.javascript.jscomp.NodeTraversal;
 import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
 import com.google.javascript.jscomp.NodeTraversal.AbstractPreOrderCallback;
@@ -55,7 +56,7 @@ public final class CheckJSDocStyle extends AbstractPostOrderCallback implements
               + " instead?");
 
   public static final DiagnosticType MISSING_PARAMETER_JSDOC =
-      DiagnosticType.disabled("JSC_MISSING_PARAMETER_JSDOC", "Parameter must have JSDoc.");
+      DiagnosticType.disabled("JSC_MISSING_PARAMETER_JSDOC", "Parameter must have JSDoc.{0}");
 
   public static final DiagnosticType MIXED_PARAM_JSDOC_STYLES =
       DiagnosticType.disabled("JSC_MIXED_PARAM_JSDOC_STYLES",
@@ -64,7 +65,7 @@ public final class CheckJSDocStyle extends AbstractPostOrderCallback implements
   public static final DiagnosticType MISSING_RETURN_JSDOC =
       DiagnosticType.disabled(
           "JSC_MISSING_RETURN_JSDOC",
-          "Function with non-trivial return must have JSDoc indicating the return type.");
+          "Function with non-trivial return must have JSDoc indicating the return type.{0}");
 
   public static final DiagnosticType MUST_BE_PRIVATE =
       DiagnosticType.disabled(
@@ -234,7 +235,7 @@ private void visitFunction(NodeTraversal t, Node function) {
         checkParams(t, function, jsDoc);
       }
       checkNoTypeOnGettersAndSetters(t, function, jsDoc);
-      checkReturn(t, function, jsDoc);
+      checkReturn(function, jsDoc);
     }
   }
 
@@ -308,10 +309,6 @@ private boolean isConstructorWithoutParameters(Node function) {
   }
 
   private void checkParams(NodeTraversal t, Node function, JSDocInfo jsDoc) {
-    if (jsDoc != null && jsDoc.isOverride()) {
-      return;
-    }
-
     if (jsDoc != null && jsDoc.getType() != null) {
       // Sometimes functions are declared with @type {function(Foo, Bar)} instead of
       //   @param {Foo} foo
@@ -329,7 +326,17 @@ private void checkParams(NodeTraversal t, Node function, JSDocInfo jsDoc) {
     } else {
       Node paramList = NodeUtil.getFunctionParameters(function);
       if (!paramList.hasXChildren(paramsFromJsDoc.size())) {
-        t.report(paramList, WRONG_NUMBER_OF_PARAMS);
+        compiler.report(
+            JSError.make(
+                paramList,
+                WRONG_NUMBER_OF_PARAMS,
+                jsDoc.isOverride()
+                    ?
+                    // moe:begin_strip
+                    " Please see go/tsjs-problematic-patterns for why @overrides require explicit"
+                        + " @param types."
+                    // moe:end_strip_and_replace ""
+                    : ""));
         return;
       }
 
@@ -429,12 +436,8 @@ private boolean hasAnyInlineJsDoc(Node function) {
     return false;
   }
 
-  private void checkReturn(NodeTraversal t, Node function, JSDocInfo jsDoc) {
-    if (jsDoc != null
-        && (jsDoc.hasType()
-            || jsDoc.isConstructor()
-            || jsDoc.hasReturnType()
-            || jsDoc.isOverride())) {
+  private void checkReturn(Node function, JSDocInfo jsDoc) {
+    if (jsDoc != null && (jsDoc.hasType() || jsDoc.isConstructor() || jsDoc.hasReturnType())) {
       return;
     }
 
@@ -450,7 +453,17 @@ private void checkReturn(NodeTraversal t, Node function, JSDocInfo jsDoc) {
     FindNonTrivialReturn finder = new FindNonTrivialReturn();
     NodeTraversal.traverse(compiler, function.getLastChild(), finder);
     if (finder.found) {
-      t.report(function, MISSING_RETURN_JSDOC);
+      compiler.report(
+          JSError.make(
+              function,
+              MISSING_RETURN_JSDOC,
+              jsDoc != null && jsDoc.isOverride()
+                  ?
+                  // moe:begin_strip
+                  " Please see go/tsjs-problematic-patterns for why @overrides require explicit"
+                      + " @return types.."
+                  // moe:end_strip_and_replace ""
+                  : ""));
     }
   }
 

test/com/google/javascript/jscomp/lint/CheckJSDocStyleTest.java

@@ -448,10 +448,6 @@ public void testMissingParam_noWarning() {
         " */",
         "function f(x, y) {}"));
 
-    testSame(lines(
-        "/** @override */",
-        "Foo.bar = function(x, y) {}"));
-
     testSame(lines(
         "/**",
         " * @param {string=} x",
@@ -1101,8 +1097,6 @@ public void testMissingReturn_functionStatement_noWarning() {
     testSame("/** @param {number} x */ function f(x) { return; }");
     testSame("/** @param {number} x\n * @return {number} */ function f(x) { return x; }");
     testSame("/** @param {number} x */ function /** number */ f(x) { return x; }");
-    testSame("/** @inheritDoc */ function f(x) { return x; }");
-    testSame("/** @override */ function f(x) { return x; }");
   }
 
   @Test
@@ -1117,8 +1111,65 @@ public void testMissingReturn_assign_noWarning() {
     testSame("/** @param {number} x */ f = function(x) { function bar() { return x; } }");
     testSame("/** @param {number} x */ f = function(x) { return; }");
     testSame("/** @param {number} x\n * @return {number} */ f = function(x) { return x; }");
-    testSame("/** @inheritDoc */ f = function(x) { return x; }");
-    testSame("/** @override */ f = function(x) { return x; }");
+  }
+
+  @Test
+  public void testMissingParamOrReturn_warnOnOverrideMethodsAndFields() {
+    testWarning(
+        lines(
+            "/** ",
+            " * @override ",
+            " * @param {string} x ",
+            " */",
+            " Foo.Bar = function(x) { return x; }"), // function assigned to a field
+        MISSING_RETURN_JSDOC);
+    testWarning(
+        lines(
+            "/** ",
+            " * @override ",
+            " * @param {string} x ",
+            " */",
+            " function f(x) { return x; }"),
+        MISSING_RETURN_JSDOC);
+    testWarning(
+        lines(
+            "/**",
+            " * @override",
+            " * @return {string}",
+            " */",
+            " Foo.Bar = function(x) { return x; }"),
+        MISSING_PARAMETER_JSDOC);
+    testWarning(
+        lines(
+            "/** ", " * @override ", " * @param {string} x ", " */", "Foo.bar = function(x, y) {}"),
+        WRONG_NUMBER_OF_PARAMS);
+
+    // also test `@inheritDoc` annotations
+    testWarning(
+        lines(
+            "/** ",
+            " * @inheritDoc ",
+            " * @return {string} x ",
+            " */",
+            "function f(x) { return x; }"),
+        MISSING_PARAMETER_JSDOC);
+    testWarning(
+        lines(
+            "/** ",
+            " * @inheritDoc ",
+            " * @return {string} x ",
+            " */",
+            "Foo.Bar = function(x) { return x; }"), // assigned to a field
+        MISSING_PARAMETER_JSDOC);
+
+    // inline param type
+    testNoWarning(
+        lines(
+            "/** ",
+            " * @override",
+            " * @return {string}",
+            " */",
+            "var f = function(/** string */ x) { return x; }"));
   }
 
   @Test
@@ -1129,8 +1180,6 @@ public void testMissingReturn_var_noWarning() {
     testSame("/** @param {number} x */ var f = function(x) { return; }");
     testSame("/** @param {number} x\n * @return {number} */ var f = function(x) { return x; }");
     testSame("/** @const {function(number): number} */ var f = function(x) { return x; }");
-    testSame("/** @inheritDoc */ var f = function(x) { return x; }");
-    testSame("/** @override */ var f = function(x) { return x; }");
   }
 
   @Test