gnodet-bot commented on code in PR #13658:
URL: https://github.com/apache/maven/pull/13658#discussion_r4346478798


##########
impl/maven-plugin-mixin/src/site/markdown/index.md:
##########
@@ -0,0 +1,112 @@
+title: Maven Plugin Mixin
+author: Maven Team
+date: ${project.version}
+
+<!--
+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.
+-->
+
+# Maven Plugin Mixin
+
+`maven-plugin-mixin` is a mixin for Maven 4 plugin projects. It bundles a JSR 
269
+annotation processor that generates Maven 4 plugin descriptors
+(`META-INF/maven/plugin.xml`) and DI factory classes at compile time — with
+**no dependency on `maven-plugin-plugin`** or `maven-plugin-tools`.
+
+## Overview
+
+When you annotate a class with `@org.apache.maven.api.plugin.annotations.Mojo`,
+the processor fires during `javac` and produces:
+
+1. **`META-INF/maven/plugin.xml`** — the plugin descriptor in v2.0.0 format,
+   describing every mojo's goal name, default phase, parameters, dependency
+   resolution requirements, `@After` ordering constraints, and more.
+
+2. **One factory class per mojo** — e.g. `TouchMojo$MojoFactory`, annotated 
with

Review Comment:
   ⚠️ **Stale class name**: the processor generates a top-level class 
`TouchMojoMojoFactory` (see `factoryName = simpleName + "MojoFactory"` in 
`MojoDescriptorProcessor.java`), but this doc uses `TouchMojo$MojoFactory` — 
the `$` notation implies a nested class, which is incorrect.



