Add notifyWhitelistedConformanceViolation() method to ErrorManager an…

…d refactor conformance violation whitelist checking.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=217924889

src/com/google/javascript/jscomp/CheckConformance.java

@@ -239,6 +239,10 @@ public static class InvalidRequirementSpec extends Exception {
     InvalidRequirementSpec(String message) {
       super(message);
     }
+
+    InvalidRequirementSpec(String message, Throwable cause) {
+      super(message, cause);
+    }
   }
 
   /**

src/com/google/javascript/jscomp/ConformanceRules.java

@@ -22,6 +22,7 @@
 import com.google.common.annotations.GwtIncompatible;
 import com.google.common.base.Ascii;
 import com.google.common.base.Joiner;
+import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
@@ -35,7 +36,6 @@
 import com.google.javascript.jscomp.CodingConvention.AssertionFunctionSpec;
 import com.google.javascript.jscomp.Requirement.Severity;
 import com.google.javascript.jscomp.Requirement.Type;
-import com.google.javascript.jscomp.Requirement.WhitelistEntry;
 import com.google.javascript.jscomp.parsing.JsDocInfoParser;
 import com.google.javascript.rhino.JSDocInfo;
 import com.google.javascript.rhino.JSDocInfo.Visibility;
@@ -115,6 +115,64 @@ public static enum ConformanceLevel {
     VIOLATION,
   }
 
+  private static class Whitelist {
+    @Nullable final ImmutableList<String> prefixes;
+    @Nullable final Pattern regexp;
+    @Nullable final Requirement.WhitelistEntry whitelistEntry;
+
+    Whitelist(List<String> prefixes, List<String> regexps) throws InvalidRequirementSpec {
+      this.prefixes = ImmutableList.<String>copyOf(prefixes);
+      this.regexp = buildPattern(regexps);
+      this.whitelistEntry = null;
+    }
+
+    Whitelist(Requirement.WhitelistEntry whitelistEntry) throws InvalidRequirementSpec {
+      this.prefixes = ImmutableList.copyOf(whitelistEntry.getPrefixList());
+      this.regexp = buildPattern(whitelistEntry.getRegexpList());
+      this.whitelistEntry = whitelistEntry;
+    }
+
+    /**
+     * Returns true if the given path matches one of the prefixes or regexps, and false otherwise
+     */
+    boolean matches(String path) {
+      if (prefixes != null) {
+        for (String prefix : prefixes) {
+          if (!path.isEmpty() && path.startsWith(prefix)) {
+            return true;
+          }
+        }
+      }
+
+      return regexp != null && regexp.matcher(path).find();
+    }
+
+    @Nullable
+    private static Pattern buildPattern(List<String> reqPatterns) throws InvalidRequirementSpec {
+      if (reqPatterns == null || reqPatterns.isEmpty()) {
+        return null;
+      }
+
+      // validate the patterns
+      for (String reqPattern : reqPatterns) {
+        try {
+          Pattern.compile(reqPattern);
+        } catch (PatternSyntaxException e) {
+          throw new InvalidRequirementSpec("invalid regex pattern", e);
+        }
+      }
+
+      Pattern pattern = null;
+      try {
+        String jointRegExp = "(" + Joiner.on("|").join(reqPatterns) + ")";
+        pattern = Pattern.compile(jointRegExp);
+      } catch (PatternSyntaxException e) {
+        throw new RuntimeException("bad joined regexp", e);
+      }
+      return pattern;
+    }
+  }
+
   /**
    * A conformance rule implementation to support things common to all rules such as whitelisting
    * and reporting.
@@ -123,10 +181,8 @@ public abstract static class AbstractRule implements Rule {
     final AbstractCompiler compiler;
     final String message;
     final Severity severity;
-    final ImmutableList<String> whitelist;
-    final ImmutableList<String> onlyApplyTo;
-    @Nullable final ImmutableList<Pattern> whitelistRegexps;
-    @Nullable final Pattern onlyApplyToRegexp;
+    final ImmutableList<Whitelist> whitelists;
+    @Nullable final Whitelist onlyApplyTo;
     final boolean reportLooseTypeViolations;
     final TypeMatchingStrategy typeMatchingStrategy;
     final Requirement requirement;
@@ -144,26 +200,25 @@ public AbstractRule(AbstractCompiler compiler, Requirement requirement)
         severity = requirement.getSeverity();
       }
 
-      ImmutableList.Builder<String> whitelistBuilder = new ImmutableList.Builder<>();
-      ImmutableList.Builder<Pattern> whitelistRegexpsBuilder = new ImmutableList.Builder<>();
-      for (WhitelistEntry entry : requirement.getWhitelistEntryList()) {
-        whitelistBuilder.addAll(entry.getPrefixList());
-        if (entry.getRegexpCount() > 0) {
-          whitelistRegexpsBuilder.add(buildPattern(entry.getRegexpList()));
-        }
+      // build whitelists
+      ImmutableList.Builder<Whitelist> whitelistsBuilder = new ImmutableList.Builder<>();
+      for (Requirement.WhitelistEntry entry : requirement.getWhitelistEntryList()) {
+        whitelistsBuilder.add(new Whitelist(entry));
       }
 
-      whitelistBuilder.addAll(requirement.getWhitelistList());
-      if (requirement.getWhitelistRegexpCount() > 0) {
-        whitelistRegexpsBuilder.add(buildPattern(requirement.getWhitelistRegexpList()));
+      if (requirement.getWhitelistCount() > 0 || requirement.getWhitelistRegexpCount() > 0) {
+        Whitelist whitelist =
+            new Whitelist(requirement.getWhitelistList(), requirement.getWhitelistRegexpList());
+        whitelistsBuilder.add(whitelist);
       }
+      whitelists = whitelistsBuilder.build();
 
-      whitelist = whitelistBuilder.build();
-      whitelistRegexps = whitelistRegexpsBuilder.build();
-
-      onlyApplyTo = ImmutableList.copyOf(requirement.getOnlyApplyToList());
-      onlyApplyToRegexp = buildPattern(
-          requirement.getOnlyApplyToRegexpList());
+      if (requirement.getOnlyApplyToCount() > 0 || requirement.getOnlyApplyToRegexpCount() > 0) {
+        onlyApplyTo =
+            new Whitelist(requirement.getOnlyApplyToList(), requirement.getOnlyApplyToRegexpList());
+      } else {
+        onlyApplyTo = null;
+      }
       reportLooseTypeViolations = requirement.getReportLooseTypeViolations();
       typeMatchingStrategy = getTypeMatchingStrategy(requirement);
       this.requirement = requirement;
@@ -184,87 +239,22 @@ private static TypeMatchingStrategy getTypeMatchingStrategy(Requirement requirem
       }
     }
 
-    @Nullable
-    private static Pattern buildPattern(List<String> reqPatterns)
-        throws InvalidRequirementSpec {
-      if (reqPatterns == null || reqPatterns.isEmpty()) {
-        return null;
-      }
-
-      // validate the patterns
-      for (String reqPattern : reqPatterns) {
-        try {
-          Pattern.compile(reqPattern);
-        } catch (PatternSyntaxException e) {
-          throw new InvalidRequirementSpec("invalid regex pattern");
-        }
-      }
-
-      Pattern pattern = null;
-      try {
-        String jointRegExp = "(" + Joiner.on("|").join(reqPatterns) + ")";
-        pattern = Pattern.compile(jointRegExp);
-      } catch (PatternSyntaxException e) {
-        throw new RuntimeException("bad joined regexp", e);
-      }
-      return pattern;
-    }
-
     /**
      * @return Whether the code represented by the Node conforms to the
      * rule.
      */
     protected abstract ConformanceResult checkConformance(
         NodeTraversal t, Node n);
 
