Restore the -Xlint category on in-process javac warnings

plexus-compiler-its/src/main/it/missing-warnings-in-process/invoker.properties

@@ -0,0 +1,19 @@
+# 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.
+
+invoker.goals = clean compile
+#invoker.buildResult = failure

plexus-compiler-its/src/main/it/missing-warnings-in-process/pom.xml

@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+	<modelVersion>4.0.0</modelVersion>
+
+	<groupId>org.apache.maven.plugins.compiler.it</groupId>
+	<artifactId>missing-warnings-in-process</artifactId>
+	<version>1.0-SNAPSHOT</version>
+
+	<build>
+		<pluginManagement>
+			<plugins>
+				<plugin>
+					<groupId>org.apache.maven.plugins</groupId>
+					<artifactId>maven-compiler-plugin</artifactId>
+					<version>@maven.compiler.version@</version>
+					<configuration>
+						<showWarnings>true</showWarnings>
+					</configuration>
+					<dependencies>
+						<dependency>
+							<groupId>org.codehaus.plexus</groupId>
+							<artifactId>plexus-compiler-api</artifactId>
+							<version>${plexus.compiler.version}</version>
+						</dependency>
+						<dependency>
+							<groupId>org.codehaus.plexus</groupId>
+							<artifactId>plexus-compiler-manager</artifactId>
+							<version>${plexus.compiler.version}</version>
+						</dependency>
+						<dependency>
+							<groupId>org.codehaus.plexus</groupId>
+							<artifactId>plexus-compiler-javac</artifactId>
+							<version>${plexus.compiler.version}</version>
+						</dependency>
+					</dependencies>
+				</plugin>
+			</plugins>
+		</pluginManagement>
+	</build>
+
+	<properties>
+		<maven.compiler.source>11</maven.compiler.source>
+		<maven.compiler.target>11</maven.compiler.target>
+		<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+		<plexus.compiler.version>@pom.version@</plexus.compiler.version>
+	</properties>
+</project>

plexus-compiler-its/src/main/it/missing-warnings-in-process/src/main/java/com/company/SomeClass.java

@@ -0,0 +1,5 @@
+package com.company;
+
+public class SomeClass
+{
+}

plexus-compiler-its/src/main/it/missing-warnings-in-process/src/main/java/module-info.java

@@ -0,0 +1,3 @@
+module testcase {
+	exports com.company to someOtherModule;
+}

plexus-compiler-its/src/main/it/missing-warnings-in-process/verify.groovy

@@ -0,0 +1,24 @@
+/*
+ * 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.
+ */
+def logFile = new File( basedir, 'build.log' )
+assert logFile.exists()
+content = logFile.text.normalize()
+
+assert content.contains( "module-info.java:[2,32] [module] module not found: someOtherModule")
+//assert content.contains( "exports com.company to someOtherModule;" )

plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavaxToolsCompiler.java

@@ -165,6 +165,10 @@ public CompilerResult compileInProcess(String[] args, final CompilerConfiguratio
                             }
                         }
                     }
+                    String lintCategory = lintCategoryOf(diagnostic, baseMessage);
+                    if (lintCategory != null && !formattedMessage.startsWith(lintCategory)) {
+                        formattedMessage = lintCategory + " " + formattedMessage;
+                    }
                     compilerMsgs.add(new CompilerMessage(
                             longFileName, kind, lineNumber, columnNumber, lineNumber, columnNumber, formattedMessage));
                 }
@@ -182,6 +186,62 @@ public CompilerResult compileInProcess(String[] args, final CompilerConfiguratio
         }
     }
 
