You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by jo...@apache.org on 2026/04/15 20:02:30 UTC
(groovy) 01/01: Defer module name resolution until later
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 <jc...@adaptavist.com>
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'
+ '''
+ }
}