From 5b444560c048b14aab16c1cf2889e559030383e1 Mon Sep 17 00:00:00 2001 From: fwcd Date: Mon, 15 Jan 2024 00:48:55 +0000 Subject: [PATCH 1/5] Model script exclusion via SourceExclusions --- .../kotlin/org/javacs/kt/CompilerClassPath.kt | 16 ++++++++-------- .../main/kotlin/org/javacs/kt/Configuration.kt | 7 ------- .../main/kotlin/org/javacs/kt/SourceFiles.kt | 14 +++++--------- .../org/javacs/kt/ScriptsConfiguration.kt | 8 ++++++++ .../kotlin/org/javacs/kt/SourceExclusions.kt | 17 +++++++++++++---- 5 files changed, 34 insertions(+), 28 deletions(-) create mode 100644 shared/src/main/kotlin/org/javacs/kt/ScriptsConfiguration.kt diff --git a/server/src/main/kotlin/org/javacs/kt/CompilerClassPath.kt b/server/src/main/kotlin/org/javacs/kt/CompilerClassPath.kt index 3b25c96ab..d3ef00e2a 100644 --- a/server/src/main/kotlin/org/javacs/kt/CompilerClassPath.kt +++ b/server/src/main/kotlin/org/javacs/kt/CompilerClassPath.kt @@ -157,20 +157,20 @@ class CompilerClassPath( private fun isBuildScript(file: Path): Boolean = file.fileName.toString().let { it == "pom.xml" || it == "build.gradle" || it == "build.gradle.kts" } + private fun findJavaSourceFiles(root: Path): Set { + val sourceMatcher = FileSystems.getDefault().getPathMatcher("glob:*.java") + return SourceExclusions(listOf(root), scriptsConfig) + .walkIncluded() + .filter { sourceMatcher.matches(it.fileName) } + .toSet() + } + override fun close() { compiler.close() outputDirectory.delete() } } -private fun findJavaSourceFiles(root: Path): Set { - val sourceMatcher = FileSystems.getDefault().getPathMatcher("glob:*.java") - return SourceExclusions(root) - .walkIncluded() - .filter { sourceMatcher.matches(it.fileName) } - .toSet() -} - private fun logAdded(sources: Collection, name: String) { when { sources.isEmpty() -> return diff --git a/server/src/main/kotlin/org/javacs/kt/Configuration.kt b/server/src/main/kotlin/org/javacs/kt/Configuration.kt index ecfa71fb0..ad3db4bb7 100644 --- a/server/src/main/kotlin/org/javacs/kt/Configuration.kt +++ b/server/src/main/kotlin/org/javacs/kt/Configuration.kt @@ -30,13 +30,6 @@ public data class DiagnosticsConfiguration( var debounceTime: Long = 250L ) -public data class ScriptsConfiguration( - /** Whether .kts scripts are handled. */ - var enabled: Boolean = false, - /** Whether .gradle.kts scripts are handled. Only considered if scripts are enabled in general. */ - var buildScriptsEnabled: Boolean = false -) - public data class JVMConfiguration( /** Which JVM target the Kotlin compiler uses. See Compiler.jvmTargetFrom for possible values. */ var target: String = "default" diff --git a/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt b/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt index 9a723234d..f5230ab66 100644 --- a/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt +++ b/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt @@ -69,7 +69,7 @@ class SourceFiles( private val scriptsConfig: ScriptsConfiguration ) { private val workspaceRoots = mutableSetOf() - private var exclusions = SourceExclusions(workspaceRoots) + private var exclusions = SourceExclusions(workspaceRoots, scriptsConfig) private val files = NotifySourcePath(sp) private val open = mutableSetOf() @@ -177,20 +177,16 @@ class SourceFiles( } private fun findSourceFiles(root: Path): Set { - val glob = if (scriptsConfig.enabled) "*.{kt,kts}" else "*.kt" - val sourceMatcher = FileSystems.getDefault().getPathMatcher("glob:$glob") - return SourceExclusions(root) + val sourceMatcher = FileSystems.getDefault().getPathMatcher("glob:*.{kt,kts}") + return SourceExclusions(listOf(root), scriptsConfig) .walkIncluded() - .filter { - sourceMatcher.matches(it.fileName) - && (scriptsConfig.buildScriptsEnabled || !it.endsWith(".gradle.kts")) - } + .filter { sourceMatcher.matches(it.fileName) } .map(Path::toUri) .toSet() } private fun updateExclusions() { - exclusions = SourceExclusions(workspaceRoots) + exclusions = SourceExclusions(workspaceRoots, scriptsConfig) } fun isOpen(uri: URI): Boolean = (uri in open) diff --git a/shared/src/main/kotlin/org/javacs/kt/ScriptsConfiguration.kt b/shared/src/main/kotlin/org/javacs/kt/ScriptsConfiguration.kt new file mode 100644 index 000000000..50cb02d90 --- /dev/null +++ b/shared/src/main/kotlin/org/javacs/kt/ScriptsConfiguration.kt @@ -0,0 +1,8 @@ +package org.javacs.kt + +public data class ScriptsConfiguration( + /** Whether .kts scripts are handled. */ + var enabled: Boolean = false, + /** Whether .gradle.kts scripts are handled. Only considered if scripts are enabled in general. */ + var buildScriptsEnabled: Boolean = false +) diff --git a/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt b/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt index b682b0fbf..93e27d036 100644 --- a/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt +++ b/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt @@ -9,10 +9,19 @@ import java.nio.file.Paths // TODO: Read exclusions from gitignore/settings.json/... instead of // hardcoding them -class SourceExclusions(private val workspaceRoots: Collection) { - private val excludedPatterns = listOf(".*", "bazel-*", "bin", "build", "node_modules", "target").map { FileSystems.getDefault().getPathMatcher("glob:$it") } - - constructor(workspaceRoot: Path) : this(listOf(workspaceRoot)) {} +class SourceExclusions( + private val workspaceRoots: Collection, + private val scriptsConfig: ScriptsConfiguration +) { + private val excludedPatterns = listOf( + ".*", "bazel-*", "bin", "build", "node_modules", "target", + *(when { + !scriptsConfig.enabled -> arrayOf("*.kts") + !scriptsConfig.buildScriptsEnabled -> arrayOf("*.gradle.kts") + else -> arrayOf() + }), + ) + .map { FileSystems.getDefault().getPathMatcher("glob:$it") } /** Finds all non-excluded files recursively. */ fun walkIncluded(): Sequence = workspaceRoots.asSequence().flatMap { root -> From 983cc6482adadd047b83b310ac89fd6628e0e59d Mon Sep 17 00:00:00 2001 From: fwcd Date: Mon, 15 Jan 2024 00:52:32 +0000 Subject: [PATCH 2/5] Skip exclusions completely instead of adding them temporarily ...to the source path. The rationale for this is that language features would be provided even for excluded files, e.g. .kts scripts when `kotlin.scripts.enabled` is disabled or source files in auto-generated directories such as `./bin`. --- server/src/main/kotlin/org/javacs/kt/SourceFiles.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt b/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt index f5230ab66..4aeecdc69 100644 --- a/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt +++ b/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt @@ -74,8 +74,10 @@ class SourceFiles( private val open = mutableSetOf() fun open(uri: URI, content: String, version: Int) { - files[uri] = SourceVersion(content, version, languageOf(uri), isTemporary = !exclusions.isURIIncluded(uri)) - open.add(uri) + if (exclusions.isURIIncluded(uri)) { + files[uri] = SourceVersion(content, version, languageOf(uri), isTemporary = false) + open.add(uri) + } } fun close(uri: URI) { From 24b57b5a0da742a709b1bc50336d6a25f413528e Mon Sep 17 00:00:00 2001 From: fwcd Date: Mon, 15 Jan 2024 01:07:44 +0000 Subject: [PATCH 3/5] Explicitly enable scripts in ScriptTest --- server/src/test/kotlin/org/javacs/kt/ScriptTest.kt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/src/test/kotlin/org/javacs/kt/ScriptTest.kt b/server/src/test/kotlin/org/javacs/kt/ScriptTest.kt index a48c204dd..9b74a8670 100644 --- a/server/src/test/kotlin/org/javacs/kt/ScriptTest.kt +++ b/server/src/test/kotlin/org/javacs/kt/ScriptTest.kt @@ -4,13 +4,17 @@ import org.hamcrest.Matchers.hasItem import org.junit.Assert.assertThat import org.junit.Test -class ScriptTest : LanguageServerTestFixture("script") { +class ScriptTest : LanguageServerTestFixture("script", Configuration().apply { + scripts.enabled = true +}) { @Test fun `open script`() { open("ExampleScript.kts") } } -class EditFunctionTest : SingleFileTestFixture("script", "FunctionScript.kts") { +class EditFunctionTest : SingleFileTestFixture("script", "FunctionScript.kts", Configuration().apply { + scripts.enabled = true +}) { @Test fun `edit a function in a script`() { replace("FunctionScript.kts", 3, 18, "2", "f") From 9592c113f21ba73fc8523c1f883c11e36025ad26 Mon Sep 17 00:00:00 2001 From: fwcd Date: Mon, 15 Jan 2024 01:23:17 +0000 Subject: [PATCH 4/5] Only recover and provide language features for non-excluded files --- .../javacs/kt/KotlinTextDocumentService.kt | 25 ++++++++++--------- .../main/kotlin/org/javacs/kt/SourceFiles.kt | 2 ++ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/server/src/main/kotlin/org/javacs/kt/KotlinTextDocumentService.kt b/server/src/main/kotlin/org/javacs/kt/KotlinTextDocumentService.kt index 9aca2ea5c..ff988545d 100644 --- a/server/src/main/kotlin/org/javacs/kt/KotlinTextDocumentService.kt +++ b/server/src/main/kotlin/org/javacs/kt/KotlinTextDocumentService.kt @@ -74,12 +74,13 @@ class KotlinTextDocumentService( ALWAYS, AFTER_DOT, NEVER } - private fun recover(position: TextDocumentPositionParams, recompile: Recompile): Pair { + private fun recover(position: TextDocumentPositionParams, recompile: Recompile): Pair? { return recover(position.textDocument.uri, position.position, recompile) } - private fun recover(uriString: String, position: Position, recompile: Recompile): Pair { + private fun recover(uriString: String, position: Position, recompile: Recompile): Pair? { val uri = parseURI(uriString) + if (!sf.isIncluded(uri)) return null val content = sp.content(uri) val offset = offset(content, position.line, position.character) val shouldRecompile = when (recompile) { @@ -92,12 +93,12 @@ class KotlinTextDocumentService( } override fun codeAction(params: CodeActionParams): CompletableFuture>> = async.compute { - val (file, _) = recover(params.textDocument.uri, params.range.start, Recompile.NEVER) + val (file, _) = recover(params.textDocument.uri, params.range.start, Recompile.NEVER) ?: return@compute emptyList() codeActions(file, sp.index, params.range, params.context) } override fun inlayHint(params: InlayHintParams): CompletableFuture> = async.compute { - val (file, _) = recover(params.textDocument.uri, params.range.start, Recompile.ALWAYS) + val (file, _) = recover(params.textDocument.uri, params.range.start, Recompile.ALWAYS) ?: return@compute emptyList() provideHints(file, config.inlayHints) } @@ -105,13 +106,13 @@ class KotlinTextDocumentService( reportTime { LOG.info("Hovering at {}", describePosition(position)) - val (file, cursor) = recover(position, Recompile.NEVER) + val (file, cursor) = recover(position, Recompile.NEVER) ?: return@compute null hoverAt(file, cursor) ?: noResult("No hover found at ${describePosition(position)}", null) } } override fun documentHighlight(position: DocumentHighlightParams): CompletableFuture> = async.compute { - val (file, cursor) = recover(position.textDocument.uri, position.position, Recompile.NEVER) + val (file, cursor) = recover(position.textDocument.uri, position.position, Recompile.NEVER) ?: return@compute emptyList() documentHighlightsAt(file, cursor) } @@ -123,7 +124,7 @@ class KotlinTextDocumentService( reportTime { LOG.info("Go-to-definition at {}", describePosition(position)) - val (file, cursor) = recover(position, Recompile.NEVER) + val (file, cursor) = recover(position, Recompile.NEVER) ?: return@compute Either.forLeft(emptyList()) goToDefinition(file, cursor, uriContentProvider.classContentProvider, tempDirectory, config.externalSources, cp) ?.let(::listOf) ?.let { Either.forLeft, List>(it) } @@ -144,19 +145,19 @@ class KotlinTextDocumentService( } override fun rename(params: RenameParams) = async.compute { - val (file, cursor) = recover(params, Recompile.NEVER) + val (file, cursor) = recover(params, Recompile.NEVER) ?: return@compute null renameSymbol(file, cursor, sp, params.newName) } - override fun completion(position: CompletionParams) = async.compute { + override fun completion(position: CompletionParams): CompletableFuture, CompletionList>> = async.compute { reportTime { LOG.info("Completing at {}", describePosition(position)) - val (file, cursor) = recover(position, Recompile.NEVER) // TODO: Investigate when to recompile + val (file, cursor) = recover(position, Recompile.NEVER) ?: return@compute Either.forRight(CompletionList()) // TODO: Investigate when to recompile val completions = completions(file, cursor, sp.index, config.completion) LOG.info("Found {} items", completions.items.size) - Either.forRight, CompletionList>(completions) + Either.forRight(completions) } } @@ -195,7 +196,7 @@ class KotlinTextDocumentService( reportTime { LOG.info("Signature help at {}", describePosition(position)) - val (file, cursor) = recover(position, Recompile.NEVER) + val (file, cursor) = recover(position, Recompile.NEVER) ?: return@compute null fetchSignatureHelpAt(file, cursor) ?: noResult("No function call around ${describePosition(position)}", null) } } diff --git a/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt b/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt index 4aeecdc69..68c0f66c8 100644 --- a/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt +++ b/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt @@ -192,6 +192,8 @@ class SourceFiles( } fun isOpen(uri: URI): Boolean = (uri in open) + + fun isIncluded(uri: URI): Boolean = exclusions.isURIIncluded(uri) } private fun patch(sourceText: String, change: TextDocumentContentChangeEvent): String { From bf9800e19bca68cc10bf3605199a7042e0f5f829 Mon Sep 17 00:00:00 2001 From: fwcd Date: Mon, 15 Jan 2024 01:24:28 +0000 Subject: [PATCH 5/5] Rewrite excludedPatterns expression without spread operator According to detekt, this would have incurred a performance penalty. --- .../main/kotlin/org/javacs/kt/SourceExclusions.kt | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt b/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt index 93e27d036..17f1d3e86 100644 --- a/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt +++ b/shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt @@ -13,14 +13,13 @@ class SourceExclusions( private val workspaceRoots: Collection, private val scriptsConfig: ScriptsConfiguration ) { - private val excludedPatterns = listOf( - ".*", "bazel-*", "bin", "build", "node_modules", "target", - *(when { - !scriptsConfig.enabled -> arrayOf("*.kts") - !scriptsConfig.buildScriptsEnabled -> arrayOf("*.gradle.kts") - else -> arrayOf() - }), - ) + private val excludedPatterns = (listOf( + ".*", "bazel-*", "bin", "build", "node_modules", "target" + ) + when { + !scriptsConfig.enabled -> listOf("*.kts") + !scriptsConfig.buildScriptsEnabled -> listOf("*.gradle.kts") + else -> emptyList() + }) .map { FileSystems.getDefault().getPathMatcher("glob:$it") } /** Finds all non-excluded files recursively. */