-    /**
-     * @return Whether the specified Node should be checked for conformance, according to this
-     *     rule's whitelist configuration.
-     */
-    protected final boolean isWhitelistedByRequirement(Node n) {
-      String srcfile = NodeUtil.getSourceName(n);
-      if (srcfile == null) {
-        return true;
-      } else if (!onlyApplyTo.isEmpty() || onlyApplyToRegexp != null) {
-        return pathIsInListOrRegexp(srcfile, onlyApplyTo, onlyApplyToRegexp)
-            && !pathIsInListOrRegexps(srcfile, whitelist, whitelistRegexps);
-      } else {
-        return !pathIsInListOrRegexps(srcfile, whitelist, whitelistRegexps);
-      }
-    }
-
-    private static boolean pathIsInListOrRegexps(
-        String srcfile, ImmutableList<String> list, @Nullable ImmutableList<Pattern> regexps) {
-      if (pathIsInList(srcfile, list)) {
-        return true;
-      }
-      if (regexps == null) {
-        return false;
-      }
-      for (Pattern regexp : regexps) {
-        if (regexp.matcher(srcfile).find()) {
-          return true;
-        }
-      }
-      return false;
-    }
-
-    private static boolean pathIsInListOrRegexp(
-        String srcfile, ImmutableList<String> list, @Nullable Pattern regexp) {
-      if (pathIsInList(srcfile, list)) {
-        return true;
-      }
-      return regexp != null && regexp.matcher(srcfile).find();
-    }
-
-    private static boolean pathIsInList(String srcfile, ImmutableList<String> list) {
-      for (int i = 0; i < list.size(); i++) {
-        String entry = list.get(i);
-        if (!entry.isEmpty() && srcfile.startsWith(entry)) {
-          return true;
+    /** Returns the first Whitelist entry that matches the given path, and null otherwise. */
+    @Nullable
+    private Whitelist findWhitelistForPath(String path) {
+      for (Whitelist whitelist : whitelists) {
+        if (whitelist.matches(path)) {
+          return whitelist;
         }
       }
-      return false;
+      return null;
     }
 
     @Override
@@ -300,10 +290,19 @@ protected void report(Node n, ConformanceResult result) {
           : "\n";
       JSError err = JSError.make(n, msg, message, separator, result.note);
 
-      // Note that shouldReportConformanceViolation has to be called first; this allows
-      // logging violations that would otherwise be whitelisted.
-      if (compiler.getErrorManager().shouldReportConformanceViolation(requirement, err)
-          && isWhitelistedByRequirement(n)) {
+      String path = NodeUtil.getSourceName(n);
+      Whitelist whitelist = path != null ? findWhitelistForPath(path) : null;
+      boolean shouldReport =
+          compiler
+              .getErrorManager()
+              .shouldReportConformanceViolation(
+                  requirement,
+                  whitelist != null
+                      ? Optional.fromNullable(whitelist.whitelistEntry)
+                      : Optional.absent(),
+                  err);
+
+      if (shouldReport && whitelist == null && (onlyApplyTo == null || onlyApplyTo.matches(path))) {
         compiler.report(err);
       }
     }

src/com/google/javascript/jscomp/ConformanceWhitelister.java

@@ -16,6 +16,7 @@
 package com.google.javascript.jscomp;
 
 import com.google.common.annotations.GwtIncompatible;
+import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.javascript.jscomp.Requirement.Severity;
@@ -63,7 +64,9 @@ public static ImmutableList<JSError> getConformanceErrors(
           new ThreadSafeDelegatingErrorManager(oldErrorManager) {
             @Override
             public synchronized boolean shouldReportConformanceViolation(
-                Requirement requirement, JSError diagnostic) {
+                Requirement requirement,
+                Optional<Requirement.WhitelistEntry> whitelistEntry,
+                JSError diagnostic) {
               errors.add(diagnostic);
               return false;
             }

src/com/google/javascript/jscomp/ErrorManager.java

@@ -16,6 +16,9 @@
 
 package com.google.javascript.jscomp;
 
+import com.google.common.base.Optional;
+import com.google.javascript.jscomp.Requirement.WhitelistEntry;
+
 /**
  * The error manager is in charge of storing, organizing and displaying
  * errors and warnings generated by the compiler.
@@ -85,10 +88,11 @@ default boolean hasHaltingErrors() {
   }
 
   /**
-   * Return true if the conformance violation should be reported. This is called before checking the
-   * whitelist inside {@code requirement}, so it can be used to log whitelisted violations.
+   * Return true if the conformance violation should be reported. This is called even if the
+   * violation is whitelisted.
    */
-  default boolean shouldReportConformanceViolation(Requirement requirement, JSError diagnostic) {
+  default boolean shouldReportConformanceViolation(
+      Requirement requirement, Optional<WhitelistEntry> whitelistEntry, JSError diagnostic) {
     return true;
   }
 }

src/com/google/javascript/jscomp/ThreadSafeDelegatingErrorManager.java

@@ -16,6 +16,8 @@
 
 package com.google.javascript.jscomp;
 
+import com.google.common.base.Optional;
+
 /**
  * A simple delegating {@link ErrorManager} that provides a thread-safe wrapper
  * for the one being delegated.
@@ -74,7 +76,9 @@ public synchronized double getTypedPercent() {
 
   @Override
   public synchronized boolean shouldReportConformanceViolation(
-      Requirement requirement, JSError diagnostic) {
-    return delegated.shouldReportConformanceViolation(requirement, diagnostic);
+      Requirement requirement,
+      Optional<Requirement.WhitelistEntry> whitelistEntry,
+      JSError diagnostic) {
+    return delegated.shouldReportConformanceViolation(requirement, whitelistEntry, diagnostic);
   }
 }

src/com/google/javascript/jscomp/gwt/super/com/google/javascript/jscomp/Requirement.java

@@ -24,4 +24,15 @@ public enum Severity {
     WARNING,
     ERROR
   }
+
+  /** No-op replacement for {@code Requirement.WhitelistEntry} */
+  public static class WhitelistEntry {
+    /** No-op replacement for {@code Requirement.WhitelistEntry.Reason} */
+    public enum Reason {
+      UNSPECIFIED,
+      LEGACY,
+      OUT_OF_SCOPE,
+      MANUALLY_REVIEWED
+    }
+  }
 }