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