Skip to content

Commit

Permalink
Remove replacing of file separator in "globs" with a "\" on Windows p…
Browse files Browse the repository at this point in the history 
…latform

Closes pinterest#1599
  • Loading branch information
@paul-dingemans
paul-dingemans committed Aug 27, 2022
1 parent 33de17a commit e843b80
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 85 deletions.
33 changes: 19 additions & 14 deletions ktlint/src/main/kotlin/com/pinterest/ktlint/internal/FileUtils.kt
View file
Expand Up @@ -28,11 +28,10 @@ internal val workDir: String = File(".").canonicalPath
private val tildeRegex = Regex("^(!)?~")
private const val NEGATION_PREFIX = "!"

private val os = System.getProperty("os.name")
private val userHome = System.getProperty("user.home")

private val defaultKotlinFileExtensions = setOf("kt", "kts")
internal val defaultPatterns = defaultKotlinFileExtensions.map { "**$globSeparator*.$it" }
internal val defaultPatterns = defaultKotlinFileExtensions.map { "**/*.$it" }

/**
* Transform the [patterns] to a sequence of files. Each element in [patterns] can be a glob, a file or directory path
Expand Down Expand Up @@ -133,8 +132,14 @@ private fun FileSystem.expand(
) =
patterns
.map { it.expandTildeToFullPath() }
.map { it.replace(File.separator, globSeparator) }
.flatMap { path -> toGlob(path, rootDir) }
.map {
if (onWindowsOS) {
// By definition the globs should use "/" as separator. Out of courtesy replace "\" with "/"
it.replace(File.separator, "/")
} else {
it
}
}.flatMap { path -> toGlob(path, rootDir) }

private fun FileSystem.toGlob(
path: String,
Expand All @@ -157,16 +162,16 @@ private fun FileSystem.toGlob(
} else if (isGlobAbsolutePath(pathWithoutNegationPrefix)) {
listOf(pathWithoutNegationPrefix)
} else {
listOf(pathWithoutNegationPrefix.prefixIfNot("**$globSeparator"))
listOf(pathWithoutNegationPrefix.prefixIfNot("**/"))
}
return expandedGlobs.map { "${negation}glob:$it" }
}

private fun getDefaultPatternsForPath(path: Path?) = defaultKotlinFileExtensions
.flatMap {
listOf(
"$path$globSeparator*.$it",
"$path$globSeparator**$globSeparator*.$it",
"$path/*.$it",
"$path/**/*.$it",
)
}

Expand All @@ -175,12 +180,6 @@ private fun FileSystem.isGlobAbsolutePath(glob: String) =
.map { it.toString() }
.any { glob.startsWith(it) }

private val globSeparator: String get() =
when {
os.startsWith("windows", ignoreCase = true) -> "\\\\"
else -> "/"
}

