Skip to content

Commit 75d9302

Browse files
jonnybot0paulk-asert
authored andcommitted
GROOVY-11896: Adjust Module import resolution
This better accommodates single-type > type-on-demand > module-import precedence of Java. See [JLS 6.4.1](https://docs.oracle.com/javase/specs/jls/se25/html/jls-6.html#jls-6.4) This also lays a little bit of groundwork for https://issues.apache.org/jira/browse/GROOVY-11916 if/when we action that.
1 parent 1024ef7 commit 75d9302

4 files changed

Lines changed: 91 additions & 11 deletions

File tree

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -410,14 +410,13 @@ public ImportNode visitImportDeclaration(final ImportDeclarationContext ctx) {
410410
* {@code module-info.class}) are supported — all packages in the JAR
411411
* are imported since automatic modules have no explicit exports.
412412
* <p>
413-
* Known differences from Java's module import behavior:
414-
* <ul>
415-
* <li>Ambiguous class names from multiple module imports silently resolve
416-
* to the last match, consistent with Groovy's existing star import
417-
* semantics. Java reports a compile-time error for such ambiguities.</li>
418-
* <li>Explicit single-type imports take priority over module-expanded
419-
* star imports (same as Java).</li>
420-
* </ul>
413+
* As per JLS 6.4.1, explicit single-type imports and type-on-demand (star)
414+
* imports take priority over module-expanded imports. Ambiguous class names
415+
* from multiple module imports produce a compile-time error (JLS 7.5.5).
416+
* <p>
417+
* Known difference from Java: Groovy's star imports (including module-expanded
418+
* ones) can resolve package-private types, whereas Java only exposes public
419+
* types from exported packages (see GROOVY-11916).
421420
*/
422421
private ImportNode expandModuleImport(final String moduleName, final List<AnnotationNode> annotations, final ImportDeclarationContext ctx) {
423422
var finder = ModuleImportHelper.moduleFinder(sourceUnit);
@@ -430,18 +429,22 @@ private ImportNode expandModuleImport(final String moduleName, final List<Annota
430429
Set<String> skip = new HashSet<>(Arrays.asList(
431430
org.codehaus.groovy.control.ResolveVisitor.DEFAULT_IMPORTS));
432431
moduleNode.getStarImports().stream().map(ImportNode::getPackageName).forEach(skip::add);
432+
moduleNode.getModuleStarImports().stream().map(ImportNode::getPackageName).forEach(skip::add);
433433
ImportNode lastImport = null;
434434
for (String pkg : packageNames) {
435435
String packageName = pkg + DOT_STR;
436436
if (!skip.contains(packageName)) {
437-
moduleNode.addStarImport(packageName, annotations);
438-
lastImport = last(moduleNode.getStarImports());
437+
// Separate list so resolution can apply JLS 6.4.1 shadowing:
438+
// a user-written `import foo.*` beats a module-expanded `foo.*`.
439+
moduleNode.addModuleStarImport(packageName, annotations);
440+
lastImport = last(moduleNode.getModuleStarImports());
439441
skip.add(packageName);
440442
}
441443
}
442444
if (lastImport == null) {
443445
// All exported packages were already covered by existing imports
444-
lastImport = last(moduleNode.getStarImports());
446+
List<ImportNode> existing = moduleNode.getModuleStarImports();
447+
lastImport = !existing.isEmpty() ? last(existing) : last(moduleNode.getStarImports());
445448
}
446449
return configureAST(lastImport, ctx);
447450
}

src/main/java/org/codehaus/groovy/ast/ModuleNode.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ public class ModuleNode extends ASTNode {
7070
private final List<MethodNode> methods = new ArrayList<>();
7171
private final List<ImportNode> imports = new ArrayList<>();
7272
private final List<ImportNode> starImports = new ArrayList<>();
73+
private final List<ImportNode> moduleStarImports = new ArrayList<>();
7374
private final Map<String, ImportNode> staticImports = new LinkedHashMap<>();
7475
private final Map<String, ImportNode> staticStarImports = new LinkedHashMap<>();
7576
private CompileUnit unit;
@@ -183,6 +184,23 @@ public void addStarImport(final String packageName, final List<AnnotationNode> a
183184
storeLastAddedImportNode(importNode);
184185
}
185186

187+
/**
188+
* @return star imports that originated from {@code import module M} declarations.
189+
* Separate from {@link #getStarImports()} so that resolution can apply
190+
* JLS 6.4.1 shadowing (type-import-on-demand shadows module-import).
191+
*/
192+
public List<ImportNode> getModuleStarImports() {
193+
return moduleStarImports;
194+
}
195+
196+
public void addModuleStarImport(final String packageName, final List<AnnotationNode> annotations) {
197+
ImportNode importNode = new ImportNode(packageName);
198+
importNode.addAnnotations(annotations);
199+
moduleStarImports.add(importNode);
200+
201+
storeLastAddedImportNode(importNode);
202+
}
203+
186204
public void addStaticImport(final ClassNode type, final String memberName, final String simpleName) {
187205
addStaticImport(type, memberName, simpleName, Collections.emptyList());
188206
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,31 @@ protected boolean resolveFromModule(final ClassNode type, final boolean testModu
768768
}
769769
}
770770
}
771+
// Module-expanded star imports — lowest precedence per JLS 6.4.1.
772+
// Only consulted if nothing above resolved the type, and ambiguity
773+
// between two module-expanded packages is a compile-time error
774+
// (matching Java's behavior per JLS 7.5.5 Example 7.5.5-3).
775+
ClassNode moduleMatch = null;
776+
String moduleMatchPkg = null;
777+
for (ImportNode importNode : module.getModuleStarImports()) {
778+
ClassNode tmp = new ConstructedClassWithPackage(importNode.getPackageName(), name);
779+
if (resolve(tmp, false, false, true)) {
780+
ClassNode resolved = tmp.redirect();
781+
if (moduleMatch != null && !moduleMatch.getName().equals(resolved.getName())) {
782+
addError("reference to " + name + " is ambiguous, both "
783+
+ moduleMatch.getName() + " (from " + moduleMatchPkg + ")"
784+
+ " and " + resolved.getName()
785+
+ " (from " + importNode.getPackageName() + ") match", type);
786+
return true;
787+
}
788+
moduleMatch = resolved;
789+
moduleMatchPkg = importNode.getPackageName();
790+
}
791+
}
792+
if (moduleMatch != null) {
793+
type.setRedirect(moduleMatch);
794+
return true;
795+
}
771796
}
772797

