Revert mutation attempts, initial error reporting solution

gradle · Aug 21, 2024 · a1151b1 · a1151b1
1 parent c37e224
commit a1151b1
Show file tree

Hide file tree

Showing 12 changed files with 197 additions and 76 deletions.
diff --git a/lsp/build.gradle.kts b/lsp/build.gradle.kts
@@ -40,15 +40,24 @@ detekt {
     config.convention(project.isolated.rootProject.projectDirectory.file("gradle/detekt.yml"))
 }
 
+java {
+    toolchain {
+        languageVersion.set(JavaLanguageVersion.of(8))
+    }
+}
+
 testing {
     suites {
         val test by getting(JvmTestSuite::class) {
-            java {
-                toolchain {
-                    languageVersion.set(JavaLanguageVersion.of(17))
-                }
-            }
             useKotlinTest("2.0.0")
         }
     }
+}
+
+tasks {
+    shadowJar {
+        manifest {
+            attributes["Main-Class"] = "org.gradle.declarative.lsp.MainKt"
+        }
+    }
 }
diff --git a/lsp/src/main/kotlin/org/gradle/declarative/lsp/DeclarativeLanguageServer.kt b/lsp/src/main/kotlin/org/gradle/declarative/lsp/DeclarativeLanguageServer.kt
@@ -27,7 +27,6 @@ import org.eclipse.lsp4j.services.LanguageClientAware
 import org.eclipse.lsp4j.services.LanguageServer
 import org.eclipse.lsp4j.services.TextDocumentService
 import org.eclipse.lsp4j.services.WorkspaceService
-import org.gradle.declarative.lsp.tapi.ConnectionHandler
 import org.slf4j.LoggerFactory
 import java.io.File
 import java.net.URI
