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.



-- 
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-unsubscr...@maven.apache.org

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

Reply via email to