Merge pull request #543 from fwcd/improve-exclusions

Model script exclusion via `SourceExclusions` and skip files entirely instead of placing them temporarily on the source path

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<Path> {
+        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<Path> {
-    val sourceMatcher = FileSystems.getDefault().getPathMatcher("glob:*.java")
-    return SourceExclusions(root)
-        .walkIncluded()
-        .filter { sourceMatcher.matches(it.fileName) }
-        .toSet()
-}
-
 private fun logAdded(sources: Collection<Path>, name: String) {
     when {
         sources.isEmpty() -> return

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"

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<CompiledFile, Int> {
+    private fun recover(position: TextDocumentPositionParams, recompile: Recompile): Pair<CompiledFile, Int>? {
         return recover(position.textDocument.uri, position.position, recompile)
     }
 
-    private fun recover(uriString: String, position: Position, recompile: Recompile): Pair<CompiledFile, Int> {
+    private fun recover(uriString: String, position: Position, recompile: Recompile): Pair<CompiledFile, Int>? {
         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,26 +93,26 @@ class KotlinTextDocumentService(
     }
 
     override fun codeAction(params: CodeActionParams): CompletableFuture<List<Either<Command, CodeAction>>> = 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<List<InlayHint>> = 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)
     }
 
     override fun hover(position: HoverParams): CompletableFuture<Hover?> = async.compute {
         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<List<DocumentHighlight>> = 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<Location>, List<LocationLink>>(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<Either<List<CompletionItem>, 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<List<CompletionItem>, 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)
         }
     }

server/src/main/kotlin/org/javacs/kt/SourceFiles.kt

@@ -69,13 +69,15 @@ class SourceFiles(
     private val scriptsConfig: ScriptsConfiguration
 ) {
     private val workspaceRoots = mutableSetOf<Path>()
-    private var exclusions = SourceExclusions(workspaceRoots)
+    private var exclusions = SourceExclusions(workspaceRoots, scriptsConfig)
     private val files = NotifySourcePath(sp)
     private val open = mutableSetOf<URI>()
 
     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) {
@@ -177,23 +179,21 @@ class SourceFiles(
     }
 
     private fun findSourceFiles(root: Path): Set<URI> {
-        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)
+
+    fun isIncluded(uri: URI): Boolean = exclusions.isURIIncluded(uri)
 }
 
 private fun patch(sourceText: String, change: TextDocumentContentChangeEvent): String {

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

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

shared/src/main/kotlin/org/javacs/kt/SourceExclusions.kt

@@ -9,10 +9,18 @@ import java.nio.file.Paths
 
 // TODO: Read exclusions from gitignore/settings.json/... instead of
 // hardcoding them
-class SourceExclusions(private val workspaceRoots: Collection<Path>) {
-	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<Path>,
+    private val scriptsConfig: ScriptsConfiguration
+) {
+	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. */
     fun walkIncluded(): Sequence<Path> = workspaceRoots.asSequence().flatMap { root ->