@@ -57,14 +56,14 @@ class DeclarativeLanguageServer : LanguageServer, LanguageClientAware {
 
     override fun initialize(params: InitializeParams?): CompletableFuture<InitializeResult> {
         val serverCapabilities = ServerCapabilities()
+        // Here we set the capabilities we support
         serverCapabilities.setTextDocumentSync(TextDocumentSyncKind.Full)
         serverCapabilities.setHoverProvider(true)
-        serverCapabilities.setCodeActionProvider(true)
 
         val workspaceFolder = params!!.workspaceFolders[0]
         val workspaceFolderFile = File(URI.create(workspaceFolder.uri))
         LOGGER.info("Fetching declarative model for workspace folder: $workspaceFolderFile")
-        ConnectionHandler(workspaceFolderFile).let {
+        TapiConnectionHandler(workspaceFolderFile).let {
             val declarativeBuildModel = it.getDomPrequisites()
             textDocumentService.setResources(declarativeBuildModel)
         }

diff --git a/lsp/src/main/kotlin/org/gradle/declarative/lsp/DeclarativeTextDocumentService.kt b/lsp/src/main/kotlin/org/gradle/declarative/lsp/DeclarativeTextDocumentService.kt
@@ -16,9 +16,6 @@
 
 package org.gradle.declarative.lsp
 
-import org.eclipse.lsp4j.CodeAction
-import org.eclipse.lsp4j.CodeActionParams
-import org.eclipse.lsp4j.Command
 import org.eclipse.lsp4j.DidChangeTextDocumentParams
 import org.eclipse.lsp4j.DidCloseTextDocumentParams
 import org.eclipse.lsp4j.DidOpenTextDocumentParams
@@ -27,18 +24,16 @@ import org.eclipse.lsp4j.Hover
 import org.eclipse.lsp4j.HoverParams
 import org.eclipse.lsp4j.MarkupContent
 import org.eclipse.lsp4j.Position
+import org.eclipse.lsp4j.PublishDiagnosticsParams
 import org.eclipse.lsp4j.Range
-import org.eclipse.lsp4j.jsonrpc.messages.Either
 import org.eclipse.lsp4j.services.LanguageClient
 import org.eclipse.lsp4j.services.LanguageClientAware
 import org.eclipse.lsp4j.services.TextDocumentService
 import org.gradle.declarative.lsp.build.model.ResolvedDeclarativeResourcesModel
-import org.gradle.declarative.lsp.storage.VersionedDocumentStore
+import org.gradle.declarative.lsp.visitor.ErrorToDiagnosticVisitor
 import org.gradle.declarative.lsp.visitor.LocationMatchingVisitor
 import org.gradle.declarative.lsp.visitor.visit
 import org.gradle.internal.declarativedsl.dom.DeclarativeDocument
-import org.gradle.internal.declarativedsl.dom.mutation.MutationApplicability
-import org.gradle.internal.declarativedsl.dom.mutation.MutationApplicabilityChecker
 import org.gradle.internal.declarativedsl.dom.operations.overlay.DocumentOverlayResult
 import org.gradle.internal.declarativedsl.evaluator.main.AnalysisDocumentUtils
 import org.gradle.internal.declarativedsl.evaluator.main.SimpleAnalysisEvaluator
@@ -49,20 +44,16 @@ import java.util.concurrent.CompletableFuture
 
 class DeclarativeTextDocumentService : TextDocumentService, LanguageClientAware {
     private val documentStore = VersionedDocumentStore()
-    private val availableMutations = listOf(
-        AddDependency()
-    )
 
     private lateinit var client: LanguageClient
     private lateinit var resources: ResolvedDeclarativeResourcesModel
     private lateinit var schemaEvaluator: SimpleAnalysisEvaluator
-    private var applicableMutations: List<MutationApplicability> = emptyList()
 
     override fun connect(client: LanguageClient?) {
         this.client = client!!
     }
 
-    // LSP Functions ----------------------------------------------------------
+    // LSP Functions ---------------------------------------------------------------------------------------------------
 
     fun setResources(resources: ResolvedDeclarativeResourcesModel) {
         this.resources = resources
@@ -72,19 +63,20 @@ class DeclarativeTextDocumentService : TextDocumentService, LanguageClientAware
     }
 
     override fun didOpen(params: DidOpenTextDocumentParams?) {
+        LOGGER.trace("Opened document: {}", params)
         params?.let {
-            LOGGER.trace("Opened document: {}", it.textDocument.uri)
-
             val uri = URI(it.textDocument.uri)
             val text = File(uri).readText()
-            documentStore.storeInitial(uri, parse(uri, text))
+            val document = parse(uri, text)
+            documentStore.storeInitial(uri, document)
+
             processDocument(uri)
         }
     }
 
     override fun didChange(params: DidChangeTextDocumentParams?) {
+        LOGGER.trace("Changed document: {}", params)
         params?.let {
-            LOGGER.trace("Changed document: ${params.textDocument.uri}")
             val uri = URI(it.textDocument.uri)
             it.contentChanges.forEach { change ->
                 documentStore.storeVersioned(
@@ -98,18 +90,19 @@ class DeclarativeTextDocumentService : TextDocumentService, LanguageClientAware
     }
 
     override fun didClose(params: DidCloseTextDocumentParams?) {
+        LOGGER.trace("Closed document: {}", params)
         params?.let {
             val uri = URI(it.textDocument.uri)
             documentStore.remove(uri)
         }
-        LOGGER.trace("Closed document: ${params?.textDocument?.uri}")
     }
 
     override fun didSave(params: DidSaveTextDocumentParams?) {
-        LOGGER.trace("Saved document: ${params?.textDocument?.uri}")
+        LOGGER.trace("Saved document: {}", params)
     }
 
     override fun hover(params: HoverParams?): CompletableFuture<Hover> {
+        LOGGER.trace("Hover requested for position: {}", params)
         val hover = params?.let {
             val uri = URI(it.textDocument.uri)
             withDom(uri) { dom ->
@@ -125,21 +118,9 @@ class DeclarativeTextDocumentService : TextDocumentService, LanguageClientAware
                         is DeclarativeDocument.DocumentNode.ElementNode -> node.name
                         else -> null
                     }?.let { name ->
-                        // We need to convert back the 1-based locations to 0-based ones
-                        val startPosition = Position(
-                            node.sourceData.lineRange.first - 1,
-                            node.sourceData.startColumn - 1
-                        )
-                        // End position is tricky, as...
-                        val endPosition = Position(
-                            node.sourceData.lineRange.last - 1,
-                            // ... the end column is exclusive, so we DON'T need to subtract 1
-                            node.sourceData.endColumn
-                        )
-
                         Hover(
                             MarkupContent("plaintext", name),
-                            Range(startPosition, endPosition)
+                            node.sourceData.toLspRange()
                         )
                     }
                 }
@@ -148,25 +129,23 @@ class DeclarativeTextDocumentService : TextDocumentService, LanguageClientAware
         return CompletableFuture.completedFuture(hover)
     }
 
-    override fun codeAction(params: CodeActionParams?): CompletableFuture<MutableList<Either<Command, CodeAction>>> {
-        params?.let {
-            // TODO: finish this
-        }
-
-        return CompletableFuture.completedFuture(null)
-    }
 
     // Common processing -----------------------------------------------------------------------------------------------
 
     private fun processDocument(uri: URI) = withDom(uri) { dom ->
-        applicableMutations = processMutationApplications(dom)
+        processErrors(uri, dom)
     }
 
-    private fun processMutationApplications(dom: DocumentOverlayResult): List<MutationApplicability> {
-        val mac = MutationApplicabilityChecker(resources.analysisSchema, dom.result)
-        return availableMutations.flatMap { mutation ->
-            mac.checkApplicability(mutation)
-        }
+    private fun processErrors(uri: URI, dom: DocumentOverlayResult) {
+        val diagnostics = dom.document.visit(ErrorToDiagnosticVisitor()).diagnostics
+        LOGGER.trace("Publishing diagnostics for document: {}", uri)
+        client.publishDiagnostics(
+            PublishDiagnosticsParams(
+                uri.toString(),
+                // Can be empty, which means that we have no errors
+                diagnostics
+            )
+        )
     }
 
     // Utility and other member functions ------------------------------------------------------------------------------
@@ -178,8 +157,12 @@ class DeclarativeTextDocumentService : TextDocumentService, LanguageClientAware
             resources.settingsFile.name, resources.settingsFile.readText()
         )
 
-        LOGGER.trace("Parsed declarative model for document: {}", uri)
         val document = AnalysisDocumentUtils.documentWithModelDefaults(settingsSchema, fileSchema)
+        when (document != null) {
+            true -> LOGGER.trace("Parsed declarative model for document: {}", uri)
+            false -> LOGGER.error("Failed to parse declarative model for document: {}", uri)
+        }
+
         return document!!
     }
 

diff --git a/...declarative/lsp/tapi/ConnectionHandler.kt → .../declarative/lsp/TapiConnectionHandler.kt b/...declarative/lsp/tapi/ConnectionHandler.kt → .../declarative/lsp/TapiConnectionHandler.kt
@@ -14,15 +14,15 @@
  * limitations under the License.
  */
 
-package org.gradle.declarative.lsp.tapi
+package org.gradle.declarative.lsp
 
 import org.gradle.declarative.lsp.build.action.GetDeclarativeResourcesModel
 import org.gradle.declarative.lsp.build.model.ResolvedDeclarativeResourcesModel
 import org.gradle.tooling.GradleConnector
 import org.gradle.tooling.ProjectConnection
 import java.io.File
 
-class ConnectionHandler(val projectRoot: File) {
+class TapiConnectionHandler(val projectRoot: File) {
     fun getDomPrequisites(): ResolvedDeclarativeResourcesModel {
         var connection: ProjectConnection? = null
         try {

diff --git a/...ive/lsp/storage/VersionedDocumentStore.kt → ...declarative/lsp/VersionedDocumentStore.kt b/...ive/lsp/storage/VersionedDocumentStore.kt → ...declarative/lsp/VersionedDocumentStore.kt
@@ -14,9 +14,10 @@
  * limitations under the License.
  */
 
-package org.gradle.declarative.lsp.storage
+package org.gradle.declarative.lsp
 
 import org.gradle.internal.declarativedsl.dom.operations.overlay.DocumentOverlayResult
+import org.slf4j.LoggerFactory
 import java.net.URI
 
 /**
@@ -68,11 +69,15 @@ class VersionedDocumentStore {
     fun remove(uri: URI) {
         store.remove(uri)
     }
-}
 
-sealed class DocumentStoreEntry {
-    abstract val document: DocumentOverlayResult
+    companion object {
+        private val LOGGER = LoggerFactory.getLogger(DeclarativeTextDocumentService::class.java)
+    }
+
+    sealed class DocumentStoreEntry {
+        abstract val document: DocumentOverlayResult
 
-    data class Initial(override val document: DocumentOverlayResult) : DocumentStoreEntry()
-    data class Versioned(val version: Int, override val document: DocumentOverlayResult) : DocumentStoreEntry()
+        data class Initial(override val document: DocumentOverlayResult) : DocumentStoreEntry()
+        data class Versioned(val version: Int, override val document: DocumentOverlayResult) : DocumentStoreEntry()
+    }
 }
diff --git a/lsp/src/main/kotlin/org/gradle/declarative/lsp/extensions.kt b/lsp/src/main/kotlin/org/gradle/declarative/lsp/extensions.kt
@@ -0,0 +1,41 @@
+/*
+ * Copyright 2024 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.gradle.declarative.lsp
+
+import org.eclipse.lsp4j.Position
+import org.eclipse.lsp4j.Range
+import org.gradle.internal.declarativedsl.language.SourceData
+
+/**
+ * Converts between DCL's [SourceData] and LSP's [Range].
+ *
+ * The conversion is needed because:
+ *  - DCL uses 1-based line and column numbers, while LSP uses 0-based line and column numbers.
+ *  - DCL uses inclusive ranges, while LSP uses exclusive ranges.
+ */
+fun SourceData.toLspRange(): Range {
+    val startPosition = Position(
+        lineRange.first - 1,
+        startColumn - 1
+    )
+    val endPosition = Position(
+        lineRange.last - 1,
+        // This missing -1 is intentional, as it makes the end position exclusive
+        endColumn
+    )
+    return Range(startPosition, endPosition)
+}
diff --git a/lsp/src/main/kotlin/org/gradle/declarative/lsp/visitor/DocumentNodeVisitor.kt b/lsp/src/main/kotlin/org/gradle/declarative/lsp/visitor/DocumentNodeVisitor.kt
@@ -21,6 +21,7 @@ import org.gradle.internal.declarativedsl.dom.DocumentNodeContainer
 
 /**
  * Classic visitor for visiting nodes in a DeclarativeDocument.
+ * The subclasses are designed to
  */
 open class DocumentNodeVisitor {
 
@@ -47,7 +48,7 @@ open class DocumentNodeVisitor {
  * In order to use this, implement a subclass of [DocumentNodeVisitor]
  * and override the methods you need to do the analysis.
  */
-fun DeclarativeDocument.visit(visitor: DocumentNodeVisitor) {
+fun <T: DocumentNodeVisitor> DeclarativeDocument.visit(visitor: T): T {
     // Initialize the list of nodes to visit with the root nodes of the forest
     val nodesToVisit = this.content.toMutableList()
 
@@ -59,14 +60,6 @@ fun DeclarativeDocument.visit(visitor: DocumentNodeVisitor) {
             nodesToVisit.addAll(node.content)
         }
         when (node) {
-            is DeclarativeDocument.DocumentNode -> {
-                visitor.visitDocumentNode(node)
-                when (node) {
-                    is DeclarativeDocument.DocumentNode.ElementNode -> visitor.visitDocumentElementNode(node)
-                    is DeclarativeDocument.DocumentNode.ErrorNode -> visitor.visitDocumentErrorNode(node)
-                    is DeclarativeDocument.DocumentNode.PropertyNode -> visitor.visitDocumentPropertyNode(node)
-                }
-            }
             is DeclarativeDocument.ValueNode -> {
                 visitor.visitValueNode(node)
                 when (node) {
@@ -75,6 +68,16 @@ fun DeclarativeDocument.visit(visitor: DocumentNodeVisitor) {
                     else -> {}
                 }
             }
+            is DeclarativeDocument.DocumentNode -> {
+                visitor.visitDocumentNode(node)
+                when (node) {
+                    is DeclarativeDocument.DocumentNode.ElementNode -> visitor.visitDocumentElementNode(node)
+                    is DeclarativeDocument.DocumentNode.ErrorNode -> visitor.visitDocumentErrorNode(node)
+                    is DeclarativeDocument.DocumentNode.PropertyNode -> visitor.visitDocumentPropertyNode(node)
+                }
+            }
         }
     }
-}
+
+    return visitor
+}