elharo commented on code in PR #996:
URL: https://github.com/apache/maven-enforcer/pull/996#discussion_r3813299014


##########
enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/modules/AbstractModuleInfoRule.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.io.InputStream;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rules.AbstractStandardEnforcerRule;
+import org.apache.maven.project.MavenProject;
+
+/**
+ * Base class for rules that inspect the {@code module-info.class} of the 
project's main output.
+ * Two output layouts are supported:
+ *
+ * <ul>
+ *   <li><b>Classic</b>: the descriptor sits directly in {@code 
${project.build.outputDirectory}}
+ *       (one Maven project = one Java module);</li>
+ *   <li><b>Module source hierarchy</b> (Maven&nbsp;4, POM model 4.1.0): one 
Maven project compiles
+ *       several modules, each to its own subdirectory
+ *       {@code 
${project.build.outputDirectory}/<module-name>/module-info.class}.</li>
+ * </ul>
+ *
+ * Subclasses call {@link #moduleOutputs()} and enforce a specific constraint 
on each returned
+ * {@link ModuleOutput}.
+ */
+abstract class AbstractModuleInfoRule extends AbstractStandardEnforcerRule {

Review Comment:
   If the subclasses are public this should probably be public too unless 
there's a reason otherwise I'm. not seeing?



##########
enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/modules/JavaModuleInfo.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.util.Collections;
+import java.util.List;
+
+/**
+ * Immutable view of the {@code Module} attribute of a {@code 
module-info.class}:

Review Comment:
   This is supposed to be immutable but it exposes potentially mutable List 
fields through the getter methods. 



##########
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&nbsp;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&nbsp;9 — is deliberately <em>not</em> used: 
multi-release JAR support
+ * in Maven&nbsp;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&nbsp;4 
(POM model 4.1.0 +
+ * {@code maven-compiler-plugin} 4.0.0-beta-3). To keep these rules usable on 
<b>Maven&nbsp;3</b>
+ * and a Java&nbsp;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&nbsp;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&nbsp;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:
   don't we or the JDK or apache commons have a utility method for this 
somewhere?>



##########
enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/modules/RequireExplicitModules.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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 javax.inject.Inject;
+import javax.inject.Named;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Stream;
+
+import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.project.MavenProject;
+
+/**
+ * Requires that a project with compiled classes is an <em>explicit</em> Java 
module, i.e. that its

Review Comment:
   run-on sentence
   also avoid latin ,like i.e. and cf.



##########
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&nbsp;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&nbsp;9 — is deliberately <em>not</em> used: 
multi-release JAR support
+ * in Maven&nbsp;3 is incomplete and can even produce invalid JARs (cf. 
MNG-6892 / MNG-6293 and

Review Comment:
   don't use nbsp



##########
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&nbsp;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&nbsp;9 — is deliberately <em>not</em> used: 
multi-release JAR support
+ * in Maven&nbsp;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&nbsp;4 
(POM model 4.1.0 +
+ * {@code maven-compiler-plugin} 4.0.0-beta-3). To keep these rules usable on 
<b>Maven&nbsp;3</b>
+ * and a Java&nbsp;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&nbsp;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&nbsp;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 {
+        ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+        byte[] chunk = new byte[8192];
+        int read;
+        while ((read = in.read(chunk)) != -1) {
+            buffer.write(chunk, 0, read);
+        }
+        return buffer.toByteArray();
+    }
+
+    /**
+     * Fail fast if the {@code module-info.class} was built for a newer Java 
release than the JVM
+     * running the enforcer, in which case {@code ModuleDescriptor.read} 
cannot parse it.
+     */
+    private static void checkReadableVersion(byte[] classFile) throws 
IOException {
+        if (classFile.length < 8
+                || (classFile[0] & 0xFF) != 0xCA
+                || (classFile[1] & 0xFF) != 0xFE
+                || (classFile[2] & 0xFF) != 0xBA
+                || (classFile[3] & 0xFF) != 0xBE) {
+            throw new IOException("Not a Java class file (bad magic)");

Review Comment:
   bad magic --> bad magic number



-- 
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]

Reply via email to