+    /**
+     * Returns the bracketed {@code -Xlint} category javac prints in front of a message, for example
+     * {@code [deprecation]}, or {@code null} when the message carries none.
+     * <p>
+     * {@link Diagnostic#getMessage(java.util.Locale)} leaves the category out, and the JDK considers that
+     * intentional (<a href="https://bugs.openjdk.org/browse/JDK-8292634">JDK-8292634</a>), so it is read back
+     * from {@link Diagnostic#toString()}. The forked compiler keeps the category because it parses javac's
+     * own output, and without this the two back-ends disagree.
+     * <p>
+     * The category is located by anchoring on the message rather than on the {@code warning:} marker in front
+     * of it, because that marker is localised while the category never is.
+     */
+    static String lintCategoryOf(Diagnostic<? extends JavaFileObject> diagnostic, String message) {
+        if (message == null || message.isEmpty()) {
+            return null;
+        }
+        String rendered;
+        try {
+            rendered = diagnostic.toString();
+        } catch (Throwable e) {
+            // same JDK-8210649 / JDK-8216202 exposure as getMessage() above
+            return null;
+        }
+        if (rendered == null) {
+            return null;
+        }
+        int newLine = message.indexOf('\n');
+        // a wrapped message continues below the source line and caret, so only its first line is contiguous here
+        String firstLine = newLine < 0 ? message : message.substring(0, newLine);
+        int messageStart = rendered.indexOf(firstLine);
+        if (messageStart <= 0) {
+            return null;
+        }
+        String head = rendered.substring(0, messageStart);
+        int close = head.lastIndexOf(']');
+        if (close < 0 || !head.substring(close + 1).trim().isEmpty()) {
+            return null;
+        }
+        int open = head.lastIndexOf('[', close);
+        if (open < 0) {
+            return null;
+        }
+        String category = head.substring(open + 1, close);
+        if (category.isEmpty()) {
+            return null;
+        }
+        // a lint category is a single token; anything else is a bracket that happens to sit in a path
+        for (int i = 0; i < category.length(); i++) {
+            char c = category.charAt(i);
+            if (!Character.isLetterOrDigit(c) && c != '-' && c != '_') {
+                return null;
+            }
+        }
+        return head.substring(open, close + 1);
+    }
+
     private CompilerMessage.Kind convertKind(Diagnostic<? extends JavaFileObject> diagnostic) {
         CompilerMessage.Kind kind;
         switch (diagnostic.getKind()) {

plexus-compilers/plexus-compiler-javac/src/test/java/org/codehaus/plexus/compiler/javac/JavacCompilerTest.java

@@ -1,5 +1,8 @@
 package org.codehaus.plexus.compiler.javac;
 
+import javax.tools.Diagnostic;
+import javax.tools.JavaFileObject;
+
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
@@ -9,6 +12,8 @@
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Locale;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import org.codehaus.plexus.compiler.CompilerConfiguration;
@@ -26,6 +31,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /*
@@ -159,6 +165,11 @@ private static String getFileName(String path) {
     }
 
     private CompilerResult compile(Path tempDirectory, Path sourceDirectory, boolean fork) throws Exception {
+        return compile(tempDirectory, sourceDirectory, fork, "-Xlint:-options");
+    }
+
+    private CompilerResult compile(Path tempDirectory, Path sourceDirectory, boolean fork, String lintArgument)
+            throws Exception {
         File buildDirectory =
                 tempDirectory.resolve(fork ? "forked" : "in-process").toFile();
         Files.createDirectories(buildDirectory.toPath());
@@ -171,7 +182,7 @@ private CompilerResult compile(Path tempDirectory, Path sourceDirectory, boolean
         configuration.addSourceLocation(sourceDirectory.toString());
         configuration.setSourceVersion("8");
         configuration.setTargetVersion("8");
-        configuration.addCompilerCustomArgument("-Xlint:-options", null);
+        configuration.addCompilerCustomArgument(lintArgument, null);
 
         return getCompiler().performCompile(configuration);
     }
@@ -200,4 +211,149 @@ private static Stream<Arguments> quoteArgumentArguments() {
                         "\"line1\\nline2\\rline3\\tline4\\fline5\""),
                 Arguments.of("backslash and newline", "line1\\\nline2", "\"line1" + "\\\\" + "\\n" + "line2\""));
     }
+
+    @Test
+    void testForkedAndInProcessLintCategoriesAreEqual(@TempDir Path tempDirectory) throws Exception {
+        Path sourceDirectory = tempDirectory.resolve("src");
+        Files.createDirectories(sourceDirectory);
+        Files.write(
+                sourceDirectory.resolve("Raw.java"),
+                Arrays.asList("class Raw {", " java.util.List raw() {", "  return null;", " }", "}"),
+                StandardCharsets.UTF_8);
+
+        CompilerResult inProcess = compile(tempDirectory, sourceDirectory, false, "-Xlint:-options,rawtypes");
+        CompilerResult forked = compile(tempDirectory, sourceDirectory, true, "-Xlint:-options,rawtypes");
+
+        assertTrue(inProcess.isSuccess());
+        assertTrue(forked.isSuccess());
+
+        CompilerMessage inProcessMessage = onlyWarning(inProcess);
+        CompilerMessage forkedMessage = onlyWarning(forked);
+        assertTrue(
+                forkedMessage.getMessage().startsWith("[rawtypes]"),
+                "forked message lost its lint category: " + forkedMessage.getMessage());
+        assertTrue(
+                inProcessMessage.getMessage().startsWith("[rawtypes]"),
+                "in-process message lost its lint category: " + inProcessMessage.getMessage());
+        assertEquals(inProcessMessage.getStartLine(), forkedMessage.getStartLine());
+        // the bodies still differ - javac's own output abbreviates the type to List where the API spells out
+        // java.util.List - which is a separate parity gap, so only the category is compared here
+    }
+
+    private static CompilerMessage onlyWarning(CompilerResult result) {
+        List<CompilerMessage> warnings = result.getCompilerMessages().stream()
+                .filter(message -> message.getKind() == CompilerMessage.Kind.WARNING)
+                .collect(Collectors.toList());
+        assertEquals(1, warnings.size(), "expected exactly one warning, got " + result.getCompilerMessages());
+        return warnings.get(0);
+    }
+
+    @ParameterizedTest(name = "{0}")
+    @MethodSource("lintCategoryArguments")
+    void lintCategoryOf(String description, String rendered, String message, String expected) {
+        assertEquals(expected, JavaxToolsCompiler.lintCategoryOf(new StubDiagnostic(rendered), message));
+    }
+
+    private static Stream<Arguments> lintCategoryArguments() {
+        return Stream.of(
+                Arguments.of(
+                        "English warning",
+                        "/src/Raw.java:2: warning: [rawtypes] found raw type: java.util.List",
+                        "found raw type: java.util.List",
+                        "[rawtypes]"),
+                Arguments.of(
+                        "localised marker",
+                        "./src/Main.java:9: \u8b66\u544a:[deprecation] java.io.File \u306e toURL()",
+                        "java.io.File \u306e toURL()",
+                        "[deprecation]"),
+                Arguments.of(
+                        "wrapped message",
+                        "/src/Raw.java:2: warning: [rawtypes] found raw type: java.util.List\n"
+                                + " java.util.List raw() {\n"
+                                + " ^\n"
+                                + "  missing type arguments for generic class java.util.List<E>",
+                        "found raw type: java.util.List\n  missing type arguments for generic class java.util.List<E>",
+                        "[rawtypes]"),
+                Arguments.of("no category", "/src/Test.java:3: error: not a statement", "not a statement", null),
+                Arguments.of(
+                        "brackets in path",
+                        "/src/build[1]/Test.java:3: error: not a statement",
+                        "not a statement",
+                        null),
+                Arguments.of("message absent from rendering", "something else entirely", "not a statement", null),
+                Arguments.of("no message", "/src/Test.java:3: warning: [module] gone", "", null));
+    }
+
+    @Test
+    void lintCategoryOfUnrenderableDiagnostic() {
+        assertNull(JavaxToolsCompiler.lintCategoryOf(
+                new StubDiagnostic(null) {
+                    @Override
+                    public String toString() {
+                        throw new IllegalStateException("JDK-8210649");
+                    }
+                },
+                "found raw type: java.util.List"));
+    }
+
+    /**
+     * A diagnostic whose only meaningful part is its rendering, which is all {@code lintCategoryOf} reads.
+     */
+    private static class StubDiagnostic implements Diagnostic<JavaFileObject> {
+        private final String rendered;
+
+        StubDiagnostic(String rendered) {
+            this.rendered = rendered;
+        }
+
+        @Override
+        public String toString() {
+            return rendered;
+        }
+
+        @Override
+        public Kind getKind() {
+            return Kind.WARNING;
+        }
+
+        @Override
+        public JavaFileObject getSource() {
+            return null;
+        }
+
+        @Override
+        public long getPosition() {
+            return NOPOS;
+        }
+
+        @Override
+        public long getStartPosition() {
+            return NOPOS;
+        }
+
+        @Override
+        public long getEndPosition() {
+            return NOPOS;
+        }
+
+        @Override
+        public long getLineNumber() {
+            return NOPOS;
+        }
+
+        @Override
+        public long getColumnNumber() {
+            return NOPOS;
+        }
+
+        @Override
+        public String getCode() {
+            return null;
+        }
+
+        @Override
+        public String getMessage(Locale locale) {
+            return null;
+        }
+    }
 }