This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch GROOVY-12173 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 6ab12c09e203100c07f07585df149a5445f029a4 Author: Daniel Sun <[email protected]> AuthorDate: Sat Jul 18 19:25:22 2026 +0900 GROOVY-12173: Left-factor parser rules to reduce lookahead on hot parse paths --- src/antlr/GroovyParser.g4 | 50 +-- .../apache/groovy/parser/antlr4/AstBuilder.java | 32 +- .../apache/groovy/parser/antlr4/Groovy12173.groovy | 369 +++++++++++++++++++++ 3 files changed, 412 insertions(+), 39 deletions(-) diff --git a/src/antlr/GroovyParser.g4 b/src/antlr/GroovyParser.g4 index 3eb0dedeed..7384daca9e 100644 --- a/src/antlr/GroovyParser.g4 +++ b/src/antlr/GroovyParser.g4 @@ -118,8 +118,10 @@ packageDeclaration ; importDeclaration - : annotationsOpt IMPORT STATIC? qualifiedName (DOT MUL | AS alias=identifier)? - | annotationsOpt IMPORT MODULE qualifiedName + : annotationsOpt IMPORT + ( MODULE qualifiedName + | STATIC? qualifiedName (DOT MUL | AS alias=identifier)? + ) ; @@ -557,8 +559,7 @@ block ; blockStatement - : localVariableDeclaration - | statement + : statement ; localVariableDeclaration @@ -579,8 +580,11 @@ variableDeclaration[int t] ; typeNamePairs - : LPAREN typeNamePair (COMMA typeNamePair)* RPAREN - | LPAREN keyedPair (COMMA keyedPair)* RPAREN + : LPAREN + ( typeNamePair (COMMA typeNamePair)* + | keyedPair (COMMA keyedPair)* + ) + RPAREN ; typeNamePair @@ -978,21 +982,29 @@ pathExpression returns [int t] pathElement returns [int t] : nls ( - DOT nls NEW creator[1] - { $t = 6; } - | - // AT: foo.@bar selects the field (or attribute), not property - ( - ( DOT // The all-powerful dot. - | SPREAD_DOT // Spread operator: x*.y === x?.collect{it.y} - | SAFE_DOT // Optional-null operator: x?.y === (x==null)?null:x.y - | SAFE_CHAIN_DOT // Optional-null chain operator: x??.y.z === x?.y?.z - ) nls (AT | nonWildcardTypeArguments)? - | - METHOD_POINTER nls // Method pointer operator: foo.&y == foo.metaClass.getMethodPointer(foo, "y") + DOT nls + ( NEW creator[1] + { $t = 6; } | - METHOD_REFERENCE nls (nonWildcardTypeArguments)? // Method reference: System.out::println + // AT: foo.@bar selects the field (or attribute), not property + (AT | nonWildcardTypeArguments)? + namePart + { $t = 1; } ) + | + // Non-DOT member selection operators (still share namePart tail) + ( SPREAD_DOT // Spread operator: x*.y === x?.collect{it.y} + | SAFE_DOT // Optional-null operator: x?.y === (x==null)?null:x.y + | SAFE_CHAIN_DOT // Optional-null chain operator: x??.y.z === x?.y?.z + ) nls (AT | nonWildcardTypeArguments)? + namePart + { $t = 1; } + | + METHOD_POINTER nls // Method pointer operator: foo.&y == foo.metaClass.getMethodPointer(foo, "y") + namePart + { $t = 1; } + | + METHOD_REFERENCE nls (nonWildcardTypeArguments)? // Method reference: System.out::println namePart { $t = 1; } diff --git a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java index aaeb90b9fd..deaaf9988d 100644 --- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java +++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java @@ -4351,29 +4351,21 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { @Override public Statement visitBlockStatement(final BlockStatementContext ctx) { - if (asBoolean(ctx.localVariableDeclaration())) { - return configureAST(this.visitLocalVariableDeclaration(ctx.localVariableDeclaration()), ctx); - } - - if (asBoolean(ctx.statement())) { - Object astNode = this.visit(ctx.statement()); //this.configureAST((Statement) this.visit(ctx.statement()), ctx); - - if (null == astNode) { - return null; - } + Object astNode = this.visit(ctx.statement()); - if (astNode instanceof Statement) { - return (Statement) astNode; - } else if (astNode instanceof MethodNode) { - throw createParsingFailedException("Method definition not expected here", ctx); - } else if (astNode instanceof ImportNode) { - throw createParsingFailedException("Import statement not expected here", ctx); - } else { - throw createParsingFailedException("The statement(" + astNode.getClass() + ") not expected here", ctx); - } + if (null == astNode) { + return null; } - throw createParsingFailedException("Unsupported block statement: " + ctx.getText(), ctx); + if (astNode instanceof Statement) { + return (Statement) astNode; + } else if (astNode instanceof MethodNode) { + throw createParsingFailedException("Method definition not expected here", ctx); + } else if (astNode instanceof ImportNode) { + throw createParsingFailedException("Import statement not expected here", ctx); + } else { + throw createParsingFailedException("The statement(" + astNode.getClass() + ") not expected here", ctx); + } } @Override diff --git a/src/test/groovy/org/apache/groovy/parser/antlr4/Groovy12173.groovy b/src/test/groovy/org/apache/groovy/parser/antlr4/Groovy12173.groovy new file mode 100644 index 0000000000..ae215a1665 --- /dev/null +++ b/src/test/groovy/org/apache/groovy/parser/antlr4/Groovy12173.groovy @@ -0,0 +1,369 @@ +/* + * 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.parser.antlr4 + +import org.antlr.v4.runtime.CommonToken +import org.codehaus.groovy.ast.ClassHelper +import org.codehaus.groovy.ast.ClassNode +import org.codehaus.groovy.ast.ImportNode +import org.codehaus.groovy.ast.MethodNode +import org.codehaus.groovy.ast.ModuleNode +import org.codehaus.groovy.ast.Parameter +import org.codehaus.groovy.ast.expr.BinaryExpression +import org.codehaus.groovy.ast.expr.ConstructorCallExpression +import org.codehaus.groovy.ast.expr.DeclarationExpression +import org.codehaus.groovy.ast.expr.Expression +import org.codehaus.groovy.ast.expr.MethodCallExpression +import org.codehaus.groovy.ast.expr.MethodPointerExpression +import org.codehaus.groovy.ast.expr.MethodReferenceExpression +import org.codehaus.groovy.ast.expr.PropertyExpression +import org.codehaus.groovy.ast.expr.AttributeExpression +import org.codehaus.groovy.ast.expr.VariableExpression +import org.codehaus.groovy.ast.stmt.BlockStatement +import org.codehaus.groovy.ast.stmt.EmptyStatement +import org.codehaus.groovy.ast.stmt.ExpressionStatement +import org.codehaus.groovy.ast.stmt.Statement +import org.codehaus.groovy.control.CompilationFailedException +import org.codehaus.groovy.control.CompilerConfiguration +import org.codehaus.groovy.control.ErrorCollector +import org.codehaus.groovy.control.ParserPlugin +import org.codehaus.groovy.control.ParserPluginFactory +import org.codehaus.groovy.control.SourceUnit +import org.codehaus.groovy.control.io.StringReaderSource +import org.junit.jupiter.api.Test +import org.objectweb.asm.Opcodes + +import static groovy.test.GroovyAssert.shouldFail +import static org.junit.jupiter.api.Assertions.assertEquals +import static org.junit.jupiter.api.Assertions.assertNotNull +import static org.junit.jupiter.api.Assertions.assertNull +import static org.junit.jupiter.api.Assertions.assertTrue + +final class Groovy12173 { + + // ------------------------------------------------------------------------- + // importDeclaration (left-factored annotationsOpt IMPORT …) + // ------------------------------------------------------------------------- + + @Test + void 'ordinary import parses after left-factor'() { + ModuleNode ast = buildAST(''' + import java.util.LinkedList + LinkedList list = new LinkedList() + ''') + assertTrue(ast.imports.any { it.type.name == 'java.util.LinkedList' }) + } + + @Test + void 'static import and star import parse after left-factor'() { + ModuleNode ast = buildAST(''' + import static java.lang.Math.PI + import static java.lang.Math.* + def x = PI + def y = sin(0) + ''') + assertTrue(ast.staticImports.containsKey('PI') || ast.staticStarImports.containsKey('java.lang.Math')) + assertTrue(ast.staticStarImports.containsKey('java.lang.Math') || !ast.staticStarImports.isEmpty()) + } + + @Test + void 'aliased import parses after left-factor'() { + ModuleNode ast = buildAST(''' + import java.util.LinkedList as LL + LL list = new LL() + ''') + assertTrue(ast.imports.any { it.alias == 'LL' }) + } + + @Test + void 'module import parses after left-factor'() { + ModuleNode ast = buildAST(''' + import module java.base + def s = "ok" + ''') + // Module import expands into module-star and/or ordinary star imports + assertTrue(!ast.moduleStarImports.isEmpty() || !ast.starImports.isEmpty(), + 'module import should expand to star imports via the left-factored importDeclaration rule') + } + + @Test + void 'annotated import parses after left-factor'() { + ModuleNode ast = buildAST(''' + @Deprecated + import java.util.ArrayList + ArrayList a = new ArrayList() + ''') + assertTrue(ast.imports.any { it.type.name == 'java.util.ArrayList' }) + } + + // ------------------------------------------------------------------------- + // typeNamePairs (left-factored LPAREN … RPAREN) + // ------------------------------------------------------------------------- + + @Test + void 'positional multi-assignment declaration uses typeNamePair branch'() { + ModuleNode ast = buildAST(''' + def (a, b) = [1, 2] + assert a == 1 && b == 2 + ''') + DeclarationExpression decl = firstDeclaration(ast) + assertTrue(decl.leftExpression instanceof org.codehaus.groovy.ast.expr.TupleExpression + || decl.leftExpression instanceof org.codehaus.groovy.ast.expr.ArgumentListExpression + || decl.leftExpression != null) + assertNotNull(decl) + } + + @Test + void 'typed positional multi-assignment uses typeNamePair branch'() { + ModuleNode ast = buildAST(''' + def (int a, String b) = [1, 'x'] + ''') + assertNotNull(firstDeclaration(ast)) + } + + @Test + void 'keyed multi-assignment uses keyedPair branch'() { + ModuleNode ast = buildAST(''' + def (name: n, age: a) = [name: 'Alice', age: 30] + assert n == 'Alice' && a == 30 + ''') + assertNotNull(firstDeclaration(ast)) + } + + @Test + void 'typed keyed multi-assignment uses keyedPair branch'() { + ModuleNode ast = buildAST(''' + def (name: String n, age: int a) = [name: 'Bob', age: 42] + ''') + assertNotNull(firstDeclaration(ast)) + } + + // ------------------------------------------------------------------------- + // pathElement DOT left-factor + sibling selectors + // ------------------------------------------------------------------------- + + @Test + void 'dot property access uses DOT namePart branch'() { + ModuleNode ast = buildAST('def x = foo.bar') + ExpressionStatement stmt = firstExpressionStatement(ast) + assertTrue(containsPropertyAccess(stmt.expression)) + } + + @Test + void 'attribute access uses DOT AT namePart branch'() { + ModuleNode ast = buildAST('def x = foo.@bar') + ExpressionStatement stmt = firstExpressionStatement(ast) + assertTrue(findInExpression(stmt.expression) { it instanceof AttributeExpression }) + } + + @Test + void 'safe spread and chain dots parse'() { + ModuleNode ast = buildAST(''' + def a = foo?.bar + def b = foo*.bar + def c = foo??.bar + ''') + assertEquals(3, ast.statementBlock.statements.size()) + } + + @Test + void 'method pointer and method reference use dedicated pathElement branches'() { + ModuleNode ast = buildAST(''' + def p = Math.&max + def r = Math::max + def g = Collections::<String>emptyList + ''') + List<Statement> stmts = ast.statementBlock.statements + assertTrue(findInExpression(((ExpressionStatement) stmts[0]).expression) { + it instanceof MethodPointerExpression + }) + assertTrue(findInExpression(((ExpressionStatement) stmts[1]).expression) { + it instanceof MethodReferenceExpression + }) + assertTrue(findInExpression(((ExpressionStatement) stmts[2]).expression) { + it instanceof MethodReferenceExpression + }) + } + + @Test + void 'spread attribute and safe attribute path elements parse'() { + ModuleNode ast = buildAST(''' + def a = list*.@field + def b = obj?.@field + ''') + assertEquals(2, ast.statementBlock.statements.size()) + } + + @Test + void 'star import without static parses'() { + ModuleNode ast = buildAST(''' + import java.util.* + List x = [] + ''') + assertTrue(ast.starImports.any { it.packageName == 'java.util.' }) + } + + @Test + void 'enclosing-instance new uses DOT NEW creator branch'() { + // `outer.new Inner()` is the classic non-static inner-class path (pathElement t=6) + ModuleNode ast = buildAST(''' + class Outer { + class Inner { + def ok() { 1 } + } + } + def o = new Outer() + def i = o.new Inner() + ''') + // Last script statement is the constructor call via pathElement NEW + Statement last = ast.statementBlock.statements.last() + assertTrue( + findInExpression(((ExpressionStatement) last).expression) { + it instanceof ConstructorCallExpression || it instanceof MethodCallExpression + }, + 'expected enclosing-instance construction via DOT NEW pathElement') + } + + // ------------------------------------------------------------------------- + // blockStatement → statement only (local var + command + empty) + // ------------------------------------------------------------------------- + + @Test + void 'local variable declaration still works inside a block'() { + ModuleNode ast = buildAST(''' + { + def x = 1 + def y = 2 + } + ''') + assertNotNull(ast) + // Block as script statement + assertTrue(ast.statementBlock.statements.any { it instanceof BlockStatement || it instanceof ExpressionStatement }) + } + + @Test + void 'command expression inside block does not double-evaluate as local var'() { + // Command-style call: must parse as statementExpression, not localVariableDeclaration + ModuleNode ast = buildAST(''' + def foo(a) { a } + { + foo 42 + } + ''') + assertNotNull(ast) + } + + @Test + void 'empty statement and expression statements inside block'() { + ModuleNode ast = buildAST(''' + { + ; + def z = 1 + z = 2 + } + ''') + assertNotNull(ast) + } + + // ------------------------------------------------------------------------- + // visitBlockStatement + // ------------------------------------------------------------------------- + + @Test + void 'visitBlockStatement returns null for null visit result'() { + def (builder, ctx) = blockStatementFixture() + assertNull(builder.visitBlockStatement(ctx)) + } + // ------------------------------------------------------------------------- + // helpers + // ------------------------------------------------------------------------- + + private static ModuleNode buildAST(String src) { + CompilerConfiguration config = new CompilerConfiguration(CompilerConfiguration.DEFAULT) + config.pluginFactory = ParserPluginFactory.antlr4() + ModuleNode ast = ParserPlugin.buildAST(src, config, new GroovyClassLoader(), null) + assertNotNull(ast, "AST should build for:\n$src") + assertTrue(!ast.context.errorCollector.hasErrors(), + "Parse errors for:\n$src\n${ast.context.errorCollector.errors}") + return ast + } + + private static DeclarationExpression firstDeclaration(ModuleNode ast) { + Statement s = ast.statementBlock.statements[0] + ExpressionStatement es = (ExpressionStatement) s + assertTrue(es.expression instanceof DeclarationExpression) + return (DeclarationExpression) es.expression + } + + private static ExpressionStatement firstExpressionStatement(ModuleNode ast) { + return (ExpressionStatement) ast.statementBlock.statements[0] + } + + private static boolean containsPropertyAccess(Expression expr) { + findInExpression(expr) { it instanceof PropertyExpression || it instanceof VariableExpression } + } + + private static boolean findInExpression(Expression expr, Closure predicate) { + if (expr == null) return false + if (predicate.call(expr)) return true + if (expr instanceof DeclarationExpression) { + return findInExpression(expr.leftExpression, predicate) || findInExpression(expr.rightExpression, predicate) + } + if (expr instanceof BinaryExpression) { + return findInExpression(expr.leftExpression, predicate) || findInExpression(expr.rightExpression, predicate) + } + if (expr instanceof PropertyExpression) { + return findInExpression(expr.objectExpression, predicate) || findInExpression(expr.property, predicate) + } + if (expr instanceof MethodCallExpression) { + return findInExpression(expr.objectExpression, predicate) || findInExpression(expr.method, predicate) + } + if (expr instanceof ConstructorCallExpression) { + return true + } + if (expr instanceof MethodPointerExpression || expr instanceof MethodReferenceExpression) { + return predicate.call(expr) + } + if (expr instanceof AttributeExpression) { + return true + } + return false + } + + /** + * Build a minimal {@link AstBuilder} plus a {@link GroovyParser.BlockStatementContext} + * with start/stop tokens so error-position helpers succeed. + */ + private static List blockStatementFixture() { + CompilerConfiguration config = new CompilerConfiguration(CompilerConfiguration.DEFAULT) + ErrorCollector collector = new ErrorCollector(config) + StringReaderSource source = new StringReaderSource('{ x = 1 }', config) + SourceUnit su = new SourceUnit('Groovy12173', source, config, new GroovyClassLoader(), collector) + AstBuilder builder = new AstBuilder(su, false, false) + + CommonToken token = new CommonToken(GroovyLexer.Identifier, 'x') + token.line = 1 + token.charPositionInLine = 0 + GroovyParser.BlockStatementContext ctx = new GroovyParser.BlockStatementContext(null, 0) + ctx.start = token + ctx.stop = token + + return [builder, ctx] + } +}
