Allow duplicate @Suppress tags by collecting the union of suppressions.

This makes it much easier for automated tools to add to the list of suppressions for a file without having to parse and merge JSDoc, just by appending to an existing @fileoverview comment. This reduces errors reported from human authored code, but it seems unlikely that a duplicate @Suppress tag is an actual mistake – human authored code usually has all suppress tags in the same line in a short comment, so it seems unlikely that anybody would accidentally add a dupe. On top of that, it seems fairly clear that taking the union of suppressions is the intended behaviour when having multiple @Suppress tags. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=204550634
google · Jul 17, 2018 · 2d724d2 · 2d724d2
1 parent 6173921
commit 2d724d2
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 25 deletions.
diff --git a/src/com/google/javascript/jscomp/parsing/JsDocInfoParser.java b/src/com/google/javascript/jscomp/parsing/JsDocInfoParser.java
@@ -1338,9 +1338,7 @@ private JsDocToken parseSuppressTag(JsDocToken token) {
         addParserWarning("msg.jsdoc.suppress");
       } else {
         token = next();
-        if (!jsdocBuilder.recordSuppressions(suppressions)) {
-          addParserWarning("msg.jsdoc.suppress.duplicate");
-        }
+        jsdocBuilder.recordSuppressions(suppressions);
       }
       return eatUntilEOLIfNotAnnotation();
     }

diff --git a/src/com/google/javascript/jscomp/resources.json b/src/com/google/javascript/jscomp/resources.json
diff --git a/src/com/google/javascript/rhino/JSDocInfo.java b/src/com/google/javascript/rhino/JSDocInfo.java
@@ -47,6 +47,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Sets;
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -1099,18 +1100,16 @@ void addSuppression(String suppression) {
   }
 
   /**
-   * Sets suppressed warnings.
+   * Adds a set of suppressions to the (possibly currently empty) set of suppressions.
    * @param suppressions A list of suppressed warning types.
    */
-  boolean setSuppressions(Set<String> suppressions) {
+  void addSuppressions(Set<String> suppressions) {
     lazyInitInfo();
 
     if (info.suppressions != null) {
-      return false;
+      suppressions = Sets.union(suppressions, info.suppressions);
     }
-
     info.suppressions = ImmutableSet.copyOf(suppressions);
-    return true;
   }
 
   /**

diff --git a/src/com/google/javascript/rhino/JSDocInfoBuilder.java b/src/com/google/javascript/rhino/JSDocInfoBuilder.java
@@ -531,15 +531,12 @@ public boolean isDeprecationReasonRecorded() {
   }
 
   /**
-   * Records the list of suppressed warnings.
+   * Records the list of suppressed warnings, possibly adding to the set of already configured
+   * warnings.
    */
-  public boolean recordSuppressions(Set<String> suppressions) {
-    if (currentInfo.setSuppressions(suppressions)) {
-      populated = true;
-      return true;
-    } else {
-      return false;
-    }
+  public void recordSuppressions(Set<String> suppressions) {
+    currentInfo.addSuppressions(suppressions);
+    populated = true;
   }
 
   public void addSuppression(String suppression) {

diff --git a/src/com/google/javascript/rhino/Messages.properties b/src/com/google/javascript/rhino/Messages.properties
@@ -262,9 +262,6 @@ msg.jsdoc.extraversion =\
 msg.jsdoc.suppress =\
     malformed @suppress tag
 
-msg.jsdoc.suppress.duplicate =\
-    duplicate @suppress tag
-
 msg.jsdoc.suppress.unknown =\
     unknown @suppress parameter: {0}
 

diff --git a/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java b/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java
@@ -2396,6 +2396,11 @@ public void testJsDocAfterSuppress() {
     assertTypeEquals(STRING_TYPE, info.getType());
   }
 
+  public void testMultipleSuppressTags() {
+    JSDocInfo info = parse("@suppress {x} \n * @suppress {y} */");
+    assertThat(info.getSuppressions()).isEqualTo(ImmutableSet.of("x", "y"));
+  }
+
   public void testBadSuppress1() {
     parse("@suppress {} */", "malformed @suppress tag");
   }
@@ -2412,10 +2417,6 @@ public void testBadSuppress4() {
     parse("@suppress {x|y */", "malformed @suppress tag");
   }
 
-  public void testBadSuppress6() {
-    parse("@suppress {x} \n * @suppress {y} */", "duplicate @suppress tag");
-  }
-
   public void testBadSuppress7() {
     parse("@suppress {impossible} */",
           "unknown @suppress parameter: impossible");

diff --git a/test/com/google/javascript/rhino/JSDocInfoTest.java b/test/com/google/javascript/rhino/JSDocInfoTest.java
@@ -521,7 +521,7 @@ public void testSetFileOverviewWithDocumentationOn() {
 
   public void testSetSuppressions() {
     JSDocInfo info = new JSDocInfo(true);
-    info.setSuppressions(ImmutableSet.of("sam", "bob"));
+    info.addSuppressions(ImmutableSet.of("sam", "bob"));
     assertEquals(ImmutableSet.of("bob", "sam"), info.getSuppressions());
   }