Skip to content

Commit

Permalink
Expand (existing) directory to globs for default kotlin extensions (#…
Browse files Browse the repository at this point in the history 
…1537)

* Expand (existing) directory to globs for default kotlin extensions 

* When an (existing) directory (without glob pattern) is specified then only process files with default kotlin extensions in that directory or its subdirectories. This is equivalent to execute ktlint in that directory without specifying any pattern.

Closes #917

* When a glob does not contain an absolute path then do not prefix it with the root directory but instead ensure that it is prefixed it with the "**/" matcher. In this way files directly inside the workdir are matched as well as files in subdirectories of the workdir.

* Print an error message and return with non-zero exit code when no files are found that match with the globs

Closes #629

* Refactor by eliminating variable project2Files

* Refactor to improve readability and consistency

* Extract constants to private values. Align name of test class with production class.

* Remove characters which may cause a problem on Windows

* Normalize paths for Windows build

* Disable tests on Windows using absolute paths as those are not supported by jimfs
  • Loading branch information
@paul-dingemans
paul-dingemans committed Jul 24, 2022
1 parent 7ae1126 commit 66ab574
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 58 deletions.
12 changes: 5 additions & 7 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,13 @@ The callback function provided as parameter to the format function is now called

* Fix cli argument "--disabled_rules" ([#1520](https://github.com/pinterest/ktlint/issue/1520)).
* A file which contains a single top level declaration of type function does not need to be named after the function but only needs to adhere to the PascalCase convention. `filename` ([#1521](https://github.com/pinterest/ktlint/issue/1521)).
* When a glob is specified then ensure that it matches files in the current directory and not only in subdirectories of the current directory ([#1533](https://github.com/pinterest/ktlint/issue/1533)).
* Disable/enable IndentationRule on blocks in middle of file. (`indent`) [#631](https://github.com/pinterest/ktlint/issues/631)
* Allow usage of letters with diacritics in enum values and filenames (`enum-entry-name-case`, `filename`) ([#1530](https://github.com/pinterest/ktlint/issue/1530)).
* Fix resolving of Java version when JAVA_TOOL_OPTIONS is set ([#1543](https://github.com/pinterest/ktlint/issues/1543))
* When a glob is specified then ensure that it matches files in the current directory and not only in subdirectories of the current directory ([#1533](https://github.com/pinterest/ktlint/issue/1533)).
* Execute `ktlint` cli on default kotlin extensions only when an (existing) path to a directory is given. ([#917](https://github.com/pinterest/ktlint/issue/917)).
* Invoke callback on `format` function for all errors including errors that are autocorrected ([#1491](https://github.com/pinterest/ktlint/issues/1491))


### Changed

Expand Down Expand Up @@ -171,16 +174,12 @@ If your project did not run with the `experimental` ruleset enabled before, you

### API Changes & RuleSet providers

If you are not an API consumer nor a RuleSet provider, then you can safely skip this section. Otherwise, please read below carefully and upgrade your usage of ktlint. In this and coming releases, we are changing and adapting important parts of our API in order to increase maintainability and flexibility for future changes. Please avoid skipping a releases as that will make it harder to migrate.
If you are not an API user nor a RuleSet provider, then you can safely skip this section. Otherwise, please read below carefully and upgrade your usage of ktlint. In this and coming releases, we are changing and adapting important parts of our API in order to increase maintainability and flexibility for future changes. Please avoid skipping a releases as that will make it harder to migrate.

#### Lint and formatting functions

The lint and formatting changes no longer accept parameters of type `Params` but only `ExperimentalParams`. Also, the VisitorProvider parameter has been removed. Because of this, your integration with KtLint breaks. Based on feedback with ktlint 0.45.x, we now prefer to break at compile time instead of trying to keep the interface backwards compatible. Please raise an issue, in case you help to convert to the new API.

#### Format callback

The callback function provided as parameter to the format function is now called for all errors regardless whether the error has been autocorrected. Existing consumers of the format function should now explicitly check the `autocorrected` flag in the callback result and handle it appropriately (in most case this will be ignoring the callback results for which `autocorrected` has value `true`).

#### Use of ".editorconfig" properties & userData

The interface `UsesEditorConfigProperties` provides method `getEditorConfigValue` to retrieve a named `.editorconfig` property for a given ASTNode. When implementing this interface, the value `editorConfigProperties` needs to be overridden. Previously it was not checked whether a retrieved property was actually recorded in this list. Now, retrieval of unregistered properties results in an exception.
Expand Down Expand Up @@ -243,7 +242,6 @@ An AssertJ style API for testing KtLint rules ([#1444](https://github.com/pinter
- Fix indentation of property getter/setter when the property has an initializer on a separate line `indent` ([#1335](https://github.com/pinterest/ktlint/issues/1335))
- When `.editorconfig` setting `indentSize` is set to value `tab` then return the default tab width as value for `indentSize` ([#1485](https://github.com/pinterest/ktlint/issues/1485))
- Allow suppressing all rules or a list of specific rules in the entire file with `@file:Suppress(...)` ([#1029](https://github.com/pinterest/ktlint/issues/1029))
- Invoke callback on `format` function for all errors including errors that are autocorrected ([#1491](https://github.com/pinterest/ktlint/issues/1491))


### Changed
Expand Down
108 changes: 72 additions & 36 deletions ktlint/src/main/kotlin/com/pinterest/ktlint/internal/FileUtils.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import java.nio.file.Path
import java.nio.file.Paths
import java.nio.file.SimpleFileVisitor
import java.nio.file.attribute.BasicFileAttributes
import kotlin.io.path.isDirectory
import kotlin.system.exitProcess
import kotlin.system.measureTimeMillis
import mu.KotlinLogging
Expand All @@ -22,23 +23,27 @@ import org.jetbrains.kotlin.util.prefixIfNot
private val logger = KotlinLogging.logger {}.initKtLintKLogger()

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")

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

/**
* Transform the [patterns] to a sequence of files. Each element in [patterns] can be a glob, a file or directory path
* relative to the [rootDir] or a absolute file or directory path.
*/
internal fun FileSystem.fileSequence(
globs: List<String>,
patterns: List<String>,
rootDir: Path = Paths.get(".").toAbsolutePath().normalize()
): Sequence<Path> {
val result = mutableListOf<Path>()

val (existingFiles, actualGlobs) = globs.partition {
val (existingFiles, patternsExclusiveExistingFiles) = patterns.partition {
try {
Files.isRegularFile(rootDir.resolve(it))
} catch (e: InvalidPathException) {
Expand All @@ -49,30 +54,28 @@ internal fun FileSystem.fileSequence(
existingFiles.mapTo(result) { rootDir.resolve(it) }

// Return early and don't traverse the file system if all the input globs are absolute paths
if (result.isNotEmpty() && actualGlobs.isEmpty()) {
if (result.isNotEmpty() && patternsExclusiveExistingFiles.isEmpty()) {
return result.asSequence()
}

val pathMatchers = if (actualGlobs.isEmpty()) {
val globs = expand(patternsExclusiveExistingFiles, rootDir)

val pathMatchers = if (globs.isEmpty()) {
defaultPatterns
.map { getPathMatcher("glob:$it") }
.toSet()
} else {
actualGlobs
.filterNot { it.startsWith("!") }
.map {
getPathMatcher(toGlob(it))
}
globs
.filterNot { it.startsWith(NEGATION_PREFIX) }
.map { getPathMatcher(it) }
}

val negatedPathMatchers = if (actualGlobs.isEmpty()) {
val negatedPathMatchers = if (globs.isEmpty()) {
emptySet()
} else {
actualGlobs
.filter { it.startsWith("!") }
.map {
getPathMatcher(toGlob(it.removePrefix("!")))
}
globs
.filter { it.startsWith(NEGATION_PREFIX) }
.map { getPathMatcher(it.removePrefix(NEGATION_PREFIX)) }
}

logger.debug {
Expand Down Expand Up @@ -123,27 +126,54 @@ internal fun FileSystem.fileSequence(
return result.asSequence()
}

private fun FileSystem.isGlobAbsolutePath(glob: String): Boolean {
val rootDirs = rootDirectories.map { it.toString() }
return rootDirs.any { glob.removePrefix("!").startsWith(it) }
}
private fun FileSystem.expand(
patterns: List<String>,
rootDir: Path
) =
patterns
.map { it.expandTildeToFullPath() }
.map { it.replace(File.separator, globSeparator) }
.flatMap { path -> toGlob(path, rootDir) }

internal fun FileSystem.toGlob(pattern: String): String {
val expandedPath = if (os.startsWith("windows", true)) {
// Windows sometimes inserts `~` into paths when using short directory names notation, e.g. `C:\Users\USERNA~1\Documents
pattern
private fun FileSystem.toGlob(
path: String,
rootDir: Path
): List<String> {
val negation = if (path.startsWith(NEGATION_PREFIX)) {
NEGATION_PREFIX
} else {
expandTilde(pattern)
}.replace(File.separator, globSeparator)

val fullPath = if (isGlobAbsolutePath(expandedPath)) {
expandedPath
""
}
val pathWithoutNegationPrefix = path.removePrefix(NEGATION_PREFIX)
val resolvedPath = try {
rootDir.resolve(pathWithoutNegationPrefix)
} 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 {
expandedPath.prefixIfNot("**$globSeparator")
listOf(pathWithoutNegationPrefix.prefixIfNot("**$globSeparator"))
}
return "glob:$fullPath"
return expandedGlobs.map { "${negation}glob:$it" }
}

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

private fun FileSystem.isGlobAbsolutePath(glob: String) =
rootDirectories
.map { it.toString() }
.any { glob.startsWith(it) }

private val globSeparator: String get() =
when {
os.startsWith("windows", ignoreCase = true) -> "\\\\"
Expand All @@ -156,7 +186,7 @@ private val globSeparator: String get() =
internal typealias JarFiles = List<String>

internal fun JarFiles.toFilesURIList() = map {
val jarFile = File(expandTilde(it))
val jarFile = File(it.expandTildeToFullPath())
if (!jarFile.exists()) {
logger.error { "File $it does not exist" }
exitProcess(1)
Expand All @@ -166,7 +196,13 @@ 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 (~/)
private fun expandTilde(path: String): String = path.replaceFirst(tildeRegex, userHome)
private fun String.expandTildeToFullPath(): String =
if (os.startsWith("windows", true)) {
// Windows sometimes inserts `~` into paths when using short directory names notation, e.g. `C:\Users\USERNA~1\Documents
this
} else {
replaceFirst(tildeRegex, userHome)
}

internal fun File.location(
relative: Boolean
Expand Down
80 changes: 65 additions & 15 deletions ktlint/src/test/kotlin/com/pinterest/ktlint/internal/FileUtilsTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ 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.OS
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.ValueSource

/**
* Tests for [fileSequence] method.
Expand Down Expand Up @@ -196,26 +200,34 @@ internal class FileUtilsFileSequenceTest {
)
}

@Test
fun `transforming globs with leading tilde`() {
assumeTrue(
System
.getProperty("os.name")
.lowercase(Locale.getDefault())
.startsWith("linux")
)

val glob = tempFileSystem.toGlob(
"~/project/src/main/kotlin/One.kt"
)
// Jimfs does not currently support the Windows syntax for an absolute path on the current drive (e.g. "\foo\bar")
@DisabledOnOs(OS.WINDOWS)
@ParameterizedTest(name = "Pattern: {0}")
@ValueSource(
strings = [
"~/project/src/main/kotlin/One.kt",
"~/project/src/main/kotlin/*.kt",
"~/project/src/main/kotlin/",
"~/project/src/main/kotlin",
"~/project/src/main/**/*.kt"
]
)
fun `Given a non-Windows OS and a pattern that starts with a tilde then transform the globs to the user home directory`(
pattern: String
) {
val homeDir = System.getProperty("user.home")
assertThat(glob).isEqualTo(
"glob:$homeDir/project/src/main/kotlin/One.kt"
val filePath = "$homeDir/project/src/main/kotlin/One.kt".normalizePath()
tempFileSystem.createFile(filePath)

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

assertThat(foundFiles).containsExactlyInAnyOrder(filePath)
}

@Test
fun `Given a pattern containing ** and a workdir without subdirectories then find all files in that workdir`() {
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()
Expand All @@ -229,6 +241,44 @@ 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`() {
val foundFiles = getFiles(
patterns = listOf("src/main/kotlin".normalizeGlob()),
rootDir = tempFileSystem.getPath("${rootDir}project1".normalizePath())
)

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

@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")
)

val foundFiles = getFiles(
patterns = listOf(
"project1\\src\\**\\*.kt".normalizeGlob(),
"!project1\\src\\**\\example\\*.kt".normalizeGlob()
)
)

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

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

Expand Down

0 comments on commit 66ab574

Please sign in to comment.