Copilot commented on code in PR #2936:
URL: https://github.com/apache/groovy/pull/2936#discussion_r4214610958


##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/GroovyLanguageServer.java:
##########
@@ -0,0 +1,141 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you 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.apache.groovy.lsp;
+
+import org.apache.groovy.lsp.internal.LanguageServerContext;
+import org.apache.groovy.lsp.internal.feature.LanguageFeatures;
+import org.apache.groovy.lsp.internal.position.PositionEncoding;
+import org.apache.groovy.lsp.internal.protocol.GroovyTextDocumentService;
+import org.apache.groovy.lsp.internal.protocol.GroovyWorkspaceService;
+import org.apache.groovy.lsp.internal.protocol.ServerCapabilityFactory;
+import org.apache.groovy.lsp.internal.util.Uris;
+import org.eclipse.lsp4j.InitializeParams;
+import org.eclipse.lsp4j.InitializeResult;
+import org.eclipse.lsp4j.InitializedParams;
+import org.eclipse.lsp4j.ServerInfo;
+import org.eclipse.lsp4j.WorkspaceFolder;
+import org.eclipse.lsp4j.services.LanguageClient;
+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 java.util.concurrent.CompletableFuture;
+
+/**
+ * Apache Groovy language server. Implements LSP 3.18 with capability
+ * negotiation for 3.19-preview clients. Compile-time execution paths
+ * ({@code @Grab}, {@code @ASTTest}) are off unless the client enables them.
+ */
+public final class GroovyLanguageServer implements LanguageServer, 
LanguageClientAware {
+
+    private final LanguageServerContext context;
+    private final LanguageFeatures features = new LanguageFeatures();
+    private final GroovyTextDocumentService textDocuments;
+    private final GroovyWorkspaceService workspace;
+    private final ServerCapabilityFactory capabilities = new 
ServerCapabilityFactory();
+
+    /**
+     * Creates a server with a fresh session.
+     */
+    public GroovyLanguageServer() {
+        this(new LanguageServerContext());
+    }
+
+    /**
+     * Creates a server around an existing session. Intended for tests.
+     *
+     * @param context session state
+     */
+    public GroovyLanguageServer(final LanguageServerContext context) {
+        this.context = context;
+        this.textDocuments = new GroovyTextDocumentService(context, features);
+        this.workspace = new GroovyWorkspaceService(context, features);
+    }
+
+    /**
+     * @return feature services for this process
+     */
+    public LanguageFeatures features() {
+        return features;
+    }
+
+    /**
+     * @return the session state
+     */
+    public LanguageServerContext getContext() {
+        return context;
+    }
+
+    @Override
+    public CompletableFuture<InitializeResult> initialize(final 
InitializeParams params) {
+        if (params.getCapabilities() != null && 
params.getCapabilities().getGeneral() != null) {
+            context.setPositionEncoding(PositionEncoding.negotiate(
+                    
params.getCapabilities().getGeneral().getPositionEncodings()));
+        } else {
+            context.setPositionEncoding(PositionEncoding.UTF16);
+        }
+        context.setClientCapabilities(params.getCapabilities());
+        if (params.getWorkspaceFolders() != null) {
+            for (WorkspaceFolder folder : params.getWorkspaceFolders()) {
+                context.addWorkspaceFolder(Uris.parse(folder.getUri()));
+            }
+        } else if (params.getRootUri() != null) {
+            context.addWorkspaceFolder(Uris.parse(params.getRootUri()));
+        }
+        InitializeResult result = new 
InitializeResult(capabilities.create(context.getPositionEncoding()));
+        result.setServerInfo(new ServerInfo("groovy-lsp", 
GroovyLanguageServer.class.getPackage() == null
+                ? "dev" : 
String.valueOf(GroovyLanguageServer.class.getPackage().getImplementationVersion())));

Review Comment:
   In normal development/test classpaths the package exists but has no 
`Implementation-Version`, so this advertises the literal string `"null"`. Use 
the same null fallback as `--version` and report `dev` when the manifest value 
is absent.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/compile/CompilationSnapshot.java:
##########
@@ -0,0 +1,116 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you 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.apache.groovy.lsp.internal.compile;
+
+import org.apache.groovy.lsp.internal.util.Uris;
+import org.apache.groovy.lsp.internal.workspace.TextDocument;
+
+import java.net.URI;
+import java.util.Collection;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+
+/**
+ * Immutable result of one workspace compile.
+ */
+public final class CompilationSnapshot {
+
+    public static final CompilationSnapshot EMPTY = new 
CompilationSnapshot(Map.of(), JavaSymbolIndex.EMPTY);
+
+    private final Map<URI, CompiledDocument> documents;
+    private final JavaSymbolIndex javaSymbols;
+    private final AtomicReference<TypeIndex> types = new AtomicReference<>();
+
+    /**
+     * @param documents compiled documents keyed by normalized URI
+     */
+    public CompilationSnapshot(final Map<URI, CompiledDocument> documents) {
+        this(documents, JavaSymbolIndex.EMPTY);
+    }
+
+    /**
+     * @param documents compiled documents keyed by normalized URI
+     * @param javaSymbols types and members from joint-compiled Java sources
+     */
+    public CompilationSnapshot(final Map<URI, CompiledDocument> documents, 
final JavaSymbolIndex javaSymbols) {
+        this.documents = Map.copyOf(documents);

Review Comment:
   `Map.copyOf` does not preserve the `LinkedHashMap` encounter order used by 
the compiler builder. Downstream indexing documents the original document order 
and limits completion candidates, so discarding it here makes ordering and even 
the selected prefix-completion subset unstable. Keep an unmodifiable 
insertion-ordered copy.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/feature/DocumentLinkService.java:
##########
@@ -0,0 +1,103 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you 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.apache.groovy.lsp.internal.feature;
+
+import org.apache.groovy.lsp.internal.compile.CompilationSnapshot;
+import org.apache.groovy.lsp.internal.compile.CompiledDocument;
+import org.apache.groovy.lsp.internal.compile.ImportSupport;
+import org.apache.groovy.lsp.internal.position.PositionEncoding;
+import org.apache.groovy.lsp.internal.position.Positions;
+import org.apache.groovy.lsp.internal.util.Uris;
+import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.ImportNode;
+import org.eclipse.lsp4j.DocumentLink;
+import org.eclipse.lsp4j.Range;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Links from import names to their source.
+ */
+public final class DocumentLinkService {
+
+    public List<DocumentLink> documentLinks(final CompiledDocument document, 
final CompilationSnapshot snapshot,
+                                            final PositionEncoding encoding) {
+        List<DocumentLink> links = new ArrayList<>();
+        if (document == null || document.getModule() == null) {
+            return links;
+        }
+        for (ImportNode imp : ImportSupport.allImports(document.getModule())) {
+            addDocumentLink(links, document, snapshot, encoding, imp);
+        }
+        return links;
+    }
+
+    private static void addDocumentLink(final List<DocumentLink> links, final 
CompiledDocument document,
+                                        final CompilationSnapshot snapshot, 
final PositionEncoding encoding,
+                                        final ImportNode imp) {
+        Range range = Positions.toRange(imp, document.getText(), encoding);
+        if (range == null) {
+            return;
+        }
+        String target = importTarget(imp, document, snapshot);
+        if (target != null) {
+            links.add(new DocumentLink(range, target));
+        }
+    }
+
+    private static String importTarget(final ImportNode imp, final 
CompiledDocument document,
+                                       final CompilationSnapshot snapshot) {
+        String className = imp.getClassName();
+        if (className == null) {
+            return null;
+        }
+        String workspace = workspaceTypeUri(className, snapshot);
+        if (workspace != null) {
+            return workspace;
+        }
+        if (document.getUri() == null || 
!"file".equalsIgnoreCase(document.getUri().getScheme())) {
+            return null;
+        }
+        Path parent = Uris.toPath(document.getUri()).getParent();
+        if (parent == null) {
+            return null;
+        }
+        return Uris.normalize(parent.resolve(className.replace('.', '/') + 
".groovy").toUri()).toString();

Review Comment:
   When an import is absent from the snapshot, this fabricates a URI relative 
to the importing file's directory. For `/src/main/groovy/demo/Use.groovy` 
importing `demo.Widget`, it links to nonexistent 
`/src/main/groovy/demo/demo/Widget.groovy`; the source root cannot be inferred 
from `parent`. Return no link unless a real workspace source URI is known, or 
resolve against configured source roots.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/GroovyLanguageServerLauncher.java:
##########
@@ -0,0 +1,135 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you 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.apache.groovy.lsp;
+
+import org.eclipse.lsp4j.jsonrpc.Launcher;
+import org.eclipse.lsp4j.services.LanguageClient;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PrintStream;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+
+/**
+ * Process entry point. Speaks LSP over stdio by default; {@code --socket
+ * <port>} listens for a single client connection. {@code System.out} is
+ * redirected to {@code System.err} so that incidental prints cannot
+ * corrupt the JSON-RPC stream.
+ */
+@SuppressWarnings("java:S106") // stdout is the JSON-RPC stream; 
--help/--version are CLI
+public final class GroovyLanguageServerLauncher {
+
+    private GroovyLanguageServerLauncher() {
+    }
+
+    /**
+     * Launches the language server.
+     *
+     * @param args {@code --help}, {@code --version}, or {@code --socket 
<port>}
+     * @throws IOException when the socket transport fails
+     * @throws InterruptedException when waiting for {@code exit} is 
interrupted
+     * @throws ExecutionException when the JSON-RPC listener fails
+     */
+    public static void main(final String[] args) throws IOException, 
InterruptedException, ExecutionException {
+        ListArgs parsed = ListArgs.parse(args);
+        if (parsed.help) {
+            System.out.println("""
+                    groovylsp - Apache Groovy Language Server (LSP 3.18)
+                    Usage: groovylsp [--stdio] [--socket <port>] [--version] 
[--help]
+                    Default transport is stdio.
+                    """);
+            return;
+        }
+        if (parsed.version) {
+            Package pkg = GroovyLanguageServer.class.getPackage();
+            String version = pkg == null || pkg.getImplementationVersion() == 
null
+                    ? "dev" : pkg.getImplementationVersion();
+            System.out.println("groovy-lsp " + version);
+            return;
+        }
+        if (parsed.port != null) {
+            try (ServerSocket serverSocket = new ServerSocket(parsed.port);
+                 Socket socket = serverSocket.accept()) {
+                run(socket.getInputStream(), socket.getOutputStream(), true);
+            }
+            return;
+        }
+        InputStream in = System.in;
+        OutputStream out = System.out;
+        System.setOut(new PrintStream(System.err, true, 
StandardCharsets.UTF_8));
+        System.exit(run(in, out, true));
+    }
+
+    /**
+     * Runs a server on the given streams. When {@code waitForExit} is true,
+     * the method blocks until {@code exit} is received.
+     *
+     * @param in JSON-RPC input
+     * @param out JSON-RPC output
+     * @param waitForExit whether to wait for the LSP {@code exit} notification
+     * @return the exit code, or {@code null} when not waiting
+     * @throws InterruptedException when waiting for {@code exit} is 
interrupted
+     * @throws ExecutionException when the JSON-RPC listener fails
+     */
+    public static Integer run(final InputStream in, final OutputStream out, 
final boolean waitForExit)
+            throws InterruptedException, ExecutionException {
+        GroovyLanguageServer server = new GroovyLanguageServer();
+        Launcher<LanguageClient> launcher = Launcher.createLauncher(server, 
LanguageClient.class, in, out);
+        server.connect(launcher.getRemoteProxy());
+        Future<Void> listening = launcher.startListening();
+        if (!waitForExit) {
+            return server.getContext().getExitCode();
+        }
+        listening.get();

Review Comment:
   `startListening()` completes when the JSON-RPC input reaches EOF, not when 
the `exit` notification invokes `GroovyLanguageServer.exit()`. Because `exit()` 
only records an exit code, a conforming client that sends `exit` but keeps the 
transport open leaves both stdio and socket launchers blocked here 
indefinitely. Add an exit signal shared with the server and wait for that 
signal rather than transport EOF.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/compile/TypeIndex.java:
##########
@@ -0,0 +1,151 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you 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.apache.groovy.lsp.internal.compile;
+
+import org.apache.groovy.lsp.internal.util.GroovydocMarkdown;
+import org.codehaus.groovy.ast.ClassNode;
+
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+
+/**
+ * Simple-name and FQCN index of types in a {@link CompilationSnapshot}.
+ */
+public final class TypeIndex {
+
+    public static final TypeIndex EMPTY = new TypeIndex(Map.of(), Map.of());
+
+    private final Map<String, List<TypeHit>> bySimpleName;
+    private final Map<String, TypeHit> byName;
+
+    private TypeIndex(final Map<String, List<TypeHit>> bySimpleName, final 
Map<String, TypeHit> byName) {
+        this.bySimpleName = bySimpleName;
+        this.byName = byName;
+    }
+
+    /**
+     * Builds an index over every non-synthetic class in {@code snapshot}.
+     *
+     * @param snapshot compiled workspace
+     * @return the index
+     */
+    public static TypeIndex of(final CompilationSnapshot snapshot) {
+        if (snapshot == null) {
+            return EMPTY;
+        }
+        Map<String, List<TypeHit>> simple = new LinkedHashMap<>();
+        Map<String, TypeHit> names = new LinkedHashMap<>();
+        for (CompiledDocument document : snapshot.documents()) {
+            if (document.getModule() == null) {
+                continue;
+            }
+            for (ClassNode classNode : document.getModule().getClasses()) {
+                if (classNode.isScript() && classNode.getLineNumber() <= 0) {
+                    continue;
+                }
+                TypeHit hit = TypeHit.of(classNode, document.getUri());
+                names.put(hit.name(), hit);
+                simple.computeIfAbsent(hit.simpleName(), key -> new 
ArrayList<>()).add(hit);
+            }
+        }
+        for (JavaSymbolIndex.JavaSymbol symbol : 
snapshot.javaSymbols().types()) {
+            TypeHit hit = new TypeHit(symbol.qualifiedName(), 
symbol.simpleName(), symbol.uri(), false, false, "");
+            names.putIfAbsent(hit.name(), hit);
+            simple.computeIfAbsent(hit.simpleName(), key -> new 
ArrayList<>()).add(hit);
+        }
+        return new TypeIndex(Map.copyOf(simple), Map.copyOf(names));

Review Comment:
   These `Map.copyOf` calls discard the insertion order assembled above. 
`matchingPrefix` iterates `byName.values()` and explicitly promises document 
order, so limited workspace-type completion becomes nondeterministic even if 
the snapshot order is preserved. Retain unmodifiable `LinkedHashMap` copies 
here.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/compile/WorkspaceLayout.java:
##########
@@ -0,0 +1,150 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you 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.apache.groovy.lsp.internal.compile;
+
+import org.apache.groovy.lsp.internal.util.Uris;
+
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Locale;
+import java.util.stream.Stream;
+
+/**
+ * Best-effort source-root and jar discovery when the client leaves
+ * {@code groovy.sourcePaths} / {@code groovy.classpath} empty. Does not
+ * invoke Gradle or Maven.
+ */
+public final class WorkspaceLayout {
+
+    private WorkspaceLayout() {
+    }
+
+    /**
+     * Search roots for Groovy sources under {@code folder}.
+     *
+     * @param folder workspace folder
+     * @param sourcePaths client source paths, possibly empty
+     * @return directories to walk
+     */
+    public static List<Path> searchRoots(final Path folder, final List<String> 
sourcePaths) {
+        List<Path> roots = new ArrayList<>();
+        if (folder == null || !Files.isDirectory(folder)) {
+            return roots;
+        }
+        if (sourcePaths != null && !sourcePaths.isEmpty()) {
+            return explicitRoots(folder, sourcePaths);
+        }
+        addIfDirectory(roots, folder.resolve("src/main/groovy"));
+        addIfDirectory(roots, folder.resolve("src/test/groovy"));
+        addIfDirectory(roots, folder.resolve("src/main/java"));
+        addIfDirectory(roots, folder.resolve("src/test/java"));
+        if (roots.isEmpty()) {
+            addIfDirectory(roots, folder.resolve("src"));
+        }
+        if (roots.isEmpty()) {
+            roots.add(folder);
+        }
+        return roots;
+    }
+
+    private static List<Path> explicitRoots(final Path folder, final 
List<String> sourcePaths) {
+        List<Path> roots = new ArrayList<>();
+        for (String sourcePath : sourcePaths) {
+            Path extra = Path.of(sourcePath);
+            roots.add(extra.isAbsolute() ? extra : folder.resolve(sourcePath));
+        }
+        return roots;
+    }
+
+    private static void addIfDirectory(final List<Path> roots, final Path 
path) {
+        if (Files.isDirectory(path)) {
+            roots.add(path);
+        }
+    }
+
+    /**
+     * Jars under {@code lib/} and {@code libs/} of {@code folder}.
+     *
+     * @param folder workspace folder
+     * @return absolute jar paths
+     */
+    public static List<String> classpathJars(final Path folder) {
+        List<String> jars = new ArrayList<>();
+        if (folder == null || !Files.isDirectory(folder)) {
+            return jars;
+        }
+        for (String dir : List.of("lib", "libs")) {
+            Path path = folder.resolve(dir);
+            if (!Files.isDirectory(path)) {
+                continue;
+            }
+            try (Stream<Path> stream = Files.list(path)) {
+                stream.filter(Files::isRegularFile)
+                        .filter(p -> 
p.getFileName().toString().toLowerCase(Locale.ROOT).endsWith(".jar"))
+                        .forEach(p -> jars.add(p.toAbsolutePath().toString()));
+            } catch (IOException ignored) {
+                // skip unreadable lib dirs
+            }
+        }
+        return jars;
+    }
+
+    /**
+     * Fills empty classpath / source-path lists from conventional layout.
+     *
+     * @param settings client settings
+     * @param folders workspace folders
+     * @return settings to compile with
+     */
+    public static CompilerSettings withInferred(final CompilerSettings 
settings, final Collection<URI> folders) {
+        CompilerSettings current = settings == null ? 
CompilerSettings.defaults() : settings;
+        List<String> classpath = new ArrayList<>(current.getClasspath());
+        List<String> sourcePaths = new ArrayList<>(current.getSourcePaths());
+        if (folders != null) {
+            for (URI folder : folders) {
+                inferFromFolder(folder, classpath, sourcePaths);
+            }
+        }
+        return new CompilerSettings(classpath, sourcePaths, 
current.getThroughPhase(),
+                current.isGrapeEnabled(), current.isAstTestEnabled(), 
current.getExtra());
+    }
+
+    private static void inferFromFolder(final URI folder, final List<String> 
classpath,
+                                        final List<String> sourcePaths) {
+        if (folder == null || !"file".equalsIgnoreCase(folder.getScheme())) {
+            return;
+        }
+        Path path = Uris.toPath(folder);
+        if (sourcePaths.isEmpty()) {
+            for (Path root : searchRoots(path, List.of())) {
+                if (!root.equals(path)) {
+                    sourcePaths.add(Uris.toUnix(path.relativize(root)));
+                }
+            }
+        }
+        if (classpath.isEmpty()) {
+            classpath.addAll(classpathJars(path));

Review Comment:
   These emptiness checks use the lists being accumulated across folders, so 
only the first workspace folder contributes inferred roots and jars. In a 
multi-root workspace, a second folder's `lib/*.jar` is always omitted, and its 
conventional source tree is missed when its layout differs from the first 
folder. Decide inference from the original settings, then aggregate candidates 
from every folder.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/feature/FoldingService.java:
##########
@@ -0,0 +1,140 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you 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.apache.groovy.lsp.internal.feature;
+
+import org.antlr.v4.runtime.Token;
+import org.apache.groovy.lsp.internal.compile.AstQuery;
+import org.apache.groovy.lsp.internal.compile.CompiledDocument;
+import org.apache.groovy.lsp.internal.compile.ImportSupport;
+import org.apache.groovy.lsp.internal.util.GroovySourceTokens;
+import org.apache.groovy.parser.antlr4.GroovyLexer;
+import org.codehaus.groovy.ast.ASTNode;
+import org.codehaus.groovy.ast.ImportNode;
+import org.codehaus.groovy.ast.ModuleNode;
+import org.eclipse.lsp4j.FoldingRange;
+import org.eclipse.lsp4j.FoldingRangeKind;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Folding ranges for types, members, imports, comments and regions.
+ */
+public final class FoldingService {
+
+    private static final String REGION = "region";
+    private static final String ENDREGION = "endregion";
+
+    public List<FoldingRange> folding(final CompiledDocument document) {
+        List<FoldingRange> ranges = new ArrayList<>();
+        if (document == null || document.getModule() == null) {
+            return ranges;
+        }
+        for (ASTNode node : AstQuery.declarations(document.getModule())) {
+            if (node.getLineNumber() > 0 && node.getLastLineNumber() > 
node.getLineNumber()) {
+                FoldingRange range = new FoldingRange(node.getLineNumber() - 
1, node.getLastLineNumber() - 1);
+                ranges.add(range);
+            }
+        }
+        addImportFolds(document.getModule(), ranges);
+        addCommentFolds(document.getText(), ranges);
+        addRegionFolds(document.getText(), ranges);
+        return ranges;
+    }
+
+    private static void addCommentFolds(final String text, final 
List<FoldingRange> ranges) {
+        int blockStart = -1;
+        String[] lines = text.split("\n", -1);
+        for (int i = 0; i < lines.length; i++) {
+            String stripped = lines[i].strip();
+            if (stripped.startsWith("/*") && blockStart < 0) {
+                blockStart = i;
+            }
+            if (stripped.contains("*/") && blockStart >= 0) {

Review Comment:
   Scanning stripped source lines treats `/*` and `*/` inside multiline string 
literals as comment delimiters, producing bogus comment folds. The module 
already tokenizes live Groovy source via `GroovySourceTokens`; derive 
block-comment ranges from lexer comment tokens so strings and slashy literals 
cannot create folds.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/workspace/DocumentStore.java:
##########
@@ -0,0 +1,134 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you 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.apache.groovy.lsp.internal.workspace;
+
+import org.apache.groovy.lsp.internal.position.PositionEncoding;
+import org.apache.groovy.lsp.internal.util.Uris;
+import org.eclipse.lsp4j.TextDocumentContentChangeEvent;
+import org.eclipse.lsp4j.TextDocumentItem;
+
+import java.net.URI;
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+/**
+ * Thread-safe store of open text documents. The client's buffer is the
+ * source of truth for open files.
+ */
+public final class DocumentStore {
+
+    private final ConcurrentMap<URI, TextDocument> documents = new 
ConcurrentHashMap<>();
+
+    /**
+     * Opens or replaces a document from {@code textDocument/didOpen}.
+     *
+     * @param item the opened document
+     * @return the stored snapshot
+     */
+    public TextDocument open(final TextDocumentItem item) {
+        URI uri = Uris.parse(item.getUri());
+        TextDocument document = new TextDocument(uri, item.getLanguageId(), 
item.getVersion(), item.getText());
+        documents.put(uri, document);
+        return document;
+    }
+
+    /**
+     * Applies {@code textDocument/didChange} events.
+     *
+     * @param uriString document URI
+     * @param version new version
+     * @param changes change events
+     * @param encoding negotiated encoding
+     * @return the updated snapshot, or {@code null} when the document is 
unknown
+     */
+    public TextDocument change(final String uriString, final int version,
+                               final List<TextDocumentContentChangeEvent> 
changes,
+                               final PositionEncoding encoding) {
+        URI uri = Uris.parse(uriString);
+        return documents.compute(uri, (key, current) -> {
+            if (current == null) {
+                return new TextDocument(uri, "groovy", version, 
applyUnknown(changes));

Review Comment:
   This contradicts the method contract and turns an out-of-order `didChange` 
into a permanently open synthetic document. Incremental changes are especially 
corrupt because `applyUnknown` treats only the last replacement text as the 
entire file; unknown documents should be ignored (return `null`) until 
`didOpen` establishes their current contents.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/feature/InlayHintService.java:
##########
@@ -0,0 +1,185 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you 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.apache.groovy.lsp.internal.feature;
+
+import org.apache.groovy.lsp.internal.compile.AstQuery;
+import org.apache.groovy.lsp.internal.compile.CompilationSnapshot;
+import org.apache.groovy.lsp.internal.compile.CompiledDocument;
+import org.apache.groovy.lsp.internal.position.PositionEncoding;
+import org.apache.groovy.lsp.internal.position.Positions;
+import org.codehaus.groovy.ast.AnnotatedNode;
+import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.MethodNode;
+import org.codehaus.groovy.ast.Parameter;
+import org.codehaus.groovy.ast.expr.DeclarationExpression;
+import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.MethodCallExpression;
+import org.codehaus.groovy.ast.expr.VariableExpression;
+import org.eclipse.lsp4j.InlayHint;
+import org.eclipse.lsp4j.InlayHintKind;
+import org.eclipse.lsp4j.Range;
+import org.eclipse.lsp4j.jsonrpc.messages.Either;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Parameter-name and inferred-type inlay hints.
+ */
+public final class InlayHintService {
+
+    public List<InlayHint> inlayHints(final CompiledDocument document, final 
Range range,
+                                      final PositionEncoding encoding) {
+        return inlayHints(document, range, encoding, null);
+    }
+
+    public List<InlayHint> inlayHints(final CompiledDocument document, final 
Range range,
+                                      final PositionEncoding encoding, final 
CompilationSnapshot snapshot) {
+        List<InlayHint> hints = new ArrayList<>();
+        if (document == null || document.getModule() == null) {
+            return hints;
+        }
+        for (ClassNode classNode : document.getModule().getClasses()) {
+            addClassHints(hints, document, range, encoding, classNode);
+        }
+        addCallSiteHints(hints, document, range, encoding, snapshot);
+        return hints;
+    }
+
+    private static void addClassHints(final List<InlayHint> hints, final 
CompiledDocument document, final Range range,
+                                      final PositionEncoding encoding, final 
ClassNode classNode) {
+        for (MethodNode method : classNode.getMethods()) {
+            addMethodHints(hints, document, range, encoding, method);
+        }
+        for (FieldNode field : classNode.getFields()) {
+            if (!field.isSynthetic() && field.isDynamicTyped()) {
+                addTypeHint(hints, field, TypeInference.of(field), 
document.getText(), encoding, range,
+                        TypeInference.usedInitializer(field));
+            }
+        }
+    }
+
+    private static void addMethodHints(final List<InlayHint> hints, final 
CompiledDocument document, final Range range,
+                                       final PositionEncoding encoding, final 
MethodNode method) {
+        if (method.isSynthetic() || method.getCode() == null) {
+            return;
+        }
+        for (Parameter parameter : method.getParameters()) {
+            if (parameter.isDynamicTyped()) {
+                addTypeHint(hints, parameter, parameter.getType(), 
document.getText(), encoding, range, false);
+            }
+        }
+        AstQuery.walk(method.getCode(), (node, ctx) -> {
+            if (node instanceof DeclarationExpression decl && 
!decl.isMultipleAssignmentDeclaration()) {
+                VariableExpression variable = decl.getVariableExpression();
+                if (variable != null && variable.isDynamicTyped()) {
+                    addTypeHint(hints, variable, TypeInference.of(variable, 
document.getModule()),
+                            document.getText(), encoding, range,
+                            TypeInference.usedInitializer(variable, 
document.getModule()));
+                }
+            }
+        });
+    }
+
+    private static void addTypeHint(final List<InlayHint> hints, final 
AnnotatedNode node, final ClassNode type,
+                                    final String text, final PositionEncoding 
encoding, final Range range,
+                                    final boolean inferred) {
+        if (!SymbolIdentity.isResolvedType(type)) {
+            return;
+        }
+        Range nameRange = Positions.toNameRange(node, text, encoding);
+        if (nameRange == null || (range != null && 
nameRange.getStart().getLine() < range.getStart().getLine())) {

Review Comment:
   This only filters hints before the requested range, so a one-line 
`textDocument/inlayHint` request returns every type hint on all later lines. 
Apply the same overlap check used for parameter hints so type hints after the 
range end are excluded.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/util/GroovydocMarkdown.java:
##########
@@ -0,0 +1,218 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you 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.apache.groovy.lsp.internal.util;
+
+import groovy.lang.groovydoc.Groovydoc;
+import org.codehaus.groovy.ast.AnnotatedNode;
+
+/**
+ * Turns a Groovydoc comment into compact Markdown for hover.
+ */
+public final class GroovydocMarkdown {
+
+    private GroovydocMarkdown() {
+    }
+
+    /**
+     * Markdown for {@code node}'s Groovydoc, or empty.
+     *
+     * @param node annotated AST node
+     * @return markdown, never {@code null}
+     */
+    public static String of(final AnnotatedNode node) {
+        if (node == null) {
+            return "";
+        }
+        Groovydoc groovydoc = node.getGroovydoc();
+        if (groovydoc == null || !groovydoc.isPresent()) {
+            return "";
+        }
+        return format(groovydoc.getContent());
+    }
+
+    /**
+     * Formats raw Groovydoc text (including {@code /**} delimiters).
+     *
+     * @param content comment text
+     * @return markdown, never {@code null}
+     */
+    public static String format(final String content) {
+        if (content == null || content.isBlank()) {
+            return "";
+        }
+        String body = stripComment(content);
+        body = inlineTags(body);
+        body = htmlToMarkdown(body);
+        body = blockTags(body);
+        return body.strip();
+    }
+
+    private static String stripComment(final String content) {
+        String text = content.strip();
+        if (text.startsWith("/**")) {
+            text = text.substring(3);
+        }
+        if (text.endsWith("*/")) {
+            text = text.substring(0, text.length() - 2);
+        }
+        StringBuilder builder = new StringBuilder();
+        for (String raw : text.split("\n", -1)) {
+            String line = raw.strip();
+            if (line.startsWith("*")) {
+                line = line.substring(1);
+                if (line.startsWith(" ")) {
+                    line = line.substring(1);
+                }
+            }
+            if (!builder.isEmpty()) {
+                builder.append('\n');
+            }
+            builder.append(line);
+        }
+        return builder.toString();
+    }
+
+    private static String inlineTags(final String text) {
+        String result = replaceTag(text, "{@code ", "`", "`");
+        result = replaceTag(result, "{@literal ", "", "");
+        result = replaceLink(result, "{@link ");
+        result = replaceLink(result, "{@linkplain ");
+        return result;
+    }
+
+    private static String replaceTag(final String text, final String open, 
final String before, final String after) {
+        StringBuilder builder = new StringBuilder();
+        int from = 0;
+        while (from < text.length()) {
+            int start = text.indexOf(open, from);
+            int end = start < 0 ? -1 : text.indexOf('}', start + 
open.length());
+            if (start < 0 || end < 0) {
+                builder.append(text, from, text.length());
+                break;
+            }
+            builder.append(text, from, start);
+            builder.append(before).append(text, start + open.length(), 
end).append(after);
+            from = end + 1;
+        }
+        return builder.toString();
+    }
+
+    private static String replaceLink(final String text, final String open) {
+        StringBuilder builder = new StringBuilder();
+        int from = 0;
+        while (from < text.length()) {
+            int start = text.indexOf(open, from);
+            int end = start < 0 ? -1 : text.indexOf('}', start + 
open.length());
+            if (start < 0 || end < 0) {
+                builder.append(text, from, text.length());
+                break;
+            }
+            builder.append(text, from, start);
+            builder.append('`').append(linkLabel(text.substring(start + 
open.length(), end))).append('`');
+            from = end + 1;
+        }
+        return builder.toString();
+    }
+
+    private static String linkLabel(final String body) {
+        String trimmed = body.strip();
+        int hash = trimmed.indexOf('#');
+        if (hash >= 0) {
+            String type = trimmed.substring(0, hash).strip();
+            String member = trimmed.substring(hash + 1).strip();
+            int space = member.indexOf(' ');
+            if (space > 0) {
+                member = member.substring(0, space);
+            }
+            int paren = member.indexOf('(');
+            if (paren > 0) {
+                member = member.substring(0, paren);
+            }
+            return type.isEmpty() ? member : type + "." + member;
+        }
+        int space = trimmed.indexOf(' ');
+        String target = space < 0 ? trimmed : trimmed.substring(0, space);
+        int lastDot = target.lastIndexOf('.');
+        return lastDot < 0 ? target : target.substring(lastDot + 1);
+    }
+
+    private static String htmlToMarkdown(final String text) {
+        String result = text.replace("<br>", "\n").replace("<br/>", 
"\n").replace("<br />", "\n");
+        result = result.replace("<p>", "\n\n").replace("</p>", "");
+        result = result.replace("<b>", "**").replace("</b>", "**");
+        result = result.replace("<strong>", "**").replace("</strong>", "**");
+        result = result.replace("<i>", "*").replace("</i>", "*");
+        result = result.replace("<em>", "*").replace("</em>", "*");
+        result = result.replace("<code>", "`").replace("</code>", "`");
+        StringBuilder builder = new StringBuilder();
+        int i = 0;
+        while (i < result.length()) {
+            char c = result.charAt(i);
+            if (c == '<') {
+                int close = result.indexOf('>', i + 1);
+                if (close < 0) {
+                    builder.append(result, i, result.length());
+                    break;
+                }
+                i = close + 1;

Review Comment:
   Every angle-bracket pair is discarded as if it were an HTML tag. Plain 
Groovydoc such as `List<String>` or `x < y && y > z` therefore loses 
user-visible text in hover output. Restrict stripping to 
recognized/syntactically valid HTML tags and preserve or escape other 
angle-bracket content.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to