From a98343e6f1ef15569bddeee2352e27c828eefea2 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Tue, 18 Dec 2018 10:56:28 -0800 Subject: [PATCH] Don't automatically add path on error There were two semantic bugs in swoval file tree views: 1) When the depth of the path search is set to -1, the file tree view is supposed to return a list that contains only the entry for the source root. For depth >= 0, the source root is not part of the results. 2) recursive traversal of the file system was halted if a subdirectory did not satisfy the filter. I think that I added the exception handler in DefaultFileTreeView to support getting the source root, but this caused problems elsewhere. I fixed the upstream bug so that now DefaultFileTreeView should do the right thing. Similarly, the logic for first listing the results with AllPass and subsequently applying the filter is unnecessary now that I fixed the upstream bug. Since there were not tests of these semantics, I added FileTreeViewSpec. --- .../sbt/internal/io/DefaultFileTreeView.scala | 11 ++++----- io/src/main/scala/sbt/io/Path.scala | 20 +++++++--------- .../test/scala/sbt/io/FileTreeViewSpec.scala | 24 +++++++++++++++++++ project/Dependencies.scala | 2 +- 4 files changed, 37 insertions(+), 20 deletions(-) create mode 100644 io/src/test/scala/sbt/io/FileTreeViewSpec.scala diff --git a/io/src/main/scala/sbt/internal/io/DefaultFileTreeView.scala b/io/src/main/scala/sbt/internal/io/DefaultFileTreeView.scala index ed949dc1..16fe8128 100644 --- a/io/src/main/scala/sbt/internal/io/DefaultFileTreeView.scala +++ b/io/src/main/scala/sbt/internal/io/DefaultFileTreeView.scala @@ -2,15 +2,12 @@ package sbt.internal.io import java.nio.file.{ NoSuchFileException, NotDirectoryException, Path } -import com.swoval.files.{ TypedPath => STypedPath } -import com.swoval.files.FileTreeViews +import com.swoval.files.{ FileTreeViews, TypedPath => STypedPath } import com.swoval.functional.Filters +import sbt.internal.io.SwovalConverters._ import sbt.io.{ FileTreeView, TypedPath } import scala.collection.JavaConverters._ -import SwovalConverters._ - -import scala.util.Try private[sbt] object DefaultFileTreeView extends FileTreeView { private[this] val fileTreeView = @@ -44,8 +41,8 @@ private[sbt] object DefaultFileTreeView extends FileTreeView { case _ => None }) } catch { - case _: NotDirectoryException | _: NoSuchFileException => - Try(Seq(TypedPath(path))).getOrElse(Nil) + case _: NoSuchFileException | _: NotDirectoryException => + Nil } } diff --git a/io/src/main/scala/sbt/io/Path.scala b/io/src/main/scala/sbt/io/Path.scala index 6fa069e3..e18d4f52 100644 --- a/io/src/main/scala/sbt/io/Path.scala +++ b/io/src/main/scala/sbt/io/Path.scala @@ -18,7 +18,6 @@ import java.nio.file.{ import com.swoval.files.FileTreeViews import com.swoval.functional.Filter -import sbt.io.FileTreeView.AllPass import scala.collection.JavaConverters._ import scala.collection.mutable @@ -308,15 +307,13 @@ object Path extends Mapper { } else { val fileTreeView = FileTreeView.DEFAULT (file, filter) => - val unfiltered = fileTreeView.list(file.toPath, 0, AllPass) - unfiltered.flatMap { tp => - val fileName = tp.toPath.toString - val file = new File(fileName) { + val typedPathFilter: TypedPath => Boolean = tp => { + filter.accept(new File(tp.toPath.toString) { override def isDirectory: Boolean = tp.isDirectory override def isFile: Boolean = tp.isFile - } - if (filter.accept(file)) Some(new File(fileName)) else None + }) } + fileTreeView.list(file.toPath, 0, typedPathFilter).map(_.toPath.toFile) } } @@ -501,16 +498,15 @@ private object DescendantOrSelfPathFinder { Integer.MAX_VALUE, new Filter[com.swoval.files.TypedPath] { override def accept(t: com.swoval.files.TypedPath): Boolean = { - val file = new File(t.getPath.toString) { + filter.accept(new File(t.getPath.toString) { override def isDirectory: Boolean = t.isDirectory - override def isFile: Boolean = t.isFile - } - if (filter.accept(file)) fileSet.add(t.getPath.toFile) - t.isDirectory // We need to accept directories for recursive traversal to work + }) } } ) + .asScala + .foreach(tp => fileSet += tp.getPath.toFile) val typedFile = new File(file.toString) { override def isDirectory: Boolean = true override def isFile: Boolean = false diff --git a/io/src/test/scala/sbt/io/FileTreeViewSpec.scala b/io/src/test/scala/sbt/io/FileTreeViewSpec.scala new file mode 100644 index 00000000..5ef0ad76 --- /dev/null +++ b/io/src/test/scala/sbt/io/FileTreeViewSpec.scala @@ -0,0 +1,24 @@ +package sbt.io + +import java.nio.file._ +import org.scalatest.FlatSpec +import sbt.io.FileTreeView.AllPass + +class FileTreeViewSpec extends FlatSpec { + val view = FileTreeView.DEFAULT + "FileTreeView" should "return the source root with depth == -1" in IO.withTemporaryDirectory { + dir => + assert(view.list(dir.toPath, -1, AllPass).map(_.toPath) == Seq(dir.toPath)) + } + "FileTreeView" should "not return the source root with depth >= 0" in IO.withTemporaryDirectory { + dir => + assert(view.list(dir.toPath, 0, AllPass).isEmpty) + assert(view.list(dir.toPath, 10, AllPass).isEmpty) + } + "FileTreeView" should "get recursive files" in IO.withTemporaryDirectory { dir => + val subdir = Files.createDirectory(dir.toPath.resolve("subdir")) + val nestedSubdir = Files.createDirectory(subdir.resolve("nested-subdir")) + val file = Files.createFile(nestedSubdir.resolve("file")) + assert(view.list(dir.toPath, 3, (_: TypedPath).toPath == file).map(_.toPath) == Seq(file)) + } +} diff --git a/project/Dependencies.scala b/project/Dependencies.scala index f70845bb..b2b9870f 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -13,5 +13,5 @@ object Dependencies { val scalatest = "org.scalatest" %% "scalatest" % "3.0.6-SNAP3" val jna = "net.java.dev.jna" % "jna" % "4.5.0" val jnaPlatform = "net.java.dev.jna" % "jna-platform" % "4.5.0" - val swovalFiles = "com.swoval" % "file-tree-views" % "2.0.5" + val swovalFiles = "com.swoval" % "file-tree-views" % "2.0.6" }