##########
impl/maven-plugin-mixin/src/test/java/org/apache/maven/plugin/processor/MojoDescriptorProcessorTest.java:
##########
@@ -0,0 +1,688 @@
+/*
+ * 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(null, 
"maven-api-annotations/4.1.0-SNAPSHOT/maven-api-annotations-4.1.0-SNAPSHOT.jar");
+
+        classpathJars = List.of(apiCore, apiAnnotations);
+    }
+
+    /**
+     * Try to resolve a JAR first from the worktree build output, then from 
the local Maven repo.
+     */
+    private static File resolveFile(String worktreeRelative, String 
m2Relative) {
+        String worktreeBase =
+                System.getProperty("processor.worktree", 
"/opt/data/oss/maven/.worktrees/plugin-processor");

Review Comment:
   🐛 **Hardcoded developer path as default**: 
`"/opt/data/oss/maven/.worktrees/plugin-processor"` is a machine-specific path. 
Any other contributor running the tests will hit this fallback and fail. The 
default should be `null` (or omitted), forcing the test to only use the Maven 
local repository path (`processor.m2`) which is already portable via 
`user.home`.



##########
impl/maven-plugin-mixin/src/main/java/org/apache/maven/plugin/processor/MojoDescriptorProcessor.java:
##########
@@ -0,0 +1,458 @@
+/*
+ * 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
+ */
+@SupportedAnnotationTypes("org.apache.maven.api.plugin.annotations.Mojo")
+@SupportedOptions({
+    MojoDescriptorProcessor.OPT_GROUP_ID,
+    MojoDescriptorProcessor.OPT_ARTIFACT_ID,
+    MojoDescriptorProcessor.OPT_VERSION,
+    MojoDescriptorProcessor.OPT_GOAL_PREFIX
+})
+public class MojoDescriptorProcessor extends AbstractProcessor {
+
+    static final String OPT_GROUP_ID = "maven.plugin.groupId";
+    static final String OPT_ARTIFACT_ID = "maven.plugin.artifactId";
+    static final String OPT_VERSION = "maven.plugin.version";
+    static final String OPT_GOAL_PREFIX = "maven.plugin.goalPrefix";
+
+    private final Map<String, TypeElement> mojoElements = new 
LinkedHashMap<>();
+    private final Set<String> generatedFactoryNames = new LinkedHashSet<>();
+    private String[] resolvedCoordinates;
+
+    @Override
+    public SourceVersion getSupportedSourceVersion() {
+        return SourceVersion.latestSupported();
+    }
+
+    @Override
+    public boolean process(Set<? extends TypeElement> annotations, 
RoundEnvironment roundEnv) {
+        for (TypeElement annotation : annotations) {
+            for (Element element : 
roundEnv.getElementsAnnotatedWith(annotation)) {
+                if (!(element instanceof TypeElement typeElement)) {
+                    continue;
+                }
+                String fqn = typeElement.getQualifiedName().toString();
+                if (generatedFactoryNames.contains(fqn)) {
+                    continue;
+                }
+                if (typeElement.getKind() == ElementKind.INTERFACE) {
+                    error("@Mojo must not be placed on an interface", element);
+                    continue;
+                }
+                if (typeElement.getModifiers().contains(Modifier.ABSTRACT)) {
+                    error("@Mojo must not be placed on an abstract class", 
element);
+                    continue;
+                }
+                if (!mojoElements.containsKey(fqn)) {
+                    mojoElements.put(fqn, typeElement);
+                    String[] coords = resolveCoordinates();
+                    if (coords != null) {
+                        try {
+                            generateFactory(typeElement, coords);
+                        } catch (IOException e) {
+                            error("Failed to generate factory for " + fqn, e);
+                        }
+                    }
+                }
+            }
+        }
+        if (roundEnv.processingOver() && !mojoElements.isEmpty()) {
+            String[] coords = resolveCoordinates();
+            if (coords != null) {
+                try {
+                    writePluginXml(coords);
+                } catch (IOException e) {
+                    error("Failed to generate plugin.xml", e);
+                }
+            }
+        }
+        return false;
+    }
+
+    String[] resolveCoordinates() {
+        if (resolvedCoordinates != null) {
+            return resolvedCoordinates;
+        }
+        String groupId = getOption(OPT_GROUP_ID);
+        String artifactId = getOption(OPT_ARTIFACT_ID);
+        String version = getOption(OPT_VERSION);
+        String goalPrefix = getOption(OPT_GOAL_PREFIX);
+        if (isBlank(goalPrefix)) {
+            goalPrefix = deriveGoalPrefix(artifactId);
+        }
+        if (isBlank(groupId) || isBlank(artifactId) || isBlank(version)) {
+            error("Could not determine plugin coordinates. "
+                    + "Supply -Amaven.plugin.groupId, 
-Amaven.plugin.artifactId and "
+                    + "-Amaven.plugin.version via the maven-plugin-mixin.");
+            return null;
+        }
+        resolvedCoordinates = new String[] {groupId, artifactId, version, 
goalPrefix};
+        return resolvedCoordinates;
+    }
+
+    private String getOption(String key) {
+        return processingEnv.getOptions().get(key);
+    }
+
+    private void writePluginXml(String[] coords) throws IOException {
+        List<MojoDescriptor> mojos = new ArrayList<>();
+        for (TypeElement mojoType : mojoElements.values()) {
+            MojoDescriptor mojo = buildMojoDescriptor(mojoType);
+            if (mojo != null) {
+                mojos.add(mojo);
+            }
+        }
+        PluginDescriptor descriptor = PluginDescriptor.newBuilder(true)
+                .name(coords[1])

Review Comment:
   🐛 **`PluginDescriptor.name` set to artifactId**: `.name(coords[1])` and 
`.artifactId(coords[1])` both use the same value. `name` should be a 
human-readable label (e.g. the Maven artifact name or goal prefix), not a 
duplicate of the artifactId. Consider reading the name from a plugin-level 
annotation or at minimum using the goal prefix instead.



##########
impl/maven-plugin-mixin/src/site/markdown/usage.md:
##########
@@ -0,0 +1,122 @@
+title: Usage
+author: Maven Team
+date: ${project.version}
+
+<!--
+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.
+-->
+
+# Usage
+
+## Minimal Maven 4 plugin
+
+A complete, functional Maven 4 plugin with one mojo and one `@Parameter`:
+
+```java
+package com.example;
+
+import org.apache.maven.api.plugin.Mojo;
+import org.apache.maven.api.plugin.annotations.Mojo;
+import org.apache.maven.api.plugin.annotations.Parameter;
+
+/**
+ * Prints a greeting.
+ *
+ * @since 1.0.0
+ */
+@Mojo(name = "greet", defaultPhase = "compile")
+public class GreetMojo implements Mojo {
+
+    /**
+     * The name to greet.
+     *
+     * @since 1.0.0
+     */
+    @Parameter(property = "greet.name", defaultValue = "World")
+    private String name;
+
+    @Override
+    public void execute() {
+        System.out.println("Hello, " + name + "!");
+    }
+}
+```
+
+POM configuration (model version 4.2.0, using the mixin):
+
+```xml
+<project xmlns="http://maven.apache.org/POM/4.2.0"; ...>
+
+  <groupId>com.example</groupId>
+  <artifactId>greet-maven-plugin</artifactId>
+  <version>1.0-SNAPSHOT</version>
+  <packaging>jar</packaging>
+
+  <mixins>
+    <mixin>
+      <groupId>org.apache.maven</groupId>
+      <artifactId>maven-plugin-mixin</artifactId>
+      <version>${maven.version}</version>
+    </mixin>
+  </mixins>
+</project>
+```
+
+The mixin configures the compiler plugin automatically — no
+`annotationProcessorPaths` or `-A` options to set up manually.
+
+At compile time, the processor generates:
+
+- `target/classes/META-INF/maven/plugin.xml` — the plugin descriptor
+- `target/generated-sources/annotations/com/example/GreetMojo$MojoFactory.java`

Review Comment:
   ⚠️ **Stale class name**: same issue — the generated class is 
`GreetMojoMojoFactory` (top-level), not `GreetMojo$MojoFactory` (nested). 
Update to match the actual output of the processor.



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