Refactoring and conformance improvements

-Expose non-conformant nodes in ConformanceWhitelister
-Provide an API to in c.g.js.refactoring.RefactoringUtils to determine if a file uses the closure library or not.

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

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

@@ -28,6 +28,22 @@ private ConformanceWhitelister() {}
 
   public static ImmutableSet<String> getViolatingPaths(
       AbstractCompiler 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) {
+    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).
@@ -45,11 +61,7 @@ public static ImmutableSet<String> getViolatingPaths(
         new CheckConformance(recordingCompiler, ImmutableList.of(cleanedConfig));
     check.process(externs, ast);
 
-    ImmutableSet.Builder<String> result = ImmutableSet.builder();
-    for (JSError e : recordingCompiler.getConformanceErrors()) {
-      result.add(e.sourceName);
-    }
-    return result.build();
+    return recordingCompiler.getConformanceErrors();
   }
 
   private static class ConformanceViolationRecordingCompiler extends ForwardingCompiler {
@@ -68,6 +80,8 @@ ImmutableList<JSError> getConformanceErrors() {
     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);
       }

src/com/google/javascript/refactoring/RefactoringUtils.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2018 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.javascript.refactoring;
+
+import com.google.javascript.jscomp.NodeUtil;
+import com.google.javascript.rhino.Node;
+
+/** Utility methods for refactoring Java code. */
+public class RefactoringUtils {
+  private RefactoringUtils() {}
+
+  /** Looks for a goog.require(), goog.provide() or goog.module() call in the node's file. */
+  public static boolean isInClosurizedFile(Node node, NodeMetadata metadata) {
+    Node script = NodeUtil.getEnclosingScript(node);
+
+    if (script == null) {
+      return false;
+    }
+
+    Node child = script.getFirstChild();
+    while (child != null) {
+      if (NodeUtil.isExprCall(child)) {
+        if (Matchers.googRequire().matches(child.getFirstChild(), metadata)) {
+          return true;
+        }
+        // goog.require or goog.module.
+      } else if (child.isVar() && child.getBooleanProp(Node.IS_NAMESPACE)) {
+        return true;
+      }
+      child = child.getNext();
+    }
+    return false;
+  }
+}

src/com/google/javascript/refactoring/SuggestedFix.java

@@ -177,7 +177,8 @@ public static final class Builder {
      */
     public Builder attachMatchedNodeInfo(Node node, AbstractCompiler compiler) {
       matchedNodeInfo =
-          MatchedNodeInfo.create(node, isInClosurizedFile(node, new NodeMetadata(compiler)));
+          MatchedNodeInfo.create(
+              node, RefactoringUtils.isInClosurizedFile(node, new NodeMetadata(compiler)));
       return this;
     }
 
@@ -917,28 +918,6 @@ public SuggestedFix build() {
           matchedNodeInfo, replacements.build(), description, alternatives.build());
     }
 
-    /** Looks for a goog.require(), goog.provide() or goog.module() call in the fix's file. */
-    private static boolean isInClosurizedFile(Node node, NodeMetadata metadata) {
-      Node script = NodeUtil.getEnclosingScript(node);
-
-      if (script == null) {
-        return false;
-      }
-
-      Node child = script.getFirstChild();
-      while (child != null) {
-        if (NodeUtil.isExprCall(child)) {
-          if (Matchers.googRequire().matches(child.getFirstChild(), metadata)) {
-            return true;
-          }
-          // goog.require or goog.module.
-        } else if (child.isVar() && child.getBooleanProp(Node.IS_NAMESPACE)) {
-          return true;
-        }
-        child = child.getNext();
-      }
-      return false;
-    }
   }
 
   /**

test/com/google/javascript/jscomp/ConformanceWhitelisterTest.java

@@ -20,9 +20,10 @@
 
 import com.google.common.annotations.GwtIncompatible;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableMultimap;
 import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
 import com.google.javascript.jscomp.Requirement.Type;
+import com.google.javascript.rhino.Node;
 import java.io.IOException;
 import java.util.List;
 import junit.framework.TestCase;
@@ -44,7 +45,7 @@ public void testConformanceWhitelistAddNew() throws IOException {
         .addValue("Object.prototype.innerHTML");
 
     assertThat(testConformanceWhitelister(sources.build(), requirement.build()))
-        .containsExactly("/entry.js");
+        .containsExactly("/entry.js", 2);
   }
 
   public void testConformanceWhitelistRemove() throws IOException {
@@ -81,12 +82,12 @@ public void testConformanceWhitelistPreserve() throws IOException {
         .addWhitelist("/entry.js");
 
     assertThat(testConformanceWhitelister(sources.build(), requirement.build()))
-        .containsExactly("/entry.js");
+        .containsExactly("/entry.js", 2);
   }
 
   // TODO(bangert): Evaluate if this is always the best behaviour.
   // The current behaviour pushes the behaviour of how to cluster the whitelist to the program
-  // driving ConformanceWhitelister. 
+  // driving ConformanceWhitelister.
   public void testConformanceWhitelistBreaksDownFolder() throws IOException {
     ImmutableList.Builder<SourceFile> sources = ImmutableList.builder();
 
@@ -103,10 +104,10 @@ public void testConformanceWhitelistBreaksDownFolder() throws IOException {
         .addWhitelist("/test/");
 
     assertThat(testConformanceWhitelister(sources.build(), requirement.build()))
-        .containsExactly("/test/entry.js");
+        .containsExactly("/test/entry.js", 2);
   }
 
-  private ImmutableSet<String> testConformanceWhitelister(
+  private ImmutableMultimap<String, Integer> testConformanceWhitelister(
       ImmutableList<SourceFile> sources, Requirement config) throws IOException {
 
     CompilerOptions options = new CompilerOptions();
@@ -121,7 +122,16 @@ private ImmutableSet<String> testConformanceWhitelister(
     Result result = compiler.compile(externs, sources, options);
     assertTrue(result.success);
 
-    return ConformanceWhitelister.getViolatingPaths(
-        compiler, compiler.getExternsRoot(), compiler.getJsRoot(), config);
+    ImmutableMultimap.Builder<String, Integer> errors = ImmutableMultimap.builder();
+    for (Node node :
+        ConformanceWhitelister.getViolatingNodes(
+            compiler, compiler.getExternsRoot(), compiler.getJsRoot(), config)) {
+      errors.put(node.getSourceFileName(), node.getLineno());
+    }
+    assertThat(errors.build().keySet())
+        .containsExactlyElementsIn(
+            ConformanceWhitelister.getViolatingPaths(
+                compiler, compiler.getExternsRoot(), compiler.getJsRoot(), config));
+    return errors.build();
   }
 }