mbien commented on code in PR #7662:
URL: https://github.com/apache/netbeans/pull/7662#discussion_r1713088554
##########
java/maven/src/org/netbeans/modules/maven/api/PluginPropertyUtils.java:
##########
@@ -693,10 +697,14 @@ public static final class PluginConfigPathParams {
/**
* Creates a query instance with mandatory parameters
+ *
* @param pluginGroupId plugin's group ID
* @param pluginArtifactId plugin's artifact ID
- * @param pathProperty name of the property (the property should
contain a list of items)
- * @param pathItemName name of the single item's element
+ * @param pathProperty name of the property (the property should
contain
+ * a list of items)
+ * @param pathItemName name of the single item's element, can be
+ * {@code null}. If {@code null} is used, all children of
+ * {@code pathProperty} are considered.
*/
public PluginConfigPathParams(String pluginGroupId, String
pluginArtifactId, String pathProperty, String pathItemName) {
Review Comment:
the 4 param constructor of this friend API should be deprecated so that
nothing calls it by accident. (analog to
https://github.com/apache/netbeans/pull/7661/files)
##########
java/maven/test/unit/src/org/netbeans/modules/maven/api/PluginPropertyUtilsTest.java:
##########
@@ -138,4 +143,144 @@ public void testGetCompilerArgs() throws Exception {
assertEquals("[--enable-preview]",
Arrays.toString(PluginPropertyUtils.getPluginPropertyList(ProjectManager.getDefault().findProject(d),
"org.apache.maven.plugins", "maven-compiler-plugin", "compilerArgs", "arg",
null)));
}
+ public void testDependencyListBuilder() throws Exception {
+ String testPom =
+ """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0";
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>let.me.reproduce</groupId>
+
<artifactId>annotation-processor-netbeans-reproducer</artifactId>
+ <version>1.0-SNAPSHOT</version>
+ <packaging>jar</packaging>
+ <properties>
+
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.release>21</maven.compiler.release>
+
<exec.mainClass>let.me.reproduce.annotation.processor.netbeans.reproducer.Main</exec.mainClass>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.mapstruct</groupId>
+ <artifactId>mapstruct</artifactId>
+ <version>1.5.5.Final</version>
+ </dependency>
+ <dependency>
+ <groupId>io.soabase.record-builder</groupId>
+ <artifactId>record-builder-core</artifactId>
+ <version>42</version>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.13.0</version>
+ <configuration>
+ <parameters>true</parameters>
+ <annotationProcessorPaths>
+ <path>
+ <groupId>org.mapstruct</groupId>
+
<artifactId>mapstruct-processor</artifactId>
+ <version>1.5.5.Final</version>
+ </path>
+ <path>
+
<groupId>io.soabase.record-builder</groupId>
+
<artifactId>record-builder-processor</artifactId>
+ <version>42</version>
+ </path>
+ </annotationProcessorPaths>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+ Xpp3Dom configRoot = Xpp3DomBuilder.build(new
StringReader(testPom)).getChild("build").getChild("plugins").getChildren()[0].getChild("configuration");
+
+ // Matching filter for propertyItemName should yield correct result
+ PluginPropertyUtils.DependencyListBuilder bld = new
PluginPropertyUtils.DependencyListBuilder(
+ "annotationProcessorPaths",
+ "path", // Implementation used prior to 2.165
+ null
+ );
+ List<Dependency> dependencies = bld.build(configRoot,
PluginPropertyUtils.DUMMY_EVALUATOR);
+ assertEquals(2, dependencies.size());
+
+ // Unmatching filter for propertyItemName should yield empty result
+ PluginPropertyUtils.DependencyListBuilder bld2 = new
PluginPropertyUtils.DependencyListBuilder(
+ "annotationProcessorPaths",
+ "annotationProcessorPath",
+ null
+ );
+ List<Dependency> dependencies2 = bld2.build(configRoot,
PluginPropertyUtils.DUMMY_EVALUATOR);
+ assertEquals(0, dependencies2.size());
+
+ String testPom2 =
+ """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0";
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>let.me.reproduce</groupId>
+
<artifactId>annotation-processor-netbeans-reproducer</artifactId>
+ <version>1.0-SNAPSHOT</version>
+ <packaging>jar</packaging>
+ <properties>
+
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.release>21</maven.compiler.release>
+
<exec.mainClass>let.me.reproduce.annotation.processor.netbeans.reproducer.Main</exec.mainClass>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.mapstruct</groupId>
+ <artifactId>mapstruct</artifactId>
+ <version>1.5.5.Final</version>
+ </dependency>
+ <dependency>
+ <groupId>io.soabase.record-builder</groupId>
+ <artifactId>record-builder-core</artifactId>
+ <version>42</version>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.13.0</version>
+ <configuration>
+ <parameters>true</parameters>
+ <annotationProcessorPaths>
+ <annotationProcessorPath>
+ <groupId>org.mapstruct</groupId>
+
<artifactId>mapstruct-processor</artifactId>
+ <version>1.5.5.Final</version>
+ </annotationProcessorPath>
+ <annotationProcessorPath>
+
<groupId>io.soabase.record-builder</groupId>
+
<artifactId>record-builder-processor</artifactId>
+ <version>42</version>
+ </annotationProcessorPath>
Review Comment:
nitpick: would be good if the elements here would have distinct names
##########
java/maven/src/org/netbeans/modules/maven/api/PluginPropertyUtils.java:
##########
@@ -632,7 +630,13 @@ public List<Dependency> build(Xpp3Dom configRoot,
ExpressionEvaluator eval) {
if (source == null) {
return null;
}
- for (Xpp3Dom ch : source.getChildren(propertyItemName)) {
+ Xpp3Dom[] children;
+ if(propertyItemName == null) {
+ children = source.getChildren();
+ } else {
+ children = source.getChildren(propertyItemName);
+ }
+ for (Xpp3Dom ch : children) {
Review Comment:
I implemented something similar at first in
https://github.com/apache/netbeans/pull/7661, but reverted that after looking
through mojo implementations. This can't really happen that list items and
other elements are interleaved.
a user could write something like this however which is valid:
```
<annotationProcessorPaths>
<path1>
...
</path1>
<path2>
...
</path2>
</annotationProcessorPaths>
```
NB had a bug. I don't see why NB should stay compatible with that bug. The
fix here should be to simply iterate over all children, since all children of
`annotationProcessorPaths` are path items. Otherwise the same bug can appear
again at some point in future.
Lets assume friend API dependencies would rely on `propertyItemName` (which
is highly unlikely), those would have the same bug today and would be fixed if
this would iterate over all children.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
