You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/10/17 12:03:10 UTC

[GitHub] [maven-plugin-tools] kwin commented on a diff in pull request #151: [MPLUGIN-419] Allow `@Parameter` on setters methods

kwin commented on code in PR #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151#discussion_r996966815


##########
maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/JavaAnnotationsMojoDescriptorExtractor.java:
##########
@@ -395,32 +397,33 @@ private DocletTag findInClassHierarchy( JavaClass javaClass, String tagName )
 
     /**
      * extract fields that are either parameters or components.
+     * also extract methods that are parameters

Review Comment:
   Upper case "Also".



##########
maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/JavaAnnotationsMojoDescriptorExtractor.java:
##########
@@ -395,32 +397,33 @@ private DocletTag findInClassHierarchy( JavaClass javaClass, String tagName )
 
     /**
      * extract fields that are either parameters or components.
+     * also extract methods that are parameters
      *
      * @param javaClass not null
      * @return map with Mojo parameters names as keys
      */
-    private Map<String, JavaField> extractFieldParameterTags( JavaClass javaClass,
-                                                              Map<String, JavaClass> javaClassesMap )
+    private Map<String, JavaAnnotatedElement> extractParameterTags( JavaClass javaClass,

Review Comment:
   I would rename this method `extractParameterAnnotations`. Tag is such an overloaded term.



##########
maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/JavaAnnotationsMojoDescriptorExtractor.java:
##########
@@ -443,6 +455,15 @@ private Map<String, JavaField> extractFieldParameterTags( JavaClass javaClass,
         }
     }
 
+    private boolean isPublicSetterMethod( JavaMethod method )
+    {
+        return method.isPublic()

Review Comment:
   Sisu also injects into methods with prefix "add". Also you need to check for the modifier (https://github.com/eclipse/sisu.plexus/blob/27a29dc633e6b03753a3c8d29a033648630c8831/org.eclipse.sisu.plexus/src/org/eclipse/sisu/plexus/CompositeBeanHelper.java#L143-L147 and https://github.com/eclipse/sisu.plexus/blob/27a29dc633e6b03753a3c8d29a033648630c8831/org.eclipse.sisu.plexus/src/org/eclipse/sisu/plexus/CompositeBeanHelper.java#L267-L282)



##########
maven-plugin-plugin/src/site/apt/examples/using-annotations.apt.vm:
##########
@@ -41,18 +41,22 @@ Using Plugin Tools Java Annotations
 
   Information for plugin descriptor generation is specified using 4 annotations:
 
-  * 2 class-level annotations:
+  * 2 class level annotations:
 
     * <<<...@Mojo>>>: This annotation will mark your class as a Mojo,
 
     * <<<...@Execute>>>: Used if your Mojo needs to fork a lifecycle,
 
     []
 
-  * 2 field-level annotations:
+  * 1 filed or method level annotations:

Review Comment:
   filed -> field



-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org