Removed ForwardingCompiler. Added a hook to log whitelisted conforman…

…ce violations.

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

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

@@ -30,11 +30,13 @@
 import java.util.Set;
 
 /**
- * Provides a framework for checking code against a set of user configured
- * conformance rules.  The rules are specified by the ConformanceConfig
- * proto, which allows for both standard checks (forbidden properties,
- * variables, or dependencies) and allow for more complex checks using
- * custom rules than specify
+ * Provides a framework for checking code against a set of user configured conformance rules. The
+ * rules are specified by the ConformanceConfig proto, which allows for both standard checks
+ * (forbidden properties, variables, or dependencies) and allow for more complex checks using custom
+ * rules than specify
+ *
+ * <p>Conformance violations are both reported as compiler errors, and are also reported separately
+ * to the {cI gue@link ErrorManager}
  *
  */
 @GwtIncompatible("com.google.protobuf")

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

@@ -128,6 +128,7 @@ public abstract static class AbstractRule implements Rule {
     @Nullable final Pattern onlyApplyToRegexp;
     final boolean reportLooseTypeViolations;
     final TypeMatchingStrategy typeMatchingStrategy;
+    final Requirement requirement;
 
     public AbstractRule(AbstractCompiler compiler, Requirement requirement)
         throws InvalidRequirementSpec {
@@ -149,6 +150,7 @@ public AbstractRule(AbstractCompiler compiler, Requirement requirement)
           requirement.getOnlyApplyToRegexpList());
       reportLooseTypeViolations = requirement.getReportLooseTypeViolations();
       typeMatchingStrategy = getTypeMatchingStrategy(requirement);
+      this.requirement = requirement;
     }
 
     private static TypeMatchingStrategy getTypeMatchingStrategy(Requirement requirement) {
@@ -200,10 +202,10 @@ 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.
+     * @return Whether the specified Node should be checked for conformance, according to this
+     *     rule's whitelist configuration.
      */
-    protected final boolean shouldCheckConformance(Node n) {
+    protected final boolean isWhitelistedByRequirement(Node n) {
       String srcfile = NodeUtil.getSourceName(n);
       if (srcfile == null) {
         return true;
@@ -229,19 +231,18 @@ private static boolean pathIsInListOrRegexp(
     @Override
     public final void check(NodeTraversal t, Node n) {
       ConformanceResult result = checkConformance(t, n);
-      if (result.level != ConformanceLevel.CONFORMANCE
-          && shouldCheckConformance(n)) {
-        report(t, n, result);
+      if (result.level != ConformanceLevel.CONFORMANCE) {
+        report(n, result);
       }
     }
 
     /**
      * Report a conformance warning for the given node.
+     *
      * @param n The node representing the violating code.
      * @param result The result representing the confidence of the violation.
      */
-    protected void report(
-        NodeTraversal t, Node n, ConformanceResult result) {
+    protected void report(Node n, ConformanceResult result) {
       DiagnosticType msg;
       if (severity == Severity.ERROR) {
         // Always report findings that are errors, even if the types are too loose to be certain.
@@ -258,7 +259,14 @@ protected void report(
       String separator = (result.note.isEmpty())
           ? ""
           : "\n";
-      t.report(n, msg, message, separator, result.note);
+      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)) {
+        compiler.report(err);
+      }
     }
   }
 

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

@@ -27,65 +27,51 @@ public class ConformanceWhitelister {
   private ConformanceWhitelister() {}
 
   public static ImmutableSet<String> getViolatingPaths(
-      AbstractCompiler compiler, Node externs, Node ast, Requirement requirement) {
+      Compiler compiler, Node externs, Node ast, Requirement requirement) {
     return getConformanceErrors(compiler, externs, ast, requirement)
         .stream()
         .map(e -> e.sourceName)
         .collect(ImmutableSet.toImmutableSet());
   }
 
   public static ImmutableSet<Node> getViolatingNodes(
-      AbstractCompiler compiler, Node externs, Node ast, Requirement requirement) {
+      Compiler compiler, Node externs, Node ast, Requirement requirement) {
     return getConformanceErrors(compiler, externs, ast, requirement)
         .stream()
         .map(e -> e.node)
         .collect(ImmutableSet.toImmutableSet());
   }
 
   public static ImmutableList<JSError> getConformanceErrors(
-      AbstractCompiler compiler, Node externs, Node ast, Requirement requirement) {
-    ConformanceViolationRecordingCompiler recordingCompiler =
-        new ConformanceViolationRecordingCompiler(compiler);
-    // Remove existing prefix whitelist entries, but keep regexps (which we don't re-add either).
-    // TODO(bangert): Check that each regex matches one entry?
+      Compiler compiler, Node externs, Node ast, Requirement requirement) {
     Requirement cleanedRequirement =
         requirement
             .toBuilder()
             .clearWhitelist()
+            .clearWhitelistRegexp()
             .setSeverity(Severity.ERROR)
             .build(); // So we only have one type of error.
-
     ConformanceConfig cleanedConfig =
         ConformanceConfig.newBuilder().addRequirement(cleanedRequirement).build();
-    CheckConformance check =
-        new CheckConformance(recordingCompiler, ImmutableList.of(cleanedConfig));
-    check.process(externs, ast);
-
-    return recordingCompiler.getConformanceErrors();
-  }
 
-  private static class ConformanceViolationRecordingCompiler extends ForwardingCompiler {
-    private final ImmutableList.Builder<JSError> conformanceErrors;
-
-    private ConformanceViolationRecordingCompiler(AbstractCompiler abstractCompiler) {
-      super(abstractCompiler);
-      conformanceErrors = ImmutableList.builder();
+    ErrorManager oldErrorManager = compiler.getErrorManager();
+    final ImmutableList.Builder<JSError> errors = ImmutableList.builder();
+    try {
+      // TODO(bangert): handle invalid conformance requirements
+      compiler.setErrorManager(
+          new ThreadSafeDelegatingErrorManager(oldErrorManager) {
+            @Override
+            public synchronized boolean shouldReportConformanceViolation(
+                Requirement requirement, JSError diagnostic) {
+              errors.add(diagnostic);
+              return false;
+            }
+          });
+      CheckConformance check = new CheckConformance(compiler, ImmutableList.of(cleanedConfig));
+      check.process(externs, ast);
+    } finally {
+      compiler.setErrorManager(oldErrorManager);
     }
-
-    ImmutableList<JSError> getConformanceErrors() {
-      return conformanceErrors.build();
-    }
-
-    @Override
-    public void report(JSError error) {
-      if (error.getType().equals(CheckConformance.CONFORMANCE_ERROR)) {
-        conformanceErrors.add(error);
-      } else if (error.getType().equals(CheckConformance.INVALID_REQUIREMENT_SPEC)) {
-        throw new IllegalArgumentException("Invalid conformance requirement" + error.description);
-      } else {
-        super.report(error);
-      }
-    }
-
+    return errors.build();
   }
 }

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

@@ -83,4 +83,12 @@ default boolean hasHaltingErrors() {
     }
     return false;
   }
+
+  /**
+   * 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.
+   */
+  default boolean shouldReportConformanceViolation(Requirement requirement, JSError diagnostic) {
+    return true;
+  }
 }