This is an automated email from the ASF dual-hosted git repository.
sjaranowski pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-enforcer.git
The following commit(s) were added to refs/heads/master by this push:
new 3e12af9 [MENFORCER-461] Fix NPE in RequirePluginVersions
3e12af9 is described below
commit 3e12af90523e9a9a795a0876aca962992b28c217
Author: Slawomir Jaranowski <s.jaranow...@gmail.com>
AuthorDate: Fri Jan 27 20:57:03 2023 +0100
[MENFORCER-461] Fix NPE in RequirePluginVersions
And a little error message improvement
---
.../enforcer/rules/RequirePluginVersions.java | 110 +++++++++++++--------
.../enforcer/rules/TestRequirePluginVersions.java | 13 +--
.../checkPluginVersionProfile/pom.xml | 4 +-
.../require-plugin-versions-ci/verify.groovy | 2 +-
.../it/projects/require-plugin-versions/pom.xml | 6 ++
.../require-plugin-versions_failure/verify.groovy | 2 +-
6 files changed, 87 insertions(+), 50 deletions(-)
diff --git
a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/RequirePluginVersions.java
b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/RequirePluginVersions.java
index ad7baf5..1c9db6e 100644
---
a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/RequirePluginVersions.java
+++
b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/RequirePluginVersions.java
@@ -24,6 +24,7 @@ import javax.inject.Named;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -39,6 +40,7 @@ import
org.apache.maven.artifact.repository.ArtifactRepository;
import org.apache.maven.artifact.resolver.ArtifactNotFoundException;
import
org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
import org.apache.maven.artifact.versioning.VersionRange;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleError;
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
import org.apache.maven.enforcer.rules.utils.EnforcerRuleUtils;
import org.apache.maven.enforcer.rules.utils.ExpressionEvaluator;
@@ -50,11 +52,14 @@ import
org.apache.maven.lifecycle.LifecycleExecutionException;
import org.apache.maven.lifecycle.mapping.LifecycleMapping;
import org.apache.maven.model.BuildBase;
import org.apache.maven.model.Model;
+import org.apache.maven.model.ModelBase;
import org.apache.maven.model.Plugin;
+import org.apache.maven.model.PluginConfiguration;
+import org.apache.maven.model.PluginContainer;
import org.apache.maven.model.Profile;
import org.apache.maven.model.ReportPlugin;
+import org.apache.maven.model.Reporting;
import org.apache.maven.plugin.InvalidPluginException;
-import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugin.PluginManager;
import org.apache.maven.plugin.PluginManagerException;
import org.apache.maven.plugin.PluginNotFoundException;
@@ -72,6 +77,8 @@ import org.eclipse.aether.RepositorySystem;
import org.eclipse.aether.resolution.ArtifactRequest;
import org.eclipse.aether.resolution.ArtifactResolutionException;
+import static java.util.Optional.ofNullable;
+
/**
* This rule will enforce that all plugins specified in the poms have a
version declared.
*
@@ -127,6 +134,7 @@ public final class RequirePluginVersions extends
AbstractStandardEnforcerRule {
/**
* Same as unCheckedPlugins but as a comma list to better support
properties. Sample form:
* <code>group:artifactId,group2:artifactId2</code>
+ *
* @since 1.0-beta-1
*/
private String unCheckedPluginList;
@@ -145,9 +153,6 @@ public final class RequirePluginVersions extends
AbstractStandardEnforcerRule {
private final RepositorySystem repositorySystem;
- /** The local. */
- private ArtifactRepository local;
-
/** The session. */
private final MavenSession session;
@@ -195,7 +200,6 @@ public final class RequirePluginVersions extends
AbstractStandardEnforcerRule {
// get the various expressions out of the helper.
lifecycles = defaultLifeCycles.getLifeCycles();
- local = session.getLocalRepository();
// get all the plugins that are bound to the specified lifecycles
Set<Plugin> allPlugins = getBoundPlugins(project, phases);
@@ -236,18 +240,20 @@ public final class RequirePluginVersions extends
AbstractStandardEnforcerRule {
if (!failures.isEmpty()) {
handleMessagesToTheUser(project, failures);
}
- } catch (Exception e) {
+ } catch (PluginNotFoundException | LifecycleExecutionException e) {
throw new EnforcerRuleException(e.getLocalizedMessage(), e);
}
}
private void handleMessagesToTheUser(MavenProject project, List<Plugin>
failures) throws EnforcerRuleException {
StringBuilder newMsg = new StringBuilder();
- newMsg.append("Some plugins are missing valid versions or depend on
Maven "
- + runtimeInformation.getMavenVersion() + " defaults:");
+ newMsg.append("Some plugins are missing valid versions or depend on
Maven ");
+ newMsg.append(runtimeInformation.getMavenVersion());
+ newMsg.append(" defaults");
handleBanMessages(newMsg);
- newMsg.append("\n");
+ newMsg.append(System.lineSeparator());
for (Plugin plugin : failures) {
+ newMsg.append(" ");
newMsg.append(plugin.getGroupId());
newMsg.append(":");
newMsg.append(plugin.getArtifactId());
@@ -280,7 +286,7 @@ public final class RequirePluginVersions extends
AbstractStandardEnforcerRule {
getLog().debug("Exception while determining plugin Version " +
e.getMessage());
newMsg.append(". Unable to determine the plugin version.");
}
- newMsg.append("\n");
+ newMsg.append(System.lineSeparator());
}
String message = getMessage();
if (StringUtils.isNotEmpty(message)) {
@@ -292,17 +298,24 @@ public final class RequirePluginVersions extends
AbstractStandardEnforcerRule {
private void handleBanMessages(StringBuilder newMsg) {
if (banLatest || banRelease || banSnapshots || banTimestamps) {
- newMsg.append(" (");
+ List<String> banList = new ArrayList<>();
if (banLatest) {
- newMsg.append("LATEST ");
+ banList.add("LATEST");
}
if (banRelease) {
- newMsg.append("RELEASE ");
+ banList.add("RELEASE");
+ }
+ if (banSnapshots) {
+ banList.add("SNAPSHOT");
+ if (banTimestamps) {
+ banList.add("TIMESTAMP SNAPSHOT");
+ }
}
- if (banSnapshots || banTimestamps) {
- newMsg.append("SNAPSHOT ");
+ if (!banList.isEmpty()) {
+ newMsg.append(" (");
+ newMsg.append(String.join(", ", banList));
+ newMsg.append(" as plugin version are not allowed)");
}
- newMsg.append("are not allowed)");
}
}
@@ -312,10 +325,9 @@ public final class RequirePluginVersions extends
AbstractStandardEnforcerRule {
* @param uncheckedPlugins
* @param plugins
* @return The plugins which have been removed.
- * @throws MojoExecutionException
*/
Set<Plugin> removeUncheckedPlugins(Collection<String> uncheckedPlugins,
Set<Plugin> plugins)
- throws MojoExecutionException {
+ throws EnforcerRuleError {
if (uncheckedPlugins != null && !uncheckedPlugins.isEmpty()) {
for (String pluginKey : uncheckedPlugins) {
Plugin plugin = parsePluginString(pluginKey,
"UncheckedPlugins");
@@ -328,7 +340,7 @@ public final class RequirePluginVersions extends
AbstractStandardEnforcerRule {
/**
* Combines the old Collection with the new comma separated list.
*
- * @param uncheckedPlugins a new collections
+ * @param uncheckedPlugins a new collections
* @param uncheckedPluginsList a list to merge
* @return List of unchecked plugins.
*/
@@ -354,10 +366,9 @@ public final class RequirePluginVersions extends
AbstractStandardEnforcerRule {
* @param existing the existing
* @param additional the additional
* @return the sets the
- * @throws MojoExecutionException the mojo execution exception
+ * @throws EnforcerRuleError the enforcer error
*/
- public Set<Plugin> addAdditionalPlugins(Set<Plugin> existing, List<String>
additional)
- throws MojoExecutionException {
+ public Set<Plugin> addAdditionalPlugins(Set<Plugin> existing, List<String>
additional) throws EnforcerRuleError {
if (additional != null) {
for (String pluginString : additional) {
Plugin plugin = parsePluginString(pluginString,
"AdditionalPlugins");
@@ -377,11 +388,10 @@ public final class RequirePluginVersions extends
AbstractStandardEnforcerRule {
* Helper method to parse and inject a Plugin.
*
* @param pluginString a plugin description to parse
- * @param field a source of pluginString
+ * @param field a source of pluginString
* @return the prepared plugin
- * @throws MojoExecutionException in case of problems
*/
- private Plugin parsePluginString(String pluginString, String field) throws
MojoExecutionException {
+ private Plugin parsePluginString(String pluginString, String field) throws
EnforcerRuleError {
if (pluginString != null) {
String[] pluginStrings = pluginString.split(":");
if (pluginStrings.length == 2) {
@@ -391,10 +401,10 @@ public final class RequirePluginVersions extends
AbstractStandardEnforcerRule {
return plugin;
} else {
- throw new MojoExecutionException("Invalid " + field + "
string: " + pluginString);
+ throw new EnforcerRuleError("Invalid " + field + " string: " +
pluginString);
}
} else {
- throw new MojoExecutionException("Invalid " + field + " string: "
+ pluginString);
+ throw new EnforcerRuleError("Invalid " + field + " string: " +
pluginString);
}
}
@@ -813,42 +823,62 @@ public final class RequirePluginVersions extends
AbstractStandardEnforcerRule {
}
private void addPluginsInProfiles(List<PluginWrapper> plugins, Model
model) {
- List<Profile> profiles = model.getProfiles();
+ List<Profile> profiles =
ofNullable(model).map(Model::getProfiles).orElseGet(Collections::emptyList);
for (Profile profile : profiles) {
- getProfilePlugins(plugins, model, profile);
- getProfileReportingPlugins(plugins, model, profile);
- getProfilePluginManagementPlugins(plugins, model, profile);
+ getProfilePlugins(plugins, profile);
+ getProfileReportingPlugins(plugins, profile);
+ getProfilePluginManagementPlugins(plugins, profile);
}
}
- private void getProfilePluginManagementPlugins(List<PluginWrapper>
plugins, Model model, Profile profile) {
- List<Plugin> modelPlugins =
profile.getBuild().getPluginManagement().getPlugins();
+ private void getProfilePluginManagementPlugins(List<PluginWrapper>
plugins, Profile profile) {
+ List<Plugin> modelPlugins = ofNullable(profile)
+ .map(Profile::getBuild)
+ .map(PluginConfiguration::getPluginManagement)
+ .map(PluginContainer::getPlugins)
+ .orElseGet(Collections::emptyList);
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins),
banMavenDefaults));
}
- private void getProfileReportingPlugins(List<PluginWrapper> plugins, Model
model, Profile profile) {
- List<ReportPlugin> modelReportPlugins =
profile.getReporting().getPlugins();
+ private void getProfileReportingPlugins(List<PluginWrapper> plugins,
Profile profile) {
+ List<ReportPlugin> modelReportPlugins = ofNullable(profile)
+ .map(ModelBase::getReporting)
+ .map(Reporting::getPlugins)
+ .orElseGet(Collections::emptyList);
// add the reporting plugins
plugins.addAll(PluginWrapper.addAll(utils.resolveReportPlugins(modelReportPlugins),
banMavenDefaults));
}
- private void getProfilePlugins(List<PluginWrapper> plugins, Model model,
Profile profile) {
- List<Plugin> modelPlugins = profile.getBuild().getPlugins();
+ private void getProfilePlugins(List<PluginWrapper> plugins, Profile
profile) {
+ List<Plugin> modelPlugins = ofNullable(profile)
+ .map(Profile::getBuild)
+ .map(PluginContainer::getPlugins)
+ .orElseGet(Collections::emptyList);
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins),
banMavenDefaults));
}
private void getPlugins(List<PluginWrapper> plugins, Model model) {
- List<Plugin> modelPlugins = model.getBuild().getPlugins();
+ List<Plugin> modelPlugins = ofNullable(model)
+ .map(Model::getBuild)
+ .map(PluginContainer::getPlugins)
+ .orElseGet(Collections::emptyList);
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins),
banMavenDefaults));
}
private void getPluginManagementPlugins(List<PluginWrapper> plugins, Model
model) {
- List<Plugin> modelPlugins =
model.getBuild().getPluginManagement().getPlugins();
+ List<Plugin> modelPlugins = ofNullable(model)
+ .map(Model::getBuild)
+ .map(PluginConfiguration::getPluginManagement)
+ .map(PluginContainer::getPlugins)
+ .orElseGet(Collections::emptyList);
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins),
banMavenDefaults));
}
private void getReportingPlugins(List<PluginWrapper> plugins, Model model)
{
- List<ReportPlugin> modelReportPlugins =
model.getReporting().getPlugins();
+ List<ReportPlugin> modelReportPlugins = ofNullable(model)
+ .map(ModelBase::getReporting)
+ .map(Reporting::getPlugins)
+ .orElseGet(Collections::emptyList);
// add the reporting plugins
plugins.addAll(PluginWrapper.addAll(utils.resolveReportPlugins(modelReportPlugins),
banMavenDefaults));
}
diff --git
a/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/TestRequirePluginVersions.java
b/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/TestRequirePluginVersions.java
index 49f6320..155c65b 100644
---
a/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/TestRequirePluginVersions.java
+++
b/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/TestRequirePluginVersions.java
@@ -26,6 +26,7 @@ import java.util.Set;
import org.apache.maven.artifact.factory.ArtifactFactory;
import org.apache.maven.enforcer.rule.api.EnforcerLogger;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleError;
import org.apache.maven.enforcer.rules.utils.EnforcerRuleUtils;
import org.apache.maven.enforcer.rules.utils.ExpressionEvaluator;
import org.apache.maven.enforcer.rules.utils.PluginWrapper;
@@ -249,7 +250,7 @@ class TestRequirePluginVersions {
* @throws MojoExecutionException the mojo execution exception
*/
@Test
- void testGetAdditionalPluginsNull() throws MojoExecutionException {
+ void testGetAdditionalPluginsNull() throws Exception {
rule.addAdditionalPlugins(null, null);
}
@@ -268,7 +269,7 @@ class TestRequirePluginVersions {
try {
rule.addAdditionalPlugins(plugins, additional);
fail("Expected Exception because the format is invalid");
- } catch (MojoExecutionException e) {
+ } catch (EnforcerRuleError e) {
}
// invalid format (too many sections)
@@ -277,7 +278,7 @@ class TestRequirePluginVersions {
try {
rule.addAdditionalPlugins(plugins, additional);
fail("Expected Exception because the format is invalid");
- } catch (MojoExecutionException e) {
+ } catch (EnforcerRuleError e) {
}
}
@@ -287,7 +288,7 @@ class TestRequirePluginVersions {
* @throws MojoExecutionException the mojo execution exception
*/
@Test
- void testGetAdditionalPluginsEmptySet() throws MojoExecutionException {
+ void testGetAdditionalPluginsEmptySet() throws Exception {
Set<Plugin> plugins = new HashSet<>();
plugins.add(EnforcerTestUtils.newPlugin("group", "a-artifact", "1.0"));
@@ -312,7 +313,7 @@ class TestRequirePluginVersions {
* @throws MojoExecutionException the mojo execution exception
*/
@Test
- void testGetAdditionalPlugins() throws MojoExecutionException {
+ void testGetAdditionalPlugins() throws Exception {
Set<Plugin> plugins = new HashSet<>();
plugins.add(EnforcerTestUtils.newPlugin("group", "a-artifact", "1.0"));
@@ -338,7 +339,7 @@ class TestRequirePluginVersions {
* @throws MojoExecutionException the mojo execution exception
*/
@Test
- void testGetUncheckedPlugins() throws MojoExecutionException {
+ void testGetUncheckedPlugins() throws Exception {
Set<Plugin> plugins = new HashSet<>();
plugins.add(EnforcerTestUtils.newPlugin("group", "a-artifact", "1.0"));
diff --git
a/enforcer-rules/src/test/resources/requirePluginVersions/checkPluginVersionProfile/pom.xml
b/enforcer-rules/src/test/resources/requirePluginVersions/checkPluginVersionProfile/pom.xml
index 48a688a..7b310fa 100644
---
a/enforcer-rules/src/test/resources/requirePluginVersions/checkPluginVersionProfile/pom.xml
+++
b/enforcer-rules/src/test/resources/requirePluginVersions/checkPluginVersionProfile/pom.xml
@@ -64,12 +64,12 @@
<executions>
<execution>
<goals><goal>site</goal></goals>
+ <phase>validate</phase>
</execution>
- <phase>validate</phase>
</executions>
</plugin>
</plugins>
</build>
- </profile>
+ </profile>
</profiles>
</project>
\ No newline at end of file
diff --git
a/maven-enforcer-plugin/src/it/projects/require-plugin-versions-ci/verify.groovy
b/maven-enforcer-plugin/src/it/projects/require-plugin-versions-ci/verify.groovy
index f2d5288..d7501f8 100644
---
a/maven-enforcer-plugin/src/it/projects/require-plugin-versions-ci/verify.groovy
+++
b/maven-enforcer-plugin/src/it/projects/require-plugin-versions-ci/verify.groovy
@@ -18,4 +18,4 @@
*/
File buildLog = new File( basedir, 'build.log' )
assert buildLog.text.contains( '[ERROR] Rule 0:
org.apache.maven.enforcer.rules.RequirePluginVersions failed with message:' )
-assert buildLog.text.contains( "Some plugins are missing valid versions or
depend on Maven ${mavenVersion} defaults: (LATEST RELEASE SNAPSHOT are not
allowed)" )
+assert buildLog.text.contains( "Some plugins are missing valid versions or
depend on Maven ${mavenVersion} defaults (LATEST, RELEASE, SNAPSHOT, TIMESTAMP
SNAPSHOT as plugin version are not allowed)" )
diff --git
a/maven-enforcer-plugin/src/it/projects/require-plugin-versions/pom.xml
b/maven-enforcer-plugin/src/it/projects/require-plugin-versions/pom.xml
index beb7c2c..ed52b00 100644
--- a/maven-enforcer-plugin/src/it/projects/require-plugin-versions/pom.xml
+++ b/maven-enforcer-plugin/src/it/projects/require-plugin-versions/pom.xml
@@ -30,6 +30,12 @@ under the License.
<description>
</description>
+ <profiles>
+ <profile>
+ <id>test</id>
+ </profile>
+ </profiles>
+
<build>
<plugins>
<plugin>
diff --git
a/maven-enforcer-plugin/src/it/projects/require-plugin-versions_failure/verify.groovy
b/maven-enforcer-plugin/src/it/projects/require-plugin-versions_failure/verify.groovy
index f2d5288..f4e8074 100644
---
a/maven-enforcer-plugin/src/it/projects/require-plugin-versions_failure/verify.groovy
+++
b/maven-enforcer-plugin/src/it/projects/require-plugin-versions_failure/verify.groovy
@@ -18,4 +18,4 @@
*/
File buildLog = new File( basedir, 'build.log' )
assert buildLog.text.contains( '[ERROR] Rule 0:
org.apache.maven.enforcer.rules.RequirePluginVersions failed with message:' )
-assert buildLog.text.contains( "Some plugins are missing valid versions or
depend on Maven ${mavenVersion} defaults: (LATEST RELEASE SNAPSHOT are not
allowed)" )
+assert buildLog.text.contains( "Some plugins are missing valid versions or
depend on Maven ${mavenVersion} defaults (LATEST, RELEASE as plugin version are
not allowed)" )
\ No newline at end of file
