Merge pull request #267 from eatkins/ordering-contract

Fix and comprehensively test glob ordering
sbt · Aug 30, 2019 · 40d5993 · 40d5993
2 parents 8742353 + 4d031e9
commit 40d5993
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 27 deletions.
diff --git a/io/src/main/scala/sbt/nio/file/Glob.scala b/io/src/main/scala/sbt/nio/file/Glob.scala
@@ -262,13 +262,16 @@ object Glob {
       case Root(leftRoot) =>
         right match {
           case Root(rightRoot) => leftRoot.compareTo(rightRoot)
-          case _: FullFileGlob => -1
-          case _               => -compare(right, left)
+          case FullFileGlob(leftBase, _, _) =>
+            leftRoot.compareTo(leftBase) match {
+              case 0 => -1
+              case i => i
+            }
+          case _ => -compare(right, left)
         }
       case FullFileGlob(leftBase, _, _) =>
         right match {
           case FullFileGlob(rightBase, _, _) => leftBase.compareTo(rightBase)
-          case _: Root                       => 1
           case _                             => -compare(right, left)
         }
     }
@@ -777,35 +780,28 @@ object RelativeGlob {
         case spm: SingleComponentMatcher =>
           y match {
             case that: SingleComponentMatcher => spm.glob.compareTo(that.glob)
-            case _                            => 1
+            case _                            => -1
           }
         case _: FunctionNameFilter =>
           y match {
-            case _: FunctionNameFilter => 0
-            case _                     => 1
+            case _: FunctionNameFilter                        => 0
+            case _: NotMatcher | _: AndMatcher | _: OrMatcher => -1
+            case _                                            => 1
           }
-        case nm: NotMatcher =>
+        case _: NotMatcher =>
           y match {
-            case that: NotMatcher => compare(nm.matcher, that.matcher)
-            case _                => 1
+            case _: NotMatcher | _: AndMatcher | _: OrMatcher => 0
+            case _                                            => 1
           }
-        case am: AndMatcher =>
+        case _: AndMatcher =>
           y match {
-            case that: AndMatcher =>
-              compare(am.left, that.left) match {
-                case 0 => compare(am.right, that.right)
-                case _ => 1
-              }
-            case _ => 1
+            case _: NotMatcher | _: AndMatcher | _: OrMatcher => 0
+            case _                                            => 1
           }
-        case om: OrMatcher =>
+        case _: OrMatcher =>
           y match {
-            case that: OrMatcher =>
-              compare(om.left, that.left) match {
-                case 0 => compare(om.right, that.right)
-                case _ => 1
-              }
-            case _ => 1
+            case _: NotMatcher | _: AndMatcher | _: OrMatcher => 0
+            case _                                            => 1
           }
       }
     }

diff --git a/io/src/test/scala/sbt/nio/GlobOrderingSpec.scala b/io/src/test/scala/sbt/nio/GlobOrderingSpec.scala
@@ -13,22 +13,66 @@ package sbt.nio
 import java.io.File
 
 import org.scalatest.FlatSpec
-import sbt.io.IO
-import sbt.nio.file.{ Glob, RecursiveGlob }
+import sbt.io.{ IO, SimpleFilter }
+import sbt.nio.file.RelativeGlob.Matcher
+import sbt.nio.file._
+import sbt.nio.file.syntax._
+
+import scala.collection.JavaConverters._
 
 class GlobOrderingSpec extends FlatSpec {
   "Globs" should "be ordered" in IO.withTemporaryDirectory { dir =>
     val subdir = new File(dir, "subdir")
     assert(Seq(Glob(subdir), Glob(dir)).sorted == Seq(Glob(dir), Glob(subdir)))
   }
   they should "fall back on depth" in IO.withTemporaryDirectory { dir =>
-    val recursive = Glob(dir, RecursiveGlob)
+    val recursive = Glob(dir, **)
     val nonRecursive = Glob(dir)
     assert(Seq(nonRecursive, recursive).sorted == Seq(recursive, nonRecursive))
   }
   they should "not stack overflow" in IO.withTemporaryDirectory { dir =>
     val exact = Glob(dir.toPath.resolve("foo"))
-    val fullFile = sbt.internal.nio.Globs(dir.toPath, true, sbt.io.HiddenFileFilter)
+    val fullFile =
+      sbt.internal.nio.Globs(dir.toPath / "foo", recursive = true, sbt.io.HiddenFileFilter)
     assert(Seq(exact, fullFile).sorted == Seq(exact, fullFile))
   }
+  they should "not violate sorting contract" in IO.withTemporaryDirectory { dir =>
+    val globs = Seq(
+      **,
+      ** / "foo",
+      ** / * / "bar",
+      ** / "foo" / "bar",
+      ** / Matcher.or(Matcher("foo"), Matcher("bar")),
+      ** / Matcher.and(Matcher("foo"), Matcher("bar")),
+      ** / Matcher.not(Matcher("foo")),
+      ** / Matcher.not(Matcher.and(Matcher("foo"), Matcher("bar"))),
+      ** / Matcher(_.contains("foo")),
+      ** / "foo" / Matcher(_.contains("bar")),
+      ** / "foo" / Matcher.not(Matcher(_.contains("bar"))),
+      ** / "foo" / "*.scala",
+      ** / **,
+      ** / *,
+      (** / "foo") / * / "*.scala",
+      (** / "foo") / * / "*.scala*",
+      (** / "foo") / ** / "*.scala*",
+      Glob(dir.toPath.resolve("foo")),
+      Glob(dir.toPath.resolve("bar")),
+      Glob(dir.toPath.resolve("bar").resolve("baz")),
+      sbt.internal.nio.Globs(dir.toPath, recursive = false, sbt.io.AllPassFilter),
+      sbt.internal.nio.Globs(dir.toPath, recursive = false, new SimpleFilter(_.contains("bar"))),
+      sbt.internal.nio.Globs(dir.toPath, recursive = true, new SimpleFilter(_.contains("baz"))),
+      sbt.internal.nio.Globs(dir.toPath, recursive = true, sbt.io.HiddenFileFilter),
+      sbt.internal.nio.Globs(dir.toPath / "foo", recursive = true, sbt.io.HiddenFileFilter),
+      sbt.internal.nio.Globs(dir.toPath, recursive = true, sbt.io.NothingFilter),
+      sbt.internal.nio.Globs(dir.toPath, recursive = true, new SimpleFilter(_.contains("foo"))),
+      Glob(dir.toPath / "scala", ** / "*.scala"),
+      Glob(dir.toPath / "java", ** / "*.java"),
+      Glob(dir.toPath / "scala", ** / "*.java"),
+    )
+    val javaGlobs = new java.util.ArrayList((globs ++ globs ++ globs).asJava)
+    1 to 1000 foreach { _ =>
+      java.util.Collections.shuffle(javaGlobs)
+      javaGlobs.asScala.sorted
+    }
+  }
 }