773798
return false;

src/test/groovy/groovy/ImportTest.groovy

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,4 +272,38 @@ final class ImportTest {
272272
assert module.toUpperCase() == 'HELLO'
273273
'''
274274
}
275+
276+
// GROOVY-11896: JLS 7.5.5 Example 7.5.5-3 — when a module import exposes
277+
// two packages that both contain a public type with the same simple name,
278+
// using that simple name must be a compile-time error, not a silent pick.
279+
// Here, java.desktop exports both javax.swing.text (with Element interface)
280+
// and javax.swing.text.html.parser (with Element class).
281+
@Test
282+
void testImportModuleAmbiguousSimpleName() {
283+
def err = shouldFail '''
284+
import module java.desktop
285+
Element e = null
286+
'''
287+
assert err.message.contains('ambiguous'),
288+
"expected ambiguity error, got: ${err.message}"
289+
}
290+
291+
// GROOVY-11896: JLS 6.4.1 — a type-import-on-demand declaration shadows
292+
// types imported by a single-module-import declaration. The `import module`
293+
// is written FIRST so that without proper tier separation, the module-
294+
// expanded java.awt.* would win by list ordering. With correct shadowing,
295+
// the user's `import java.util.*` wins regardless of source order, and
296+
// List resolves to java.util.List rather than java.awt.List.
297+
@Test
298+
void testImportModuleShadowedByStarImport() {
299+
assertScript '''
300+
import module java.desktop
301+
assert List.name == 'java.awt.List'
302+
'''
303+
assertScript '''
304+
import module java.desktop
305+
import java.util.*
306+
assert List.name == 'java.util.List'
307+
'''
308+
}
275309
}

0 commit comments

Comments
 (0)