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


##########
subprojects/groovy-console/src/main/groovy/groovy/console/ui/text/MatchingHighlighter.java:
##########
@@ -239,7 +239,7 @@ private void clearHighlighted() {
                 doc.setCharacterAttributes(
                         highlightedTokenInfo.getV2().getOffset(),
                         highlightedTokenInfo.getV3(),
-                        findStyleByTokenType(highlightedTokenInfo.getV1()),
+                        defaultStyle,

Review Comment:
   Clearing a temporary match highlight by applying `defaultStyle` will 
overwrite whatever syntax style was previously applied at that span (e.g., from 
`SmartDocumentFilter`). This can leave braces/regions de-highlighted until the 
next full reparse. A safer approach is to restore the prior attributes (store 
them when applying the highlight), or to re-run the syntax styling for just 
that span when clearing.



##########
subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/language/GroovyshLanguageSession.groovy:
##########
@@ -0,0 +1,344 @@
+/*
+ *  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.groovysh.language
+
+import groovy.transform.CompileStatic
+import groovy.transform.Internal
+import groovy.transform.PackageScope
+import org.apache.groovy.groovysh.jline.GroovyEngine
+import org.apache.groovy.lsp.internal.engine.GroovyLanguageEngine
+import org.codehaus.groovy.control.CompilerConfiguration
+import org.jline.console.CmdDesc
+import org.jline.console.CmdLine
+import org.jline.reader.Candidate
+import org.jline.reader.Completer
+import org.jline.reader.LineReader
+import org.jline.reader.ParsedLine
+import org.jline.utils.AttributedString
+
+import java.lang.reflect.Modifier
+import java.util.regex.Pattern
+
+/**
+ * Compiler-backed session for groovysh. Does not import LSP4J. The adapter
+ * composes the REPL buffer, binding type declarations, and the current line;
+ * the engine compiles that synthetic script through semantic analysis.
+ * Implementation detail of groovysh, not an embedding API.
+ */
+@CompileStatic
+@Internal
+final class GroovyshLanguageSession implements AutoCloseable, Completer {
+
+    /** This session's unsaved buffer. The engine does not assign the name. */
+    @PackageScope
+    static final URI BUFFER_URI = URI.create('groovy-buffer:repl')
+
+    private static final Pattern SYSTEM_VAR = Pattern.compile('[A-Z]+[A-Z_]*')
+    private static final Pattern IDENTIFIER = 
Pattern.compile('[\\p{L}_$][\\p{L}\\p{N}_$]*')
+    private static final Set<String> SKIP = ['GROOVYSH_OPTIONS', 
'GROOVY_OPTIONS', 'CONSOLE_OPTIONS',
+            'ROOT', 'PWD', 'NANORC', '_args'] as Set
+
+    private final GroovyEngine groovy
+    private final GroovyLanguageEngine engine
+    private String namesPreamble
+    private Set<String> namesCache
+    private CompilerConfiguration boundConfiguration
+
+    GroovyshLanguageSession(GroovyEngine groovy) {
+        this.groovy = groovy
+        this.engine = new GroovyLanguageEngine()
+        bindEngine()
+        engine.openBuffer(BUFFER_URI, 'groovy', '')
+    }
+
+    /**
+     * Compiles the session buffer plus {@code line} and returns diagnostics.
+     *
+     * @param line current input
+     * @return hits
+     */
+    @PackageScope
+    List<GroovyLanguageEngine.Hit> diagnosticsFor(String line) {
+        bindEngine()
+        engine.diagnostics(BUFFER_URI, compose(line ?: ''))
+    }
+
+    /**
+     * Completions for the current line at end-of-buffer.
+     *
+     * @param line current input
+     * @return candidates
+     */
+    @PackageScope
+    List<GroovyLanguageEngine.Candidate> completeLine(String line) {
+        completeLine(line, -1)
+    }
+
+    /**
+     * Completions for {@code line} at {@code cursor} (0-based index into
+     * {@code line}). A negative cursor means end of the composed script.
+     *
+     * @param line current input
+     * @param cursor offset in {@code line}
+     * @return candidates
+     */
+    @PackageScope
+    List<GroovyLanguageEngine.Candidate> completeLine(String line, int cursor) 
{
+        bindEngine()
+        String current = line ?: ''
+        String script = compose(current)
+        int[] caret = caretInComposed(script, current, cursor)
+        engine.complete(BUFFER_URI, script, caret[0], caret[1])
+    }
+
+    /**
+     * Tail-tip description. {@code SYNTAX} reports compiler diagnostics and
+     * does not evaluate. {@code METHOD} asks for signatures on the full line
+     * (so an unclosed {@code (} is visible) and falls back to Inspector only
+     * when the compiler has none.
+     *
+     * @param line JLine command line
+     * @return description, or {@code null}
+     */
+    CmdDesc scriptDescription(CmdLine line) {
+        if (line != null && line.descriptionType == 
CmdLine.DescriptionType.SYNTAX) {
+            try {
+                def hits = diagnosticsFor(line.line ?: '')
+                if (hits) {
+                    def desc = new CmdDesc()
+                    desc.setMainDesc(hits.take(5).collect { new 
AttributedString(it.message() ?: '') })
+                    return desc
+                }
+            } catch (Exception ignored) {
+            }
+            return null
+        }
+        try {
+            if (line != null && line.descriptionType == 
CmdLine.DescriptionType.METHOD) {
+                String raw = line.line ?: ''
+                String head = line.head ?: ''
+                int cursor = Math.min(head.length() + 1, raw.length())
+                def desc = describeSignatures(signaturesFor(raw, cursor))
+                if (desc != null) {
+                    return desc
+                }
+            }
+        } catch (Exception ignored) {
+        }
+        try {
+            return groovy.scriptDescription(line)
+        } catch (Exception ignored) {
+            return null
+        }
+    }
+
+    @PackageScope
+    GroovyLanguageEngine.SignatureSet signaturesFor(String line) {
+        signaturesFor(line, -1)
+    }
+
+    /**
+     * Signatures for {@code line} at {@code cursor} (0-based index into
+     * {@code line}). A negative cursor means end of the composed script.
+     *
+     * @param line current input
+     * @param cursor offset in {@code line}
+     * @return signatures, never {@code null}
+     */
+    @PackageScope
+    GroovyLanguageEngine.SignatureSet signaturesFor(String line, int cursor) {
+        bindEngine()
+        String current = line ?: ''
+        String script = compose(current)
+        int[] caret = caretInComposed(script, current, cursor)
+        engine.signatures(BUFFER_URI, script, caret[0], caret[1])
+    }
+
+    /**
+     * {@code compose} always appends {@code current} last, so a cursor into
+     * {@code current} maps by prefix length of the composed script.
+     */
+    private static int[] caretInComposed(String script, String current, int 
cursor) {
+        if (cursor < 0 || cursor > current.length()) {
+            return GroovyLanguageEngine.positionOf(script, script.length())
+        }
+        int currentStart = script.length() - current.length()
+        return GroovyLanguageEngine.positionOf(script, currentStart + cursor)
+    }
+
+    /**
+     * Builds a METHOD tail-tip from compiler signatures.
+     *
+     * @param set engine signatures
+     * @return description, or {@code null} when there are none
+     */
+    @PackageScope
+    static CmdDesc describeSignatures(GroovyLanguageEngine.SignatureSet set) {
+        if (set == null || !set.signatures()) {
+            return null
+        }
+        def desc = new CmdDesc()
+        desc.setMainDesc(set.signatures().collect { new 
AttributedString(it.label() ?: '') })
+        desc
+    }
+
+    @Override
+    void complete(LineReader reader, ParsedLine line, List<Candidate> 
candidates) {
+        if (candidates == null || line == null) {
+            return
+        }
+        String raw = line.line() ?: ''
+        if (raw.startsWith('/')) {
+            return
+        }
+        try {
+            def items = completeLine(raw, line.cursor())
+            mapEngineCandidates(items, line, candidates)
+        } catch (RuntimeException ignored) {
+        }
+    }
+
+    @Override
+    void close() {
+        engine.close()
+    }
+
+    /**
+     * Synthetic script: remembered snippets, then binding declarations for
+     * names the buffer does not already introduce, then the current line.
+     *
+     * @param current current input
+     * @return script text
+     */
+    @PackageScope
+    String compose(String current) {
+        String preamble = groovy.buffer ?: ''
+        String decls = bindingDeclarations(namesIn(preamble))
+        def parts = []
+        if (preamble) {
+            parts << preamble
+        }
+        if (decls) {
+            parts << decls
+        }
+        parts << (current ?: '')
+        parts.join('\n')
+    }
+
+    private Set<String> namesIn(String preamble) {
+        if (!preamble) {
+            return [] as Set
+        }
+        if (preamble == namesPreamble) {
+            return namesCache
+        }
+        bindEngine()
+        namesCache = engine.declaredNames(BUFFER_URI, preamble)
+        namesPreamble = preamble
+        namesCache
+    }
+
+    private void bindEngine() {
+        engine.setParentLoader(groovy.classLoader)
+        CompilerConfiguration configuration = groovy.compilerConfiguration
+        // setHostConfiguration drops the compile cache. Publish only when
+        // the instance changes so a repeated completion can reuse it.
+        if (configuration != boundConfiguration) {
+            boundConfiguration = configuration
+            engine.setHostConfiguration(configuration)
+        }
+    }

Review Comment:
   This only republishes host `CompilerConfiguration` when the instance 
identity changes. If groovysh or an embedder mutates the same 
`CompilerConfiguration` in place (e.g., editing an `ImportCustomizer`), the 
engine won't be told to drop its compile cache and won't observe the change. If 
in-place mutation is a supported/expected path, consider tracking a lightweight 
fingerprint of relevant config state (script base class + customizer 
identities/count), or providing a hook to force-republish when commands mutate 
customizers.



##########
subprojects/groovy-console/src/main/groovy/groovy/console/ui/text/SmartDocumentFilter.java:
##########
@@ -195,235 +108,169 @@ public void remove(DocumentFilter.FilterBypass fb, int 
offset, int length)
     public void replace(DocumentFilter.FilterBypass fb, int offset,
                         int length, String text, AttributeSet attrs)
             throws BadLocationException {
-
-        // text might be null and indicates no replacement text
-        if (text == null) text = "";
-
-        // remove problem meta characters returns
+        if (text == null) {
+            text = "";
+        }
         text = replaceMetaCharacters(text);
-
         fb.replace(offset, length, text, attrs);
-
         parseDocument();
     }
 
     private String replaceMetaCharacters(String string) {
-        // just in case remove carriage returns
-        string = string.replace("\r\n", "\n");
-        return string;
+        return string.replace("\r\n", "\n");
     }
 
     private void parseDocument() throws BadLocationException {
-        GroovyLangLexer lexer;
-        try {
-            lexer = createLexer(styledDocument.getText(0, 
styledDocument.getLength()));
-        } catch (IOException e) {
-            e.printStackTrace();
-            this.latest = false;
-            return;
-        }
-
-        CommonTokenStream tokenStream = new CommonTokenStream(lexer);
-
-        try {
-            tokenStream.fill();
-        } catch (LexerNoViableAltException | GroovySyntaxError e) {
-            // ignore
-            this.latest = false;
-            return;
-        } catch (Exception e) {
-            e.printStackTrace();
-            this.latest = false;
-            return;
-        }
-
-        List<Token> tokenList = tokenStream.getTokens();
-        List<Token> tokenListToRender;
+        String text = styledDocument.getText(0, styledDocument.getLength());
+        List<Token> tokenList = GroovySourceTokens.tokenizeLenient(text);
+        this.latestTokenList = tokenList;
+        this.latest = true;
+        int[] dirty = dirtyRange(text);
         try {
-            tokenListToRender = findTokensToRender(tokenList);
+            render(text, tokenList, dirty[0], dirty[1]);
         } finally {
             this.setRenderRange(null);
         }
+    }
 
-        boolean inAnnotation = false;
-        for (Token token : tokenListToRender) {
-            int tokenType = token.getType();
-
-            if (EOF == tokenType) {
-                continue;
-            }
-
-            int tokenStartIndex = token.getStartIndex();
-            int tokenStopIndex = token.getStopIndex();
-            int tokenLength = tokenStopIndex - tokenStartIndex + 1;
-
-            // color annotation names (identifier/dot tokens after @) with 
annotation style
-            // handles @ToString, @groovy.transform.CompileStatic, etc.
-            boolean isAnnotationPart = inAnnotation
-                    && (tokenType == Identifier || tokenType == 
CapitalizedIdentifier || tokenType == DOT);
-            if (tokenType == AT) {
-                inAnnotation = true;
-            } else if (isAnnotationPart) {
-                // stay in annotation for dotted names
-                inAnnotation = (tokenType == DOT);
-            } else {
-                inAnnotation = false;
-            }
+    private int[] dirtyRange(String text) {
+        int last = text.length() - 1;
+        if (last < 0) {
+            return new int[] {0, -1};
+        }
+        Tuple2<Integer, Integer> range = getRenderRange();
+        if (range == null) {
+            return new int[] {0, last};
+        }
+        int start = range.getV1();
+        int stop = range.getV2();
+        if (start < 0 || stop < 0) {
+            return new int[] {0, last};
+        }
+        int from = Math.max(0, Math.min(start, stop));
+        int to = Math.min(last, Math.max(start, stop));
+        return new int[] {from, to};
+    }
 
-            styledDocument.setCharacterAttributes(tokenStartIndex,
-                    tokenLength,
-                    isAnnotationPart ? findStyleByTokenType(AT) : 
findStyleByTokenType(tokenType),
-                    true);
+    private void render(String text, List<Token> antlrTokens, int dirtyStart, 
int dirtyStop) {
+        if (text.isEmpty() || dirtyStart > dirtyStop) {
+            return;
+        }
+        styledDocument.setCharacterAttributes(dirtyStart, dirtyStop - 
dirtyStart + 1, defaultStyle, true);
+        paintSemantic(text, antlrTokens, 
GroovyLanguageEngine.lexicalTokens(text, antlrTokens), dirtyStart, dirtyStop);
+    }
 
-            if (GStringBegin == tokenType || GStringPart == tokenType) {
-                styledDocument.setCharacterAttributes(
-                        tokenStartIndex + tokenLength - 1,
-                        1,
-                        defaultStyle,
-                        true);
+    private void paintSemantic(String text, List<Token> antlrTokens, 
List<GroovyLanguageEngine.Token> tokens,
+                               int dirtyStart, int dirtyStop) {
+        for (GroovyLanguageEngine.Token token : tokens) {
+            int[] span = GroovyLanguageEngine.spanOf(text, token);
+            int start = span[0];
+            int end = span[1];
+            if (end > dirtyStart && start <= dirtyStop && end > start) {
+                Style style = styleFor(token, antlrTokens, start);
+                styledDocument.setCharacterAttributes(start, end - start, 
style, true);
+                unstyleGStringDollar(antlrTokens, start, end);
             }
         }
-
-        this.latestTokenList = tokenList;
-        this.latest = true;
     }
 
-    private List<Token> findTokensToRender(List<Token> tokenList) {
-        final Tuple2<Integer, Integer> renderRange = getRenderRange();
-        if (null != renderRange) {
-            long startLine = lineNumber(renderRange.getV1());
-            long stopLine = lineNumber(renderRange.getV2());
-
-            if (startLine < 0 || stopLine < 0) return tokenList; // should 
never happen
-
-            return tokenList.stream()
-                    .filter(e -> e.getLine() >= startLine && 
PositionConfigureUtils.endPosition(e).getV1() <= stopLine)
-                    .collect(Collectors.toList());
+    private Style styleFor(GroovyLanguageEngine.Token token, List<Token> 
antlrTokens, int start) {
+        String type = token.type();
+        if ("keyword".equals(type)) {
+            return styleNamed(STYLE_RESERVED);
         }
-
-        List<Token> tmpLatestTokenList = filterNewlines(this.latestTokenList);
-        int latestTokenListSize = tmpLatestTokenList.size();
-        if (0 == latestTokenListSize) {
-            return tokenList;
+        if ("string".equals(type)) {
+            return styleNamed(stringTheme(antlrTokens, start));
         }
-
-        List<Token> tmpTokenList = filterNewlines(tokenList);
-        int tmpTokenListSize = tmpTokenList.size();
-        if (0 == tmpTokenListSize) {
-            return tokenList;
+        if ("number".equals(type)) {
+            return styleNamed(STYLE_DIGIT);
         }
-
-        int startTokenIndex = 0;
-        int minSize = Math.min(tmpTokenListSize, latestTokenListSize);
-        for (int i = 0; i < minSize; i++) {
-            Token token = tmpTokenList.get(i);
-            Token latestToken = tmpLatestTokenList.get(i);
-
-            if (token.getType() == latestToken.getType()
-                    && token.getStartIndex() == latestToken.getStartIndex()
-                    && token.getStopIndex() == latestToken.getStopIndex()) {
-                continue;
-            }
-
-            startTokenIndex = i;
-            break;
+        if ("comment".equals(type)) {
+            return styleNamed(STYLE_COMMENT);
         }
-
-        List<Token> newTokenList = new ReversedList<>(tmpTokenList);
-        List<Token> newLatestTokenList = new 
ReversedList<>(tmpLatestTokenList);
-
-        int stopTokenIndex = tmpTokenListSize;
-
-        Token lastToken = newTokenList.get(0);
-        Token lastLatestToken = newLatestTokenList.get(0);
-
-        for (int i = 0; i < minSize; i++) {
-            Token token = newTokenList.get(i);
-            Token latestToken = newLatestTokenList.get(i);
-
-            if ((token.getType() == latestToken.getType())
-                    && (token.getStartIndex() - lastToken.getStartIndex()) == 
(latestToken.getStartIndex() - lastLatestToken.getStartIndex())
-                    && ((token.getStopIndex() - lastToken.getStopIndex()) == 
(latestToken.getStopIndex() - lastLatestToken.getStopIndex()))) {
-                continue;
-            }
-
-            stopTokenIndex = tmpTokenListSize - i;
-            break;
+        if ("decorator".equals(type)) {
+            return styleNamed(STYLE_ANNOTATION);
         }
-
-        if (startTokenIndex == stopTokenIndex) {
-            return tmpTokenListSize != latestTokenListSize ? tokenList : 
Collections.emptyList();
-        } else if (startTokenIndex < stopTokenIndex) {
-            return tmpTokenList.subList(startTokenIndex, stopTokenIndex);
+        if ("operator".equals(type)) {
+            return styleNamed(STYLE_OPERATION);
         }
-
-        return tokenList; // should never reach here. If unexpected error 
occurred, it's better to render all tokens
+        return styleNamed(STYLE_IDENT);
     }
 
     /**
-     * Returns line number starting with 1
+     * Line-split semantic spans do not repeat the opening delimiter.
+     * The theme comes from the lexer token that opened the literal.
      */
-    private long lineNumber(int position) {
-        long result = -1;
-
-        if (position < 0) return result;
-
-        try {
-            String text = this.styledDocument.getText(0, position + 1);
-            result = StringUtils.countChar(text, '\n') + 1;
-        } catch (BadLocationException e) {
-            e.printStackTrace(System.err);
+    private static String stringTheme(List<Token> lexerTokens, int offset) {
+        int index = indexCovering(lexerTokens, offset);
+        if (index < 0) {
+            return STYLE_SINGLE_QUOTES;
         }
-
-        return result;
+        for (int i = index; i >= 0; i--) {
+            Token token = lexerTokens.get(i);
+            int type = token.getType();
+            if (type == GroovyLexer.StringLiteral || type == 
GroovyLexer.GStringBegin) {
+                return themeOfDelimiter(token.getText());
+            }
+            if (type != GroovyLexer.GStringPart && type != 
GroovyLexer.GStringEnd) {
+                break;
+            }
+        }
+        return STYLE_QUOTES;
     }
 
-    private List<Token> filterNewlines(List<Token> tokenList) {
-        return tokenList.stream()
-                .filter(e -> !(NL == e.getType() && 
e.getText().trim().isEmpty()))
-                .collect(Collectors.toList());
+    private static String themeOfDelimiter(String body) {
+        if (body != null && (body.startsWith("$/") || body.startsWith("/"))) {
+            return STYLE_SLASHY_QUOTES;
+        }
+        if (body != null && body.startsWith("\"")) {
+            return STYLE_QUOTES;
+        }
+        return STYLE_SINGLE_QUOTES;
     }
 
-    private Style findStyleByTokenType(int tokenType) {
-        Style style = styleContext.getStyle(String.valueOf(tokenType));
-
-        return null == style ? defaultStyle : style;
+    private void unstyleGStringDollar(List<Token> lexerTokens, int spanStart, 
int spanEnd) {
+        int index = indexCovering(lexerTokens, spanStart);
+        if (index < 0) {
+            return;
+        }
+        Token token = lexerTokens.get(index);
+        int type = token.getType();
+        if (type != GroovyLexer.GStringBegin && type != 
GroovyLexer.GStringPart) {
+            return;
+        }
+        int dollar = token.getStopIndex();
+        if (dollar >= spanStart && dollar < spanEnd) {
+            styledDocument.setCharacterAttributes(dollar, 1, defaultStyle, 
true);
+        }
     }
 
-    private Style createDefaultStyleByTokenType(int tokenType) {
-        return styleContext.addStyle(String.valueOf(tokenType), defaultStyle);
+    private static int indexCovering(List<Token> lexerTokens, int offset) {
+        if (lexerTokens == null) {
+            return -1;
+        }
+        for (int i = 0; i < lexerTokens.size(); i++) {
+            Token token = lexerTokens.get(i);
+            int start = token.getStartIndex();
+            int stop = token.getStopIndex();
+            if (start >= 0 && start <= offset && offset <= stop) {
+                return i;
+            }
+        }
+        return -1;
     }

Review Comment:
   `indexCovering` does a linear scan from the beginning for each lookup, and 
it's called per semantic token during painting (`stringTheme`, 
`unstyleGStringDollar`). For large documents this can become O(n²) and cause UI 
lag on the EDT. Consider using a binary search over token start/stop indices, 
or a single forward cursor/two-pointer approach since token spans are processed 
in increasing order.



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