gnodet-bot commented on code in PR #13658: URL: https://github.com/apache/maven/pull/13658#discussion_r4466989597
########## impl/maven-plugin-mixin/src/test/java/org/apache/maven/plugin/processor/MojoDescriptorProcessorTest.java: ########## @@ -0,0 +1,698 @@ +/* + * 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.plugin.processor; + +import javax.tools.Diagnostic; +import javax.tools.DiagnosticCollector; +import javax.tools.FileObject; +import javax.tools.ForwardingJavaFileManager; +import javax.tools.JavaCompiler; +import javax.tools.JavaFileManager; +import javax.tools.JavaFileObject; +import javax.tools.SimpleJavaFileObject; +import javax.tools.StandardJavaFileManager; +import javax.tools.StandardLocation; +import javax.tools.ToolProvider; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.OutputStream; +import java.io.StringWriter; +import java.net.URI; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +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.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Unit tests for {@link MojoDescriptorProcessor} using the JDK in-process compiler. + * + * <p>Each test compiles a small in-memory source file that uses the Maven plugin + * annotations ({@code @Mojo}, {@code @Parameter}, etc.) from maven-api-core on + * the classpath, with the processor JAR on the processor path. Generated + * resources (plugin.xml) and source files (factory classes) are intercepted by + * a {@link CapturingFileManager} and asserted in-memory — no disk I/O.</p> + */ +class MojoDescriptorProcessorTest { + + /** Resolved classpath jars needed for compiling test sources. */ + private static List<File> classpathJars; + + /** The processor JAR (contains the compiled MojoDescriptorProcessor). */ + private static File processorJar; + + @BeforeAll + static void resolveClasspath() { + // Processor JAR — built from this module + processorJar = resolveFile( + "impl/maven-plugin-mixin/target/maven-plugin-mixin-4.1.0-SNAPSHOT.jar", + "maven-plugin-mixin/4.1.0-SNAPSHOT/maven-plugin-mixin-4.1.0-SNAPSHOT.jar"); Review Comment: ⚠️ **Stale version after parent bump to `4.2.0-SNAPSHOT`**: these paths reference `4.1.0-SNAPSHOT` jars. After the version bump in commit `467c413e`, the build produces `maven-plugin-mixin-4.2.0-SNAPSHOT.jar` — `resolveFile()` will silently miss the worktree JAR and fall back to the m2 repo, which also has no `4.1.0-SNAPSHOT` entry for the new build. `processorJar` ends up `null`, causing an NPE or `@BeforeAll` failure that aborts the entire test class. ```suggestion "impl/maven-plugin-mixin/target/maven-plugin-mixin-${project.version}.jar", "maven-plugin-mixin/${project.version}/maven-plugin-mixin-${project.version}.jar"); ``` Since this is a Java file (not a POM), Maven filtering won't help here. Use a system property injected via Surefire instead — add to the POM's Surefire config: ```xml <systemPropertyVariables> <maven.plugin.mixin.version>${project.version}</maven.plugin.mixin.version> </systemPropertyVariables> ``` Then in the test: ```java private static final String VERSION = System.getProperty("maven.plugin.mixin.version", "4.2.0-SNAPSHOT"); ``` And use `VERSION` in the path strings. ########## impl/maven-plugin-mixin/src/test/java/org/apache/maven/plugin/processor/MojoDescriptorProcessorTest.java: ########## @@ -0,0 +1,698 @@ +/* + * 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.plugin.processor; + +import javax.tools.Diagnostic; +import javax.tools.DiagnosticCollector; +import javax.tools.FileObject; +import javax.tools.ForwardingJavaFileManager; +import javax.tools.JavaCompiler; +import javax.tools.JavaFileManager; +import javax.tools.JavaFileObject; +import javax.tools.SimpleJavaFileObject; +import javax.tools.StandardJavaFileManager; +import javax.tools.StandardLocation; +import javax.tools.ToolProvider; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.OutputStream; +import java.io.StringWriter; +import java.net.URI; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +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.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Unit tests for {@link MojoDescriptorProcessor} using the JDK in-process compiler. + * + * <p>Each test compiles a small in-memory source file that uses the Maven plugin + * annotations ({@code @Mojo}, {@code @Parameter}, etc.) from maven-api-core on + * the classpath, with the processor JAR on the processor path. Generated + * resources (plugin.xml) and source files (factory classes) are intercepted by + * a {@link CapturingFileManager} and asserted in-memory — no disk I/O.</p> + */ +class MojoDescriptorProcessorTest { + + /** Resolved classpath jars needed for compiling test sources. */ + private static List<File> classpathJars; + + /** The processor JAR (contains the compiled MojoDescriptorProcessor). */ + private static File processorJar; + + @BeforeAll + static void resolveClasspath() { + // Processor JAR — built from this module + processorJar = resolveFile( + "impl/maven-plugin-mixin/target/maven-plugin-mixin-4.1.0-SNAPSHOT.jar", + "maven-plugin-mixin/4.1.0-SNAPSHOT/maven-plugin-mixin-4.1.0-SNAPSHOT.jar"); + + // Annotation JARs needed on the compilation classpath + File apiCore = resolveFile( + "api/maven-api-core/target/maven-api-core-4.1.0-SNAPSHOT.jar", + "maven-api-core/4.1.0-SNAPSHOT/maven-api-core-4.1.0-SNAPSHOT.jar"); + File apiAnnotations = resolveFile( + "api/maven-api-annotations/target/maven-api-annotations-4.1.0-SNAPSHOT.jar", + "maven-api-annotations/4.1.0-SNAPSHOT/maven-api-annotations-4.1.0-SNAPSHOT.jar"); Review Comment: ⚠️ **Same stale version issue** for `maven-api-annotations`. ```suggestion "api/maven-api-annotations/target/maven-api-annotations-${project.version}.jar", "maven-api-annotations/${project.version}/maven-api-annotations-${project.version}.jar"); ``` ########## impl/maven-plugin-mixin/src/main/mixin/mixin.xml: ########## @@ -0,0 +1,154 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + 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. +--> +<!-- + Mixin POM for Maven 4 plugin projects. + Use this as a mixin to get zero-config plugin descriptor generation: + + <mixins> + <mixin> + <groupId>org.apache.maven</groupId> + <artifactId>maven-plugin-mixin</artifactId> + <version>4.1.0-SNAPSHOT</version> Review Comment: **Usage comment shows stale `4.1.0-SNAPSHOT`**. Update the example version to match the current release. ```suggestion <version>4.2.0-SNAPSHOT</version> ``` (Or use a placeholder like `<version>@VERSION@</version>` with a note pointing to the Maven Central release page — SNAPSHOT versions are unsuitable examples for end-user docs.) ########## impl/maven-plugin-mixin/src/test/java/org/apache/maven/plugin/processor/MojoDescriptorProcessorTest.java: ########## @@ -0,0 +1,698 @@ +/* + * 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.plugin.processor; + +import javax.tools.Diagnostic; +import javax.tools.DiagnosticCollector; +import javax.tools.FileObject; +import javax.tools.ForwardingJavaFileManager; +import javax.tools.JavaCompiler; +import javax.tools.JavaFileManager; +import javax.tools.JavaFileObject; +import javax.tools.SimpleJavaFileObject; +import javax.tools.StandardJavaFileManager; +import javax.tools.StandardLocation; +import javax.tools.ToolProvider; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.OutputStream; +import java.io.StringWriter; +import java.net.URI; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +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.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Unit tests for {@link MojoDescriptorProcessor} using the JDK in-process compiler. + * + * <p>Each test compiles a small in-memory source file that uses the Maven plugin + * annotations ({@code @Mojo}, {@code @Parameter}, etc.) from maven-api-core on + * the classpath, with the processor JAR on the processor path. Generated + * resources (plugin.xml) and source files (factory classes) are intercepted by + * a {@link CapturingFileManager} and asserted in-memory — no disk I/O.</p> + */ +class MojoDescriptorProcessorTest { + + /** Resolved classpath jars needed for compiling test sources. */ + private static List<File> classpathJars; + + /** The processor JAR (contains the compiled MojoDescriptorProcessor). */ + private static File processorJar; + + @BeforeAll + static void resolveClasspath() { + // Processor JAR — built from this module + processorJar = resolveFile( + "impl/maven-plugin-mixin/target/maven-plugin-mixin-4.1.0-SNAPSHOT.jar", + "maven-plugin-mixin/4.1.0-SNAPSHOT/maven-plugin-mixin-4.1.0-SNAPSHOT.jar"); + + // Annotation JARs needed on the compilation classpath + File apiCore = resolveFile( + "api/maven-api-core/target/maven-api-core-4.1.0-SNAPSHOT.jar", + "maven-api-core/4.1.0-SNAPSHOT/maven-api-core-4.1.0-SNAPSHOT.jar"); Review Comment: ⚠️ **Same stale version issue** for `maven-api-core`. After the bump all API jars are also `4.2.0-SNAPSHOT`. ```suggestion "api/maven-api-core/target/maven-api-core-${project.version}.jar", "maven-api-core/${project.version}/maven-api-core-${project.version}.jar"); ``` ########## impl/maven-plugin-mixin/src/main/java/org/apache/maven/plugin/processor/MojoDescriptorProcessor.java: ########## @@ -0,0 +1,468 @@ +/* + * 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.plugin.processor; + +import javax.annotation.processing.AbstractProcessor; +import javax.annotation.processing.RoundEnvironment; +import javax.annotation.processing.SupportedAnnotationTypes; +import javax.annotation.processing.SupportedOptions; +import javax.lang.model.SourceVersion; +import javax.lang.model.element.Element; +import javax.lang.model.element.ElementKind; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.Modifier; +import javax.lang.model.element.TypeElement; +import javax.lang.model.element.VariableElement; +import javax.lang.model.type.DeclaredType; +import javax.lang.model.type.TypeKind; +import javax.lang.model.type.TypeMirror; +import javax.tools.Diagnostic; +import javax.tools.StandardLocation; +import javax.xml.stream.XMLStreamException; + +import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.io.Writer; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; + +import org.apache.maven.api.plugin.annotations.After; +import org.apache.maven.api.plugin.annotations.Execute; +import org.apache.maven.api.plugin.annotations.Mojo; +import org.apache.maven.api.plugin.annotations.Resolution; +import org.apache.maven.api.plugin.descriptor.AfterLink; +import org.apache.maven.api.plugin.descriptor.MojoDescriptor; +import org.apache.maven.api.plugin.descriptor.PluginDescriptor; +import org.apache.maven.plugin.descriptor.io.PluginDescriptorStaxWriter; + +/** + * JSR 269 annotation processor that generates Maven plugin descriptors ({@code META-INF/maven/plugin.xml}) + * and DI factory classes at compile time from {@link Mojo @Mojo}-annotated classes. + * + * <p>Plugin coordinates are read from compiler {@code -A} options, typically provided by the + * {@code maven-plugin-mixin}. + * + * @since 4.1.0 Review Comment: **`@since 4.1.0` is incorrect** after the version bump. `master` is now at `4.2.0-SNAPSHOT` — the first published release that will contain this class is `4.2.0`. ```suggestion * @since 4.2.0 ``` -- 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]
