[
https://issues.apache.org/jira/browse/GROOVY-11896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18073822#comment-18073822
]
ASF GitHub Bot commented on GROOVY-11896:
-----------------------------------------
jonnybot0 commented on PR #2429:
URL: https://github.com/apache/groovy/pull/2429#issuecomment-4255122812
I think this is good to merge, but I have found some other things we might
want to fix-forward. I'm going to note them here, but can open up another PR,
as they may be worth discussing separately and I don't want to be a blocker.
I've combed over this by doing some comparisons with the relevant JEPs
([476](https://openjdk.org/jeps/476), [494](https://openjdk.org/jeps/494),
[511](https://openjdk.org/jeps/511)) and the [Java Language Specification for
Module Import
Declarations](https://docs.oracle.com/en/java/javase/24/docs/specs/module-import-declarations-jls.html).
Besides importing the package-private stuff (which is fine and appropriately
backlogged), there are a handful of other differences with how Java handles
module imports.
# Issue 1: Ambiguous Imports
The most significant difference is how this would handle ambiguity between
module imports. In Java, this is a compile-time error, but Groovy will just
pick one silently and run with it. There are a few specific examples of this in
the Java specification (7.5.5,
https://docs.oracle.com/javase/specs/jls/se25/html/jls-7.html#jls-7.5.5). You
can reproduce these in jshell:
```
import module java.base;
import module java.desktop;
List l; // Error - Ambiguous name!
```
Granted, we're already more chill about static star imports than Java, so
maybe *not* throwing that error is the consistent thing? But given that module
support is new, I think we could go the more "conservative" way and be okay,
since no one is going to depend on the more lax behavior.
# Issue 2: Import priority
The Java Language specification specifies a priority for imports of
single-type > type-on-demand > module-import. See
[6.4.1](https://docs.oracle.com/javase/specs/jls/se25/html/jls-6.html#jls-6.4.1)
of the spec.
On this branch, if a user writes import foo.* and also import module M where
M exports foo, the list ordering decides which import wins, not the
specification precedence.
# Addressing the issues
There's a couple of ways Claude & I could hack up that would address these
two problems.
The first is a smaller change: simply make a separate list of module
imports, then resolve them later.
https://github.com/jonnybot0/groovy/pull/new/groovy11896-ammendments
The second is a bigger departure from this branch, and actually represents
the module imports within the AST.
https://github.com/jonnybot0/groovy/pull/new/groovy11896-bigger-refactor
Both approaches solve these two problems (as shown by having the same
passing test in their diff). I suspect that the second path is a better
direction to head. It looks like it would better support other aspects of and
expansions to the JPMS down the road. That said, it's a more significant
departure, and may have more knock-on effects; there's a lot of code using
`isStar() || isStatic()` that might want auditing. Also, it should probably be
benchmarked, as I suspect it could perform worse.
# Way Forward
I think the best approach is probably to merge this branch, then open a PR
from my second branch and take feedback on that separately. That said, it might
be quickest for @paulk-asert to just take the diff from the first branch,
incorporate it in, and then do a PR from something like the second branch at
our leisure, while still pocketing the win today.
Let me know what you think is best. Thanks for giving a generous period for
feedback!
> Support module import declarations
> ----------------------------------
>
> Key: GROOVY-11896
> URL: https://issues.apache.org/jira/browse/GROOVY-11896
> Project: Groovy
> Issue Type: New Feature
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> In Java, _module import declarations_ allow you to import all exported
> packages of a module with a single statement. It was introduced as a preview
> feature in Java 23 ([JEP 476|https://openjdk.org/jeps/476]) and finalized in
> Java 25 ([JEP 511|https://openjdk.org/jeps/511]).
> The syntax is as follows:
> {code:java}
> import module java.base; // Java
> {code}
> h3. Implementation highlights
> * Grammar extended with a dedicated import module <qualifiedName>
> alternative (no .* or as alias — rejected at parse time)
>
>
> * Modules resolved via ModuleFinder.compose(ofSystem(),
> classpathModuleFinder()) — system/JDK modules first, then modular JARs on the
> compilation classpath
>
> * Explicit JPMS modules (with module-info.class) import all unqualified
> exports; automatic modules (with Automatic-Module-Name manifest entry) import
> all packages
>
> * Default imports and existing star imports are skipped to avoid redundant
> resolution
>
>
> * module remains usable as an identifier (context-sensitive keyword, like
> record/sealed)
>
>
> Passing test scenarios:
> {code}
>
>
>
> // System module — JDK classes available without individual imports
> import module java.base
>
>
>
> LocalDate date = LocalDate.now()
> CountDownLatch latch = new CountDownLatch(1)
>
>
>
>
> // System module — java.sql
>
>
>
> import module java.sql
> assert Connection.name == 'java.sql.Connection'
>
>
>
> assert DriverManager.name == 'java.sql.DriverManager'
>
>
>
>
> // JPMS module from classpath (module-info.class)
>
>
>
> import module org.junit.jupiter.api
>
>
>
> assert Test.name == 'org.junit.jupiter.api.Test'
>
>
>
> assert DisabledIf.name == 'org.junit.jupiter.api.condition.DisabledIf'
>
>
>
>
> // Automatic module from classpath (Automatic-Module-Name in MANIFEST.MF)
>
>
>
> import module org.apache.groovy.jmx
>
>
>
> assert GroovyMBean.name == 'groovy.jmx.GroovyMBean'
>
>
>
>
>
>
>
> // Unknown module — compile error
> import module no.such.module // fails: "Unknown module: no.such.module"
>
>
>
>
>
>
>
> // Star/alias not permitted with module imports — parse error
>
>
>
> import module java.base.* // fails
>
>
>
> import module java.base as jb // fails
>
>
>
>
>
>
>
> // 'module' still works as an identifier
> def module = 'hello'
>
>
>
> assert module.toUpperCase() == 'HELLO'
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
