You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2026/07/18 01:59:19 UTC

(groovy) branch GROOVY-12171 created (now 4e6ac8fa3a)

This is an automated email from the ASF dual-hosted git repository.

asf-gitbox-commits pushed a change to branch GROOVY-12171
in repository https://gitbox.apache.org/repos/asf/groovy.git


      at 4e6ac8fa3a GROOVY-12171: Improve GString syntax error when '$' is not followed by a valid interpolation

This branch includes the following new commits:

     new 4e6ac8fa3a GROOVY-12171: Improve GString syntax error when '$' is not followed by a valid interpolation

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(groovy) 01/01: GROOVY-12171: Improve GString syntax error when '$' is not followed by a valid interpolation

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

asf-gitbox-commits pushed a commit to branch GROOVY-12171
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 4e6ac8fa3a5505ca5799e6cfd3747bb6465c9242
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sat Jul 18 10:58:51 2026 +0900

    GROOVY-12171: Improve GString syntax error when '$' is not followed by a valid interpolation
---
 .../groovy/parser/antlr4/GroovyLangLexer.java      |  91 +++++++++++++++++-
 .../groovy/parser/antlr4/SyntaxErrorTest.groovy    |  65 +++++++++++++
 .../codehaus/groovy/antlr/GStringEndTest.groovy    | 106 ++++++++++++++++++---
 3 files changed, 245 insertions(+), 17 deletions(-)

diff --git a/src/main/java/org/apache/groovy/parser/antlr4/GroovyLangLexer.java b/src/main/java/org/apache/groovy/parser/antlr4/GroovyLangLexer.java
index 00af5045ef..888f84507e 100644
--- a/src/main/java/org/apache/groovy/parser/antlr4/GroovyLangLexer.java
+++ b/src/main/java/org/apache/groovy/parser/antlr4/GroovyLangLexer.java
@@ -22,6 +22,7 @@ import org.antlr.v4.runtime.CharStream;
 import org.antlr.v4.runtime.CharStreams;
 import org.antlr.v4.runtime.Lexer;
 import org.antlr.v4.runtime.LexerNoViableAltException;
+import org.antlr.v4.runtime.Token;
 import org.antlr.v4.runtime.atn.ATN;
 import org.antlr.v4.runtime.atn.LexerATNSimulator;
 import org.apache.groovy.parser.antlr4.internal.atnmanager.LexerAtnManager;
@@ -43,9 +44,67 @@ public class GroovyLangLexer extends GroovyLexer {
         this.setInterpreter(new PositionAdjustingLexerATNSimulator(this, LexerAtnManager.INSTANCE.getATN()));
     }
 
+    /**
+     * Returns the next token, with a special case for EOF after a bare {@code $} in a GString.
+     * <p>
+     * After {@code GStringBegin} or {@code GStringPart} matches a trailing {@code $} at the
+     * end of input, Antlr marks {@code _hitEOF} and would skip
+     * {@link #GSTRING_TYPE_SELECTOR_MODE} entirely on the subsequent call.  Detect that
+     * situation and report the dollar-body error instead of a generic unexpected-EOF parse
+     * failure.
+     *
+     * @return the next token from the input
+     */
     @Override
