You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2026/09/14 10:44:12 UTC
(groovy) branch master updated: GROOVY-12401: require a type argument before advising a missing '>'
This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 7d3d238ea8 GROOVY-12401: require a type argument before advising a missing '>'
7d3d238ea8 is described below
commit 7d3d238ea80a9cfd696410dc4944d8a8c0132f70
Author: Paul King <pa...@asert.com.au>
AuthorDate: Mon Sep 14 20:35:13 2026 +1000
GROOVY-12401: require a type argument before advising a missing '>'
`unmatchedTypeArgument` concluded a type argument list was unclosed from the
absence of a closing `>` in its scan window. A comparison never has one
either, so any comparison whose left operand is a capitalized identifier was
rewritten when an unrelated syntax error followed:
int MAX = 10
if (MAX < x) { println 1 + -> Missing '>'
`Integer.MAX_VALUE < x` behaved the same way, as did the shift in `X << 2`:
the guard written for `<<` skips the second `<`, but the first has already
counted, so the shift still reached the conclusion.
Decide from the token after the `<` instead. It must look like a type
argument - a capitalized name, a primitive, or a wildcard - which the
genuine `List<Integer name` shape satisfies and a comparison does not. A
newline between the two does not settle the question, and a `<` that ends
the input keeps the advice because nothing follows to disprove it.
A lowercase type argument (`List<foo`, or a fully qualified
`List<java.util.Date`) now falls back to the generic sentence rather than
risk the message on every comparison.
---
.../internal/AbstractFriendlyErrorStrategy.java | 44 ++++++++++++--
.../parser/antlr4/internal/ErrorStrategyTest.java | 69 +++++++++++++++++++++-
2 files changed, 104 insertions(+), 9 deletions(-)
diff --git a/src/main/java/org/apache/groovy/parser/antlr4/internal/AbstractFriendlyErrorStrategy.java b/src/main/java/org/apache/groovy/parser/antlr4/internal/AbstractFriendlyErrorStrategy.java
index 9b0bf127c6..5fc90d6c68 100644
--- a/src/main/java/org/apache/groovy/parser/antlr4/internal/AbstractFriendlyErrorStrategy.java
+++ b/src/main/java/org/apache/groovy/parser/antlr4/internal/AbstractFriendlyErrorStrategy.java
@@ -511,9 +511,17 @@ abstract class AbstractFriendlyErrorStrategy extends DefaultErrorStrategy {
/**
* {@code List<Integer name} / {@code new ArrayList<Integer(} — an
- * unmatched {@code <} after a capitalized type name is a missing
- * {@code >}, not a comparison. {@code x < y z} (lowercase left
- * operand) is left to the generic sentence.
+ * unmatched {@code <} between a capitalized type name and a type
+ * argument is a missing {@code >}, not a comparison.
+ * <p>
+ * Both sides have to look like a type, because the absence of a closing
+ * {@code >} proves nothing on its own: a comparison never has one
+ * either. {@code MAX < x} and {@code Integer.MAX_VALUE < x} would
+ * otherwise be rewritten whenever an unrelated error follows, and so
+ * would the shift in {@code X << 2}. See {@link #isTypeArgumentStart}.
+ * A {@code <} that ends the input ({@code class Foo<}) keeps the
+ * {@code >} advice: there is no following token to disprove it.
+ * </p>
* <p>
* ANTLR's offending token for {@code List<Integer name} is often
* {@code List} (the NVAE start), so a prefix-up-to-offender scan
@@ -546,6 +554,7 @@ abstract class AbstractFriendlyErrorStrategy extends DefaultErrorStrategy {
}
int depth = 0;
boolean typeOpener = false;
+ boolean pendingOpener = false;
Token prev = null;
int n = tokens.size();
int seen = 0;
@@ -555,8 +564,18 @@ abstract class AbstractFriendlyErrorStrategy extends DefaultErrorStrategy {
continue;
}
int type = t.getType();
+ // a `<` opens a type argument list only if a type argument follows;
+ // a newline may separate the two, so it does not settle the question.
+ // A `<` that ends the input (`class Foo<`) has nothing to disprove it
+ if (pendingOpener && type != NL) {
+ pendingOpener = false;
+ if (isTypeArgumentStart(type) || type == Token.EOF) {
+ depth++;
+ typeOpener = true;
+ }
+ }
if (type == Token.EOF || type == SEMI || type == ASSIGN
- || (type == NL && depth == 0 && i > lo)) {
+ || (type == NL && depth == 0 && !pendingOpener && i > lo)) {
if (i > lo) {
break;
}
@@ -570,8 +589,7 @@ abstract class AbstractFriendlyErrorStrategy extends DefaultErrorStrategy {
continue;
}
if (prev != null && prev.getType() == CapitalizedIdentifier) {
- depth++;
- typeOpener = true;
+ pendingOpener = true;
} else if (depth > 0) {
depth++;
}
@@ -774,6 +792,20 @@ abstract class AbstractFriendlyErrorStrategy extends DefaultErrorStrategy {
};
}
+ /**
+ * What may begin a type argument: a capitalized type name, a primitive
+ * (for an array type argument) or a wildcard. A lowercase name is not
+ * accepted, so a fully qualified {@code List<java.util.Date name} keeps
+ * the generic sentence rather than risk claiming a missing {@code >} for
+ * every comparison.
+ */
+ private static boolean isTypeArgumentStart(final int type) {
+ return switch (type) {
+ case CapitalizedIdentifier, BuiltInPrimitiveType, QUESTION -> true;
+ default -> false;
+ };
+ }
+
private static boolean isCreatedNameToken(final int type) {
return switch (type) {
case AT, Identifier, CapitalizedIdentifier, BuiltInPrimitiveType,
diff --git a/src/test/java/org/apache/groovy/parser/antlr4/internal/ErrorStrategyTest.java b/src/test/java/org/apache/groovy/parser/antlr4/internal/ErrorStrategyTest.java
index 83fbea5284..cf91f104dc 100644
--- a/src/test/java/org/apache/groovy/parser/antlr4/internal/ErrorStrategyTest.java
+++ b/src/test/java/org/apache/groovy/parser/antlr4/internal/ErrorStrategyTest.java
@@ -904,6 +904,67 @@ final class ErrorStrategyTest {
assertNull(AbstractFriendlyErrorStrategy.unmatchedTypeArgument(e, z));
}
+ @Test
+ void unmatchedTypeArgumentIgnoresComparisonAgainstACapitalizedName() {
+ // an uppercase constant on the left is not a type argument list:
+ // `MAX < x` has no closing '>' because it never had an opening one
+ var tokens = lex("if (MAX < x) { println 1 +\n}");
+ Token brace = requireType(tokens, GroovyParser.LBRACE);
+ RecognitionException e = stubException(new IntervalSet(), brace, tokens);
+ assertNull(AbstractFriendlyErrorStrategy.unmatchedTypeArgument(e, brace));
+ }
+
+ @Test
+ void unmatchedTypeArgumentIgnoresComparisonAgainstAConstantField() {
+ var tokens = lex("if (Integer.MAX_VALUE < x) { println 1 +\n}");
+ Token brace = requireType(tokens, GroovyParser.LBRACE);
+ RecognitionException e = stubException(new IntervalSet(), brace, tokens);
+ assertNull(AbstractFriendlyErrorStrategy.unmatchedTypeArgument(e, brace));
+ }
+
+ @Test
+ void unmatchedTypeArgumentIgnoresLeftShiftWithinTheScanWindow() {
+ // the shift sits next to the offender, so the scan really does reach it
+ var tokens = lex("def y = X << 2 +");
+ Token plus = requireText(tokens, "+");
+ RecognitionException e = stubException(new IntervalSet(), plus, tokens);
+ assertNull(AbstractFriendlyErrorStrategy.unmatchedTypeArgument(e, plus));
+ }
+
+ @Test
+ void unmatchedTypeArgumentAcceptsAPrimitiveTypeArgument() {
+ var tokens = lex("List<int[] list = null");
+ Token list = tokens.get(0);
+ RecognitionException e = stubException(new IntervalSet(), list, tokens);
+ assertEquals("Missing '>'", AbstractFriendlyErrorStrategy.unmatchedTypeArgument(e, list));
+ }
+
+ @Test
+ void unmatchedTypeArgumentAcceptsAWildcardTypeArgument() {
+ var tokens = lex("List<? extends Number list = null");
+ Token list = tokens.get(0);
+ RecognitionException e = stubException(new IntervalSet(), list, tokens);
+ assertEquals("Missing '>'", AbstractFriendlyErrorStrategy.unmatchedTypeArgument(e, list));
+ }
+
+ @Test
+ void unmatchedTypeArgumentAcceptsATypeArgumentOnTheNextLine() {
+ // a newline between '<' and the type argument does not settle the question
+ var tokens = lex("List<\nInteger x");
+ Token x = requireText(tokens, "x");
+ RecognitionException e = stubException(new IntervalSet(), x, tokens);
+ assertEquals("Missing '>'", AbstractFriendlyErrorStrategy.unmatchedTypeArgument(e, x));
+ }
+
+ @Test
+ void unmatchedTypeArgumentAcceptsATrailingLtAtEndOfInput() {
+ // `class Foo<` — nothing follows the '<' to disprove a type argument list
+ var tokens = lex("class Foo<");
+ Token eof = requireType(tokens, Token.EOF);
+ RecognitionException e = stubException(new IntervalSet(), eof, tokens);
+ assertEquals("Missing '>'", AbstractFriendlyErrorStrategy.unmatchedTypeArgument(e, eof));
+ }
+
@Test
void unmatchedTypeArgumentNulls() {
assertNull(AbstractFriendlyErrorStrategy.unmatchedTypeArgument(null, token(GroovyParser.Identifier, "x")));
@@ -1213,13 +1274,15 @@ final class ErrorStrategyTest {
}
@Test
- void unmatchedTypeArgumentNestedLowercaseNameIncrementsDepth() {
- // `foo<` is not itself a type opener; depth is already > 0 from `List<`.
+ void unmatchedTypeArgumentDeclinesALowercaseTypeArgument() {
+ // A type argument must look like a type. Declining `List<foo` costs a
+ // less specific sentence for an unconventionally named class; accepting
+ // any lowercase name would rewrite every `MAX < x` comparison.
var tokens = lex("List<foo<Integer x");
Token x = requireText(tokens, "x");
Token list = tokens.get(0);
NoViableAltException nvae = nvaeStartingAt(tokens, list, x);
- assertEquals("Missing '>'", AbstractFriendlyErrorStrategy.unmatchedTypeArgument(nvae, x));
+ assertNull(AbstractFriendlyErrorStrategy.unmatchedTypeArgument(nvae, x));
}
@Test