Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove irregular files, including symbolic links, from git ls-files. #1559

Merged
merged 3 commits into from
Nov 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ trait ScalafmtRunner {
val files = options.fileFetchMode match {
case m @ (GitFiles | RecursiveSearch) =>
val fetchFiles: AbsoluteFile => Seq[AbsoluteFile] =
if (m == GitFiles) options.gitOps.lsTree(_)
else FileOps.listFiles(_)
if (m == GitFiles) options.gitOps.lsTree
else FileOps.listFiles

options.files.flatMap {
case d if d.jfile.isDirectory => fetchFiles(d).filter(canFormat)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
package org.scalafmt.util

import scala.io.Codec
import java.io._
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths
import java.nio.file.{Files, LinkOption, Path, Paths}
import scala.io.Codec

object FileOps {

def makeAbsolute(workingDir: File)(file: File): File =
if (file.isAbsolute) file
else new File(workingDir, file.getPath)

def isRegularFile(file: File): Boolean =
Files.isRegularFile(file.toPath, LinkOption.NOFOLLOW_LINKS)

def listFiles(path: String): Vector[String] = {
listFiles(new File(path))
}
Expand Down
23 changes: 13 additions & 10 deletions scalafmt-core/shared/src/main/scala/org/scalafmt/util/GitOps.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package org.scalafmt.util

import scala.sys.process.ProcessLogger
import scala.util.{Failure, Success, Try}
import scala.util.Try
import scala.util.control.NonFatal
import java.io.File

Expand Down Expand Up @@ -41,17 +41,20 @@ class GitOpsImpl(private[util] val workingDirectory: AbsoluteFile)
gitRes.map(augmentString(_).lines.toSeq)
}

override def lsTree(dir: AbsoluteFile): Seq[AbsoluteFile] =
override def lsTree(dir: AbsoluteFile): Seq[AbsoluteFile] = {
val cmd = Seq(
"git",
"ls-files",
"--full-name",
dir.path
)
rootDir.fold(Seq.empty[AbsoluteFile]) { rtDir =>
exec(
Seq(
"git",
"ls-files",
"--full-name",
dir.path
)
).getOrElse(Seq.empty).map(f => rtDir / f)
exec(cmd)
.getOrElse(Seq.empty)
.map(f => rtDir / f)
.filter(file => FileOps.isRegularFile(file.jfile))
}
}

override def rootDir: Option[AbsoluteFile] = {
val cmd = Seq(
Expand Down
44 changes: 44 additions & 0 deletions scalafmt-tests/src/test/scala/org/scalafmt/util/DeleteTree.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.scalafmt.util

import java.io.IOException
import java.nio.file.{FileVisitResult, FileVisitor, Files, Path}
import java.nio.file.attribute.BasicFileAttributes

object DeleteTree {
SamirTalwar marked this conversation as resolved.
Show resolved Hide resolved
def deleteTree(path: Path): Unit = {
Files.walkFileTree(path, new DeleteTree)
()
}
}

class DeleteTree extends FileVisitor[Path] {
override def preVisitDirectory(
dir: Path,
attrs: BasicFileAttributes
): FileVisitResult = {
FileVisitResult.CONTINUE
}

override def visitFile(
file: Path,
attrs: BasicFileAttributes
): FileVisitResult = {
Files.delete(file)
FileVisitResult.CONTINUE
}

override def visitFileFailed(
file: Path,
exc: IOException
): FileVisitResult = {
throw exc
}

override def postVisitDirectory(
dir: Path,
exc: IOException
): FileVisitResult = {
Files.delete(dir)
FileVisitResult.CONTINUE
}
}
36 changes: 28 additions & 8 deletions scalafmt-tests/src/test/scala/org/scalafmt/util/GitOpsTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ package org.scalafmt.util
import java.io.File
import java.nio.charset.StandardCharsets
import java.nio.file.{Files, Paths}

import scala.util._
import org.scalatest._
import org.scalactic.source.Position
import org.scalafmt.util.DeleteTree.deleteTree
import org.scalatest._

class GitOpsTest extends fixture.FunSuite {

import Matchers._
import GitOpsTest._
import Matchers._

val root = AbsoluteFile.userDir
val dirName = "gitTestDir"
Expand All @@ -20,7 +20,7 @@ class GitOpsTest extends fixture.FunSuite {

// DESNOTE(2017-08-16, pjrt): Create a temporary git directory for each
// test.
override def withFixture(test: OneArgTest) = {
override def withFixture(test: OneArgTest): Outcome = {
val f = Files.createTempDirectory(dirName)
val absFile = AbsoluteFile.fromPath(f.toString).get
val ops = new GitOpsImpl(absFile)
Expand All @@ -29,9 +29,8 @@ class GitOpsTest extends fixture.FunSuite {
val initF = touch("initialfile")(ops)
add(initF)(ops)
commit(ops)
val t = try test.toNoArgTest(ops)
finally f.toFile.delete
withFixture(t)
try withFixture(test.toNoArgTest(ops))
finally deleteTree(f)
}

def touch(
Expand All @@ -42,6 +41,19 @@ class GitOpsTest extends fixture.FunSuite {
AbsoluteFile.fromPath(f.toString).get
}

def symbolicLinkTo(
file: AbsoluteFile,
name: String = Random.alphanumeric.take(10).mkString,
dir: Option[AbsoluteFile] = None
)(implicit ops: GitOpsImpl): AbsoluteFile = {
val linkFile =
File.createTempFile(name, ".ext", dir.orElse(ops.rootDir).get.jfile)
linkFile.delete()
val link = AbsoluteFile.fromPath(linkFile.toString).get
Files.createSymbolicLink(linkFile.toPath, file.jfile.toPath)
link
}

def mv(f: AbsoluteFile, dir: Option[AbsoluteFile] = None)(
implicit ops: GitOpsImpl
): AbsoluteFile = {
Expand Down Expand Up @@ -97,6 +109,15 @@ class GitOpsTest extends fixture.FunSuite {
ls should contain only (f)
}

test("lsTree should exclude symbolic links") { implicit ops =>
val f = touch()
add(f)
val g = symbolicLinkTo(f)
add(g)
commit
ls should contain only (f)
}

test("lsTree should not return committed files that have been deleted") {
implicit ops =>
val f = touch()
Expand Down Expand Up @@ -252,7 +273,6 @@ class GitOpsTest extends fixture.FunSuite {

private object GitOpsTest {

import OptionValues._
import Matchers._

// Filesystem commands
Expand Down