-    public void recover(LexerNoViableAltException e) {
-        throw e; // if some lexical error occurred, stop parsing!
+    public Token nextToken() {
+        if (_hitEOF && isGStringDollarSelectorMode()) {
+            throw new GroovySyntaxError(
+                    illegalGStringDollarMessage(Token.EOF),
+                    GroovySyntaxError.LEXER,
+                    getLine(),
+                    getCharPositionInLine() + 1);
+        }
+        return super.nextToken();
+    }
+
+    /**
+     * Report a lexical error.  When the failure is inside
+     * {@link #GSTRING_TYPE_SELECTOR_MODE} (after a {@code $} in a GString), emit the
+     * dedicated dollar-body diagnostic instead of Antlr's generic token-recognition message.
+     *
+     * @param e the recognition failure raised by the ATN simulator
+     */
+    @Override
+    public void notifyListeners(final LexerNoViableAltException e) {
+        if (isGStringDollarSelectorMode()) {
+            getErrorListenerDispatch().syntaxError(
+                    this, null, _tokenStartLine, _tokenStartCharPositionInLine,
+                    illegalGStringDollarMessage(), e);
+            return;
+        }
+        super.notifyListeners(e);
+    }
+
+    /**
+     * Abort lexing on the first lexical error.  For an invalid character after {@code $}
+     * in a GString, throw {@link GroovySyntaxError} so the parser layer can surface a
+     * source-located syntax error without retrying SLL→LL or falling back to a generic
+     * exception message.
+     *
+     * @param e the recognition failure raised by the ATN simulator
+     */
+    @Override
+    public void recover(final LexerNoViableAltException e) {
+        if (isGStringDollarSelectorMode()) {
+            // Column is 1-based; _tokenStartCharPositionInLine is 0-based (Antlr convention).
+            throw new GroovySyntaxError(
+                    illegalGStringDollarMessage(),
+                    GroovySyntaxError.LEXER,
+                    _tokenStartLine,
+                    _tokenStartCharPositionInLine + 1);
+        }
+        // if some lexical error occurred, stop parsing!
+        throw e;
     }
 
     @Override
@@ -53,6 +112,34 @@ public class GroovyLangLexer extends GroovyLexer {
         ((PositionAdjustingLexerATNSimulator) getInterpreter()).resetAcceptPosition(getInputStream(), _tokenStartCharIndex - 1, _tokenStartLine, _tokenStartCharPositionInLine - 1);
     }
 
+    /**
+     * Whether the lexer is choosing between {@code ${...}} and {@code $identifier} after a
+     * dollar in a GString (double-quoted, triple-double-quoted, slashy or dollar-slashy).
+     */
+    private boolean isGStringDollarSelectorMode() {
+        return GSTRING_TYPE_SELECTOR_MODE == _mode;
+    }
+
+    /**
+     * Build the user-facing message for an illegal character (or EOF) immediately after
+     * {@code $} in a GString.  When a concrete character is available it is appended so the
+     * diagnostic names both the rule and the offending input.
+     */
+    private String illegalGStringDollarMessage() {
+        return illegalGStringDollarMessage(_input.LA(1));
+    }
+
+    /**
+     * @param c the code point at the failure site, or {@link Token#EOF}
+     */
+    private String illegalGStringDollarMessage(final int c) {
+        // Antlr leaves the input index at the unrecognised character for LexerNoViableAltException.
+        if (Token.EOF == c) {
+            return "Illegal string body character after dollar sign";
+        }
+        return "Illegal string body character after dollar sign: " + getCharErrorDisplay(c);
+    }
+
     private static class PositionAdjustingLexerATNSimulator extends LexerATNSimulator {
         public PositionAdjustingLexerATNSimulator(Lexer recog, ATN atn) {
             super(recog, atn);
diff --git a/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy b/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
index bbe0175f72..9593d78133 100644
--- a/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
+++ b/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
@@ -131,6 +131,71 @@ final class SyntaxErrorTest {
             |'''.stripMargin()
     }
 
+    @Test
+    void 'groovy core - GString illegal character after dollar'() {
+        // Trailing bare `$` in double-quoted and triple-double-quoted strings.
+        // Must name the dollar (Groovy 2 message) rather than only the closing quote.
+        expectParseError '''\
+            |def Target = "releases$"
+            |'''.stripMargin(), '''\
+            |Illegal string body character after dollar sign: '"' @ line 1, column 24.
+            |   def Target = "releases$"
+            |                          ^
+            |
+            |1 error
+            |'''.stripMargin()
+
+        expectParseError '''\
+            |def Target = """releases$"""
+            |'''.stripMargin(), '''\
+            |Illegal string body character after dollar sign: '"' @ line 1, column 26.
+            |   def Target = """releases$"""
+            |                            ^
+            |
+            |1 error
+            |'''.stripMargin()
+
+        expectParseError '''\
+            |def x = "$"
+            |'''.stripMargin(), '''\
+            |Illegal string body character after dollar sign: '"' @ line 1, column 11.
+            |   def x = "$"
+            |             ^
+            |
+            |1 error
+            |'''.stripMargin()
+
+        expectParseError '''\
+            |def x = "a$ b"
+            |'''.stripMargin(), '''\
+            |Illegal string body character after dollar sign: ' ' @ line 1, column 12.
+            |   def x = "a$ b"
+            |              ^
+            |
+            |1 error
+            |'''.stripMargin()
+
+        // newline immediately after `$` (unclosed GString line)
+        expectParseError '''\
+            |def x = "hello$
+            |'''.stripMargin(), '''\
+            |Illegal string body character after dollar sign: '\\n' @ line 1, column 16.
+            |   def x = "hello$
+            |                  ^
+            |
+            |1 error
+            |'''.stripMargin()
+
+        // true EOF immediately after `$` — no character to display in the message
+        expectParseError 'def x = "hello$', '''\
+            |Illegal string body character after dollar sign @ line 1, column 16.
+            |   def x = "hello$
+            |                  ^
+            |
+            |1 error
+            |'''.stripMargin()
+    }
+
     @Test
     void 'groovy core - ParExpression'() {
         TestUtils.doRunAndShouldFail('fail/ParExpression_01x.groovy')
diff --git a/src/test/groovy/org/codehaus/groovy/antlr/GStringEndTest.groovy b/src/test/groovy/org/codehaus/groovy/antlr/GStringEndTest.groovy
index d9062f5293..70833a4b94 100644
--- a/src/test/groovy/org/codehaus/groovy/antlr/GStringEndTest.groovy
+++ b/src/test/groovy/org/codehaus/groovy/antlr/GStringEndTest.groovy
@@ -22,31 +22,107 @@ import org.codehaus.groovy.control.MultipleCompilationErrorsException
 import org.junit.jupiter.api.Test
 
 import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
 
+/**
+ * Error diagnostics for GStrings that end with a bare {@code $}
+ * (or otherwise place an illegal character immediately after {@code $}).
+ * <p>
+ * Historical Groovy 2 reported
+ * {@code illegal string body character after dollar sign} with a precise
+ * caret.  The Antlr4 lexer must preserve that clarity rather than emitting
+ * the generic {@code token recognition error at: '"'} that points at the
+ * closing quote without mentioning the dollar.
+ */
 class GStringEndTest {
+
+    private static final String ILLEGAL_AFTER_DOLLAR = 'Illegal string body character after dollar sign'
+
     @Test
-    void testInvalidEndContainsLineNumber(){
-        try {
-            assertScript '''
+    void testInvalidEndContainsLineNumber() {
+        def err = shouldFail(MultipleCompilationErrorsException,
+             '''
                 def Target = "releases$"
-            '''
-        } catch (MultipleCompilationErrorsException mcee) {
-            def text = mcee.toString();
-            assert text.contains("line 2, column 40")
-        }
+            ''')
+        def text = err.toString()
+        assert text.contains('line 2, column 40')
+        assert text.contains(ILLEGAL_AFTER_DOLLAR)
+        // caret sample still points at the illegal character (the closing quote after $)
+        assert text.contains('def Target = "releases$"')
+        assert !text.contains('token recognition error')
+    }
+
+    @Test
+    void testInvalidEndInTripleDoubleQuotedString() {
+        def err = shouldFail(MultipleCompilationErrorsException,
+             '''
+                def Target = """releases$"""
+            ''')
+        def text = err.toString()
+        assert text.contains(ILLEGAL_AFTER_DOLLAR)
+        assert text.contains('line 2, column 42')
+        assert !text.contains('token recognition error')
+    }
+
+    @Test
+    void testDollarOnlyGString() {
+        def err = shouldFail(MultipleCompilationErrorsException,
+             'def x = "$"')
+        def text = err.toString()
+        assert text.contains(ILLEGAL_AFTER_DOLLAR)
+        assert text.contains('line 1, column 11')
+        assert !text.contains('token recognition error')
+    }
+
+    @Test
+    void testIllegalCharacterAfterDollarNotClosingQuote() {
+        // space after $ is also illegal (not an identifier / not ${)
+        def err = shouldFail(MultipleCompilationErrorsException,
+             'def x = "a$ b"')
+        def text = err.toString()
+        assert text.contains(ILLEGAL_AFTER_DOLLAR)
+        assert text.contains('line 1, column 12')
+        assert !text.contains('token recognition error')
+    }
+
+    @Test
+    void testMidStringDollarPartWithTrailingDollar() {
+        def err = shouldFail(MultipleCompilationErrorsException,
+             'def x = "hello $name$"')
+        def text = err.toString()
+        assert text.contains(ILLEGAL_AFTER_DOLLAR)
+        assert !text.contains('token recognition error')
+    }
+
+    @Test
+    void testEscapedTrailingDollarIsLegal() {
+        assertScript '''
+            def Target = "releases\\$"
+            assert Target == 'releases$'
+        '''
+    }
+
+    @Test
+    void testDollarAtEndOfFile() {
+        // GStringBegin consumes up through `$`, then EOF — no character to quote in the message
+        def err = shouldFail(MultipleCompilationErrorsException,
+             'def x = "hello$')
+        def text = err.toString()
+        assert text.contains(ILLEGAL_AFTER_DOLLAR)
+        assert !text.contains('token recognition error')
+        // EOF form has no trailing ": '…'" character display
+        assert !(text =~ /Illegal string body character after dollar sign: '/)
     }
 
     @Test
     void testErrorReportOnStringEndWithOutParser() {
         // GROOVY-6608: the code did throw a NPE
-        try {
-            assertScript '''
+        def err = shouldFail(MultipleCompilationErrorsException,
+             '''
             def scanFolders()
             { doThis( ~"(?i)^sometext$",
-            '''
-        } catch (MultipleCompilationErrorsException mcee) {
-            def text = mcee.toString();
-            assert text.contains("line 3, column 39")
-        }
+            ''')
+        def text = err.toString()
+        assert text.contains('line 3, column 39')
     }
 }