Skip to content

Commit 96b0192

Browse files
paulk-asertpaulk-asert
committed
GROOVY-10355: restore binary reading of (name) +/- x and (name) in/as x (design sketch)
A parenthesized bare name whose final segment starts lowercase is by convention a value, not a class, so a cast mis-parse of the ambiguous shapes is rebuilt in AstBuilder as the binary expression the syntax visually suggests, preserving textual left-to-right grouping across precedence levels. The binary-only keywords in/as captured as cast operand identifiers are restored to their relational reading for any capitalization. Unresolvable bare-name cast types now carry a hint explaining the ambiguity and the ((name)) workaround. The grammar is unchanged: a predicate-gated castExprAlt is not viable because adaptive prediction under the me.sunlan antlr4 fork only consults semantic predicates when a decision conflict is registered, which this decision never produces, so the predicate would only fire as a parse-time FailedPredicateException.
1 parent 63be01f commit 96b0192

3 files changed

Lines changed: 430 additions & 10 deletions

File tree

‎src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java‎

Lines changed: 244 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@
127127
import org.codehaus.groovy.control.CompilationFailedException;
128128
import org.codehaus.groovy.control.CompilePhase;
129129
import org.codehaus.groovy.control.ModuleImportHelper;
130+
import org.codehaus.groovy.control.ResolveVisitor;
130131
import org.codehaus.groovy.control.SourceUnit;
131132
import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
132133
import org.codehaus.groovy.transform.AsyncTransformHelper;
@@ -154,6 +155,7 @@
154155
import java.util.Map;
155156
import java.util.Objects;
156157
import java.util.Optional;
158+
import java.util.regex.Pattern;
157159
import java.util.Set;
158160
import java.util.function.Function;
159161
import java.util.stream.Collectors;
@@ -2371,6 +2373,18 @@ public Expression visitCommandExpression(final CommandExpressionContext ctx) {
23712373

23722374
Expression baseExpr = (Expression) this.visit(ctx.expression());
23732375

2376+
// GROOVY-10355: "(name) in x" / "(name) as T" arrive as a cast of the keyword identifier
2377+
// plus an argument list; restore the binary reading
2378+
if (hasArgumentList && !hasCommandArgument && baseExpr instanceof CastExpression) {
2379+
String keyword = baseExpr.getNodeMetaData(CAST_OF_BINARY_KEYWORD);
2380+
if (keyword != null) {
2381+
Expression repaired = this.repairBinaryKeywordCast((CastExpression) baseExpr, keyword, ctx);
2382+
if (repaired != null) {
2383+
return repaired;
2384+
}
2385+
}
2386+
}
2387+
23742388
if ((hasArgumentList || hasCommandArgument) && !isInsideParentheses(baseExpr)
23752389
&& baseExpr instanceof BinaryExpression && !"[".equals(((BinaryExpression) baseExpr).getOperation().getText())) {
23762390
throw createParsingFailedException("Unexpected input: '" + getOriginalText(ctx.expression()) + "'", ctx.expression());
@@ -2426,6 +2440,49 @@ public Expression visitCommandExpression(final CommandExpressionContext ctx) {
24262440
ctx);
24272441
}
24282442

2443+
/**
2444+
* Rebuilds {@code (name) in x} as a binary {@code in} expression and {@code (name) as T}
2445+
* as a coercion, from the mis-parsed cast-plus-argument-list shape (GROOVY-10355).
2446+
* Returns {@code null} when the argument shape does not permit a faithful rewrite.
2447+
*/
2448+
private Expression repairBinaryKeywordCast(final CastExpression cast, final String keyword, final CommandExpressionContext ctx) {
2449+
Expression argumentList = this.visitEnhancedArgumentListInPar(ctx.enhancedArgumentListInPar());
2450+
if (!(argumentList instanceof ArgumentListExpression)) return null;
2451+
List<Expression> arguments = ((ArgumentListExpression) argumentList).getExpressions();
2452+
if (arguments.size() != 1) return null;
2453+
2454+
Expression left = this.createParenNameExpression(cast.getType().getName(), ctx.expression());
2455+
Expression right = arguments.get(0);
2456+
Expression keywordSource = cast.getExpression();
2457+
2458+
if ("in".equals(keyword)) {
2459+
org.codehaus.groovy.syntax.Token inToken = new org.codehaus.groovy.syntax.Token(
2460+
Types.KEYWORD_IN, keyword, keywordSource.getLineNumber(), keywordSource.getColumnNumber());
2461+
return configureAST(new BinaryExpression(left, inToken, right), ctx);
2462+
}
2463+
2464+
String typeName = qualifiedNameOf(right);
2465+
if (typeName == null) return null;
2466+
return configureAST(CastExpression.asExpression(ClassHelper.make(typeName), left), ctx);
2467+
}
2468+
2469+
/** The dotted name an expression spells, when it is a plain variable/property chain. */
2470+
private static String qualifiedNameOf(final Expression expression) {
2471+
if (expression instanceof VariableExpression) {
2472+
return ((VariableExpression) expression).getName();
2473+
}
2474+
if (expression instanceof PropertyExpression) {
2475+
PropertyExpression property = (PropertyExpression) expression;
2476+
if (property.getProperty() instanceof ConstantExpression) {
2477+
String qualifier = qualifiedNameOf(property.getObjectExpression());
2478+
if (qualifier != null) {
2479+
return qualifier + "." + property.getPropertyAsString();
2480+
}
2481+
}
2482+
}
2483+
return null;
2484+
}
2485+
24292486
/* Validate the following invalid cases:
24302487
* 1) void m() {}
24312488
* 2) String m() {}
@@ -3070,31 +3127,185 @@ public Expression visitPostfixExpression(final PostfixExpressionContext ctx) {
30703127

30713128
@Override
30723129
public Expression visitUnaryNotExprAlt(final UnaryNotExprAltContext ctx) {
3130+
Expression expression = (Expression) this.visit(ctx.expression());
3131+
3132+
// GROOVY-10355: the unary operator applies to the leading operand of a repaired
3133+
// "(name) +/- x" binary, not to the whole binary
3134+
if (isRepairedParenNameBinary(expression) && !isInsideParentheses(expression)) {
3135+
BinaryExpression repaired = (BinaryExpression) expression;
3136+
Expression wrapped = asBoolean(ctx.NOT())
3137+
? new NotExpression(repaired.getLeftExpression())
3138+
: new BitwiseNegationExpression(repaired.getLeftExpression());
3139+
repaired.setLeftExpression(configureAST(wrapped, ctx));
3140+
return configureAST(repaired, ctx);
3141+
}
3142+
30733143
if (asBoolean(ctx.NOT())) {
3074-
return configureAST(
3075-
new NotExpression((Expression) this.visit(ctx.expression())),
3076-
ctx);
3144+
return configureAST(new NotExpression(expression), ctx);
30773145
}
30783146

30793147
if (asBoolean(ctx.BITNOT())) {
3080-
return configureAST(
3081-
new BitwiseNegationExpression((Expression) this.visit(ctx.expression())),
3082-
ctx);
3148+
return configureAST(new BitwiseNegationExpression(expression), ctx);
30833149
}
30843150

30853151
throw createParsingFailedException("Unsupported unary expression: " + ctx.getText(), ctx);
30863152
}
30873153

30883154
@Override
3089-
public CastExpression visitCastExprAlt(final CastExprAltContext ctx) {
3155+
public Expression visitCastExprAlt(final CastExprAltContext ctx) {
3156+
String bareName = this.bareCastTypeName(ctx.castParExpression());
3157+
3158+
// GROOVY-10355: "(name) + x" / "(name) - x" — a bare name with a lowercase-initial final
3159+
// segment is by convention a value, not a class, so build the binary expression the
3160+
// syntax visually suggests instead of a cast of a unary expression
3161+
if (bareName != null && isLowercaseFinalSegment(bareName) && ctx.expression() instanceof UnaryAddExprAltContext) {
3162+
UnaryAddExprAltContext uctx = (UnaryAddExprAltContext) ctx.expression();
3163+
int opType = uctx.op.getType();
3164+
if (ADD == opType || SUB == opType) {
3165+
Expression left = this.createParenNameExpression(bareName, ctx.castParExpression());
3166+
Expression right = (Expression) this.visit(uctx.expression());
3167+
Expression repaired = this.combineRebalancing(left, this.createGroovyToken(uctx.op), right, opType);
3168+
repaired.putNodeMetaData(REPAIRED_PAREN_NAME_BINARY, Boolean.TRUE);
3169+
return configureAST(repaired, ctx);
3170+
}
3171+
}
3172+
3173+
// GROOVY-10355: "(name) in [x, y]" / "(name) in [k: v]" — the collection literal is
3174+
// captured as a subscript on the keyword identifier; restore the binary reading
3175+
if (bareName != null) {
3176+
Expression subscriptRepair = this.tryRepairKeywordCastSubscript(bareName, ctx);
3177+
if (subscriptRepair != null) {
3178+
return subscriptRepair;
3179+
}
3180+
}
3181+
30903182
Expression expr = (Expression) this.visit(ctx.expression());
30913183
if (expr instanceof VariableExpression && ((VariableExpression) expr).isSuperExpression()) {
30923184
throw this.createParsingFailedException("Cannot cast or coerce `super`", ctx); // GROOVY-9391
30933185
}
3094-
CastExpression cast = new CastExpression(this.visitCastParExpression(ctx.castParExpression()), expr);
3186+
ClassNode type = this.visitCastParExpression(ctx.castParExpression());
3187+
3188+
// GROOVY-10355: "(name) in x" / "(name) as T" — the binary-only keyword is captured as the
3189+
// cast operand identifier; mark it so visitCommandExpression can restore the binary reading
3190+
if (bareName != null && expr instanceof VariableExpression) {
3191+
String operandName = ((VariableExpression) expr).getName();
3192+
if ("in".equals(operandName) || "as".equals(operandName)) {
3193+
CastExpression keywordCast = new CastExpression(type, expr);
3194+
keywordCast.putNodeMetaData(CAST_OF_BINARY_KEYWORD, operandName);
3195+
return configureAST(keywordCast, ctx);
3196+
}
3197+
}
3198+
3199+
// GROOVY-10355: "(int)(name) + 10" — the operand was repaired to a binary expression, so
3200+
// the cast applies to that expression's leftmost operand, not to the whole binary
3201+
if (isRepairedParenNameBinary(expr) && !isInsideParentheses(expr)) {
3202+
BinaryExpression repaired = (BinaryExpression) expr;
3203+
repaired.setLeftExpression(configureAST(new CastExpression(type, repaired.getLeftExpression()), ctx));
3204+
return configureAST(repaired, ctx);
3205+
}
3206+
3207+
CastExpression cast = new CastExpression(type, expr);
3208+
if (bareName != null && isAmbiguousCastOperand(expr)) {
3209+
// GROOVY-10355: should the type fail to resolve, the error explains the ambiguity
3210+
cast.putNodeMetaData(ResolveVisitor.CAST_RESOLVE_HINT,
3211+
"; '(" + bareName + ")' followed by an operand is parsed as a cast - if '" + bareName
3212+
+ "' was meant as a value, wrap it in a second set of parentheses, e.g. ((" + bareName + "))");
3213+
}
30953214
return configureAST(cast, ctx);
30963215
}
30973216

3217+
/**
3218+
* The content of the parentheses when it is a bare, possibly-qualified class-or-variable
3219+
* name — no primitives, generics, array dimensions, annotations or intersections.
3220+
*/
3221+
private String bareCastTypeName(final CastParExpressionContext ctx) {
3222+
String text = ctx.intersectionType().getText();
3223+
if (!BARE_NAME_PATTERN.matcher(text).matches()) return null;
3224+
if (ClassHelper.isPrimitiveType(ClassHelper.make(text))) return null;
3225+
return text;
3226+
}
3227+
3228+
private static boolean isLowercaseFinalSegment(final String name) {
3229+
return !Character.isUpperCase(name.codePointAt(name.lastIndexOf('.') + 1));
3230+
}
3231+
3232+
private Expression createParenNameExpression(final String qualifiedName, final GroovyParserRuleContext ctx) {
3233+
String[] parts = qualifiedName.split("\\.");
3234+
Expression expression = new VariableExpression(parts[0]);
3235+
for (int i = 1; i < parts.length; i += 1) {
3236+
expression = new PropertyExpression(expression, parts[i]);
3237+
}
3238+
return configureAST(expression, ctx);
3239+
}
3240+
3241+
private static boolean isRepairedParenNameBinary(final Expression expression) {
3242+
return expression instanceof BinaryExpression && expression.getNodeMetaData(REPAIRED_PAREN_NAME_BINARY) != null;
3243+
}
3244+
3245+
/**
3246+
* Rebuilds {@code (name) in [x, y]} / {@code (name) in [k: v]} as a binary {@code in}
3247+
* expression. The collection literal after {@code in} parses as a subscript on the
3248+
* keyword identifier, so the literal is rebuilt from the subscript arguments (GROOVY-10355).
3249+
*/
3250+
private Expression tryRepairKeywordCastSubscript(final String bareName, final CastExprAltContext ctx) {
3251+
if (!(ctx.expression() instanceof PostfixExprAltContext)) return null;
3252+
PostfixExpressionContext postfixCtx = ((PostfixExprAltContext) ctx.expression()).postfixExpression();
3253+
if (postfixCtx.op != null) return null;
3254+
PathExpressionContext pathCtx = postfixCtx.pathExpression();
3255+
if (!(pathCtx.primary() instanceof IdentifierPrmrAltContext) || pathCtx.pathElement().size() != 1) return null;
3256+
IdentifierPrmrAltContext identifierCtx = (IdentifierPrmrAltContext) pathCtx.primary();
3257+
if (identifierCtx.typeArguments() != null || !"in".equals(identifierCtx.identifier().getText())) return null;
3258+
3259+
PathElementContext elementCtx = pathCtx.pathElement(0);
3260+
Expression right;
3261+
if (elementCtx.indexPropertyArgs() != null && elementCtx.indexPropertyArgs().LBRACK() != null) {
3262+
IndexPropertyArgsContext indexCtx = elementCtx.indexPropertyArgs();
3263+
List<Expression> elements = indexCtx.expressionList() == null
3264+
? new ArrayList<>() : this.visitExpressionList(indexCtx.expressionList());
3265+
right = configureAST(new ListExpression(elements), indexCtx);
3266+
} else if (elementCtx.namedPropertyArgs() != null && elementCtx.namedPropertyArgs().LBRACK() != null) {
3267+
NamedPropertyArgsContext namedCtx = elementCtx.namedPropertyArgs();
3268+
right = configureAST(new MapExpression(this.visitNamedPropertyArgs(namedCtx)), namedCtx);
3269+
} else {
3270+
return null;
3271+
}
3272+
3273+
Expression left = this.createParenNameExpression(bareName, ctx.castParExpression());
3274+
org.codehaus.groovy.syntax.Token inToken = new org.codehaus.groovy.syntax.Token(Types.KEYWORD_IN, "in",
3275+
identifierCtx.getStart().getLine(), identifierCtx.getStart().getCharPositionInLine() + 1);
3276+
return configureAST(new BinaryExpression(left, inToken, right), ctx);
3277+
}
3278+
3279+
/**
3280+
* Composes {@code left op right} preserving textual left-to-right grouping when either operand
3281+
* is a repaired "(name) +/- x" binary whose leading operand should bind with this operator first:
3282+
* {@code L op (name +/- x)} becomes {@code (L op name) +/- x} when {@code op} binds at least as
3283+
* tightly as +/-, and {@code (name +/- x) op R} becomes {@code name +/- (x op R)} when
3284+
* {@code op} binds more tightly.
3285+
*/
3286+
private Expression combineRebalancing(final Expression left, final org.codehaus.groovy.syntax.Token op, final Expression right, final int opType) {
3287+
boolean atLeastAdditive = POWER == opType || MUL == opType || DIV == opType || MOD == opType || ADD == opType || SUB == opType;
3288+
if (atLeastAdditive && isRepairedParenNameBinary(right) && !isInsideParentheses(right)) {
3289+
BinaryExpression repaired = (BinaryExpression) right;
3290+
repaired.setLeftExpression(this.combineRebalancing(left, op, repaired.getLeftExpression(), opType));
3291+
return repaired;
3292+
}
3293+
boolean tighterThanAdditive = POWER == opType || MUL == opType || DIV == opType || MOD == opType;
3294+
if (tighterThanAdditive && isRepairedParenNameBinary(left) && !isInsideParentheses(left)) {
3295+
BinaryExpression repaired = (BinaryExpression) left;
3296+
repaired.setRightExpression(this.combineRebalancing(repaired.getRightExpression(), op, right, opType));
3297+
return repaired;
3298+
}
3299+
return new BinaryExpression(left, op, right);
3300+
}
3301+
3302+
private boolean isAmbiguousCastOperand(final Expression expr) {
3303+
return expr instanceof UnaryPlusExpression || expr instanceof UnaryMinusExpression
3304+
|| expr instanceof PrefixExpression || expr instanceof ListExpression
3305+
|| expr instanceof MapExpression || expr instanceof ClosureExpression
3306+
|| isInsideParentheses(expr);
3307+
}
3308+
30983309
@Override
30993310
public Expression visitAwaitExprAlt(final AwaitExprAltContext ctx) {
31003311
List<? extends ExpressionContext> exprCtxs = ctx.expression();
@@ -3128,6 +3339,21 @@ public BinaryExpression visitPowerExprAlt(final PowerExprAltContext ctx) {
31283339
@Override
31293340
public Expression visitUnaryAddExprAlt(final UnaryAddExprAltContext ctx) {
31303341
Expression expression = (Expression) this.visit(ctx.expression());
3342+
3343+
// GROOVY-10355: the unary operator applies to the leading operand of a repaired
3344+
// "(name) +/- x" binary, not to the whole binary
3345+
if (isRepairedParenNameBinary(expression) && !isInsideParentheses(expression)) {
3346+
BinaryExpression repaired = (BinaryExpression) expression;
3347+
Expression wrapped;
3348+
switch (ctx.op.getType()) {
3349+
case ADD: wrapped = new UnaryPlusExpression(repaired.getLeftExpression()); break;
3350+
case SUB: wrapped = new UnaryMinusExpression(repaired.getLeftExpression()); break;
3351+
default: wrapped = new PrefixExpression(this.createGroovyToken(ctx.op), repaired.getLeftExpression()); break;
3352+
}
3353+
repaired.setLeftExpression(configureAST(wrapped, ctx));
3354+
return configureAST(repaired, ctx);
3355+
}
3356+
31313357
switch (ctx.op.getType()) {
31323358
case ADD:
31333359
if (this.isNonStringConstantOutsideParentheses(expression)) {
@@ -4673,7 +4899,7 @@ private ConstantExpression createConstantExpression(final Expression expression)
46734899
}
46744900

46754901
private BinaryExpression createBinaryExpression(final ExpressionContext left, final Token op, final ExpressionContext right) {
4676-
return new BinaryExpression((Expression) this.visit(left), this.createGroovyToken(op), (Expression) this.visit(right));
4902+
return (BinaryExpression) this.combineRebalancing((Expression) this.visit(left), this.createGroovyToken(op), (Expression) this.visit(right), op.getType());
46774903
}
46784904

46794905
private BinaryExpression createBinaryExpression(final ExpressionContext left, final Token op, final ExpressionContext right, final ExpressionContext ctx) {
@@ -5034,6 +5260,15 @@ public List<DeclarationExpression> getDeclarationExpressions() {
50345260
private static final String PACKAGE_INFO = "package-info";
50355261
private static final String PACKAGE_INFO_FILE_NAME = PACKAGE_INFO + ".groovy";
50365262

5263+
// GROOVY-10355: a binary expression rebuilt from a "(name) +/- x" cast mis-parse; parents
5264+
// re-associate it so the textual left-to-right grouping is preserved
5265+
private static final String REPAIRED_PAREN_NAME_BINARY = "_REPAIRED_PAREN_NAME_BINARY";
5266+
5267+
// GROOVY-10355: a cast whose operand is the binary-only keyword identifier "in" or "as"
5268+
private static final String CAST_OF_BINARY_KEYWORD = "_CAST_OF_BINARY_KEYWORD";
5269+
5270+
private static final Pattern BARE_NAME_PATTERN = Pattern.compile("[A-Za-z_$][A-Za-z0-9_$]*(\\.[A-Za-z_$][A-Za-z0-9_$]*)*");
5271+
50375272
private static final String CLASS_NAME = "_CLASS_NAME";
50385273
private static final String INSIDE_PARENTHESES_LEVEL = "_INSIDE_PARENTHESES_LEVEL";
50395274
private static final String IS_INSIDE_INSTANCEOF_EXPR = "_IS_INSIDE_INSTANCEOF_EXPR";

‎src/main/java/org/codehaus/groovy/control/ResolveVisitor.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,14 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
109109
* Placeholder name used for wildcard generic arguments.
110110
*/
111111
public static final String QUESTION_MARK = "?";
112+
/**
113+
* Node-metadata key under which a parser may store an explanatory suffix for a
114+
* {@link CastExpression} whose type it suspects may fail to resolve — for example a
115+
* cast produced by a grammar ambiguity. When the cast's type cannot be resolved, the
116+
* stored text is appended to the {@code unable to resolve class} error message.
117+
* The value is the complete hint text; this visitor attaches no meaning to it.
118+
*/
119+
public static final String CAST_RESOLVE_HINT = "_CAST_RESOLVE_HINT";
112120

113121
private final CompilationUnit compilationUnit;
114122
private ClassNodeResolver classNodeResolver;
@@ -974,7 +982,8 @@ public Expression transform(final Expression exp) {
974982
} else if (exp instanceof AnnotationConstantExpression) {
975983
ret = transformAnnotationConstantExpression((AnnotationConstantExpression) exp);
976984
} else {
977-
resolveOrFail(exp.getType(), exp);
985+
String hint = exp instanceof CastExpression ? exp.getNodeMetaData(CAST_RESOLVE_HINT) : null;
986+
resolveOrFail(exp.getType(), hint != null ? hint : "", exp);
978987
ret = exp.transformExpression(this);
979988
}
980989
if (ret != null && ret != exp) {

0 commit comments

Comments
 (0)