gnodet-bot commented on code in PR #610:
URL: https://github.com/apache/maven-jar-plugin/pull/610#discussion_r4224696041
##########
src/main/java/org/apache/maven/plugins/jar/ToolExecutor.java:
##########
@@ -69,6 +69,108 @@ final class ToolExecutor {
*/
private static final String CREATED_BY = "Created-By";
+ /**
+ * Sanitizes a candidate automatic module name by replacing all characters
that are neither
+ * valid Java identifier start/part characters nor {@code '.'} separators,
then collapsing
+ * consecutive separators and stripping leading/trailing dots.
+ *
+ * <p>Unlike the JDK's {@code ModulePath.cleanModuleName()} which is
ASCII-only, this method
+ * accepts any character that {@link Character#isJavaIdentifierStart} or
+ * {@link Character#isJavaIdentifierPart} considers valid, including
supplementary Unicode
+ * code points (the encoding of {@code MANIFEST.MF} files is UTF-8).</p>
+ *
+ * @param name the raw, potentially invalid module name
+ * @return the sanitized name, or an empty string if nothing remains after
sanitization
+ */
+ private static String cleanModuleName(final String name) {
+ final int length = name.length();
+ final var b = new StringBuilder(length);
+ boolean wantStart = true;
+ int n;
+ for (int i = 0; i < length; i += n) {
+ final int c = name.codePointAt(i);
+ n = Character.charCount(c);
+ if (wantStart) {
+ if (!Character.isJavaIdentifierStart(c)) {
+ continue;
+ }
+ wantStart = false;
+ } else {
+ if (!Character.isJavaIdentifierPart(c)) {
+ wantStart = true;
+ b.append('.');
+ continue;
+ }
+ }
+ b.appendCodePoint(c);
+ }
+ int end = b.length() - 1;
+ if (end >= 0 && b.charAt(end) == '.') {
+ b.setLength(end);
+ }
+ return b.toString();
+ }
+
+ /**
+ * Validates the {@code Automatic-Module-Name} attribute in the given
manifest.
+ * If the attribute is absent or already valid, this method does nothing
and returns {@code false}.
+ *
+ * <p>If the name is invalid and was explicitly declared in the POM via
+ * {@code <archive><manifestEntries><Automatic-Module-Name>}, the build
fails immediately
+ * (the developer chose that name and must fix it).</p>
+ *
+ * <p>If the name is invalid and was read from a {@code MANIFEST.MF} file
(e.g. from the
+ * compiled output directory), {@link #cleanModuleName(String)} is applied
and a warning is
+ * logged. If the sanitized name is still invalid the attribute is
removed and a warning is
+ * logged (MJAR-596).</p>
+ *
+ * @param manifest the merged manifest to inspect and potentially modify
+ * @return {@code true} if the manifest was modified and a temporary
manifest file must be written
+ * @throws MojoException if the name was explicitly declared in POM
configuration and is invalid
+ */
+ private boolean sanitizeAutomaticModuleName(Manifest manifest) {
+ String name =
manifest.getMainAttributes().getValue("Automatic-Module-Name");
+ if (name == null || SourceVersion.isName(name)) {
+ return false;
+ }
+ /*
+ * If the invalid name originates from an explicit
<archive><manifestEntries> declaration
+ * in the POM, fail the build so that the developer is alerted
immediately — they set an
+ * invalid name on purpose and should fix it. This matches the
historical maven-archiver
+ * behaviour (MJAR-260).
+ *
+ * If the name was read from a MANIFEST.MF file (either from the
compiled output directory
+ * or from <archive><manifestFile>), apply cleanModuleName() to
replace characters that are
+ * not valid Java identifier start/part characters with '.'
separators. This handles the
+ * common case where a project sets Automatic-Module-Name to
+ * "${project.groupId}.${project.artifactId}" and the artifactId
contains hyphens (MJAR-596).
+ */
+ String pluginName = (manifestFromPlugin != null)
+ ?
manifestFromPlugin.getMainAttributes().getValue("Automatic-Module-Name")
+ : null;
+ if (name.equals(pluginName)) {
+ throw new MojoException("Invalid automatic module name: \"" + name
+ "\".");
+ }
+ String sanitized = cleanModuleName(name);
+ if (!sanitized.isEmpty() && SourceVersion.isName(sanitized)) {
+ logger.warn("Automatic-Module-Name \"" + name + "\" is not a valid
Java module name."
+ + " It has been sanitized to \"" + sanitized + "\""
+ + " using the same algorithm the JDK uses to derive
automatic module names"
+ + " from JAR file names."
+ + " Consider setting a valid name explicitly"
Review Comment:
💡 **Nit:** The warning says "using the same algorithm the JDK uses" but the
Javadoc directly above (`cleanModuleName`) says _"Unlike the JDK's
`ModulePath.cleanModuleName()` which is ASCII-only"_. The algorithm was
intentionally broadened — the warning text should reflect that.
```suggestion
logger.warn("Automatic-Module-Name \"" + name + "\" is not a
valid Java module name."
+ " It has been sanitized to \"" + sanitized + "\""
+ " using an algorithm similar to the one the JDK uses
to derive automatic module names"
+ " from JAR file names."
```
--
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]
