Copilot commented on code in PR #1106:
URL:
https://github.com/apache/maven-plugin-tools/pull/1106#discussion_r3699389928
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugins/plugin/descriptor/PluginDescriptorBuilder.java:
##########
@@ -40,6 +40,9 @@
* @author Jason van Zyl
*/
public class PluginDescriptorBuilder {
+
+ boolean isV4;
+
Review Comment:
The `isV4` flag is mutable parsing state, but it's currently
package-visible. That makes it part of the public surface within the package
and increases the risk of accidental external mutation; it should be
encapsulated as a private field.
##########
maven-plugin-report-plugin/src/it/plugin-report-400/src/main/java/org/DummyReport.java:
##########
@@ -0,0 +1,199 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org;
+
+import java.io.File;
+import java.util.Locale;
+import java.util.ResourceBundle;
+
+import org.apache.maven.doxia.sink.Sink;
+import org.apache.maven.doxia.siterenderer.Renderer;
+import org.apache.maven.plugins.annotations.Component;
+import org.apache.maven.plugins.annotations.Execute;
+import org.apache.maven.plugins.annotations.LifecyclePhase;
+import org.apache.maven.plugins.annotations.Mojo;
+import org.apache.maven.plugins.annotations.Parameter;
+import org.apache.maven.project.MavenProject;
+import org.apache.maven.reporting.AbstractMavenReport;
+import org.apache.maven.reporting.AbstractMavenReportRenderer;
+import org.apache.maven.reporting.MavenReportException;
+
+/**
+ * Dummy Reporting Plugin.
+ */
+@Mojo(name = "report", requiresReports = true)
+@Execute(phase = LifecyclePhase.COMPILE)
+public class DummyReport extends AbstractMavenReport {
+ /**
+ * Report output directory.
+ */
+ @Parameter(defaultValue = "${project.build.directory}/generated-site/xdoc")
+ private File outputDirectory;
+
+ /**
+ * Doxia Site Renderer.
+ */
+ @Component
+ private Renderer siteRenderer;
+
+ /**
+ * The Maven Project.
+ */
+ @Parameter(property = "project", readonly = true, required = true)
+ private MavenProject project;
+
+ /**
+ * The goal prefix that will appear before the ":".
+ *
+ * @since 2.4
+ */
+ @Parameter(property = "goalPrefix")
+ protected String goalPrefix;
+
+ /**
+ * Set this to "true" to skip invoking any goals or reports of the plugin.
+ *
+ * @since 2.8
+ */
+ @Parameter(defaultValue = "false", property = "maven.plugin.skip")
+ private boolean skip;
+
+ /**
+ * Set this to "true" to skip generating the report.
+ *
+ * @since 2.8
+ */
+ @Parameter(defaultValue = "false", property = "maven.plugin.report.skip")
+ private boolean skipReport;
+
+ /**
+ * {@inheritDoc}
+ */
+ protected Renderer getSiteRenderer() {
+ return siteRenderer;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ protected String getOutputDirectory() {
+ return outputDirectory.getPath();
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ protected MavenProject getProject() {
+ return project;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ public boolean canGenerateReport() {
+ if (skip || skipReport) {
+ return false;
+ }
+
+ return true;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ protected void executeReport(Locale locale) throws MavenReportException {
+ // Generate the plugin's documentation
+ generatePluginDocumentation(locale);
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ public String getDescription(Locale locale) {
+ return getBundle(locale).getString("report.plugin.description");
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ public String getName(Locale locale) {
+ return getBundle(locale).getString("report.plugin.name");
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ public String getOutputName() {
+ return "plugin-info";
+ }
+
+ /**
+ * @param pluginDescriptor not null
+ * @param locale not null
+ * @throws MavenReportException if any
+ */
Review Comment:
The Javadoc for `generatePluginDocumentation` documents a `pluginDescriptor`
parameter that the method does not take. This is misleading for readers and
should be corrected to match the actual signature.
##########
maven-plugin-report-plugin/src/main/java/org/apache/maven/plugins/plugin/descriptor/PluginDescriptorBuilder.java:
##########
@@ -70,6 +73,11 @@ public PluginDescriptor build(Reader reader, String source)
throws PlexusConfigu
pluginDescriptor.setInheritedByDefault(Boolean.parseBoolean(inheritedByDefault));
}
+ String requiredMavenVersion =
c.getChild("requiredMavenVersion").getValue();
+ if (requiredMavenVersion != null) {
+ isV4 = requiredMavenVersion.startsWith("4");
+ }
Review Comment:
`isV4` is only updated when `requiredMavenVersion` is non-null. If a
`PluginDescriptorBuilder` instance is reused, a previous value can leak into
subsequent builds (e.g., a Maven 4 build followed by a descriptor without
`requiredMavenVersion`). Reset the flag at the start of each `build(...)` call
before reading the version.
--
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]
