diff --git a/CHANGELOG.md b/CHANGELOG.md index 14764dea80..f6a67c13f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ This project adheres to [Semantic Versioning](https://semver.org/). Do not show deprecation warning about property "disabled_rules" when using CLi-parameter `--disabled-rules` ([#1599](https://github.com/pinterest/ktlint/issues/1599)) +* Traversing directory hierarchy at Windows ([#1600](https://github.com/pinterest/ktlint/issues/1600)) +* Ant-style path pattern support ([#1601](https://github.com/pinterest/ktlint/issues/1601)) + ### Added ### Changed diff --git a/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/FileUtils.kt b/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/FileUtils.kt index aee7a34fda..cbf3a6028d 100644 --- a/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/FileUtils.kt +++ b/ktlint/src/main/kotlin/com/pinterest/ktlint/internal/FileUtils.kt @@ -15,7 +15,11 @@ import java.nio.file.Path import java.nio.file.Paths import java.nio.file.SimpleFileVisitor import java.nio.file.attribute.BasicFileAttributes +import java.util.Deque +import java.util.LinkedList +import kotlin.io.path.absolutePathString import kotlin.io.path.isDirectory +import kotlin.io.path.pathString import kotlin.system.exitProcess import kotlin.system.measureTimeMillis import mu.KotlinLogging @@ -28,11 +32,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 @@ -84,7 +87,7 @@ internal fun FileSystem.fileSequence( Start walkFileTree for rootDir: '$rootDir' include: ${pathMatchers.map { " - $it" }} - exlcude: + exclude: ${negatedPathMatchers.map { " - $it" }} """.trimIndent() } @@ -96,13 +99,27 @@ internal fun FileSystem.fileSequence( filePath: Path, fileAttrs: BasicFileAttributes, ): FileVisitResult { - if (negatedPathMatchers.none { it.matches(filePath) } && - pathMatchers.any { it.matches(filePath) } + val path = + if (onWindowsOS) { + Paths.get( + filePath + .absolutePathString() + .replace(File.separatorChar, '/'), + ).also { + if (it != filePath) { + logger.trace { "On WindowsOS transform '$filePath' to '$it'" } + } + } + } else { + filePath + } + if (negatedPathMatchers.none { it.matches(path) } && + pathMatchers.any { it.matches(path) } ) { - logger.trace { "- File: $filePath: Include" } - result.add(filePath) + logger.trace { "- File: $path: Include" } + result.add(path) } else { - logger.trace { "- File: $filePath: Ignore" } + logger.trace { "- File: $path: Ignore" } } return FileVisitResult.CONTINUE } @@ -127,14 +144,32 @@ internal fun FileSystem.fileSequence( return result.asSequence() } -private fun FileSystem.expand( +internal fun FileSystem.expand( patterns: List, rootDir: Path, ) = patterns - .map { it.expandTildeToFullPath() } - .map { it.replace(File.separator, globSeparator) } - .flatMap { path -> toGlob(path, rootDir) } + .mapNotNull { + if (onWindowsOS) { + it.normalizeWindowsPattern() + } else { + it + } + }.map { it.expandTildeToFullPath() } + .map { + if (onWindowsOS) { + // By definition the globs should use "/" as separator. Out of courtesy replace "\" with "/" + it + .replace(File.separator, "/") + .also { transformedPath -> + if (it != transformedPath) { + logger.trace { "On WindowsOS transform '$it' to '$transformedPath'" } + } + } + } else { + it + } + }.flatMap { path -> toGlob(path, rootDir) } private fun FileSystem.toGlob( path: String, @@ -145,42 +180,135 @@ private fun FileSystem.toGlob( } else { "" } - val pathWithoutNegationPrefix = path.removePrefix(NEGATION_PREFIX) - val resolvedPath = try { - rootDir.resolve(pathWithoutNegationPrefix) + val pathWithoutNegationPrefix = + path + .removePrefix(NEGATION_PREFIX) + val expandedPatterns = try { + val resolvedPath = + rootDir + .resolve(pathWithoutNegationPrefix) + .normalize() + if (resolvedPath.isDirectory()) { + resolvedPath + .expandPathToDefaultPatterns() + .also { + logger.trace { "Expanding resolved directory path '$resolvedPath' to patterns: [$it]" } + } + } else { + resolvedPath + .pathString + .expandDoubleStarPatterns() + .also { + logger.trace { "Expanding resolved path '$resolvedPath` to patterns: [$it]" } + } + } } catch (e: InvalidPathException) { - // Windows throws an exception when you pass a glob to Path#resolve. - null - } - val expandedGlobs = if (resolvedPath != null && resolvedPath.isDirectory()) { - getDefaultPatternsForPath(resolvedPath) - } else if (isGlobAbsolutePath(pathWithoutNegationPrefix)) { - listOf(pathWithoutNegationPrefix) - } else { - listOf(pathWithoutNegationPrefix.prefixIfNot("**$globSeparator")) + if (onWindowsOS) { + // Windows throws an exception when passing a wildcard (*) to Path#resolve. + pathWithoutNegationPrefix + .expandDoubleStarPatterns() + .also { + logger.trace { "On WindowsOS: expanding unresolved path '$pathWithoutNegationPrefix` to patterns: [$it]" } + } + } else { + emptyList() + } } - return expandedGlobs.map { "${negation}glob:$it" } -} -private fun getDefaultPatternsForPath(path: Path?) = defaultKotlinFileExtensions - .flatMap { - listOf( - "$path$globSeparator*.$it", - "$path$globSeparator**$globSeparator*.$it", - ) - } + return expandedPatterns + .map { originalPattern -> + if (onWindowsOS) { + originalPattern + // Replace "\" with "/" + .replace(this.separator, "/") + // Remove drive letter (and colon) from path as this will lead to invalid globs and replace it with a double + // star pattern. Technically this is not functionally identical as the pattern could match on multiple drives. + .substringAfter(":") + .removePrefix("/") + .prefixIfNot("**/") + .also { transformedPattern -> + if (transformedPattern != originalPattern) { + logger.trace { "On WindowsOS, transform '$originalPattern' to '$transformedPattern'" } + } + } + } else { + originalPattern + } + }.map { "${negation}glob:$it" } +} -private fun FileSystem.isGlobAbsolutePath(glob: String) = - rootDirectories - .map { it.toString() } - .any { glob.startsWith(it) } +/** + * For each double star pattern in the path, create and additional path in which the double start pattern is removed. + * In this way a pattern like some-directory/**/*.kt will match wile files in some-directory or any of its + * subdirectories. + */ +private fun String?.expandDoubleStarPatterns(): Set { + val paths = mutableSetOf(this) + val parts = this?.split("/").orEmpty() + parts + .filter { it == "**" } + .forEach { doubleStarPart -> + run { + val expandedPath = + parts + .filter { it !== doubleStarPart } + .joinToString(separator = "/") + // The original path can contain multiple double star patterns. Replace only one double start pattern + // with an additional path patter and call recursively for remain double star patterns + paths.addAll(expandedPath.expandDoubleStarPatterns()) + } + } + return paths.filterNotNull().toSet() +} -private val globSeparator: String get() = - when { - os.startsWith("windows", ignoreCase = true) -> "\\\\" - else -> "/" +private fun String?.normalizeWindowsPattern() = + if (onWindowsOS) { + val parts: Deque = LinkedList() + // Replace "\" with "/" + this + ?.replace("\\", "/") + ?.split("/") + ?.filterNot { + // Reference to current directory can simply be ignored + it == "." + }?.forEach { + if (it == "..") { + // Whenever the parent directory reference follows a part not containing a wildcard, then the parent + // reference and the preceding element can be ignored. In other cases, the result pattern can not be + // cleaned. If that pattern would be transformed to a glob then the result regular expression of + // that glob results in a pattern that will never be matched as the ".." reference will not occur in + // the filepath that is being checked with the regular expression. + if (parts.isEmpty()) { + logger.warn { + "On WindowsOS the pattern '$this' can not be used as it refers to a path outside of the current directory" + } + return@normalizeWindowsPattern null + } else if (parts.peekLast().contains('*')) { + logger.warn { + "On WindowsOS the pattern '$this' can not be used as '/..' follows the wildcard pattern ${parts.peekLast()}" + } + return@normalizeWindowsPattern null + } else { + parts.removeLast() + } + } else { + parts.addLast(it) + } + } + parts.joinToString(separator = "/") + } else { + this } +private fun Path.expandPathToDefaultPatterns() = + defaultKotlinFileExtensions + .flatMap { + listOf( + "$this/*.$it", + "$this/**/*.$it", + ) + } + /** * List of paths to Java `jar` files. */ @@ -198,13 +326,24 @@ 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) + .also { + if (it != this) { + logger.trace { "On non-WindowsOS expand '$this' to '$it'" } + } + } } +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 diff --git a/ktlint/src/test/kotlin/com/pinterest/ktlint/internal/FileUtilsTest.kt b/ktlint/src/test/kotlin/com/pinterest/ktlint/internal/FileUtilsTest.kt index 20ab8639da..be13780523 100644 --- a/ktlint/src/test/kotlin/com/pinterest/ktlint/internal/FileUtilsTest.kt +++ b/ktlint/src/test/kotlin/com/pinterest/ktlint/internal/FileUtilsTest.kt @@ -2,26 +2,27 @@ package com.pinterest.ktlint.internal import com.google.common.jimfs.Configuration import com.google.common.jimfs.Jimfs -import java.io.File +import com.pinterest.ktlint.core.initKtLintKLogger 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 +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() @@ -87,8 +88,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", ), ) @@ -104,49 +105,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()), ) @@ -157,39 +134,52 @@ 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`() { + @ParameterizedTest(name = "Pattern: {0}") + @ValueSource( + strings = [ + // Redundant path below should resolve to "**/main/**/*.kt" for the test to succeed + "./**/main/**/*.kt", + "**/./main/**/*.kt", + "**/main/./**/*.kt", + "**/main/**/./*.kt", + "xx/../**/main/**/./*.kt", + "**/xx/../main/**/*.kt", + "**/main/xx/../**/*.kt", + "**/main/**/./xx/../*.kt", + ], + ) + fun `Given a pattern containing redundant elements then find all files in that workdir and all its sub directories that match the pattern without the redundant items`( + pattern: String, + ) { val foundFiles = getFiles( - patterns = listOf("src/main/kotlin/One.kt".normalizeGlob()), + patterns = listOf(pattern), rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath()), ) assertThat(foundFiles).containsExactlyInAnyOrder( ktFile1InProjectSubDirectory, + ktFile2InProjectSubDirectory, ) } @Test - 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`() { + 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(), - ktFile2InProjectSubDirectory.normalizeGlob(), - ), + patterns = listOf("src/main/kotlin/One.kt"), rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath()), ) assertThat(foundFiles).containsExactlyInAnyOrder( ktFile1InProjectSubDirectory, - ktFile2InProjectSubDirectory, ) } @Test - fun `Given a glob containing an (absolute) file path and a workdir then find all files match the pattern`() { + 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( - "${rootDir}project1/src/**/*.kt".normalizeGlob(), + "src/main/kotlin/One.kt", + ktFile2InProjectSubDirectory, ), rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath()), ) @@ -220,7 +210,7 @@ internal class FileUtilsFileSequenceTest { tempFileSystem.createFile(filePath) val foundFiles = getFiles( - patterns = listOf(pattern.normalizeGlob()), + patterns = listOf(pattern), ) assertThat(foundFiles).containsExactlyInAnyOrder(filePath) @@ -230,7 +220,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()), ) @@ -241,12 +231,32 @@ 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 pattern containing multiple double star patters and a workdir without subdirectories then find all files in that workdir`() { + val foundFiles = getFiles( + patterns = listOf( + "src/**/kotlin/**/*.kt", + ), + rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath()), + ) + + assertThat(foundFiles).containsExactlyInAnyOrder( + ktFile1InProjectSubDirectory, + ktFile2InProjectSubDirectory, + ) + } + @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" + 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()), ) @@ -258,19 +268,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", ), ) @@ -279,8 +283,95 @@ internal class FileUtilsFileSequenceTest { .doesNotContain(ktFile2InProjectSubDirectory) } - private fun String.normalizePath() = replace('/', File.separatorChar) - private fun String.normalizeGlob(): String = replace("/", rawGlobSeparator) + @DisabledOnOs(OS.WINDOWS) + @ParameterizedTest(name = "Pattern: {0}") + @ValueSource( + strings = [ + "../**/*.kt", + "../**/src/main/kotlin/One.kt", + "src/../../project1/src/**/*.kt", + "src/../../project1/src/main/kotlin/*.kt", + ], + ) + fun `On non-WindowsOS, a pattern containing a double-dot (parent directory) reference may leave the current directory`( + pattern: String, + ) { + val foundFiles = getFiles( + patterns = listOf(pattern), + rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath()), + ) + + assertThat(foundFiles).contains(ktFile1InProjectSubDirectory) + } + + @EnabledOnOs(OS.WINDOWS) + @ParameterizedTest(name = "Pattern: {0}") + @ValueSource( + strings = [ + "../**/*.kt", + "../**/src/main/kotlin/One.kt", + "src/../../project1/src/**/*.kt", + "src/../../project1/src/main/kotlin/*.kt", + ], + ) + fun `On WindowsOS, a pattern containing a double-dot (parent directory) reference may not leave the current directory`( + pattern: String, + ) { + val foundFiles = getFiles( + patterns = listOf( + pattern, + "/some/non/existing/file", // This prevents the default patterns to be added + ), + rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath()), + ) + + assertThat(foundFiles).isEmpty() + } + + @DisabledOnOs(OS.WINDOWS) + @ParameterizedTest(name = "Pattern: {0}") + @ValueSource( + strings = [ + "**/../**/*.kt", + "**/../src/main/kotlin/One.kt", + "src/main/k*/../kotlin/One.kt", + ], + ) + fun `On non-WindowsOS, a pattern containing a wildcard may followed by a double-dot (parent directory) reference`( + pattern: String, + ) { + val foundFiles = getFiles( + patterns = listOf(pattern), + rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath()), + ) + + assertThat(foundFiles).contains(ktFile1InProjectSubDirectory) + } + + @EnabledOnOs(OS.WINDOWS) + @ParameterizedTest(name = "Pattern: {0}") + @ValueSource( + strings = [ + "**/../**/*.kt", + "**/../src/main/kotlin/One.kt", + "src/main/k*/../kotlin/One.kt", + ], + ) + fun `On WindowsOS, a pattern containing a wildcard may followed by a double-dot (parent directory) reference`( + pattern: String, + ) { + val foundFiles = getFiles( + patterns = listOf( + pattern, + "/some/non/existing/file", // This prevents the default patterns to be added + ), + rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath()), + ) + + assertThat(foundFiles).isEmpty() + } + + private fun String.normalizePath() = replace("/", tempFileSystem.separator) private fun FileSystem.createFile(it: String) { val filePath = getPath(it.normalizePath()) @@ -294,14 +385,11 @@ internal class FileUtilsFileSequenceTest { rootDir: Path = tempFileSystem.rootDirectories.first(), ): List = tempFileSystem .fileSequence(patterns, rootDir) - .map { it.toString() } + .map { it.normalize().toString() } .toList() -} - -internal val rawGlobSeparator: String get() { - val os = System.getProperty("os.name") - return when { - os.startsWith("windows", ignoreCase = true) -> "\\" - else -> "/" - } + .also { + logger.info { + "Getting files with [patterns = $patterns] and [rootdir = $rootDir] returns [files = $it]" + } + } }