Add lint check that warns when adding @Suppress.

------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=213481890
google · Sep 19, 2018 · 70f7b8c · 70f7b8c
1 parent dd47783
commit 70f7b8c
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 6 deletions.
diff --git a/src/com/google/javascript/jscomp/lint/CheckJSDocStyle.java b/src/com/google/javascript/jscomp/lint/CheckJSDocStyle.java
@@ -75,6 +75,12 @@ public final class CheckJSDocStyle extends AbstractPostOrderCallback implements
       DiagnosticType.disabled("JSC_OPTIONAL_PARAM_NOT_MARKED_OPTIONAL",
           "Parameter {0} is optional so its type must end with =");
 
+  public static final DiagnosticType WARN_EXCESSIVE_AT_SUPPRESS =
+      DiagnosticType.disabled(
+          "JSC_WARN_EXCESSIVE_AT_SUPPRESS",
+          "Prefer to address the underlying issue"
+              + " rather than using @suppress annotations when possible.");
+
   public static final DiagnosticType WRONG_NUMBER_OF_PARAMS =
       DiagnosticType.disabled("JSC_WRONG_NUMBER_OF_PARAMS", "Wrong number of @param annotations");
 
@@ -102,6 +108,7 @@ public final class CheckJSDocStyle extends AbstractPostOrderCallback implements
           MUST_BE_PRIVATE,
           MUST_HAVE_TRAILING_UNDERSCORE,
           OPTIONAL_PARAM_NOT_MARKED_OPTIONAL,
+          WARN_EXCESSIVE_AT_SUPPRESS,
           WRONG_NUMBER_OF_PARAMS,
           INCORRECT_PARAM_NAME,
           EXTERNS_FILES_SHOULD_BE_ANNOTATED,
@@ -135,7 +142,10 @@ public void visit(NodeTraversal t, Node n, Node parent) {
       case LET:
       case CONST:
       case STRING_KEY:
+        break;
       case SCRIPT:
+        JSDocInfo jsDoc = n.getJSDocInfo();
+        warnIfAtCodeOrAtSuppressPresent(t, n, jsDoc);
         break;
       case MEMBER_FUNCTION_DEF:
       case GETTER_DEF:
@@ -151,25 +161,28 @@ public void visit(NodeTraversal t, Node n, Node parent) {
     }
   }
 