/**
* List of paths to Java `jar` files.
*/
Expand All @@ -198,13 +197,19 @@ internal fun JarFiles.toFilesURIList() = map {
// a complete solution would be to implement https://www.gnu.org/software/bash/manual/html_node/Tilde-Expansion.html
// this implementation takes care only of the most commonly used case (~/)
internal fun String.expandTildeToFullPath(): String =
if (os.startsWith("windows", true)) {
if (onWindowsOS) {
// Windows sometimes inserts `~` into paths when using short directory names notation, e.g. `C:\Users\USERNA~1\Documents
this
} else {
replaceFirst(tildeRegex, userHome)
}

private val onWindowsOS
get() =
System
.getProperty("os.name")
.startsWith("windows", true)

internal fun File.location(
relative: Boolean,
) = if (relative) this.toRelativeString(File(workDir)) else this.path
Expand Down
99 changes: 28 additions & 71 deletions ktlint/src/test/kotlin/com/pinterest/ktlint/internal/FileUtilsTest.kt
View file
Expand Up @@ -7,15 +7,13 @@ import java.io.File
import java.nio.file.FileSystem
import java.nio.file.Files
import java.nio.file.Path
import java.util.Locale
import mu.KotlinLogging
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Assumptions.assumeTrue
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.condition.DisabledOnOs
import org.junit.jupiter.api.condition.EnabledOnOs
import org.junit.jupiter.api.condition.OS
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.ValueSource
Expand All @@ -25,7 +23,7 @@ private val logger = KotlinLogging.logger {}.initKtLintKLogger()
/**
* Tests for [fileSequence] method.
*/
internal class FileUtilsFileSequenceTest {
internal class FileUtilsTest {
private val tempFileSystem = Jimfs.newFileSystem(Configuration.forCurrentPlatform())

private val rootDir = tempFileSystem.rootDirectories.first().toString()
Expand Down Expand Up @@ -91,8 +89,8 @@ internal class FileUtilsFileSequenceTest {
fun `Given some patterns and no workdir then ignore all files in hidden directories`() {
val foundFiles = getFiles(
patterns = listOf(
"project1/**/*.kt".normalizeGlob(),
"project1/*.kt".normalizeGlob(),
"project1/**/*.kt",
"project1/*.kt",
),
)

Expand All @@ -108,49 +106,25 @@ internal class FileUtilsFileSequenceTest {
)
}

@Nested
inner class NegatePattern {
@Test
fun `Given some patterns including a negate pattern and no workdir then select all files except files in the negate pattern`() {
val foundFiles = getFiles(
patterns = listOf(
"project1/src/**/*.kt".normalizeGlob(),
"!project1/src/**/example/*.kt".normalizeGlob(),
),
)

assertThat(foundFiles)
.containsExactlyInAnyOrder(ktFile1InProjectSubDirectory)
.doesNotContain(ktFile2InProjectSubDirectory)
}

@Test
fun `Given the Windows OS and some unescaped patterns including a negate pattern and no workdir then ignore all files in the negate pattern`() {
assumeTrue(
System
.getProperty("os.name")
.lowercase(Locale.getDefault())
.startsWith("windows"),
)

val foundFiles = getFiles(
patterns = listOf(
"project1\\src\\**\\*.kt".normalizeGlob(),
"!project1\\src\\**\\example\\*.kt".normalizeGlob(),
),
)
@Test
fun `Given some patterns including a negate pattern and no workdir then select all files except files in the negate pattern`() {
val foundFiles = getFiles(
patterns = listOf(
"project1/src/**/*.kt",
"!project1/src/**/example/*.kt",
),
)

assertThat(foundFiles)
.containsExactlyInAnyOrder(ktFile1InProjectSubDirectory)
.doesNotContain(ktFile2InProjectSubDirectory)
}
assertThat(foundFiles)
.containsExactlyInAnyOrder(ktFile1InProjectSubDirectory)
.doesNotContain(ktFile2InProjectSubDirectory)
}

@Test
fun `Given a pattern and a workdir then find all files in that workdir and all its sub directories that match the pattern`() {
val foundFiles = getFiles(
patterns = listOf(
"**/main/**/*.kt".normalizeGlob(),
"**/main/**/*.kt",
),
rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath()),
)
Expand All @@ -164,7 +138,7 @@ internal class FileUtilsFileSequenceTest {
@Test
fun `Given an (relative) file path from the workdir then find all files in that workdir and all its sub directories that match the pattern`() {
val foundFiles = getFiles(
patterns = listOf("src/main/kotlin/One.kt".normalizeGlob()),
patterns = listOf("src/main/kotlin/One.kt"),
rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath()),
)

Expand All @@ -177,8 +151,8 @@ internal class FileUtilsFileSequenceTest {
fun `Given an (absolute) file path and a workdir then find that absolute path and all files in the workdir and all its sub directories that match the pattern`() {
val foundFiles = getFiles(
patterns = listOf(
"src/main/kotlin/One.kt".normalizeGlob(),
ktFile2InProjectSubDirectory.normalizeGlob(),
"src/main/kotlin/One.kt",
ktFile2InProjectSubDirectory,
),
rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath()),
)
Expand All @@ -193,7 +167,7 @@ internal class FileUtilsFileSequenceTest {
fun `Given a glob containing an (absolute) file path and a workdir then find all files match the pattern`() {
val foundFiles = getFiles(
patterns = listOf(
"${rootDir}project1/src/**/*.kt".normalizeGlob(),
"${rootDir}project1/src/**/*.kt",
),
rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath()),
)
Expand Down Expand Up @@ -224,7 +198,7 @@ internal class FileUtilsFileSequenceTest {
tempFileSystem.createFile(filePath)

val foundFiles = getFiles(
patterns = listOf(pattern.normalizeGlob()),
patterns = listOf(pattern),
)

assertThat(foundFiles).containsExactlyInAnyOrder(filePath)
Expand All @@ -234,7 +208,7 @@ internal class FileUtilsFileSequenceTest {
fun `Given a pattern containing a double star and a workdir without subdirectories then find all files in that workdir`() {
val foundFiles = getFiles(
patterns = listOf(
"**/*.kt".normalizeGlob(),
"**/*.kt",
),
rootDir = tempFileSystem.getPath("${rootDir}project1/src/main/kotlin/".normalizePath()),
)
Expand All @@ -245,19 +219,17 @@ internal class FileUtilsFileSequenceTest {
)
}

// Jimfs does not currently support the Windows syntax for an absolute path on the current drive (e.g. "\foo\bar")
// @DisabledOnOs(OS.WINDOWS)
@Test
fun `Given a (relative) directory path (but not a glob) from the workdir then find all files in that workdir and it subdirectories having the default kotlin extensions`() {
logger.info {
val patterns = "src/main/kotlin".normalizeGlob()
val patterns = "src/main/kotlin"
val dir = "${rootDir}project1".normalizePath()
"`Given a (relative) directory path (but not a glob) from the workdir then find all files in that workdir and it subdirectories having the default kotlin extensions`\n" +
"\tpatterns = $patterns\n" +
"\trootDir = $dir"
}
val foundFiles = getFiles(
patterns = listOf("src/main/kotlin".normalizeGlob()),
patterns = listOf("src/main/kotlin"),
rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath()),
)

Expand All @@ -269,19 +241,13 @@ internal class FileUtilsFileSequenceTest {
)
}

@EnabledOnOs(OS.WINDOWS)
@Test
fun `Given the Windows OS and some unescaped globs including a negate pattern and no workdir then ignore all files in the negate pattern`() {
assumeTrue(
System
.getProperty("os.name")
.lowercase(Locale.getDefault())
.startsWith("windows"),
)

fun `Given the Windows OS and some globs using backslash as file separator the convert the globs to using a forward slash`() {
val foundFiles = getFiles(
patterns = listOf(
"project1\\src\\**\\*.kt".normalizeGlob(),
"!project1\\src\\**\\example\\*.kt".normalizeGlob(),
"project1\\src\\**\\*.kt",
"!project1\\src\\**\\example\\*.kt",
),
)

Expand All @@ -291,7 +257,6 @@ internal class FileUtilsFileSequenceTest {
}

private fun String.normalizePath() = replace('/', File.separatorChar)
private fun String.normalizeGlob(): String = replace("/", rawGlobSeparator)

private fun FileSystem.createFile(it: String) {
val filePath = getPath(it.normalizePath())
Expand All @@ -313,11 +278,3 @@ internal class FileUtilsFileSequenceTest {
}
}
}

internal val rawGlobSeparator: String get() {
val os = System.getProperty("os.name")
return when {
os.startsWith("windows", ignoreCase = true) -> "\\"
else -> "/"
}
}

0 comments on commit e843b80

Please sign in to comment.