From 1eb78da370719d6122ea77d1b8f1ed6bc92f14c2 Mon Sep 17 00:00:00 2001 From: Alexandros Alexiou Date: Fri, 6 Dec 2024 09:36:39 +0200 Subject: [PATCH 1/3] feat: allow walking generated sources inside target folder --- .../kotlin/org/javacs/kt/CompilerClassPath.kt | 2 +- .../kotlin/org/javacs/kt/SourceExclusions.kt | 86 +++++++++++++------ 2 files changed, 59 insertions(+), 29 deletions(-) diff --git a/server/src/main/kotlin/org/javacs/kt/CompilerClassPath.kt b/server/src/main/kotlin/org/javacs/kt/CompilerClassPath.kt index 794377454..2a4492841 100644 --- a/server/src/main/kotlin/org/javacs/kt/CompilerClassPath.kt +++ b/server/src/main/kotlin/org/javacs/kt/CompilerClassPath.kt @@ -45,7 +45,7 @@ class CompilerClassPath( compiler.updateConfiguration(config) } - /** Updates and possibly reinstantiates the compiler using new paths. */ + /** Updates and possibly re-instantiates the compiler using new paths. */ private fun refresh( updateClassPath: Boolean = true, updateBuildScriptClassPath: Boolean = true, diff --git a/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt b/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt index de4a512c2..bf7b8197d 100644 --- a/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt +++ b/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt @@ -1,48 +1,78 @@ package org.javacs.kt -import org.javacs.kt.util.filePath -import java.io.File import java.net.URI import java.nio.file.FileSystems import java.nio.file.Path -import java.nio.file.Paths +import org.javacs.kt.util.filePath // TODO: Read exclusions from gitignore/settings.json/... instead of // hardcoding them class SourceExclusions( private val workspaceRoots: Collection, - private val scriptsConfig: ScriptsConfiguration + scriptsConfig: ScriptsConfiguration, ) { - val excludedPatterns = (listOf( - ".git", ".hg", ".svn", // Version control systems - ".idea", ".idea_modules", ".vs", ".vscode", ".code-workspace", ".settings", // IDEs - "bazel-*", "bin", "build", "node_modules", "target", // Build systems - ) + when { - !scriptsConfig.enabled -> listOf("*.kts") - !scriptsConfig.buildScriptsEnabled -> listOf("*.gradle.kts") - else -> emptyList() - }) + val excludedPatterns = + (listOf( + ".git", + ".hg", + ".svn", // Version control systems + ".idea", + ".idea_modules", + ".vs", + ".vscode", + ".code-workspace", + ".settings", // IDEs + "bazel-*", + "bin", + "build", + "node_modules", + ) + + when { + !scriptsConfig.enabled -> listOf("*.kts") + !scriptsConfig.buildScriptsEnabled -> listOf("*.gradle.kts") + else -> emptyList() + }) private val exclusionMatchers = excludedPatterns + .filter { !it.startsWith("!") } .map { FileSystems.getDefault().getPathMatcher("glob:$it") } - /** Finds all non-excluded files recursively. */ - fun walkIncluded(): Sequence = workspaceRoots.asSequence().flatMap { root -> - root.toFile() - .walk() - .onEnter { isPathIncluded(it.toPath()) } - .map { it.toPath() } - } + fun walkIncluded(): Sequence = + workspaceRoots.asSequence().flatMap { root -> + root.toFile().walk().onEnter { isPathIncluded(it.toPath()) }.map { it.toPath() } + } - /** Tests whether the given URI is not excluded. */ fun isURIIncluded(uri: URI) = uri.filePath?.let(this::isPathIncluded) ?: false - /** Tests whether the given path is not excluded. */ - fun isPathIncluded(file: Path): Boolean = workspaceRoots.any { file.startsWith(it) } - && exclusionMatchers.none { matcher -> - workspaceRoots - .mapNotNull { if (file.startsWith(it)) it.relativize(file) else null } - .flatMap { it } // Extract path segments - .any(matcher::matches) + fun isPathIncluded(file: Path): Boolean { + if (!workspaceRoots.any { file.startsWith(it) }) { + return false + } + + val relativePaths = workspaceRoots + .mapNotNull { if (file.startsWith(it)) it.relativize(file) else null } + .flatten() + + // Check if we're in a target directory + if (relativePaths.contains(Path.of("target"))) { + val pathList = relativePaths.toList() + val targetIndex = pathList.indexOf(Path.of("target")) + + // Allow only target directory itself or if next directory is generated-sources + return pathList.size <= targetIndex + 1 || + pathList[targetIndex + 1] == Path.of("generated-sources") + } + + // If path matches any exclusion pattern, exclude it + if (exclusionMatchers.any { matcher -> + relativePaths.any(matcher::matches) + }) { + return false } + + // Include paths outside target directory by default + return true + } + } + From f20c645c3d351232bd71af591cf7b2d9b14c6c2a Mon Sep 17 00:00:00 2001 From: Alexandros Alexiou Date: Fri, 6 Dec 2024 10:11:53 +0200 Subject: [PATCH 2/3] fix: return statements should be less than 2 --- .../kotlin/org/javacs/kt/SourceExclusions.kt | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt b/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt index bf7b8197d..121b147d1 100644 --- a/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt +++ b/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt @@ -53,26 +53,21 @@ class SourceExclusions( .mapNotNull { if (file.startsWith(it)) it.relativize(file) else null } .flatten() - // Check if we're in a target directory - if (relativePaths.contains(Path.of("target"))) { - val pathList = relativePaths.toList() - val targetIndex = pathList.indexOf(Path.of("target")) - - // Allow only target directory itself or if next directory is generated-sources - return pathList.size <= targetIndex + 1 || - pathList[targetIndex + 1] == Path.of("generated-sources") + val isIncluded = when { + // Check if we're in a target directory + relativePaths.contains(Path.of("target")) -> { + val pathList = relativePaths.toList() + val targetIndex = pathList.indexOf(Path.of("target")) + // Allow only target directory itself or if next directory is generated-sources + pathList.size <= targetIndex + 1 || pathList[targetIndex + 1] == Path.of("generated-sources") + } + // Check exclusion patterns + exclusionMatchers.any { matcher -> relativePaths.any(matcher::matches) } -> false + // Include paths outside target directory by default + else -> true } - // If path matches any exclusion pattern, exclude it - if (exclusionMatchers.any { matcher -> - relativePaths.any(matcher::matches) - }) { - return false - } - - // Include paths outside target directory by default - return true + return isIncluded } - } From ff7be1c8f5634b3ddea7852c84b207940c816110 Mon Sep 17 00:00:00 2001 From: Alexandros Alexiou Date: Sat, 7 Dec 2024 12:31:34 +0200 Subject: [PATCH 3/3] feat(server): add gitignore support to SourceExclusions - Add test setup/cleanup with temp workspace and gitignore file - Add new test cases for gitignore pattern exclusions - Update existing tests for cross-platform compatibility - Test various gitignore patterns (wildcards, directories, extensions) --- .../kotlin/org/javacs/kt/SourceExclusions.kt | 96 +++++-- .../org/javacs/kt/SourceExclusionsTest.kt | 257 ++++++++++++++++++ 2 files changed, 324 insertions(+), 29 deletions(-) create mode 100644 shared/src/test/kotlin/org/javacs/kt/SourceExclusionsTest.kt diff --git a/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt b/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt index 121b147d1..28f68bbe0 100644 --- a/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt +++ b/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt @@ -2,17 +2,17 @@ package org.javacs.kt import java.net.URI import java.nio.file.FileSystems +import java.nio.file.Files import java.nio.file.Path import org.javacs.kt.util.filePath +import java.io.IOException -// TODO: Read exclusions from gitignore/settings.json/... instead of -// hardcoding them class SourceExclusions( private val workspaceRoots: Collection, scriptsConfig: ScriptsConfiguration, ) { - val excludedPatterns = - (listOf( + private val defaultPatterns = + listOf( ".git", ".hg", ".svn", // Version control systems @@ -26,16 +26,52 @@ class SourceExclusions( "bin", "build", "node_modules", - ) + - when { - !scriptsConfig.enabled -> listOf("*.kts") - !scriptsConfig.buildScriptsEnabled -> listOf("*.gradle.kts") - else -> emptyList() - }) + ) + + private val scriptPatterns = + when { + !scriptsConfig.enabled -> listOf("*.kts") + !scriptsConfig.buildScriptsEnabled -> listOf("*.gradle.kts") + else -> emptyList() + } - private val exclusionMatchers = excludedPatterns - .filter { !it.startsWith("!") } - .map { FileSystems.getDefault().getPathMatcher("glob:$it") } + private val gitignorePatterns: List = readGitignorePatterns() + + val excludedPatterns = defaultPatterns + scriptPatterns + gitignorePatterns + + private val exclusionMatchers = + excludedPatterns + .filter { !it.startsWith("!") } // Skip negated patterns for now + .map { FileSystems.getDefault().getPathMatcher("glob:$it") } + + private fun readGitignorePatterns(): List { + return workspaceRoots + .flatMap { root -> + val gitignore = root.resolve(".gitignore") + if (Files.exists(gitignore)) { + try { + Files.readAllLines(gitignore) + .asSequence() + .map { it.trim() } + .filter { it.isNotEmpty() && !it.startsWith("#") } + .map { it.removeSuffix("/") } + .toList() + .also { patterns -> + LOG.debug("Read {} patterns from {}", patterns.size, gitignore) + } + } catch (e: IOException) { + LOG.warn("Could not read .gitignore at $gitignore: ${e.message}") + emptyList() + } catch (e: SecurityException) { + LOG.warn("Security error reading .gitignore at $gitignore: ${e.message}") + emptyList() + } + } else { + emptyList() + } + } + .distinct() // Remove duplicates across workspace roots + } fun walkIncluded(): Sequence = workspaceRoots.asSequence().flatMap { root -> @@ -49,25 +85,27 @@ class SourceExclusions( return false } - val relativePaths = workspaceRoots - .mapNotNull { if (file.startsWith(it)) it.relativize(file) else null } - .flatten() + val relativePaths = + workspaceRoots + .mapNotNull { if (file.startsWith(it)) it.relativize(file) else null } + .flatten() - val isIncluded = when { - // Check if we're in a target directory - relativePaths.contains(Path.of("target")) -> { - val pathList = relativePaths.toList() - val targetIndex = pathList.indexOf(Path.of("target")) - // Allow only target directory itself or if next directory is generated-sources - pathList.size <= targetIndex + 1 || pathList[targetIndex + 1] == Path.of("generated-sources") + val isIncluded = + when { + // Check if we're in a target directory + relativePaths.contains(Path.of("target")) -> { + val pathList = relativePaths.toList() + val targetIndex = pathList.indexOf(Path.of("target")) + // Allow only target directory itself or if next directory is generated-sources + pathList.size <= targetIndex + 1 || + pathList[targetIndex + 1] == Path.of("generated-sources") + } + // Check exclusion patterns + exclusionMatchers.any { matcher -> relativePaths.any(matcher::matches) } -> false + // Include paths outside target directory by default + else -> true } - // Check exclusion patterns - exclusionMatchers.any { matcher -> relativePaths.any(matcher::matches) } -> false - // Include paths outside target directory by default - else -> true - } return isIncluded } } - diff --git a/shared/src/test/kotlin/org/javacs/kt/SourceExclusionsTest.kt b/shared/src/test/kotlin/org/javacs/kt/SourceExclusionsTest.kt new file mode 100644 index 000000000..63074622c --- /dev/null +++ b/shared/src/test/kotlin/org/javacs/kt/SourceExclusionsTest.kt @@ -0,0 +1,257 @@ +package org.javacs.kt + +import java.io.File +import java.nio.file.Files +import java.nio.file.attribute.PosixFilePermissions +import org.hamcrest.Matchers.equalTo +import org.junit.After +import org.junit.Assert.assertThat +import org.junit.Before +import org.junit.Test + +class SourceExclusionsTest { + private val workspaceRoot = + File(System.getProperty("java.io.tmpdir"), "test-workspace").toPath() + private lateinit var sourceExclusions: SourceExclusions + + @Before + fun setup() { + Files.createDirectories(workspaceRoot) + + val gitignore = workspaceRoot.resolve(".gitignore") + Files.write( + gitignore, + listOf("*.log", "output/", "temp/", "# comment to ignore", "build-output/", "*.tmp"), + ) + + sourceExclusions = + SourceExclusions( + workspaceRoots = listOf(workspaceRoot), + scriptsConfig = ScriptsConfiguration(enabled = true, buildScriptsEnabled = true), + ) + } + + @After + fun cleanup() { + workspaceRoot.toFile().deleteRecursively() + } + + @Test + fun `test path exclusions`() { + assertThat(sourceExclusions.isPathIncluded(workspaceRoot.resolve(".git")), equalTo(false)) + assertThat( + sourceExclusions.isPathIncluded(workspaceRoot.resolve("node_modules")), + equalTo(false), + ) + assertThat(sourceExclusions.isPathIncluded(workspaceRoot.resolve(".idea")), equalTo(false)) + + assertThat( + sourceExclusions.isPathIncluded( + workspaceRoot.resolve("src").resolve("main").resolve("kotlin") + ), + equalTo(true), + ) + assertThat( + sourceExclusions.isPathIncluded( + workspaceRoot.resolve("src").resolve("test").resolve("kotlin") + ), + equalTo(true), + ) + } + + @Test + fun `test gitignore patterns`() { + assertThat( + sourceExclusions.isPathIncluded(workspaceRoot.resolve("debug.log")), + equalTo(false), + ) + assertThat( + sourceExclusions.isPathIncluded(workspaceRoot.resolve("output").resolve("file.txt")), + equalTo(false), + ) + assertThat(sourceExclusions.isPathIncluded(workspaceRoot.resolve("temp")), equalTo(false)) + assertThat( + sourceExclusions.isPathIncluded(workspaceRoot.resolve("build-output")), + equalTo(false), + ) + assertThat( + sourceExclusions.isPathIncluded(workspaceRoot.resolve("data.tmp")), + equalTo(false), + ) + + assertThat( + sourceExclusions.isPathIncluded(workspaceRoot.resolve("src").resolve("main.kt")), + equalTo(true), + ) + assertThat( + sourceExclusions.isPathIncluded(workspaceRoot.resolve("README.md")), + equalTo(true), + ) + } + + @Test + fun `test target directory handling`() { + assertThat(sourceExclusions.isPathIncluded(workspaceRoot.resolve("target")), equalTo(true)) + assertThat( + sourceExclusions.isPathIncluded( + workspaceRoot.resolve("target").resolve("generated-sources") + ), + equalTo(true), + ) + assertThat( + sourceExclusions.isPathIncluded(workspaceRoot.resolve("target").resolve("classes")), + equalTo(false), + ) + } + + @Test + fun `test URI inclusion`() { + val includedUri = + workspaceRoot + .resolve("src") + .resolve("main") + .resolve("kotlin") + .resolve("Example.kt") + .toUri() + val excludedUri = workspaceRoot.resolve(".git").resolve("config").toUri() + val gitignoreExcludedUri = workspaceRoot.resolve("output").resolve("test.txt").toUri() + + assertThat(sourceExclusions.isURIIncluded(includedUri), equalTo(true)) + assertThat(sourceExclusions.isURIIncluded(excludedUri), equalTo(false)) + assertThat(sourceExclusions.isURIIncluded(gitignoreExcludedUri), equalTo(false)) + } + + @Test + fun `test paths outside workspace root`() { + val outsidePath = + File(System.getProperty("java.io.tmpdir"), "outside-workspace") + .toPath() + .resolve("file.kt") + assertThat(sourceExclusions.isPathIncluded(outsidePath), equalTo(false)) + } + + @Test + fun `test script handling based on configuration`() { + val restrictedScriptsExclusions = + SourceExclusions( + workspaceRoots = listOf(workspaceRoot), + scriptsConfig = ScriptsConfiguration(enabled = false), + ) + + assertThat( + restrictedScriptsExclusions.isPathIncluded(workspaceRoot.resolve("build.gradle.kts")), + equalTo(false), + ) + + assertThat( + sourceExclusions.isPathIncluded(workspaceRoot.resolve("build.gradle.kts")), + equalTo(true), + ) + } + + @Test + fun `test gitignore handling with IO errors`() { + val ioErrorWorkspace = workspaceRoot.resolve("io-error-workspace") + Files.createDirectories(ioErrorWorkspace) + + val problematicGitignore = ioErrorWorkspace.resolve(".gitignore") + Files.write(problematicGitignore, listOf("test-pattern")) + + try { + // Make the file unreadable to simulate IO error + Files.setPosixFilePermissions( + problematicGitignore, + PosixFilePermissions.fromString("--x------"), + ) + + val exclusionsWithIOError = + SourceExclusions( + workspaceRoots = listOf(ioErrorWorkspace), + scriptsConfig = ScriptsConfiguration(enabled = true, buildScriptsEnabled = true), + ) + + assertThat( + exclusionsWithIOError.isPathIncluded(ioErrorWorkspace.resolve(".git")), + equalTo(false), + ) + assertThat( + exclusionsWithIOError.isPathIncluded( + ioErrorWorkspace.resolve("src/main/kotlin/Test.kt") + ), + equalTo(true), + ) + } catch (e: UnsupportedOperationException) { + // Skip test if POSIX permissions are not supported + } + } + + @Test + fun `test multiple gitignore files`() { + val subdir = workspaceRoot.resolve("subproject") + Files.createDirectories(subdir) + Files.write(subdir.resolve(".gitignore"), listOf("subproject-specific.log", "local-temp/")) + + val multiRootExclusions = + SourceExclusions( + workspaceRoots = listOf(workspaceRoot, subdir), + scriptsConfig = ScriptsConfiguration(enabled = true, buildScriptsEnabled = true), + ) + + assertThat( + multiRootExclusions.isPathIncluded(workspaceRoot.resolve("debug.log")), + equalTo(false), + ) + assertThat( + multiRootExclusions.isPathIncluded(subdir.resolve("subproject-specific.log")), + equalTo(false), + ) + assertThat(multiRootExclusions.isPathIncluded(subdir.resolve("local-temp")), equalTo(false)) + } + + @Test + fun `test empty gitignore handling`() { + val emptyGitignoreWorkspace = workspaceRoot.resolve("empty-gitignore-workspace") + Files.createDirectories(emptyGitignoreWorkspace) + Files.write(emptyGitignoreWorkspace.resolve(".gitignore"), listOf()) + + val exclusionsWithEmptyGitignore = + SourceExclusions( + workspaceRoots = listOf(emptyGitignoreWorkspace), + scriptsConfig = ScriptsConfiguration(enabled = true, buildScriptsEnabled = true), + ) + + assertThat( + exclusionsWithEmptyGitignore.isPathIncluded(emptyGitignoreWorkspace.resolve(".git")), + equalTo(false), + ) + assertThat( + exclusionsWithEmptyGitignore.isPathIncluded( + emptyGitignoreWorkspace.resolve("src/main/kotlin/Test.kt") + ), + equalTo(true), + ) + } + + @Test + fun `test non-existent gitignore handling`() { + val noGitignoreWorkspace = workspaceRoot.resolve("no-gitignore-workspace") + Files.createDirectories(noGitignoreWorkspace) + + val exclusionsWithoutGitignore = + SourceExclusions( + workspaceRoots = listOf(noGitignoreWorkspace), + scriptsConfig = ScriptsConfiguration(enabled = true, buildScriptsEnabled = true), + ) + + assertThat( + exclusionsWithoutGitignore.isPathIncluded(noGitignoreWorkspace.resolve(".git")), + equalTo(false), + ) + assertThat( + exclusionsWithoutGitignore.isPathIncluded( + noGitignoreWorkspace.resolve("src/main/kotlin/Test.kt") + ), + equalTo(true), + ) + } +}