From 70f7b8c84dc6d8de32002eee11e2c64e69bfcb26 Mon Sep 17 00:00:00 2001 From: yitingwang Date: Tue, 18 Sep 2018 11:16:15 -0700 Subject: [PATCH] Add lint check that warns when adding @suppress. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=213481890 --- .../jscomp/lint/CheckJSDocStyle.java | 23 +++++++++++++++---- .../testing/RefasterJsTestUtils.java | 8 ++++++- .../jscomp/lint/CheckJSDocStyleTest.java | 23 +++++++++++++++++++ 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/com/google/javascript/jscomp/lint/CheckJSDocStyle.java b/src/com/google/javascript/jscomp/lint/CheckJSDocStyle.java index a6cb3a3bc69..eef3b7fb23b 100644 --- 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 index 64999505414..66360e04422 100644 --- 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 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 index 97e5b9e2cd2..b8cb09c2446 100644 --- 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); + } }