elharo commented on code in PR #996: URL: https://github.com/apache/maven-enforcer/pull/996#discussion_r3829908950
########## enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/modules/JavaModuleInfoReader.java: ########## @@ -0,0 +1,172 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.enforcer.rules.modules; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Set; + +/** + * Reads the {@code Module} attribute of a {@code module-info.class} (name, {@code requires}, + * {@code exports}, {@code opens}) by delegating to {@link java.lang.module.ModuleDescriptor}. + * + * <p><b>Design decision.</b> This plugin compiles with {@code --release 8}, so it cannot + * 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 (see MNG-6892 / MNG-6293 and + * {@code maven-jar-plugin#484}); it is only cleanly solved in Maven 4 (POM model 4.1.0 + + * {@code maven-compiler-plugin} 4.0.0-beta-3). To keep these rules usable on <b>Maven 3</b> + * and a Java 8 source baseline, we instead access {@code ModuleDescriptor} <b>reflectively</b> + * through this small wrapper class: the API is present at runtime whenever a + * {@code module-info.class} exists (such a project is necessarily built on Java 9+), and the + * rules simply do nothing when there is no module descriptor. See {@code apache/maven-enforcer#995}. + */ +final class JavaModuleInfoReader { + + private static final String MODULE_DESCRIPTOR = "java.lang.module.ModuleDescriptor"; + private static final String INVALID_DESCRIPTOR = "java.lang.module.InvalidModuleDescriptorException"; + + private JavaModuleInfoReader() {} + + /** + * Parse a {@code module-info.class}. + * + * @param in the class-file bytes of a {@code module-info.class} + * @return the parsed module info, or {@code null} if the bytes are not a valid module descriptor + * @throws IOException if the bytes cannot be read, or if {@code java.lang.module} is unavailable Review Comment: So the concern here is that we're checking Java 9+ code with a Java 9 JDK? Is this likely or even possible? Does the enforcer check code it doesn't build? I wonder if there's a better way to handle this. Maybe an earlier failure if you're trying to use these rules in a JDK 8 environment. ########## enforcer-rules/src/site/markdown/requireExplicitModules.md.vm: ########## @@ -0,0 +1,73 @@ +--- +title: Require Explicit Modules +author: + - Gerd Aschemann +date: 2026-07-16 +--- + +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +# Require Explicit Modules + +This rule requires that a project which compiles classes is an *explicit* Java module, i.e. that its main output contains a `module-info.class`. This prevents a modular application from silently consuming the artifact as an *automatic module* (a plain JAR placed on the module path), whose auto-derived name and "exports everything" semantics are unstable across releases. + +The rule does nothing for projects that compile no classes (e.g. `pom` aggregators, or a module that only carries resources), so it can be enabled across a whole build without special-casing. Review Comment: e.g. --> for example ########## enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/modules/ModuleInfoFixtures.java: ########## @@ -0,0 +1,130 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.enforcer.rules.modules; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.List; + +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.ModuleVisitor; +import org.objectweb.asm.Opcodes; + +/** + * Test-only helper that generates real {@code module-info.class} fixtures with ASM and writes them + * (plus optional dummy classes) into an output directory, so the module rules can be exercised against + * a {@code project.build.outputDirectory} exactly as they see it at build time. + */ +final class ModuleInfoFixtures { + + private ModuleInfoFixtures() {} + + static Builder module(String name) { + return new Builder(name, false); + } + + static Builder openModule(String name) { + return new Builder(name, true); + } + + /** Write an arbitrary (empty) compiled class so an output directory looks non-empty to a rule. */ + 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(); + File classFile = new File(outputDirectory, binaryName.replace('.', '/') + ".class"); + Files.createDirectories(classFile.getParentFile().toPath()); + Files.write(classFile.toPath(), cw.toByteArray()); + } + + static final class Builder { + private final String name; + private final boolean open; + private final List<String> requires = new ArrayList<>(); + private final List<Directive> exports = new ArrayList<>(); + private final List<Directive> opens = new ArrayList<>(); + + private Builder(String name, boolean open) { + this.name = name; + this.open = open; + } + + Builder requires(String module) { + requires.add(module); + return this; + } + + Builder exports(String packageName, String... targets) { + exports.add(new Directive(packageName, targets)); + return this; + } + + Builder opens(String packageName, String... targets) { + opens.add(new Directive(packageName, targets)); + return this; + } + + byte[] toBytes() { + ClassWriter cw = new ClassWriter(0); Review Comment: cw --> classWriter ########## enforcer-rules/src/site/markdown/requireExplicitModules.md.vm: ########## @@ -0,0 +1,73 @@ +--- +title: Require Explicit Modules +author: + - Gerd Aschemann +date: 2026-07-16 +--- + +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +# Require Explicit Modules + +This rule requires that a project which compiles classes is an *explicit* Java module, i.e. that its main output contains a `module-info.class`. This prevents a modular application from silently consuming the artifact as an *automatic module* (a plain JAR placed on the module path), whose auto-derived name and "exports everything" semantics are unstable across releases. Review Comment: avoid i,.e. and split sentence ########## enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/modules/RequireExplicitModulesTest.java: ########## @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.enforcer.rules.modules; + +import java.io.File; + +import org.apache.maven.enforcer.rule.api.EnforcerLogger; +import org.apache.maven.enforcer.rule.api.EnforcerRuleException; +import org.apache.maven.model.Build; +import org.apache.maven.project.MavenProject; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledForJreRange; +import org.junit.jupiter.api.condition.JRE; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +// Reads module-info via java.lang.module.ModuleDescriptor, which exists only on Java 9+. +@EnabledForJreRange(min = JRE.JAVA_9) +class RequireExplicitModulesTest { + + @TempDir + File outputDirectory; + + private final MavenProject project = mock(MavenProject.class); + private RequireExplicitModules rule; + + @BeforeEach + void setUp() { + Build build = mock(Build.class); + when(build.getOutputDirectory()).thenReturn(outputDirectory.getAbsolutePath()); + when(project.getBuild()).thenReturn(build); + rule = new RequireExplicitModules(project); + rule.setLog(mock(EnforcerLogger.class)); Review Comment: might not have to use a mock here ########## enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/modules/JavaModuleInfoReader.java: ########## @@ -0,0 +1,170 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.enforcer.rules.modules; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Set; + +/** + * Reads the {@code Module} attribute of a {@code module-info.class} (name, {@code requires}, + * {@code exports}, {@code opens}) by delegating to {@link java.lang.module.ModuleDescriptor}. + * + * <p><b>Design decision.</b> This plugin compiles with {@code --release 8}, so it cannot + * 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 + * {@code maven-jar-plugin#484}); it is only cleanly solved in Maven 4 (POM model 4.1.0 + + * {@code maven-compiler-plugin} 4.0.0-beta-3). To keep these rules usable on <b>Maven 3</b> + * and a Java 8 source baseline, we instead access {@code ModuleDescriptor} <b>reflectively</b> + * through this small wrapper class: the API is present at runtime whenever a + * {@code module-info.class} exists (such a project is necessarily built on Java 9+), and the + * rules simply do nothing when there is no module descriptor. See {@code apache/maven-enforcer#995}. + */ +final class JavaModuleInfoReader { + + private static final String MODULE_DESCRIPTOR = "java.lang.module.ModuleDescriptor"; + private static final String INVALID_DESCRIPTOR = "java.lang.module.InvalidModuleDescriptorException"; + + private JavaModuleInfoReader() {} + + /** + * Parse a {@code module-info.class}. + * + * @param in the class-file bytes of a {@code module-info.class} + * @return the parsed module info, or {@code null} if the bytes are not a valid module descriptor + * @throws IOException if the bytes cannot be read, or if {@code java.lang.module} is unavailable + * (i.e. running on a Java 8 runtime) + */ + static JavaModuleInfo read(InputStream in) throws IOException { + byte[] classFile = readAllBytes(in); + // ModuleDescriptor.read cannot parse a class file newer than the running JVM; without this + // check it would throw InvalidModuleDescriptorException and we would wrongly treat the module + // as "not present". Surface a clear diagnostic instead. + checkReadableVersion(classFile); + try { + Class<?> descriptorType = Class.forName(MODULE_DESCRIPTOR); + Object descriptor = descriptorType + .getMethod("read", InputStream.class) + .invoke(null, new ByteArrayInputStream(classFile)); + + String name = (String) descriptorType.getMethod("name").invoke(descriptor); + boolean open = (Boolean) descriptorType.getMethod("isOpen").invoke(descriptor); + List<String> requires = requireNames(descriptor, descriptorType); + List<JavaModuleInfo.Directive> exports = directives(descriptor, descriptorType, "exports", "Exports"); + List<JavaModuleInfo.Directive> opens = directives(descriptor, descriptorType, "opens", "Opens"); + return new JavaModuleInfo(name, open, requires, exports, opens); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause != null && INVALID_DESCRIPTOR.equals(cause.getClass().getName())) { + return null; // not a valid module-info.class + } + throw new IOException("Could not read module descriptor", cause != null ? cause : e); + } catch (ClassNotFoundException | NoSuchMethodException e) { + throw new IOException( + "java.lang.module is not available; reading module-info requires a Java 9+ runtime", e); + } catch (ReflectiveOperationException e) { + throw new IOException("Could not read module descriptor", e); + } + } + + private static byte[] readAllBytes(InputStream in) throws IOException { Review Comment: as above, I'm not sure we actually need these rules in a Java 8 environment. I think requiring Java 9+ for these rules (though not the entire enforcer) is reasonable. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
