This is an automated email from the ASF dual-hosted git repository. jonnybot pushed a commit to branch groovy11896-bigger-refactor in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 4b912f4578d75308f0eaf213dcaf057277639afd Author: Jonny Carter <[email protected]> AuthorDate: Wed Apr 15 14:57:25 2026 -0500 Defer module name resolution until later This actually changes the AST to make module names available. This could be a win for tooling & navigation. --- .../apache/groovy/parser/antlr4/AstBuilder.java | 25 ++++-------- .../java/org/codehaus/groovy/ast/ImportNode.java | 44 ++++++++++++++++++++++ .../java/org/codehaus/groovy/ast/ModuleNode.java | 19 ++++++++++ .../codehaus/groovy/control/ResolveVisitor.java | 26 +++++++++++++ src/test/groovy/groovy/ImportTest.groovy | 30 +++++++++++++++ 5 files changed, 127 insertions(+), 17 deletions(-) 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 5cf282905c..191013c3f8 100644 --- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java +++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java @@ -434,26 +434,17 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { if (moduleRef == null) { throw createParsingFailedException("Unknown module: " + moduleName, ctx); } - Set<String> skip = new HashSet<>(Arrays.asList( - org.codehaus.groovy.control.ResolveVisitor.DEFAULT_IMPORTS)); - moduleNode.getStarImports().stream().map(ImportNode::getPackageName).forEach(skip::add); - ImportNode lastImport = null; - // Collect exported packages from this module and its transitive dependencies + // Collect exported packages from this module and its transitive dependencies. + // Unlike the eager approach, we do NOT expand these into star imports here — + // the module import stays first-class in the AST, and ResolveVisitor consults + // it as a distinct (lowest-precedence) tier per JLS 6.4.1. Set<String> visited = new HashSet<>(); List<String> packageNames = new ArrayList<>(); collectModuleExports(moduleRef, finder, packageNames, visited); - for (String pkg : packageNames) { - String packageName = pkg + DOT_STR; - if (!skip.contains(packageName)) { - moduleNode.addStarImport(packageName, annotations); - lastImport = last(moduleNode.getStarImports()); - skip.add(packageName); - } - } - if (lastImport == null) { - // All exported packages were already covered by existing imports - lastImport = last(moduleNode.getStarImports()); - } + List<String> packagesWithDot = new ArrayList<>(packageNames.size()); + for (String pkg : packageNames) packagesWithDot.add(pkg + DOT_STR); + moduleNode.addModuleImport(moduleName, packagesWithDot, annotations); + ImportNode lastImport = last(moduleNode.getModuleImports()); return configureAST(lastImport, ctx); } diff --git a/src/main/java/org/codehaus/groovy/ast/ImportNode.java b/src/main/java/org/codehaus/groovy/ast/ImportNode.java index 02b341e795..94d442e921 100644 --- a/src/main/java/org/codehaus/groovy/ast/ImportNode.java +++ b/src/main/java/org/codehaus/groovy/ast/ImportNode.java @@ -18,6 +18,8 @@ */ package org.codehaus.groovy.ast; +import java.util.Collections; +import java.util.List; import java.util.Objects; /** @@ -31,6 +33,8 @@ public class ImportNode extends AnnotatedNode { private final String packageName; private final boolean isStar; private final boolean isStatic; + private final String moduleName; // non-null for module imports + private final List<String> modulePackages; // exported packages of the module private transient int hashCode; /** @@ -46,6 +50,8 @@ public class ImportNode extends AnnotatedNode { this.isStatic = false; this.packageName = null; this.fieldName = null; + this.moduleName = null; + this.modulePackages = null; } /** @@ -60,6 +66,8 @@ public class ImportNode extends AnnotatedNode { this.isStatic = false; this.packageName = Objects.requireNonNull(packageName); this.fieldName = null; + this.moduleName = null; + this.modulePackages = null; } /** @@ -74,6 +82,8 @@ public class ImportNode extends AnnotatedNode { this.isStatic = true; this.packageName = null; this.fieldName = null; + this.moduleName = null; + this.modulePackages = null; } /** @@ -90,6 +100,37 @@ public class ImportNode extends AnnotatedNode { this.isStatic = true; this.packageName = null; this.fieldName = Objects.requireNonNull(fieldName); + this.moduleName = null; + this.modulePackages = null; + } + + /** + * An {@code import module M} declaration (JEP 476 / 511). + * + * @param moduleName the JPMS module name, e.g. {@code "java.sql"} + * @param modulePackages exported packages (with trailing dot), resolved at parse time + */ + public ImportNode(final String moduleName, final List<String> modulePackages) { + this.type = null; + this.alias = null; + this.isStar = false; + this.isStatic = false; + this.packageName = null; + this.fieldName = null; + this.moduleName = Objects.requireNonNull(moduleName); + this.modulePackages = Collections.unmodifiableList(modulePackages); + } + + public boolean isModule() { + return moduleName != null; + } + + public String getModuleName() { + return moduleName; + } + + public List<String> getModulePackages() { + return modulePackages; } /** @@ -97,6 +138,9 @@ public class ImportNode extends AnnotatedNode { */ @Override public String getText() { + if (isModule()) { + return "import module " + moduleName; + } String simpleName = getAlias(); String memberName = getFieldName(); diff --git a/src/main/java/org/codehaus/groovy/ast/ModuleNode.java b/src/main/java/org/codehaus/groovy/ast/ModuleNode.java index 6c0760ba69..4d31d2135f 100644 --- a/src/main/java/org/codehaus/groovy/ast/ModuleNode.java +++ b/src/main/java/org/codehaus/groovy/ast/ModuleNode.java @@ -70,6 +70,7 @@ public class ModuleNode extends ASTNode { private final List<MethodNode> methods = new ArrayList<>(); private final List<ImportNode> imports = new ArrayList<>(); private final List<ImportNode> starImports = new ArrayList<>(); + private final List<ImportNode> moduleImports = new ArrayList<>(); private final Map<String, ImportNode> staticImports = new LinkedHashMap<>(); private final Map<String, ImportNode> staticStarImports = new LinkedHashMap<>(); private CompileUnit unit; @@ -183,6 +184,24 @@ public class ModuleNode extends ASTNode { storeLastAddedImportNode(importNode); } + /** + * @return {@code import module M} declarations (JEP 511). + * Separate from {@link #getStarImports()} so resolution can apply + * JLS 6.4.1 shadowing (module-import is a lower tier than on-demand). + */ + public List<ImportNode> getModuleImports() { + return moduleImports; + } + + public void addModuleImport(final String moduleName, final List<String> packages, + final List<AnnotationNode> annotations) { + ImportNode node = new ImportNode(moduleName, packages); + node.addAnnotations(annotations); + moduleImports.add(node); + + storeLastAddedImportNode(node); + } + public void addStaticImport(final ClassNode type, final String memberName, final String simpleName) { addStaticImport(type, memberName, simpleName, Collections.emptyList()); } diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java index ec4335ceaf..3925af240d 100644 --- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java +++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java @@ -768,6 +768,32 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer { } } } + // Module imports — lowest precedence per JLS 6.4.1. + // Ambiguity between two module-sourced packages is a compile-time + // error (JLS 7.5.5 Example 7.5.5-3), unlike regular star imports. + ClassNode moduleMatch = null; + String moduleMatchPkg = null; + for (ImportNode moduleImport : module.getModuleImports()) { + for (String pkg : moduleImport.getModulePackages()) { + ClassNode tmp = new ConstructedClassWithPackage(pkg, name); + if (resolve(tmp, false, false, true)) { + ClassNode resolved = tmp.redirect(); + if (moduleMatch != null && !moduleMatch.getName().equals(resolved.getName())) { + addError("reference to " + name + " is ambiguous, both " + + moduleMatch.getName() + " (from " + moduleMatchPkg + ")" + + " and " + resolved.getName() + " (from " + pkg + ") match", + type); + return true; + } + moduleMatch = resolved; + moduleMatchPkg = pkg; + } + } + } + if (moduleMatch != null) { + type.setRedirect(moduleMatch); + return true; + } } return false; diff --git a/src/test/groovy/groovy/ImportTest.groovy b/src/test/groovy/groovy/ImportTest.groovy index 8d07b5c03b..ada46a45a9 100644 --- a/src/test/groovy/groovy/ImportTest.groovy +++ b/src/test/groovy/groovy/ImportTest.groovy @@ -272,4 +272,34 @@ final class ImportTest { assert module.toUpperCase() == 'HELLO' ''' } + + // GROOVY-11896: JLS 7.5.5 Example 7.5.5-3 — when a module import exposes + // two packages that both contain a public type with the same simple name, + // using that simple name must be a compile-time error, not a silent pick. + // Here, java.desktop exports both javax.swing.text (with Element interface) + // and javax.swing.text.html.parser (with Element class). + @Test + void testImportModuleAmbiguousSimpleName() { + def err = shouldFail ''' + import module java.desktop + Element e = null + ''' + assert err.message.contains('ambiguous'), + "expected ambiguity error, got: ${err.message}" + } + + // GROOVY-11896: JLS 6.4.1 — a type-import-on-demand declaration shadows + // types imported by a single-module-import declaration. The `import module` + // is written FIRST so that without proper tier separation, the module- + // expanded java.awt.* would win by list ordering. With correct shadowing, + // the user's `import java.util.*` wins regardless of source order, and + // List resolves to java.util.List rather than java.awt.List. + @Test + void testImportModuleShadowedByStarImport() { + assertScript ''' + import module java.desktop + import java.util.* + assert List.name == 'java.util.List' + ''' + } }
