[ 
https://issues.apache.org/jira/browse/MNG-6776?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17801907#comment-17801907
 ] 

ASF GitHub Bot commented on MNG-6776:
-------------------------------------

slawekjaranowski commented on code in PR #1361:
URL: https://github.com/apache/maven/pull/1361#discussion_r1439751661


##########
maven-plugin-api/src/main/java/org/apache/maven/plugin/descriptor/MojoDescriptor.java:
##########
@@ -198,31 +200,25 @@ public void setParameters(List<Parameter> parameters) 
throws DuplicateParameterE
      * @throws DuplicateParameterException if any
      */
     public void addParameter(Parameter parameter) throws 
DuplicateParameterException {
-        if (parameters != null && parameters.contains(parameter)) {
+        if (parameters.contains(parameter)) {
             throw new DuplicateParameterException(parameter.getName()
                     + " has been declared multiple times in mojo with goal: " 
+ getGoal() + " (implementation: "
                     + getImplementation() + ")");
         }
 
-        if (parameters == null) {
-            parameters = new LinkedList<>();
-        }
-
         parameters.add(parameter);
     }
 
     /**
-     * @return the list parameters as a Map
+     * @return the list parameters as a Map (keyed by {@link 
Parameter#getName()}) that is built from
+     * {@link #parameters} list on each call. In other words, the map returned 
is built on fly and is a copy.
+     * Any change to this map is NOT reflected on list and other way around!

Review Comment:
   unmodifiable?



##########
maven-plugin-api/src/main/java/org/apache/maven/plugin/descriptor/MojoDescriptor.java:
##########
@@ -163,31 +163,33 @@ public void setLanguage(String language) {
     }
 
     /**
-     * @return Description with reason of a Mojo deprecation.
+     * @return <code>true</code> if the Mojo is deprecated, <code>false</code> 
otherwise.
      */
     public String getDeprecated() {
         return deprecated;
     }
 
     /**
-     * @param deprecated Description with reason of a Mojo deprecation.
+     * @param deprecated <code>true</code> to deprecate the Mojo, 
<code>false</code> otherwise.
      */
     public void setDeprecated(String deprecated) {
         this.deprecated = deprecated;
     }
 
     /**
-     * @return the list of parameters
+     * @return the list of parameters copy. Any change to returned list is NOT 
reflected on this instance. To add
+     * parameters, use {@link #addParameter(Parameter)} method.
      */
     public List<Parameter> getParameters() {
-        return parameters;
+        return new ArrayList<>(parameters);

Review Comment:
   Maybe we should return unmodifiable collections, it will be difficult to 
detect if someone try to change this.





> Inconsistent list of parameters in MojoDescriptor
> -------------------------------------------------
>
>                 Key: MNG-6776
>                 URL: https://issues.apache.org/jira/browse/MNG-6776
>             Project: Maven
>          Issue Type: Bug
>          Components: Plugin API
>    Affects Versions: 3.6.2
>            Reporter: Sylwester Lachiewicz
>            Assignee: Tamas Cservenak
>            Priority: Minor
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> While working with maven-plugin-tools I discovered inconsistent results 
> returned from MojoDescriptor getParameters() and getParametersMap().
> In 
> [AntMojoDescriptorExtractor.java#L101|https://github.com/apache/maven-plugin-tools/blob/master/maven-script/maven-plugin-tools-ant/src/main/java/org/apache/maven/tools/plugin/extractor/ant/AntMojoDescriptorExtractor.java#L101]
> If first I call MojoDescriptor.getParameterMap() and then later add parameter 
> via MojoDescriptor.addParameter - MojoDescriptor.getParameterMap() will still 
> return map with old (cached) list with parameters.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to