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


##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/LanguageServerContext.java:
##########
@@ -0,0 +1,513 @@
+/*
+ *  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;
+
+import org.apache.groovy.lsp.internal.compile.CompilationSnapshot;
+import org.apache.groovy.lsp.internal.compile.CompiledDocument;
+import org.apache.groovy.lsp.internal.compile.CompilerSettings;
+import org.apache.groovy.lsp.internal.compile.GroovyCompiler;
+import org.apache.groovy.lsp.internal.compile.WorkspaceScanner;
+import org.apache.groovy.lsp.internal.diagnostic.DiagnosticConverter;
+import org.apache.groovy.lsp.internal.position.PositionEncoding;
+import org.apache.groovy.lsp.internal.compile.ImportSupport;
+import org.apache.groovy.lsp.internal.compile.WorkspaceLayout;
+import org.apache.groovy.lsp.internal.protocol.ClientFeatures;
+import org.apache.groovy.lsp.internal.util.Uris;
+import org.apache.groovy.lsp.internal.workspace.DocumentStore;
+import org.apache.groovy.lsp.internal.workspace.TextDocument;
+
+import org.eclipse.lsp4j.ApplyWorkspaceEditParams;
+import org.eclipse.lsp4j.WorkspaceEdit;
+import org.eclipse.lsp4j.ClientCapabilities;
+import org.eclipse.lsp4j.Diagnostic;
+import org.eclipse.lsp4j.ProgressParams;
+import org.eclipse.lsp4j.PublishDiagnosticsParams;
+import org.eclipse.lsp4j.WorkDoneProgressBegin;
+import org.eclipse.lsp4j.WorkDoneProgressCreateParams;
+import org.eclipse.lsp4j.WorkDoneProgressEnd;
+import org.eclipse.lsp4j.jsonrpc.messages.Either;
+import org.eclipse.lsp4j.services.LanguageClient;
+
+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.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+
+/**
+ * Mutable session state for one language-server process.
+ */
+public final class LanguageServerContext {
+
+    private final DocumentStore documents = new DocumentStore();
+    private final GroovyCompiler compiler = new GroovyCompiler();
+    private final WorkspaceScanner scanner = new WorkspaceScanner();
+    private final DiagnosticConverter diagnostics = new DiagnosticConverter();
+    private final Object compileLock = new Object();
+
+    private final AtomicReference<LanguageClient> client = new 
AtomicReference<>();
+    private final AtomicReference<ClientCapabilities> clientCapabilities = new 
AtomicReference<>();
+    private final AtomicReference<PositionEncoding> positionEncoding = new 
AtomicReference<>(PositionEncoding.UTF16);
+    private final AtomicReference<CompilerSettings> settings = new 
AtomicReference<>(CompilerSettings.defaults());
+    private final AtomicReference<CompilationSnapshot> snapshot = new 
AtomicReference<>(CompilationSnapshot.EMPTY);
+    private final AtomicReference<ClassLoader> parentLoader = new 
AtomicReference<>();
+    private volatile boolean initialized;
+    private volatile boolean shutdown;
+    private volatile Integer exitCode;
+    private final Set<URI> workspaceFolders = Collections.synchronizedSet(new 
LinkedHashSet<>());
+
+    private final ScheduledExecutorService debounce = 
Executors.newSingleThreadScheduledExecutor(r -> {
+        Thread thread = new Thread(r, "groovy-lsp-compile");
+        thread.setDaemon(true);
+        return thread;
+    });
+    private ScheduledFuture<?> pendingCompile;
+    private long compileGeneration;
+    private String compileFingerprint;
+
+    /**
+     * Creates a session.
+     */
+    public LanguageServerContext() {
+        // Fields are initialized at declaration; the constructor exists so 
tests can build a session directly.
+    }
+
+    /**
+     * Compiles open documents plus workspace sources and publishes 
diagnostics.
+     * Unchanged inputs reuse the last snapshot.
+     */
+    public CompilationSnapshot recompile() {
+        synchronized (compileLock) {
+            final long generation;
+            final CompilerSettings effective;
+            final List<Path> extra;
+            final Collection<TextDocument> open;
+            final ClassLoader loader;
+            final String fingerprint;
+            final boolean reuse;
+            synchronized (this) {
+                if (shutdown) {
+                    return snapshot.get();
+                }
+                if (pendingCompile != null) {
+                    pendingCompile.cancel(false);
+                    pendingCompile = null;
+                }
+                List<URI> folders;
+                synchronized (workspaceFolders) {
+                    folders = new ArrayList<>(workspaceFolders);
+                }
+                effective = WorkspaceLayout.withInferred(settings.get(), 
folders);
+                extra = scanner.scan(folders, effective.getSourcePaths());
+                open = documents.snapshot();
+                loader = compileParentLoader();
+                fingerprint = fingerprint(open, extra, effective, loader);
+                if (fingerprint.equals(compileFingerprint) && snapshot.get() 
!= CompilationSnapshot.EMPTY) {
+                    // Another compile already committed this snapshot but may
+                    // not have published yet. Fall through to 
publishDiagnostics.
+                    reuse = true;
+                    generation = compileGeneration;
+                } else {
+                    reuse = false;
+                    generation = ++compileGeneration;
+                }
+            }
+            if (!reuse) {
+                String token = "groovy-compile-" + generation;
+                beginProgress(token, "Compiling Groovy");
+                CompilationSnapshot compiled;
+                try {
+                    compiled = compiler.compile(open, extra, effective, 
loader);
+                } finally {
+                    endProgress(token);
+                }
+                synchronized (this) {
+                    if (generation == compileGeneration) {
+                        compileFingerprint = fingerprint;
+                        snapshot.set(compiled);

Review Comment:
   Replacing the snapshot drops URIs for deleted files or removed workspace 
folders, but `publishDiagnostics()` only publishes documents in the new 
snapshot. Any diagnostics previously sent for a dropped URI remain visible 
indefinitely; compare old/new snapshot URIs and publish an empty diagnostic 
list for removed entries.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/compile/JavaSymbolIndex.java:
##########
@@ -0,0 +1,263 @@
+/*
+ *  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 com.sun.source.tree.ClassTree;
+import com.sun.source.tree.CompilationUnitTree;
+import com.sun.source.tree.LineMap;
+import com.sun.source.tree.MethodTree;
+import com.sun.source.tree.Tree;
+import com.sun.source.tree.VariableTree;
+import com.sun.source.util.SourcePositions;
+import com.sun.source.util.TreePathScanner;
+import com.sun.source.util.Trees;
+import org.apache.groovy.lsp.internal.util.Uris;
+
+import javax.lang.model.element.Name;
+import javax.tools.JavaFileObject;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Types and members compiled from workspace {@code .java} sources.
+ * Coordinates are Groovy 1-based carets.
+ */
+public final class JavaSymbolIndex {
+
+    public static final JavaSymbolIndex EMPTY = new JavaSymbolIndex(Map.of(), 
Map.of(), Map.of());
+
+    private final Map<String, JavaSymbol> byName;
+    private final Map<String, List<JavaSymbol>> bySimpleName;
+    private final Map<String, List<JavaSymbol>> members;
+
+    private JavaSymbolIndex(final Map<String, JavaSymbol> byName, final 
Map<String, List<JavaSymbol>> bySimpleName,
+                            final Map<String, List<JavaSymbol>> members) {
+        this.byName = byName;
+        this.bySimpleName = bySimpleName;
+        this.members = members;
+    }
+
+    /**
+     * Walks {@code units} using {@code trees} captured from the javac task
+     * before {@code generate()}.
+     *
+     * @param trees javac tree API
+     * @param units parsed compilation units
+     * @return an index, never {@code null}
+     */
+    public static JavaSymbolIndex of(final Trees trees, final Iterable<? 
extends CompilationUnitTree> units) {
+        if (trees == null || units == null) {
+            return EMPTY;
+        }
+        Map<String, JavaSymbol> names = new LinkedHashMap<>();
+        Map<String, List<JavaSymbol>> simple = new LinkedHashMap<>();
+        Map<String, List<JavaSymbol>> members = new LinkedHashMap<>();
+        SourcePositions positions = trees.getSourcePositions();
+        for (CompilationUnitTree unit : units) {
+            if (unit == null || isStub(unit.getSourceFile())) {
+                continue;
+            }
+            URI uri = Uris.normalize(unit.getSourceFile().toUri());
+            LineMap lineMap = unit.getLineMap();
+            new IndexingScanner(uri, unit, positions, lineMap, new 
IndexMaps(names, simple, members)).scan(unit, null);
+        }
+        return new JavaSymbolIndex(Collections.unmodifiableMap(new 
LinkedHashMap<>(names)),
+                copyLists(simple), copyLists(members));
+    }
+
+    /**
+     * @param qualifiedName FQCN
+     * @return the type, or {@code null}
+     */
+    public JavaSymbol type(final String qualifiedName) {
+        return qualifiedName == null ? null : byName.get(qualifiedName);
+    }
+
+    /**
+     * @return every indexed type, in compilation-unit order
+     */
+    public List<JavaSymbol> types() {
+        return List.copyOf(byName.values());
+    }
+
+    /**
+     * @param simpleName unqualified name
+     * @return hits, possibly empty
+     */
+    public List<JavaSymbol> typesNamed(final String simpleName) {
+        if (simpleName == null) {
+            return List.of();
+        }
+        List<JavaSymbol> hits = bySimpleName.get(simpleName);
+        return hits == null ? List.of() : hits;
+    }
+
+    /**
+     * Members of {@code owner} with {@code simpleName}.
+     *
+     * @param owner FQCN
+     * @param simpleName member name
+     * @return hits, possibly empty
+     */
+    public List<JavaSymbol> members(final String owner, final String 
simpleName) {
+        if (owner == null || simpleName == null) {
+            return List.of();
+        }
+        List<JavaSymbol> hits = members.get(owner + "#" + simpleName);
+        return hits == null ? List.of() : hits;
+    }
+
+    private static boolean isStub(final JavaFileObject file) {
+        if (file == null || file.toUri() == null) {
+            return true;
+        }
+        String scheme = file.toUri().getScheme();
+        return scheme != null && "string".equalsIgnoreCase(scheme);
+    }
+
+    private static Map<String, List<JavaSymbol>> copyLists(final Map<String, 
List<JavaSymbol>> source) {
+        Map<String, List<JavaSymbol>> copy = new LinkedHashMap<>();
+        for (Map.Entry<String, List<JavaSymbol>> entry : source.entrySet()) {
+            copy.put(entry.getKey(), List.copyOf(entry.getValue()));
+        }
+        return Map.copyOf(copy);
+    }
+
+    /**
+     * One Java type or member.
+     *
+     * @param qualifiedName FQCN for types; {@code owner#name} for members
+     * @param simpleName identifier
+     * @param owner FQCN of the enclosing type, or {@code null} for a 
top-level type
+     * @param uri source file
+     * @param line 1-based start line
+     * @param column 1-based start column
+     * @param endLine 1-based end line
+     * @param endColumn exclusive 1-based end column
+     */
+    public record JavaSymbol(String qualifiedName, String simpleName, String 
owner, URI uri,
+                             int line, int column, int endLine, int endColumn) 
{
+    }
+
+    private record IndexMaps(Map<String, JavaSymbol> names, Map<String, 
List<JavaSymbol>> simple,
+                             Map<String, List<JavaSymbol>> members) {
+    }
+
+    private static final class IndexingScanner extends TreePathScanner<Void, 
Void> {
+        private final URI uri;
+        private final CompilationUnitTree unit;
+        private final SourcePositions positions;
+        private final LineMap lineMap;
+        private final IndexMaps maps;
+        private final List<String> enclosing = new ArrayList<>();
+
+        private IndexingScanner(final URI uri, final CompilationUnitTree unit, 
final SourcePositions positions,
+                                final LineMap lineMap, final IndexMaps maps) {
+            this.uri = uri;
+            this.unit = unit;
+            this.positions = positions;
+            this.lineMap = lineMap;
+            this.maps = maps;
+        }
+
+        @Override
+        public Void visitClass(final ClassTree node, final Void unused) {
+            String simpleName = text(node.getSimpleName());
+            if (simpleName.isEmpty()) {
+                return super.visitClass(node, unused);
+            }
+            String qualified = qualified(simpleName);
+            JavaSymbol symbol = symbol(qualified, simpleName, 
enclosing.isEmpty() ? null : enclosing.get(enclosing.size() - 1),
+                    node);
+            maps.names().put(qualified, symbol);
+            maps.simple().computeIfAbsent(simpleName, key -> new 
ArrayList<>()).add(symbol);
+            enclosing.add(qualified);
+            try {
+                return super.visitClass(node, unused);
+            } finally {
+                enclosing.remove(enclosing.size() - 1);
+            }
+        }
+
+        @Override
+        public Void visitMethod(final MethodTree node, final Void unused) {
+            addMember(text(node.getName()), node);
+            return super.visitMethod(node, unused);
+        }
+
+        @Override
+        public Void visitVariable(final VariableTree node, final Void unused) {
+            Tree.Kind parent = getCurrentPath() == null || 
getCurrentPath().getParentPath() == null
+                    ? null : 
getCurrentPath().getParentPath().getLeaf().getKind();
+            if (parent == Tree.Kind.CLASS || parent == Tree.Kind.INTERFACE || 
parent == Tree.Kind.ENUM
+                    || parent == Tree.Kind.RECORD) {
+                addMember(text(node.getName()), node);
+            }
+            return super.visitVariable(node, unused);
+        }
+
+        private void addMember(final String simpleName, final Tree node) {
+            if (simpleName.isEmpty() || "<init>".equals(simpleName) || 
enclosing.isEmpty()) {
+                return;
+            }
+            String owner = enclosing.get(enclosing.size() - 1);
+            JavaSymbol symbol = symbol(owner + "#" + simpleName, simpleName, 
owner, node);
+            maps.members().computeIfAbsent(owner + "#" + simpleName, key -> 
new ArrayList<>()).add(symbol);
+        }
+
+        private String qualified(final String simpleName) {
+            if (enclosing.isEmpty()) {
+                String pkg = packageName();
+                return pkg.isEmpty() ? simpleName : pkg + "." + simpleName;
+            }
+            return enclosing.get(enclosing.size() - 1) + "$" + simpleName;
+        }
+
+        private String packageName() {
+            if (unit.getPackageName() == null) {
+                return "";
+            }
+            return unit.getPackageName().toString();
+        }
+
+        private JavaSymbol symbol(final String qualified, final String 
simpleName, final String owner, final Tree node) {
+            long start = positions.getStartPosition(unit, node);
+            long end = positions.getEndPosition(unit, node);
+            if (start < 0) {
+                start = 0;
+            }
+            if (end < start) {
+                end = start;
+            }
+            int line = (int) lineMap.getLineNumber(start);
+            int column = (int) lineMap.getColumnNumber(start);
+            int endLine = (int) lineMap.getLineNumber(end);
+            int endColumn = (int) lineMap.getColumnNumber(end);
+            return new JavaSymbol(qualified, simpleName, owner, uri, line, 
column, endLine, endColumn);

Review Comment:
   Javac `LineMap` columns are UTF-16 columns, while `JavaSymbol` documents 
them as Groovy code-point columns and navigation later passes them through 
`Positions.toRange` as code points. Definitions after supplementary characters 
are therefore shifted; convert using the Java source line or preserve the 
UTF-16 coordinate semantics through link construction.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/LanguageServerContext.java:
##########
@@ -0,0 +1,513 @@
+/*
+ *  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;
+
+import org.apache.groovy.lsp.internal.compile.CompilationSnapshot;
+import org.apache.groovy.lsp.internal.compile.CompiledDocument;
+import org.apache.groovy.lsp.internal.compile.CompilerSettings;
+import org.apache.groovy.lsp.internal.compile.GroovyCompiler;
+import org.apache.groovy.lsp.internal.compile.WorkspaceScanner;
+import org.apache.groovy.lsp.internal.diagnostic.DiagnosticConverter;
+import org.apache.groovy.lsp.internal.position.PositionEncoding;
+import org.apache.groovy.lsp.internal.compile.ImportSupport;
+import org.apache.groovy.lsp.internal.compile.WorkspaceLayout;
+import org.apache.groovy.lsp.internal.protocol.ClientFeatures;
+import org.apache.groovy.lsp.internal.util.Uris;
+import org.apache.groovy.lsp.internal.workspace.DocumentStore;
+import org.apache.groovy.lsp.internal.workspace.TextDocument;
+
+import org.eclipse.lsp4j.ApplyWorkspaceEditParams;
+import org.eclipse.lsp4j.WorkspaceEdit;
+import org.eclipse.lsp4j.ClientCapabilities;
+import org.eclipse.lsp4j.Diagnostic;
+import org.eclipse.lsp4j.ProgressParams;
+import org.eclipse.lsp4j.PublishDiagnosticsParams;
+import org.eclipse.lsp4j.WorkDoneProgressBegin;
+import org.eclipse.lsp4j.WorkDoneProgressCreateParams;
+import org.eclipse.lsp4j.WorkDoneProgressEnd;
+import org.eclipse.lsp4j.jsonrpc.messages.Either;
+import org.eclipse.lsp4j.services.LanguageClient;
+
+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.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+
+/**
+ * Mutable session state for one language-server process.
+ */
+public final class LanguageServerContext {
+
+    private final DocumentStore documents = new DocumentStore();
+    private final GroovyCompiler compiler = new GroovyCompiler();
+    private final WorkspaceScanner scanner = new WorkspaceScanner();
+    private final DiagnosticConverter diagnostics = new DiagnosticConverter();
+    private final Object compileLock = new Object();
+
+    private final AtomicReference<LanguageClient> client = new 
AtomicReference<>();
+    private final AtomicReference<ClientCapabilities> clientCapabilities = new 
AtomicReference<>();
+    private final AtomicReference<PositionEncoding> positionEncoding = new 
AtomicReference<>(PositionEncoding.UTF16);
+    private final AtomicReference<CompilerSettings> settings = new 
AtomicReference<>(CompilerSettings.defaults());
+    private final AtomicReference<CompilationSnapshot> snapshot = new 
AtomicReference<>(CompilationSnapshot.EMPTY);
+    private final AtomicReference<ClassLoader> parentLoader = new 
AtomicReference<>();
+    private volatile boolean initialized;
+    private volatile boolean shutdown;
+    private volatile Integer exitCode;
+    private final Set<URI> workspaceFolders = Collections.synchronizedSet(new 
LinkedHashSet<>());
+
+    private final ScheduledExecutorService debounce = 
Executors.newSingleThreadScheduledExecutor(r -> {
+        Thread thread = new Thread(r, "groovy-lsp-compile");
+        thread.setDaemon(true);
+        return thread;
+    });
+    private ScheduledFuture<?> pendingCompile;
+    private long compileGeneration;
+    private String compileFingerprint;
+
+    /**
+     * Creates a session.
+     */
+    public LanguageServerContext() {
+        // Fields are initialized at declaration; the constructor exists so 
tests can build a session directly.
+    }
+
+    /**
+     * Compiles open documents plus workspace sources and publishes 
diagnostics.
+     * Unchanged inputs reuse the last snapshot.
+     */
+    public CompilationSnapshot recompile() {
+        synchronized (compileLock) {
+            final long generation;
+            final CompilerSettings effective;
+            final List<Path> extra;
+            final Collection<TextDocument> open;
+            final ClassLoader loader;
+            final String fingerprint;
+            final boolean reuse;
+            synchronized (this) {
+                if (shutdown) {
+                    return snapshot.get();
+                }
+                if (pendingCompile != null) {
+                    pendingCompile.cancel(false);
+                    pendingCompile = null;
+                }
+                List<URI> folders;
+                synchronized (workspaceFolders) {
+                    folders = new ArrayList<>(workspaceFolders);
+                }
+                effective = WorkspaceLayout.withInferred(settings.get(), 
folders);
+                extra = scanner.scan(folders, effective.getSourcePaths());
+                open = documents.snapshot();
+                loader = compileParentLoader();
+                fingerprint = fingerprint(open, extra, effective, loader);
+                if (fingerprint.equals(compileFingerprint) && snapshot.get() 
!= CompilationSnapshot.EMPTY) {
+                    // Another compile already committed this snapshot but may
+                    // not have published yet. Fall through to 
publishDiagnostics.
+                    reuse = true;
+                    generation = compileGeneration;
+                } else {
+                    reuse = false;
+                    generation = ++compileGeneration;
+                }
+            }
+            if (!reuse) {
+                String token = "groovy-compile-" + generation;
+                beginProgress(token, "Compiling Groovy");
+                CompilationSnapshot compiled;
+                try {
+                    compiled = compiler.compile(open, extra, effective, 
loader);
+                } finally {
+                    endProgress(token);
+                }
+                synchronized (this) {
+                    if (generation == compileGeneration) {
+                        compileFingerprint = fingerprint;
+                        snapshot.set(compiled);
+                    }
+                }
+            }
+        }
+        publishDiagnostics();
+        return snapshot.get();
+    }
+
+    /**
+     * Schedules a recompile after {@code delayMs} of quiet.
+     *
+     * @param delayMs debounce delay
+     */
+    public synchronized void scheduleRecompile(final long delayMs) {
+        if (shutdown || debounce.isShutdown()) {
+            return;
+        }
+        if (pendingCompile != null) {
+            pendingCompile.cancel(false);
+        }
+        pendingCompile = debounce.schedule(this::recompile, delayMs, 
TimeUnit.MILLISECONDS);
+    }
+
+    /**
+     * Publishes diagnostics for every compiled document.
+     */
+    public void publishDiagnostics() {
+        LanguageClient languageClient = client.get();
+        if (languageClient == null) {
+            return;
+        }
+        for (var document : snapshot.get().documents()) {
+            languageClient.publishDiagnostics(new PublishDiagnosticsParams(
+                    document.getUri().toString(), diagnosticsFor(document)));
+        }

Review Comment:
   These pushed diagnostics omit the document version. Because compilation runs 
asynchronously, a newer `didChange` can arrive while an older snapshot is 
compiling, and the client cannot identify and discard the stale publication. 
Set `PublishDiagnosticsParams.version` for open-document versions.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/feature/CompletionService.java:
##########
@@ -0,0 +1,519 @@
+/*
+ *  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 groovy.lang.groovydoc.Groovydoc;
+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.compile.ImportSupport;
+import org.apache.groovy.lsp.internal.compile.TypeIndex;
+import org.apache.groovy.lsp.internal.position.PositionEncoding;
+import org.apache.groovy.lsp.internal.position.Positions;
+import org.apache.groovy.lsp.internal.util.GroovyKeywords;
+import org.apache.groovy.lsp.internal.workspace.TextDocument;
+import org.codehaus.groovy.ast.ASTNode;
+import org.codehaus.groovy.ast.AnnotatedNode;
+import org.codehaus.groovy.ast.ClassHelper;
+import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.MethodNode;
+import org.codehaus.groovy.ast.ModuleNode;
+import org.codehaus.groovy.ast.Parameter;
+import org.codehaus.groovy.ast.PropertyNode;
+import org.codehaus.groovy.ast.expr.ConstantExpression;
+import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.MapEntryExpression;
+import org.codehaus.groovy.ast.expr.MapExpression;
+import org.codehaus.groovy.ast.expr.PropertyExpression;
+import org.codehaus.groovy.ast.expr.VariableExpression;
+import org.codehaus.groovy.runtime.DefaultGroovyMethods;
+import org.eclipse.lsp4j.CompletionItem;
+import org.eclipse.lsp4j.CompletionItemKind;
+import org.eclipse.lsp4j.CompletionList;
+import org.eclipse.lsp4j.InsertTextFormat;
+import org.eclipse.lsp4j.MarkupContent;
+import org.eclipse.lsp4j.MarkupKind;
+import org.eclipse.lsp4j.Position;
+import org.eclipse.lsp4j.jsonrpc.messages.Either;
+
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Keyword, snippet, member, workspace-type and GDK completions.
+ * Unimported workspace types carry {@code additionalTextEdits}.
+ */
+public final class CompletionService {
+
+    static final int MAX_ITEMS = 80;
+    private static final List<String> GDK_NAMES = gdkMethodNames();
+
+    /**
+     * Completes at {@code position} in {@code document}.
+     *
+     * @param document open document
+     * @param snapshot latest compile
+     * @param position LSP position
+     * @param encoding negotiated encoding
+     * @return a completion list (incomplete when truncated)
+     */
+    public CompletionList complete(final TextDocument document, final 
CompilationSnapshot snapshot,
+                                   final Position position, final 
PositionEncoding encoding) {
+        List<CompletionItem> items = new ArrayList<>();
+        String prefix = prefixAt(document, position, encoding);
+        boolean memberAccess = isMemberAccess(document, position, encoding);
+
+        CompiledDocument compiled = snapshot == null ? null : 
snapshot.get(document);
+        ModuleNode module = compiled == null ? null : compiled.getModule();
+
+        if (!memberAccess) {
+            addMatching(items, GroovyKeywords.KEYWORDS, prefix, 
CompletionItemKind.Keyword);
+            addSnippets(items, prefix);
+        }
+
+        if (module != null) {
+            addFromAst(items, document, compiled, snapshot, position, 
encoding, module);
+        } else if (memberAccess && prefix.length() >= 2) {
+            addGdk(items, prefix);
+        }
+
+        boolean incomplete = items.size() > MAX_ITEMS;
+        if (incomplete) {
+            items = new ArrayList<>(items.subList(0, MAX_ITEMS));
+        }
+        return new CompletionList(incomplete, items);
+    }
+
+    /**
+     * Fills documentation from the compiled snapshot using {@code item.data}.
+     *
+     * @param item unresolved item
+     * @param snapshot latest compile
+     * @return the same item, possibly with documentation
+     */
+    public CompletionItem resolve(final CompletionItem item, final 
CompilationSnapshot snapshot) {
+        if (item == null || item.getData() == null || snapshot == null) {
+            return item;
+        }
+        Map<String, String> data = stringMap(item.getData());
+        if (data.isEmpty()) {
+            return item;
+        }
+        String kind = data.get("k");
+        String name = data.get("n");
+        if ("t".equals(kind) && name != null) {
+            TypeIndex.TypeHit hit = snapshot.types().byName(name);
+            if (hit != null && !hit.documentation().isEmpty()) {
+                item.setDocumentation(Either.forRight(new 
MarkupContent(MarkupKind.MARKDOWN, hit.documentation())));
+            }
+        }
+        if ("m".equals(kind) && name != null) {
+            String owner = data.get("o");
+            MethodNode method = MethodBinding.findMethod(snapshot, owner, 
name, data.get("d"));
+            if (method != null) {
+                String docs = documentation(method);
+                if (!docs.isEmpty()) {
+                    item.setDocumentation(Either.forRight(new 
MarkupContent(MarkupKind.MARKDOWN, docs)));
+                }
+            }
+        }
+        return item;
+    }
+
+    static String prefixAt(final TextDocument document, final Position 
position,
+                           final PositionEncoding encoding) {
+        if (document == null || position == null) {
+            return "";
+        }
+        int[] groovy = document.toGroovy(position, encoding);
+        String line = Positions.lineText(document.getText(), groovy[0]);
+        int end = caretOffset(line, groovy[1]);
+        int start = skipIdentifierBack(line, end);
+        return line.substring(start, end);
+    }
+
+    static boolean isMemberAccess(final TextDocument document, final Position 
position,
+                                  final PositionEncoding encoding) {
+        if (document == null || position == null) {
+            return false;
+        }
+        int[] groovy = document.toGroovy(position, encoding);
+        String line = Positions.lineText(document.getText(), groovy[0]);
+        int idx = skipWhitespaceBack(line, caretOffset(line, groovy[1]));
+        idx = skipIdentifierBack(line, idx);
+        return idx > 0 && line.codePointBefore(idx) == '.';
+    }
+
+    private static int caretOffset(final String line, final int groovyColumn) {
+        int col = Math.max(groovyColumn - 1, 0);
+        int cpCount = line.codePointCount(0, line.length());
+        if (col > cpCount) {
+            col = cpCount;
+        }
+        return line.offsetByCodePoints(0, col);
+    }
+
+    private static int skipWhitespaceBack(final String line, int idx) {
+        while (idx > 0 && Character.isWhitespace(line.codePointBefore(idx))) {
+            idx -= Character.charCount(line.codePointBefore(idx));
+        }
+        return idx;
+    }
+
+    private static int skipIdentifierBack(final String line, int idx) {
+        while (idx > 0) {
+            int cp = line.codePointBefore(idx);
+            if (!Character.isJavaIdentifierPart(cp) && cp != '$') {
+                break;
+            }
+            idx -= Character.charCount(cp);
+        }
+        return idx;
+    }
+
+    private static void addOverrideCompletions(final List<CompletionItem> 
items, final ModuleNode module,
+                                               final TextDocument document, 
final Position position,
+                                               final PositionEncoding 
encoding, final String prefix) {
+        ClassNode enclosing = enclosingClass(module, document, position, 
encoding);
+        if (enclosing == null || enclosing.isInterface()) {
+            return;
+        }
+        for (MethodNode method : 
CodeActionService.missingAbstracts(enclosing)) {
+            if (!prefixMatches(method.getName(), prefix)) {
+                continue;
+            }
+            CompletionItem item = new CompletionItem(method.getName());
+            item.setKind(CompletionItemKind.Method);
+            item.setDetail("override " + signature(method));
+            item.setSortText("0" + method.getName());
+            item.setInsertText(CodeActionService.overrideSnippet(method));
+            item.setInsertTextFormat(InsertTextFormat.Snippet);
+            items.add(item);
+        }
+    }
+
+    private static ClassNode enclosingClass(final ModuleNode module, final 
TextDocument document,
+                                            final Position position, final 
PositionEncoding encoding) {
+        ClassNode enclosing = null;
+        for (ASTNode node : AstQuery.containing(module, document, position, 
encoding)) {
+            if (node instanceof ClassNode classNode) {
+                enclosing = classNode;
+            }
+        }
+        return enclosing;

Review Comment:
   `AstQuery.containing` returns nodes innermost-first, but this loop keeps 
overwriting the match and returns the outermost class. Override completion 
inside a nested class therefore inspects the outer class and suggests the wrong 
stubs; return the first `ClassNode` instead.



##########
subprojects/groovy-lsp/src/test/groovy/org/apache/groovy/lsp/internal/compile/CompilerSafetyTest.groovy:
##########
@@ -0,0 +1,524 @@
+/*
+ *  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 com.sun.source.tree.CompilationUnitTree
+import com.sun.source.util.JavacTask
+import com.sun.source.util.Trees
+import groovy.lang.GroovyClassLoader
+import java.lang.reflect.InvocationHandler
+import java.lang.reflect.Proxy
+import java.nio.file.FileSystems
+import java.nio.file.Path
+import java.nio.file.attribute.PosixFilePermissions
+import javax.tools.JavaFileObject
+import javax.tools.ToolProvider
+import org.codehaus.groovy.control.CompilationUnit
+import javax.tools.Diagnostic
+import org.junit.jupiter.api.Assumptions
+import org.junit.jupiter.api.io.TempDir
+
+import org.apache.groovy.lsp.internal.diagnostic.DiagnosticConverter
+import org.apache.groovy.lsp.internal.position.PositionEncoding
+import org.apache.groovy.lsp.internal.workspace.TextDocument
+import org.codehaus.groovy.control.CompilerConfiguration
+import org.codehaus.groovy.control.ErrorCollector
+import org.codehaus.groovy.control.SourceUnit
+import org.codehaus.groovy.control.messages.SyntaxErrorMessage
+import org.codehaus.groovy.syntax.SyntaxException
+import org.apache.groovy.lsp.internal.LanguageServerContext
+import org.apache.groovy.lsp.internal.util.Uris
+import org.eclipse.lsp4j.TextDocumentItem
+import org.junit.jupiter.api.Test
+
+import java.nio.file.Files
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+import java.util.concurrent.atomic.AtomicInteger
+import java.util.concurrent.atomic.AtomicReference
+import java.util.concurrent.locks.LockSupport
+
+final class CompilerSafetyTest {
+
+    @TempDir
+    Path folder
+
+
+    @Test
+    void astTestClosureDoesNotRunWhenDisabled() {
+        System.clearProperty('groovy.asttest.enable')
+        def compiler = new GroovyCompiler()
+        def settings = CompilerSettings.defaults()
+        def uri = URI.create('file:///tmp/AstTestSafety.groovy')
+        def src = '''
+            import groovy.transform.ASTTest
+            import org.codehaus.groovy.control.CompilePhase
+            @ASTTest(phase = CompilePhase.SEMANTIC_ANALYSIS, value = { throw 
new RuntimeException('asttest-executed') })
+            class Victim {}
+            '''
+        def snapshot = compiler.compile(
+                [new TextDocument(uri, 'groovy', 1, src)],
+                [],
+                settings,
+                CompilerSafetyTest.classLoader)
+        def compiled = snapshot.get(uri)
+        assert compiled != null
+        assert System.getProperty('groovy.asttest.enable') == null
+        assert compiled.errors.every { 
!it.toString().contains('asttest-executed') }
+    }
+
+    @Test
+    void diagnosticsAreScopedToOwningFile() {
+        def config = new CompilerConfiguration()
+        def a = new SourceUnit('file:///tmp/A.groovy', 'class A {', config, 
null, null)
+        def b = new SourceUnit('file:///tmp/B.groovy', 'class B {}', config, 
null, null)
+        def collector = new ErrorCollector(config)
+        collector.addErrorAndContinue(new SyntaxErrorMessage(new 
SyntaxException('bad A', 1, 1), a))
+        def docA = new CompiledDocument(URI.create('file:///tmp/A.groovy'), 1, 
'class A {', null, a, collector)
+        def docB = new CompiledDocument(URI.create('file:///tmp/B.groovy'), 1, 
'class B {}', null, b, collector)
+        def converter = new DiagnosticConverter()
+        assert converter.convert(docA, PositionEncoding.UTF16).size() == 1
+        assert converter.convert(docB, PositionEncoding.UTF16).isEmpty()
+    }
+
+    @Test
+    void compilerReusesThenClosesClassLoader() {
+        def compiler = new GroovyCompiler()
+        def settings = CompilerSettings.defaults()
+        def uri = URI.create('file:///tmp/Loader.groovy')
+        def src = 'class Loader {}\n'
+        compiler.compile([new TextDocument(uri, 'groovy', 1, src)], [], 
settings, CompilerSafetyTest.classLoader)
+        compiler.compile([new TextDocument(uri, 'groovy', 2, src)], [], 
settings, CompilerSafetyTest.classLoader)
+        compiler.close()
+        def again = compiler.compile([new TextDocument(uri, 'groovy', 3, 
src)], [], settings, CompilerSafetyTest.classLoader)
+        assert again.get(uri).module != null
+        compiler.close()
+    }
+
+    @Test
+    void compilerReadsExtraFilesAndRestoresProcessFlags() {
+        def dir = Files.createTempDirectory('groovy-lsp-extra')
+        def extra = dir.resolve('Extra.groovy')
+        Files.writeString(extra, 'class Extra {}\n')
+        def note = dir.resolve('readme.txt')
+        Files.writeString(note, 'not groovy')
+        def nested = dir.resolve('nested')
+        Files.createDirectory(nested)
+        System.setProperty('groovy.asttest.enable', 'true')
+        System.setProperty('groovy.grape.enable', 'true')

Review Comment:
   These global properties are cleared only after compilation and assertions 
complete. If setup or compilation throws, the test leaves both flags enabled 
for subsequent tests; capture their prior values and restore them in the 
existing `finally` block.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/feature/RenameService.java:
##########
@@ -0,0 +1,297 @@
+/*
+ *  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.Identifiers;
+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.apache.groovy.lsp.internal.util.Uris;
+import org.apache.groovy.lsp.internal.workspace.TextDocument;
+import org.codehaus.groovy.ast.ASTNode;
+import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.InnerClassNode;
+import org.codehaus.groovy.ast.ModuleNode;
+import org.eclipse.lsp4j.FileRename;
+import org.eclipse.lsp4j.Location;
+import org.eclipse.lsp4j.Position;
+import org.eclipse.lsp4j.PrepareRenameResult;
+import org.eclipse.lsp4j.Range;
+import org.eclipse.lsp4j.RenameFile;
+import org.eclipse.lsp4j.ResourceOperation;
+import org.eclipse.lsp4j.SnippetTextEdit;
+import org.eclipse.lsp4j.TextDocumentEdit;
+import org.eclipse.lsp4j.TextEdit;
+import org.eclipse.lsp4j.VersionedTextDocumentIdentifier;
+import org.eclipse.lsp4j.WorkspaceEdit;
+import org.eclipse.lsp4j.jsonrpc.messages.Either;
+
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Prepare-rename and rename using reference locations. Calls whose
+ * {@code methodTarget} is empty are refused: a name-and-arity guess is
+ * good enough for hover, not for rewriting source.
+ */
+public final class RenameService {
+
+    private final NavigationService navigation = new NavigationService();
+
+    /**
+     * @param document open document
+     * @param snapshot latest compile
+     * @param position LSP position
+     * @param encoding negotiated encoding
+     * @return the range of the identifier, or {@code null} when it cannot be 
renamed
+     */
+    public Either<Range, PrepareRenameResult> prepareRename(final TextDocument 
document,
+                                                            final 
CompilationSnapshot snapshot,
+                                                            final Position 
position,
+                                                            final 
PositionEncoding encoding) {
+        CompiledDocument compiled = snapshot == null ? null : 
snapshot.get(document);
+        if (compiled == null || compiled.getModule() == null) {
+            return null;
+        }
+        ASTNode node = AstQuery.nodeAt(compiled.getModule(), document, 
position, encoding);
+        if (!MethodBinding.isRenameSafe(node, snapshot)) {
+            return null;
+        }
+        String name = Identifiers.nameOf(node);
+        if (name == null) {
+            return null;
+        }
+        Range range = Positions.toIdentifierRange(node, document.getText(), 
encoding);
+        if (range == null) {
+            return null;
+        }
+        return Either.forRight(new PrepareRenameResult(range, name));
+    }
+
+    /**
+     * @param document open document
+     * @param snapshot latest compile
+     * @param position LSP position
+     * @param newName replacement identifier
+     * @param encoding negotiated encoding
+     * @return a workspace edit
+     */
+    public WorkspaceEdit rename(final TextDocument document, final 
CompilationSnapshot snapshot,
+                                final Position position, final String newName, 
final PositionEncoding encoding) {
+        ASTNode node = renameTarget(document, snapshot, position, newName, 
encoding);
+        if (node == null) {
+            return emptyEdit();
+        }
+        Map<String, List<TextEdit>> changes = referenceEdits(document, 
snapshot, position, encoding, newName);
+        WorkspaceEdit edit = new WorkspaceEdit(changes);
+        addFileRename(edit, document, node, newName, changes);
+        return edit;
+    }
+
+    /**
+     * Inverse of class→file rename: rewrite the type whose name equals the
+     * old stem. The client already moves the file; this returns text edits
+     * only.
+     *
+     * @param files rename pairs
+     * @param snapshot latest compile
+     * @param encoding negotiated encoding
+     * @return a workspace edit, never {@code null}
+     */
+    public WorkspaceEdit willRenameFiles(final List<FileRename> files, final 
CompilationSnapshot snapshot,
+                                         final PositionEncoding encoding) {
+        WorkspaceEdit combined = new WorkspaceEdit(new HashMap<>());
+        if (files == null || snapshot == null) {
+            return combined;
+        }
+        for (FileRename file : files) {
+            if (file == null) {
+                continue;
+            }
+            WorkspaceEdit one = willRenameFile(file.getOldUri(), 
file.getNewUri(), snapshot, encoding);
+            if (one.getChanges() != null) {
+                one.getChanges().forEach((uri, edits) ->
+                        combined.getChanges().computeIfAbsent(uri, key -> new 
ArrayList<>()).addAll(edits));
+            }
+        }
+        return combined;
+    }
+
+    private WorkspaceEdit willRenameFile(final String oldUriString, final 
String newUriString,
+                                         final CompilationSnapshot snapshot, 
final PositionEncoding encoding) {
+        if (oldUriString == null || newUriString == null) {
+            return emptyEdit();
+        }
+        URI oldUri = Uris.parse(oldUriString);
+        URI newUri = Uris.parse(newUriString);
+        String oldName = Uris.fileName(oldUri);
+        String newName = Uris.fileName(newUri);
+        String oldExt = Uris.groovyExtension(oldName);
+        String newExt = Uris.groovyExtension(newName);
+        if (oldExt == null || newExt == null || !oldExt.equals(newExt)) {
+            return emptyEdit();
+        }
+        String oldStem = oldName.substring(0, oldName.length() - 
oldExt.length());
+        String newStem = newName.substring(0, newName.length() - 
newExt.length());
+        if (!isIdentifier(newStem) || !sameDirectory(oldUri, newUri)) {
+            return emptyEdit();
+        }
+        CompiledDocument compiled = snapshot.get(oldUri);
+        if (compiled == null || compiled.getModule() == null) {
+            return emptyEdit();
+        }
+        ClassNode type = uniqueStemClass(compiled.getModule(), oldStem);
+        if (type == null) {
+            return emptyEdit();
+        }
+        Range nameRange = Positions.toNameRange(type, compiled.getText(), 
encoding);
+        if (nameRange == null) {
+            return emptyEdit();
+        }
+        TextDocument document = compiled.toTextDocument();
+        return renameText(document, snapshot, nameRange.getStart(), newStem, 
encoding);
+    }
+
+    private WorkspaceEdit renameText(final TextDocument document, final 
CompilationSnapshot snapshot,
+                                     final Position position, final String 
newName,
+                                     final PositionEncoding encoding) {
+        if (renameTarget(document, snapshot, position, newName, encoding) == 
null) {
+            return emptyEdit();
+        }
+        return new WorkspaceEdit(referenceEdits(document, snapshot, position, 
encoding, newName));
+    }
+
+    private ASTNode renameTarget(final TextDocument document, final 
CompilationSnapshot snapshot,
+                                 final Position position, final String newName,
+                                 final PositionEncoding encoding) {
+        if (document == null || newName == null || newName.isBlank() || 
!isIdentifier(newName)) {
+            return null;
+        }
+        CompiledDocument compiled = snapshot == null ? null : 
snapshot.get(document);
+        if (compiled == null || compiled.getModule() == null) {
+            return null;
+        }
+        ASTNode node = AstQuery.nodeAt(compiled.getModule(), document, 
position, encoding);
+        return MethodBinding.isRenameSafe(node, snapshot) ? node : null;
+    }
+
+    private Map<String, List<TextEdit>> referenceEdits(final TextDocument 
document,
+                                                       final 
CompilationSnapshot snapshot,
+                                                       final Position position,
+                                                       final PositionEncoding 
encoding,
+                                                       final String newName) {
+        List<Location> locations = navigation.references(document, snapshot, 
position, encoding, true);
+        Map<String, List<TextEdit>> changes = new HashMap<>();
+        for (Location location : locations) {
+            changes.computeIfAbsent(location.getUri(), key -> new 
ArrayList<>())
+                    .add(new TextEdit(location.getRange(), newName));
+        }
+        return changes;
+    }
+
+    private static WorkspaceEdit emptyEdit() {
+        return new WorkspaceEdit(Map.of());
+    }
+
+    private static ClassNode uniqueStemClass(final ModuleNode module, final 
String stem) {
+        ClassNode match = null;
+        for (ClassNode classNode : module.getClasses()) {
+            if (classNode instanceof InnerClassNode || classNode.isScript() || 
classNode.getLineNumber() <= 0) {
+                continue;
+            }
+            if (stem.equals(classNode.getNameWithoutPackage())) {
+                if (match != null) {
+                    return null;
+                }
+                match = classNode;
+            }
+        }
+        return match;
+    }
+
+    private static boolean sameDirectory(final URI oldUri, final URI newUri) {
+        String oldPath = oldUri.getPath();
+        String newPath = newUri.getPath();
+        if (oldPath == null || newPath == null) {
+            return false;
+        }
+        int oldSlash = Uris.lastSeparator(oldPath);
+        int newSlash = Uris.lastSeparator(newPath);
+        if (oldSlash < 0 || newSlash < 0) {
+            return false;
+        }
+        return oldPath.substring(0, oldSlash).equals(newPath.substring(0, 
newSlash));
+    }
+
+    private static void addFileRename(final WorkspaceEdit edit, final 
TextDocument document, final ASTNode node,
+                                      final String newName, final Map<String, 
List<TextEdit>> changes) {
+        if (!(node instanceof ClassNode classNode)) {
+            return;
+        }
+        String fileName = Uris.fileName(document.getUri());
+        String extension = Uris.groovyExtension(fileName);
+        if (extension == null) {
+            return;
+        }
+        String stem = fileName.substring(0, fileName.length() - 
extension.length());
+        if (!stem.equals(classNode.getNameWithoutPackage())) {
+            return;
+        }
+        String oldUri = document.getUri().toString();
+        int slash = Uris.lastSeparator(oldUri);
+        if (slash < 0) {
+            return;
+        }
+        String newUri = oldUri.substring(0, slash + 1) + newName + extension;
+        List<Either<TextDocumentEdit, ResourceOperation>> documentChanges = 
new ArrayList<>();
+        for (Map.Entry<String, List<TextEdit>> entry : changes.entrySet()) {
+            List<Either<TextEdit, SnippetTextEdit>> eitherEdits = 
entry.getValue().stream()
+                    .map(textEdit -> Either.<TextEdit, 
SnippetTextEdit>forLeft(textEdit))
+                    .toList();
+            documentChanges.add(Either.forLeft(new TextDocumentEdit(
+                    new VersionedTextDocumentIdentifier(entry.getKey(), null), 
eitherEdits)));
+        }
+        documentChanges.add(Either.forRight(new RenameFile(oldUri, newUri)));
+        edit.setDocumentChanges(documentChanges);
+    }
+
+    static boolean isIdentifier(final String name) {
+        if (name == null || name.isEmpty()) {
+            return false;
+        }
+        int first = name.codePointAt(0);
+        if (!Character.isJavaIdentifierStart(first)) {
+            return false;
+        }
+        int i = Character.charCount(first);
+        while (i < name.length()) {
+            int cp = name.codePointAt(i);
+            if (!Character.isJavaIdentifierPart(cp)) {
+                return false;
+            }
+            i += Character.charCount(cp);
+        }
+        return true;

Review Comment:
   Reserved words such as `class`, `def`, and `true` satisfy the Java 
identifier checks, so rename currently accepts them and produces invalid Groovy 
source. Reject names in `GroovyKeywords` after validating their code points.



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/compile/WorkspaceScanner.java:
##########
@@ -0,0 +1,107 @@
+/*
+ *  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.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Stream;
+
+/**
+ * Finds Groovy and Java sources under workspace folders and configured source 
paths.
+ */
+public final class WorkspaceScanner {
+
+    private static final int MAX_FILES = 4000;
+
+    /**
+     * Scans {@code roots} plus {@code sourcePaths} (resolved against each 
root)
+     * for Groovy files, skipping {@code build}, {@code .git} and similar.
+     *
+     * @param roots workspace folders
+     * @param sourcePaths extra relative or absolute source roots
+     * @return discovered files, each path at most once
+     */
+    public List<Path> scan(final Collection<URI> roots, final List<String> 
sourcePaths) {
+        List<Path> files = new ArrayList<>();
+        List<Path> searchRoots = new ArrayList<>();
+        if (roots != null) {
+            for (URI root : roots) {
+                if (root == null || 
!"file".equalsIgnoreCase(root.getScheme())) {
+                    continue;
+                }
+                
searchRoots.addAll(WorkspaceLayout.searchRoots(Uris.toPath(root), sourcePaths));
+            }
+        }
+        Set<Path> seen = new HashSet<>();
+        Set<Path> walked = new HashSet<>();
+        for (Path root : searchRoots) {
+            if (files.size() >= MAX_FILES) {
+                break;
+            }
+            if (!walked.add(normalized(root))) {
+                continue;
+            }
+            addGroovyFiles(files, root, seen);
+        }
+        return files;
+    }
+
+    private void addGroovyFiles(final List<Path> files, final Path root, final 
Set<Path> seen) {
+        if (!Files.isDirectory(root)) {
+            return;
+        }
+        try (Stream<Path> stream = Files.walk(root)) {
+            stream.filter(Files::isRegularFile)
+                    .filter(p -> {
+                        String name = p.getFileName().toString();
+                        return Uris.isGroovyFileName(name) || 
Uris.isJavaFileName(name);
+                    })
+                    .filter(p -> !isIgnored(root.relativize(p)))

Review Comment:
   Filtering ignored paths after `Files.walk` does not prune those directory 
subtrees: the walker still descends through all of `.git`, `node_modules`, 
`build`, and similar trees before rejecting their files. On a workspace-root 
fallback this can make scanning very slow or exhaust resources; use 
`walkFileTree` and return `SKIP_SUBTREE` from `preVisitDirectory`.



##########
subprojects/groovy-lsp/src/test/groovy/org/apache/groovy/lsp/internal/compile/CompilerSafetyTest.groovy:
##########
@@ -0,0 +1,524 @@
+/*
+ *  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 com.sun.source.tree.CompilationUnitTree
+import com.sun.source.util.JavacTask
+import com.sun.source.util.Trees
+import groovy.lang.GroovyClassLoader
+import java.lang.reflect.InvocationHandler
+import java.lang.reflect.Proxy
+import java.nio.file.FileSystems
+import java.nio.file.Path
+import java.nio.file.attribute.PosixFilePermissions
+import javax.tools.JavaFileObject
+import javax.tools.ToolProvider
+import org.codehaus.groovy.control.CompilationUnit
+import javax.tools.Diagnostic
+import org.junit.jupiter.api.Assumptions
+import org.junit.jupiter.api.io.TempDir
+
+import org.apache.groovy.lsp.internal.diagnostic.DiagnosticConverter
+import org.apache.groovy.lsp.internal.position.PositionEncoding
+import org.apache.groovy.lsp.internal.workspace.TextDocument
+import org.codehaus.groovy.control.CompilerConfiguration
+import org.codehaus.groovy.control.ErrorCollector
+import org.codehaus.groovy.control.SourceUnit
+import org.codehaus.groovy.control.messages.SyntaxErrorMessage
+import org.codehaus.groovy.syntax.SyntaxException
+import org.apache.groovy.lsp.internal.LanguageServerContext
+import org.apache.groovy.lsp.internal.util.Uris
+import org.eclipse.lsp4j.TextDocumentItem
+import org.junit.jupiter.api.Test
+
+import java.nio.file.Files
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+import java.util.concurrent.atomic.AtomicInteger
+import java.util.concurrent.atomic.AtomicReference
+import java.util.concurrent.locks.LockSupport
+
+final class CompilerSafetyTest {

Review Comment:
   This test class mutates JVM system properties but does not acquire the 
repository's system-property resource lock. Parallel JUnit execution can race 
with other tests; the established convention is 
`@ResourceLock(Resources.SYSTEM_PROPERTIES)` (for example, 
`InvokerFactoryTest.groovy:286-302` and 
`CachedMethodDirectInvokerTest.java:60-72`).



##########
subprojects/groovy-lsp/src/main/java/org/apache/groovy/lsp/internal/compile/LspJavaCompiler.java:
##########
@@ -0,0 +1,245 @@
+/*
+ *  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 com.sun.source.tree.CompilationUnitTree;
+import com.sun.source.util.JavacTask;
+import com.sun.source.util.Trees;
+import org.apache.groovy.lsp.internal.util.Uris;
+import groovy.lang.GroovyObject;
+import org.codehaus.groovy.control.CompilationUnit;
+import org.codehaus.groovy.control.CompilerConfiguration;
+import org.codehaus.groovy.tools.javac.JavaCompiler;
+import org.codehaus.groovy.tools.javac.JavaCompilerFactory;
+
+import javax.tools.Diagnostic;
+import javax.tools.DiagnosticCollector;
+import javax.tools.JavaFileObject;
+import javax.tools.SimpleJavaFileObject;
+import javax.tools.StandardJavaFileManager;
+import javax.tools.ToolProvider;
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.charset.Charset;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Joint-compilation javac backend for groovy-lsp. Uses the public
+ * {@code javax.tools} / {@code com.sun.source} APIs (no {@code 
--add-exports}).
+ * Open buffers overlay the filesystem; stub sources stay in memory
+ * ({@code memStub}).
+ */
+public final class LspJavaCompiler implements JavaCompiler, 
JavaCompilerFactory {
+
+    private final Map<URI, String> overlays;
+    private JavaSymbolIndex index = JavaSymbolIndex.EMPTY;
+    private final Map<URI, List<JavaMessage>> messages = new LinkedHashMap<>();
+
+    /**
+     * @param overlays open Java buffers keyed by normalized URI
+     */
+    public LspJavaCompiler(final Map<URI, String> overlays) {
+        this.overlays = overlays == null ? Map.of() : Map.copyOf(overlays);
+    }
+
+    /**
+     * @return types and members from the last javac run
+     */
+    public JavaSymbolIndex index() {
+        return index;
+    }
+
+    /**
+     * @return javac diagnostics from the last run, keyed by source URI
+     */
+    public Map<URI, List<JavaMessage>> messages() {
+        return Map.copyOf(messages);
+    }
+
+    @Override
+    public JavaCompiler createCompiler(final CompilerConfiguration config) {
+        return this;
+    }
+
+    @Override
+    public void compile(final List<String> files, final CompilationUnit cu) {
+        index = JavaSymbolIndex.EMPTY;
+        messages.clear();
+        var javac = ToolProvider.getSystemJavaCompiler();
+        if (javac == null || cu == null) {
+            return;
+        }
+        CompilerConfiguration config = cu.getConfiguration();
+        DiagnosticCollector<JavaFileObject> diagnostics = new 
DiagnosticCollector<>();
+        Charset charset = Charset.forName(config.getSourceEncoding());
+        try (StandardJavaFileManager fileManager = 
javac.getStandardFileManager(diagnostics, Locale.ENGLISH, charset)) {
+            Set<JavaFileObject> units = new LinkedHashSet<>();
+            if (cu.getJavaCompilationUnitSet() != null) {
+                units.addAll(cu.getJavaCompilationUnitSet());
+            }
+            addSources(units, files, fileManager);
+            JavacTask task = (JavacTask) javac.getTask(null, fileManager, 
diagnostics, parameters(config),
+                    null, units);
+            Trees treeApi = Trees.instance(task);
+            Iterable<? extends CompilationUnitTree> trees = task.parse();
+            task.analyze();
+            task.generate();
+            index = JavaSymbolIndex.of(treeApi, trees);
+            collectDiagnostics(diagnostics);
+        } catch (IOException | RuntimeException ignored) {
+            // keep whatever diagnostics were collected
+        }
+    }
+
+    private void addSources(final Set<JavaFileObject> units, final 
List<String> files,
+                            final StandardJavaFileManager fileManager) {
+        Set<URI> seen = recordedUris(units);
+        addOverlays(units, seen);
+        addRemainingFiles(units, files, fileManager, seen);
+    }
+
+    private static Set<URI> recordedUris(final Set<JavaFileObject> units) {
+        return units.stream()
+                .filter(unit -> unit != null && unit.toUri() != null)
+                .map(unit -> Uris.normalize(unit.toUri()))
+                .collect(Collectors.toCollection(LinkedHashSet::new));
+    }
+
+    private void addOverlays(final Set<JavaFileObject> units, final Set<URI> 
seen) {
+        for (Map.Entry<URI, String> overlay : overlays.entrySet()) {
+            URI uri = Uris.normalize(overlay.getKey());
+            if (seen.add(uri)) {
+                units.add(new BufferJavaFileObject(uri, overlay.getValue()));
+            }
+        }
+    }
+
+    private void addRemainingFiles(final Set<JavaFileObject> units, final 
List<String> files,
+                                   final StandardJavaFileManager fileManager, 
final Set<URI> seen) {
+        if (files == null) {
+            return;
+        }
+        List<File> remaining = new ArrayList<>();
+        for (String file : files) {
+            if (file != null) {
+                addIfNewFile(remaining, file, seen);
+            }
+        }
+        if (!remaining.isEmpty()) {
+            for (JavaFileObject object : 
fileManager.getJavaFileObjectsFromFiles(remaining)) {
+                units.add(object);
+            }
+        }
+    }
+
+    private void addIfNewFile(final List<File> remaining, final String file, 
final Set<URI> seen) {
+        Path path = Path.of(file);
+        URI uri = Uris.normalize(path.toUri());
+        if (!overlays.containsKey(uri) && seen.add(uri) && 
Files.isRegularFile(path)) {
+            remaining.add(path.toFile());
+        }
+    }
+
+    private void collectDiagnostics(final DiagnosticCollector<JavaFileObject> 
diagnostics) {
+        for (Diagnostic<? extends JavaFileObject> diagnostic : 
diagnostics.getDiagnostics()) {
+            URI uri = diagnosticUri(diagnostic);
+            if (uri != null) {
+                messages.computeIfAbsent(uri, key -> new 
ArrayList<>()).add(toMessage(diagnostic));
+            }
+        }
+    }
+
+    private static URI diagnosticUri(final Diagnostic<? extends 
JavaFileObject> diagnostic) {
+        if (diagnostic == null || diagnostic.getKind() == 
Diagnostic.Kind.NOTE) {
+            return null;
+        }
+        JavaFileObject source = diagnostic.getSource();
+        if (source == null || source.toUri() == null) {
+            return null;
+        }
+        URI uri = Uris.normalize(source.toUri());
+        return "string".equalsIgnoreCase(uri.getScheme()) ? null : uri;
+    }
+
+    private static JavaMessage toMessage(final Diagnostic<? extends 
JavaFileObject> diagnostic) {
+        int line = bounded(diagnostic.getLineNumber());
+        int column = bounded(diagnostic.getColumnNumber());
+        String text = diagnostic.getMessage(Locale.ENGLISH);
+        boolean error = diagnostic.getKind() == Diagnostic.Kind.ERROR;
+        return new JavaMessage(line, column, line, column + 1, text == null ? 
"" : text, error);

Review Comment:
   `Diagnostic.getColumnNumber()` is a 1-based UTF-16 column, but `JavaMessage` 
declares these values as Groovy code-point coordinates and 
`DiagnosticConverter` converts them as code points. Any javac diagnostic after 
a supplementary character is shifted; convert from UTF-16 using the source line 
here, or carry the coordinate encoding explicitly.



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