-  private void checkForAtSignCodePresence(NodeTraversal t, Node n, @Nullable JSDocInfo jsDoc) {
+  private void warnIfAtCodeOrAtSuppressPresent(NodeTraversal t, Node n, @Nullable JSDocInfo jsDoc) {
     if (jsDoc == null) {
       return;
     }
     if (jsDoc.isAtSignCodePresent()) {
       t.report(n, PREFER_BACKTICKS_TO_AT_SIGN_CODE);
     }
+    if (!jsDoc.getSuppressions().isEmpty()) {
+      t.report(n, WARN_EXCESSIVE_AT_SUPPRESS);
+    }
   }
 
   private void visitNonFunction(NodeTraversal t, Node n) {
     JSDocInfo jsDoc = n.getJSDocInfo();
 
-    checkForAtSignCodePresence(t, n, jsDoc);
+    warnIfAtCodeOrAtSuppressPresent(t, n, jsDoc);
   }
 
   private void checkStyleForPrivateProperties(NodeTraversal t, Node n) {
     JSDocInfo jsDoc = NodeUtil.getBestJSDocInfo(n);
 
-    checkForAtSignCodePresence(t, n, jsDoc);
+    warnIfAtCodeOrAtSuppressPresent(t, n, jsDoc);
 
     String name;
     if (n.isMemberFunctionDef() || n.isGetterDef() || n.isSetterDef()) {
@@ -201,7 +214,7 @@ private void checkStyleForPrivateProperties(NodeTraversal t, Node n) {
   private void visitFunction(NodeTraversal t, Node function, Node parent) {
     JSDocInfo jsDoc = NodeUtil.getBestJSDocInfo(function);
 
-    checkForAtSignCodePresence(t, function, jsDoc);
+    warnIfAtCodeOrAtSuppressPresent(t, function, jsDoc);
 
     if (jsDoc == null && !hasAnyInlineJsDoc(function)) {
       checkMissingJsDoc(t, function);
@@ -226,7 +239,7 @@ private void visitFunction(NodeTraversal t, Node function, Node parent) {
   private void visitClass(NodeTraversal t, Node cls) {
     JSDocInfo jsDoc = NodeUtil.getBestJSDocInfo(cls);
 
-    checkForAtSignCodePresence(t, cls, jsDoc);
+    warnIfAtCodeOrAtSuppressPresent(t, cls, jsDoc);
 
     if (jsDoc == null) {
       return;

diff --git a/src/com/google/javascript/refactoring/testing/RefasterJsTestUtils.java b/src/com/google/javascript/refactoring/testing/RefasterJsTestUtils.java
@@ -21,8 +21,10 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.io.Files;
 import com.google.common.truth.Correspondence;
+import com.google.javascript.jscomp.CheckLevel;
 import com.google.javascript.jscomp.CommandLineRunner;
 import com.google.javascript.jscomp.CompilerOptions;
+import com.google.javascript.jscomp.DiagnosticGroups;
 import com.google.javascript.refactoring.ApplySuggestedFixes;
 import com.google.javascript.refactoring.RefactoringDriver;
 import com.google.javascript.refactoring.RefasterJsScanner;
@@ -69,9 +71,13 @@ public static void assertFileRefactoring(
     }
     final ImmutableList<String> expectedCode = expectedCodeBuilder.build();
 
+    CompilerOptions options = RefactoringDriver.getCompilerOptions();
+    // turn off lint checks for the tests
+    options.setWarningLevel(DiagnosticGroups.LINT_CHECKS, CheckLevel.OFF);
     RefactoringDriver.Builder driverBuilder =
         new RefactoringDriver.Builder()
-            .addExterns(CommandLineRunner.getBuiltinExterns(CompilerOptions.Environment.BROWSER));
+            .addExterns(CommandLineRunner.getBuiltinExterns(CompilerOptions.Environment.BROWSER))
+            .withCompilerOptions(options);
 
     for (String additionalSource : additionalSourceFiles) {
       driverBuilder.addInputsFromFile(testDataPathPrefix + File.separator + additionalSource);

diff --git a/test/com/google/javascript/jscomp/lint/CheckJSDocStyleTest.java b/test/com/google/javascript/jscomp/lint/CheckJSDocStyleTest.java
@@ -27,6 +27,7 @@
 import static com.google.javascript.jscomp.lint.CheckJSDocStyle.MUST_HAVE_TRAILING_UNDERSCORE;
 import static com.google.javascript.jscomp.lint.CheckJSDocStyle.OPTIONAL_PARAM_NOT_MARKED_OPTIONAL;
 import static com.google.javascript.jscomp.lint.CheckJSDocStyle.PREFER_BACKTICKS_TO_AT_SIGN_CODE;
+import static com.google.javascript.jscomp.lint.CheckJSDocStyle.WARN_EXCESSIVE_AT_SUPPRESS;
 import static com.google.javascript.jscomp.lint.CheckJSDocStyle.WRONG_NUMBER_OF_PARAMS;
 
 import com.google.javascript.jscomp.CheckLevel;
@@ -1076,4 +1077,26 @@ public void testAtSignCodeDetectedWhenPresent() {
         "/** blah blah {@code blah blah} blah blah */ function f() {}",
         PREFER_BACKTICKS_TO_AT_SIGN_CODE);
   }
+
+  public void testAtCodeInFileOverview() {
+    testWarning(
+        "/** @fileoverview {@code code} */ /** @const */ var example;",
+        PREFER_BACKTICKS_TO_AT_SIGN_CODE);
+  }
+
+  public void testAtSuppressDetectedForFunction() {
+    testWarning("/** @suppress {underscore} */ function f() {}", WARN_EXCESSIVE_AT_SUPPRESS);
+  }
+
+  public void testAtSuppressDetectedWithinFunction() {
+    testWarning(
+        "/** blah */ function f() { /** @suppress {underscore} @private */ obj.private = f; }",
+        WARN_EXCESSIVE_AT_SUPPRESS);
+  }
+
+  public void testAtSuppressInFileOverview() {
+    testWarning(
+        "/** @fileoverview  @suppress {visibility} */ /** @const */ var example;",
+        WARN_EXCESSIVE_AT_SUPPRESS);
+  }
 }