Fix glob on Windows and fix regression

Also fix regression on patterns like '!**\./node_modules**.js' introduced by #1409 (dfd1853). Fixes #1407. Fixes #1477.
google · Feb 2, 2016 · 46e309e · 46e309e
1 parent f00239a
commit 46e309e
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 16 deletions.
diff --git a/src/com/google/javascript/jscomp/CommandLineRunner.java b/src/com/google/javascript/jscomp/CommandLineRunner.java
@@ -1491,8 +1491,11 @@ public static List<String> findJsFiles(Collection<String> patterns) throws IOExc
         File matchedFile = new File(pattern);
         if (matchedFile.isDirectory()) {
           matchPaths(new File(matchedFile, "**.js").toString(), allJsInputs, excludes);
-        } else if (!excludes.contains(pattern)) {
-          allJsInputs.add(pattern);
+        } else {
+          String pathString = Paths.get(pattern).normalize().toString();
+          if (!excludes.contains(pathString)) {
+            allJsInputs.add(pathString);
+          }
         }
       } else {
         matchPaths(pattern, allJsInputs, excludes);
@@ -1511,34 +1514,29 @@ private static void matchPaths(String pattern, final Set<String> allJsInputs,
       pattern = pattern.substring(1);
     }
 
-    if (File.separator.equals("\\")) {
-      pattern = pattern.replace('\\', '/');
-    }
+    String separator = File.separator.equals("\\") ? "\\\\" : File.separator;
 
     // Split the pattern into two pieces: the globbing part
     // and the non-globbing prefix.
-    List<String> patternParts = Splitter.on('/').splitToList(pattern);
+    List<String> patternParts = Splitter.on(File.separator).splitToList(pattern);
     String prefix = ".";
     for (int i = 0; i < patternParts.size(); i++) {
       if (patternParts.get(i).contains("*")) {
-        if (i == 0) {
-          break;
-        } else {
-          prefix = Joiner.on(File.separator).join(patternParts.subList(0, i));
-          pattern = Joiner.on(File.separator).join(patternParts.subList(i, patternParts.size()));
+        if (i > 0) {
+          prefix = Joiner.on(separator).join(patternParts.subList(0, i));
+          pattern = Joiner.on(separator).join(patternParts.subList(i, patternParts.size()));
         }
+        break;
       }
     }
 
-    final PathMatcher matcher = fs.getPathMatcher("glob:" + (prefix.equals(".")
-        ? pattern
-        : prefix + File.separator + pattern));
+    final PathMatcher matcher = fs.getPathMatcher("glob:" + prefix + separator + pattern);
     java.nio.file.Files.walkFileTree(
         fs.getPath(prefix), new SimpleFileVisitor<Path>() {
           @Override public FileVisitResult visitFile(
               Path p, BasicFileAttributes attrs) {
-            if (matcher.matches(p.normalize())) {
-              String pathString = p.toString();
+            if (matcher.matches(p) || matcher.matches(p.normalize())) {
+              String pathString = p.normalize().toString();
               if (remove) {
                 excludes.add(pathString);
                 allJsInputs.remove(pathString);

diff --git a/test/com/google/javascript/jscomp/CommandLineRunnerTest.java b/test/com/google/javascript/jscomp/CommandLineRunnerTest.java
@@ -43,6 +43,7 @@
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.PrintStream;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
@@ -1143,6 +1144,48 @@ public void testGlobJs4() throws IOException, FlagUsageException {
         new FlagEntry<>(JsSourceType.JS, glob2));
   }
 
+  public void testGlobJs5() throws IOException, FlagUsageException {
+    FlagEntry<JsSourceType> jsFile1 = createJsFile("test1", "var a;");
+    FlagEntry<JsSourceType> jsFile2 = createJsFile("test2", "var b;");
+    File ignoreDir = new File("./ignore");
+    if (ignoreDir.isDirectory()) {
+      for (File f : ignoreDir.listFiles()) {
+        f.delete();
+      }
+    } else {
+      ignoreDir.delete();
+      ignoreDir = java.nio.file.Files.createDirectory(Paths.get("./ignore")).toFile();
+    }
+    File ignoredFile = new File(ignoreDir + File.separator + "test2.js");
+    new File(jsFile2.value).renameTo(ignoredFile);
+    // Make sure patterns like "!**\./ignore**.js" work
+    String glob1 = "!**\\." + File.separator + "ignore**.js";
+    String glob2 = new File(jsFile1.value).getParent() + File.separator + "**.js";
+    compileFiles(
+        "var a;", new FlagEntry<>(JsSourceType.JS, glob1),
+        new FlagEntry<>(JsSourceType.JS, glob2));
+    ignoredFile.delete();
+    ignoreDir.delete();
+  }
+
+  public void testGlobJs6() throws IOException, FlagUsageException {
+    FlagEntry<JsSourceType> jsFile1 = createJsFile("test1", "var a;");
+    FlagEntry<JsSourceType> jsFile2 = createJsFile("test2", "var b;");
+    File temp1 = Files.createTempDir();
+    File temp2 = Files.createTempDir();
+    File jscompTempDir = new File(jsFile1.value).getParentFile();
+    File newTemp1 = new File(jscompTempDir + File.separator + "temp1");
+    File newTemp2 = new File(jscompTempDir + File.separator + "temp2");
+    temp1.renameTo(newTemp1);
+    temp2.renameTo(newTemp2);
+    new File(jsFile1.value).renameTo(new File(newTemp1 + File.separator + "test1.js"));
+    new File(jsFile2.value).renameTo(new File(newTemp2 + File.separator + "test2.js"));
+    // Test multiple segments with glob patterns, like /foo/bar/**/*.js
+    String glob = jscompTempDir + File.separator + "**" + File.separator + "*.js";
+    compileFiles(
+        "var a;var b;", new FlagEntry<>(JsSourceType.JS, glob));
+  }
+
   public void testSourceMapInputs() throws Exception {
     args.add("--js_output_file");
     args.add("/path/to/out.js");