Skip to content

Commit

Permalink
[MENFORCER-461] Fix NPE in RequirePluginVersions
Browse files Browse the repository at this point in the history 
And a little error message improvement
  • Loading branch information
@slawekjaranowski
slawekjaranowski committed Jan 28, 2023
1 parent 86c0cc5 commit 3e12af9
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 50 deletions.
110 changes: 70 additions & 40 deletions enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/RequirePluginVersions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
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;
Expand All @@ -39,6 +40,7 @@
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;
Expand All @@ -50,11 +52,14 @@
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;
Expand All @@ -72,6 +77,8 @@
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.
*
Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -195,7 +200,6 @@ public void execute() throws EnforcerRuleException {
// 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);
Expand Down Expand Up @@ -236,18 +240,20 @@ public void execute() throws EnforcerRuleException {
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());
Expand Down Expand Up @@ -280,7 +286,7 @@ private void handleMessagesToTheUser(MavenProject project, List<Plugin> failures
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)) {
Expand All @@ -292,17 +298,24 @@ private void handleMessagesToTheUser(MavenProject project, List<Plugin> failures

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)");
}
}

Expand All @@ -312,10 +325,9 @@ private void handleBanMessages(StringBuilder newMsg) {
* @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");
Expand All @@ -328,7 +340,7 @@ Set<Plugin> removeUncheckedPlugins(Collection<String> uncheckedPlugins, Set<Plug
/**
* 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.
*/
Expand All @@ -354,10 +366,9 @@ public Collection<String> combineUncheckedPlugins(
* @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");
Expand All @@ -377,11 +388,10 @@ public Set<Plugin> addAdditionalPlugins(Set<Plugin> existing, List<String> addit
* 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) {
Expand All @@ -391,10 +401,10 @@ private Plugin parsePluginString(String pluginString, String field) throws MojoE

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);
}
}

Expand Down Expand Up @@ -813,42 +823,62 @@ private List<PluginWrapper> getAllPluginEntries(MavenProject project) {
}

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));
}
Expand Down
13 changes: 7 additions & 6 deletions enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/TestRequirePluginVersions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

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;
Expand Down Expand Up @@ -249,7 +250,7 @@ void testHasVersionSpecifiedWithProperties() throws Exception {
* @throws MojoExecutionException the mojo execution exception
*/
@Test
void testGetAdditionalPluginsNull() throws MojoExecutionException {
void testGetAdditionalPluginsNull() throws Exception {
rule.addAdditionalPlugins(null, null);
}

Expand All @@ -268,7 +269,7 @@ void testGetAdditionalPluginsInvalidFormat() {
try {
rule.addAdditionalPlugins(plugins, additional);
fail("Expected Exception because the format is invalid");
} catch (MojoExecutionException e) {
} catch (EnforcerRuleError e) {
}

// invalid format (too many sections)
Expand All @@ -277,7 +278,7 @@ void testGetAdditionalPluginsInvalidFormat() {
try {
rule.addAdditionalPlugins(plugins, additional);
fail("Expected Exception because the format is invalid");
} catch (MojoExecutionException e) {
} catch (EnforcerRuleError e) {
}
}

Expand All @@ -287,7 +288,7 @@ void testGetAdditionalPluginsInvalidFormat() {
* @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"));
Expand All @@ -312,7 +313,7 @@ void testGetAdditionalPluginsEmptySet() throws MojoExecutionException {
* @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"));
Expand All @@ -338,7 +339,7 @@ void testGetAdditionalPlugins() throws MojoExecutionException {
* @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"));
Expand Down
4 changes: 2 additions & 2 deletions enforcer-rules/src/test/resources/requirePluginVersions/checkPluginVersionProfile/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
2 changes: 1 addition & 1 deletion maven-enforcer-plugin/src/it/projects/require-plugin-versions-ci/verify.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)" )
6 changes: 6 additions & 0 deletions maven-enforcer-plugin/src/it/projects/require-plugin-versions/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ under the License.
<description>
</description>

<profiles>
<profile>
<id>test</id>
</profile>
</profiles>

<build>
<plugins>
<plugin>
Expand Down
2 changes: 1 addition & 1 deletion maven-enforcer-plugin/src/it/projects/require-plugin-versions_failure/verify.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)" )

0 comments on commit 3e12af9

Please sign in to comment.