Add Java module-info rules: requireExplicitModules, requireMinimalExports, banUnjustifiedOpens - #996
Add Java module-info rules: requireExplicitModules, requireMinimalExports, banUnjustifiedOpens#996ascheman wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds three new built-in Maven Enforcer rules that enforce deterministic Java Platform Module System (JPMS) descriptor policies by reading the compiled module-info.class (including Maven 4 “module source hierarchy” outputs), along with unit tests, integration tests, and site documentation to make the new rules usable and discoverable.
Changes:
- Implement new rules:
requireExplicitModules,requireMinimalExports, andbanUnjustifiedOpens(backed by a reflectiveModuleDescriptorreader compatible with a Java 8 compilation baseline). - Add unit tests (Java 9+ gated) and multiple Maven Invoker IT projects covering pass/fail cases and Maven 4 module-source-hierarchy scenarios.
- Add site pages for the rules and index entries; add ASM as a test-scope dependency for generating
module-info.classfixtures.
Reviewed changes
Copilot reviewed 53 out of 53 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| maven-enforcer-plugin/src/it/projects/require-minimal-exports-fail/verify.groovy | IT verifier asserting requireMinimalExports fails with expected diagnostics. |
| maven-enforcer-plugin/src/it/projects/require-minimal-exports-fail/src/main/java/module-info.java | IT module descriptor that improperly exports an internal package to trigger failure. |
| maven-enforcer-plugin/src/it/projects/require-minimal-exports-fail/src/main/java/com/example/it/internal/Impl.java | IT internal type used to populate an internal package. |
| maven-enforcer-plugin/src/it/projects/require-minimal-exports-fail/src/main/java/com/example/it/api/Service.java | IT public API type for a “mixed exports” module. |
| maven-enforcer-plugin/src/it/projects/require-minimal-exports-fail/pom.xml | IT build configuring requireMinimalExports bound after compilation. |
| maven-enforcer-plugin/src/it/projects/require-minimal-exports-fail/invoker.properties | IT gating (JDK 11+) and expected failure configuration. |
| maven-enforcer-plugin/src/it/projects/require-explicit-modules-fail/verify.groovy | IT verifier asserting requireExplicitModules fails with expected diagnostics. |
| maven-enforcer-plugin/src/it/projects/require-explicit-modules-fail/src/main/java/com/example/it/App.java | IT compiled class without module-info to force “automatic module” detection. |
| maven-enforcer-plugin/src/it/projects/require-explicit-modules-fail/pom.xml | IT build configuring requireExplicitModules bound after compilation. |
| maven-enforcer-plugin/src/it/projects/require-explicit-modules-fail/invoker.properties | IT gating (JDK 11+) and expected failure configuration. |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-pass/verify.groovy | IT verifier for Maven 4 module source hierarchy “pass” case. |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-pass/src/com.example.core/main/java/module-info.java | Core module descriptor for Maven 4 hierarchy “pass” case. |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-pass/src/com.example.core/main/java/com/example/core/service/Service.java | Core module compiled class for hierarchy test. |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-pass/src/com.example.api/main/java/module-info.java | API module descriptor for Maven 4 hierarchy “pass” case. |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-pass/src/com.example.api/main/java/com/example/api/Api.java | API module compiled class for hierarchy test. |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-pass/pom.xml | Maven 4 (model 4.1.0) IT project using module source hierarchy and all three rules. |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-pass/invoker.properties | IT gating (Maven 4 + JDK 17+) and expected success configuration. |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-fail/verify.groovy | IT verifier for Maven 4 module source hierarchy “fail” case. |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-fail/src/com.example.core/main/java/module-info.java | Core module descriptor exporting an internal package to force failure. |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-fail/src/com.example.core/main/java/com/example/core/internal/Impl.java | Core internal type for hierarchy failure scenario. |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-fail/src/com.example.api/main/java/module-info.java | API module descriptor for hierarchy failure scenario (expected to remain clean). |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-fail/src/com.example.api/main/java/com/example/api/Api.java | API module compiled class for hierarchy failure scenario. |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-fail/pom.xml | Maven 4 (model 4.1.0) IT project configured to fail requireMinimalExports. |
| maven-enforcer-plugin/src/it/projects/module-source-hierarchy-fail/invoker.properties | IT gating (Maven 4 + JDK 17+) and expected failure configuration. |
| maven-enforcer-plugin/src/it/projects/module-info-rules-pass/verify.groovy | IT verifier for a classic single-module project passing all three rules. |
| maven-enforcer-plugin/src/it/projects/module-info-rules-pass/src/main/java/module-info.java | “Pass” module descriptor with qualified opens ... to. |
| maven-enforcer-plugin/src/it/projects/module-info-rules-pass/src/main/java/com/example/it/internal/Helper.java | Internal package type (not exported/opened) for encapsulation test. |
| maven-enforcer-plugin/src/it/projects/module-info-rules-pass/src/main/java/com/example/it/dto/Dto.java | DTO type in a package opened only to a named framework module. |
| maven-enforcer-plugin/src/it/projects/module-info-rules-pass/src/main/java/com/example/it/api/Service.java | Public API type in exported package for “pass” scenario. |
| maven-enforcer-plugin/src/it/projects/module-info-rules-pass/pom.xml | IT build configuring all three module rules bound after compilation. |
| maven-enforcer-plugin/src/it/projects/module-info-rules-pass/invoker.properties | IT gating (JDK 11+) and expected success configuration. |
| maven-enforcer-plugin/src/it/projects/ban-unjustified-opens-fail/verify.groovy | IT verifier asserting banUnjustifiedOpens fails with expected diagnostics. |
| maven-enforcer-plugin/src/it/projects/ban-unjustified-opens-fail/src/main/java/module-info.java | IT module descriptor with an unqualified opens to trigger failure. |
| maven-enforcer-plugin/src/it/projects/ban-unjustified-opens-fail/src/main/java/com/example/it/dto/Dto.java | DTO type in unqualified opened package for failure scenario. |
| maven-enforcer-plugin/src/it/projects/ban-unjustified-opens-fail/src/main/java/com/example/it/api/Service.java | API type for the banUnjustifiedOpens failure IT. |
| maven-enforcer-plugin/src/it/projects/ban-unjustified-opens-fail/pom.xml | IT build configuring banUnjustifiedOpens bound after compilation. |
| maven-enforcer-plugin/src/it/projects/ban-unjustified-opens-fail/invoker.properties | IT gating (JDK 11+) and expected failure configuration. |
| enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/modules/RequireMinimalExportsTest.java | Unit tests for RequireMinimalExports including hierarchy scenarios and configuration toggles. |
| enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/modules/RequireExplicitModulesTest.java | Unit tests for RequireExplicitModules including hierarchy scenarios and custom message. |
| enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/modules/ModuleInfoFixtures.java | Test helper generating module-info.class fixtures (and dummy classes) via ASM. |
| enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/modules/JavaModuleInfoReaderTest.java | Unit tests for the reflective module descriptor reader, including “too new classfile” diagnostics. |
| enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/modules/BanUnjustifiedOpensTest.java | Unit tests for BanUnjustifiedOpens including open-module and hierarchy scenarios. |
| enforcer-rules/src/site/apt/requireMinimalExports.apt.vm | Site documentation for requireMinimalExports rule and configuration. |
| enforcer-rules/src/site/apt/requireExplicitModules.apt.vm | Site documentation for requireExplicitModules rule and configuration. |
| enforcer-rules/src/site/apt/index.apt | Rule index updated to list the three new module rules. |
| enforcer-rules/src/site/apt/banUnjustifiedOpens.apt.vm | Site documentation for banUnjustifiedOpens rule and configuration. |
| enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/modules/RequireMinimalExports.java | New requireMinimalExports rule implementation. |
| enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/modules/RequireExplicitModules.java | New requireExplicitModules rule implementation. |
| enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/modules/JavaModuleInfoReader.java | Reflective ModuleDescriptor-based reader for compiled module-info.class. |
| enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/modules/JavaModuleInfo.java | Immutable in-memory representation of module directives. |
| enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/modules/BanUnjustifiedOpens.java | New banUnjustifiedOpens rule implementation. |
| enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/modules/AbstractModuleInfoRule.java | Shared base for module-info-based rules including output layout detection. |
| enforcer-rules/pom.xml | Adds ASM dependency in test scope for fixture generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| List<String> result = new ArrayList<String>(requires.size()); | ||
| for (Object require : requires) { | ||
| result.add((String) name.invoke(require)); | ||
| } | ||
| return result; | ||
| } |
| List<JavaModuleInfo.Directive> result = new ArrayList<JavaModuleInfo.Directive>(entries.size()); | ||
| for (Object entry : entries) { | ||
| String pkg = (String) source.invoke(entry); | ||
| Set<?> to = (Set<?>) targets.invoke(entry); // empty for an unqualified directive | ||
| List<String> moduleTargets = new ArrayList<String>(to.size()); | ||
| for (Object target : to) { | ||
| moduleTargets.add((String) target); | ||
| } | ||
| result.add(new JavaModuleInfo.Directive(pkg, moduleTargets)); | ||
| } | ||
| return result; |
| public void setAllowedOpens(List<String> allowedOpens) { | ||
| this.allowedOpens = allowedOpens; | ||
| } |
| public void setAllowedExports(List<String> allowedExports) { | ||
| this.allowedExports = allowedExports; | ||
| } |
| static void writeDummyClass(File outputDirectory, String binaryName) throws IOException { | ||
| ClassWriter cw = new ClassWriter(0); | ||
| cw.visit(Opcodes.V1_8, Opcodes.ACC_PUBLIC, binaryName.replace('.', '/'), null, "java/lang/Object", null); | ||
| cw.visitEnd(); | ||
| Files.createDirectories(outputDirectory.toPath()); | ||
| Files.write(new File(outputDirectory, binaryName + ".class").toPath(), cw.toByteArray()); | ||
| } |
|
Addressed all five review findings in b82335d: sorted |
|
Successfully tested locally on the AssertJ build, see output logs at assertj/assertj#4331 (comment). |
| * Subclasses call {@link #moduleOutputs()} and enforce a specific constraint on each returned | ||
| * {@link ModuleOutput}. | ||
| */ | ||
| abstract class AbstractModuleInfoRule extends AbstractStandardEnforcerRule { |
There was a problem hiding this comment.
If the subclasses are public this should probably be public too unless there's a reason otherwise I'm. not seeing?
There was a problem hiding this comment.
Agreed — made AbstractModuleInfoRule public.
| import java.util.List; | ||
|
|
||
| /** | ||
| * Immutable view of the {@code Module} attribute of a {@code module-info.class}: |
There was a problem hiding this comment.
This is supposed to be immutable but it exposes potentially mutable List fields through the getter methods.
There was a problem hiding this comment.
Good catch. The lists were wrapped with Collections.unmodifiableList, but that is only a view over the caller's list, so a retained reference on the construction side could still mutate it. Switched both constructors to a defensive copy — Collections.unmodifiableList(new ArrayList<>(...)) — so the view is fully decoupled and truly immutable. (List.copyOf would be cleaner but is JDK 10; this class compiles with --release 8.)
| * reference {@code java.lang.module.ModuleDescriptor} (a Java 9 API) directly. The obvious | ||
| * alternative — a multi-release JAR overlay ({@code src/main/java9}) so the module-reading code | ||
| * could be compiled for Java 9 — is deliberately <em>not</em> used: multi-release JAR support | ||
| * in Maven 3 is incomplete and can even produce invalid JARs (cf. MNG-6892 / MNG-6293 and |
There was a problem hiding this comment.
Removed all entities from the rule javadoc (plain spaces now).
| } | ||
| } | ||
|
|
||
| private static byte[] readAllBytes(InputStream in) throws IOException { |
There was a problem hiding this comment.
don't we or the JDK or apache commons have a utility method for this somewhere?>
There was a problem hiding this comment.
InputStream.readAllBytes() is exactly it — but it landed in JDK 9, and this class compiles with --release 8 (the class javadoc explains why the module-reading path is kept on the Java 8 baseline), so it is not available at compile time. commons-io isn't a current dependency of enforcer-rules and pulling one in for a six-line drain felt heavier than the loop. I added a comment on the method noting this so the next reader doesn't wonder.
| || (classFile[1] & 0xFF) != 0xFE | ||
| || (classFile[2] & 0xFF) != 0xBA | ||
| || (classFile[3] & 0xFF) != 0xBE) { | ||
| throw new IOException("Not a Java class file (bad magic)"); |
There was a problem hiding this comment.
bad magic --> bad magic number
| import org.apache.maven.project.MavenProject; | ||
|
|
||
| /** | ||
| * Requires that a project with compiled classes is an <em>explicit</em> Java module, i.e. that its |
There was a problem hiding this comment.
run-on sentence
also avoid latin ,like i.e. and cf.
There was a problem hiding this comment.
Split the run-on sentence and removed every i.e./e.g./cf. from the rule javadoc.
Add a small reader that parses a module-info.class into an immutable JavaModuleInfo (name, open flag, requires, exports, opens). It delegates to java.lang.module.ModuleDescriptor via reflection so the code keeps the plugin's Java 8 source baseline, and guards against a class file newer than the running JVM with a clear diagnostic instead of a silent failure. ASM is added in test scope only, to generate module-info.class fixtures; production stays dependency-free. Part of apache#995. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Gerd Aschemann <ascheman@apache.org>
Add requireExplicitModules (no automatic-module fallback), requireMinimalExports (do not export internal/impl packages) and banUnjustifiedOpens (no unqualified opens or open module). They share an AbstractModuleInfoRule base whose moduleOutputs() supports both the classic layout (one module-info.class in the output directory) and the Maven 4 module source hierarchy (one module per output subdirectory). Rules bind after compile (e.g. process-classes) since they read the compiled module-info.class. Unit tests are guarded to Java 9+. Part of apache#995. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Gerd Aschemann <ascheman@apache.org>
Four classic single-module ITs (one pass, three fail) plus two Maven 4 module source hierarchy ITs (pass and fail). The classic ITs are gated to JDK 11+; the module-source-hierarchy ITs to Maven 4.0.0-rc-5+ and JDK 17+, so they skip cleanly on older toolchains. Each binds the enforce goal to process-classes so module-info.class exists when the rule runs. Part of apache#995. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Gerd Aschemann <ascheman@apache.org>
Add site pages for requireExplicitModules, requireMinimalExports and banUnjustifiedOpens, and list them in the built-in rules index. Each page notes the two supported output layouts and that the enforce execution must run after compile (e.g. process-classes). Part of apache#995. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Gerd Aschemann <ascheman@apache.org>
Sort requires/exports/opens (and their targets) read from ModuleDescriptor sets so diagnostics are stable across JDKs; null-normalize allowedOpens/allowedExports setters; write test fixture classes to package-directory layout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review feedback from Elliott Rusty Harold (elharo) on apache#996: - Make AbstractModuleInfoRule public (matches its public subclasses) - Defensive-copy the JavaModuleInfo lists (unmodifiableList over a copy) - Fix "bad magic" -> "bad magic number" in the class-file check - Drop entities and i.e./e.g./cf. from the rule javadoc - Note why readAllBytes is hand-rolled (Java 8 baseline) Part of apache#995. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Gerd Aschemann <ascheman@apache.org>
b82335d to
e9a4e35
Compare
|
@elharo Thanks for the review — addressed all six points in e9a4e35 (rebased onto current master, which also moved the three rule doc pages from apt to the new markdown site format). Reasoning for the two non-mechanical ones (defensive copy, hand-rolled |
Implements the three deterministic Java module rules proposed in #995.
Rules
requireExplicitModules— fails a project that compiles classes but has nomodule-info.class, so an artifact never silently resolves as an automatic module.requireMinimalExports— fails a module thatexportsan internal/impl package (configurableinternalPackagePattern,allowedExportswhitelist,ignoreQualifiedExports).banUnjustifiedOpens— fails an unqualifiedopens(or anopen module); qualifiedopens … toand anallowedOpenswhitelist pass.Each reads the compiled
module-info.class, so itsenforceexecution must run after compilation (e.g. bound toprocess-classes). They support both the classic layout (onemodule-info.classin the output directory) and the Maven 4 module source hierarchy (POM model 4.1.0), where one project compiles several modules, each to${project.build.outputDirectory}/<module-name>/.module-inforeader — design noteThe rules read
module-info.classthroughjava.lang.module.ModuleDescriptor, accessed reflectively. The plugin compiles with--release 8and so cannot reference the Java 9ModuleDescriptorAPI directly. The obvious alternative — a multi-release JAR overlay compiled for Java 9 — is deliberately avoided: MRJAR support in Maven 3 is incomplete and can produce invalid JARs (cf. MNG-6892 / MNG-6293,maven-jar-plugin#484), and is only cleanly solved in Maven 4. Reflection keeps the rules usable on Maven 3 with a Java 8 baseline: the API is present at runtime whenever amodule-info.classexists (such a project is necessarily built on Java 9+), and the rules do nothing when there is no descriptor. The reader also fails with a clear diagnostic — rather than silently — when amodule-info.classis a newer class-file version than the JVM running the build.Tests
module-info.classfixtures — production stays dependency-free).maven-enforcer-plugin/src/it/projects/: four classic single-module (one pass, three fail) plus two Maven 4 module-source-hierarchy (pass/fail), gated by JDK / Maven version so they skip cleanly on older toolchains.A follow-up rule for split-package detection across the module path is planned separately.