Skip to content

Commit 3e12af9

Browse files
slawekjaranowskislawekjaranowski
committed
[MENFORCER-461] Fix NPE in RequirePluginVersions
And a little error message improvement
1 parent 86c0cc5 commit 3e12af9

File tree

6 files changed

+87
-50
lines changed

6 files changed

+87
-50
lines changed

‎enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/RequirePluginVersions.java

Lines changed: 70 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.ArrayList;
2525
import java.util.Arrays;
2626
import java.util.Collection;
27+
import java.util.Collections;
2728
import java.util.HashMap;
2829
import java.util.HashSet;
2930
import java.util.List;
@@ -39,6 +40,7 @@
3940
import org.apache.maven.artifact.resolver.ArtifactNotFoundException;
4041
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
4142
import org.apache.maven.artifact.versioning.VersionRange;
43+
import org.apache.maven.enforcer.rule.api.EnforcerRuleError;
4244
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
4345
import org.apache.maven.enforcer.rules.utils.EnforcerRuleUtils;
4446
import org.apache.maven.enforcer.rules.utils.ExpressionEvaluator;
@@ -50,11 +52,14 @@
5052
import org.apache.maven.lifecycle.mapping.LifecycleMapping;
5153
import org.apache.maven.model.BuildBase;
5254
import org.apache.maven.model.Model;
55+
import org.apache.maven.model.ModelBase;
5356
import org.apache.maven.model.Plugin;
57+
import org.apache.maven.model.PluginConfiguration;
58+
import org.apache.maven.model.PluginContainer;
5459
import org.apache.maven.model.Profile;
5560
import org.apache.maven.model.ReportPlugin;
61+
import org.apache.maven.model.Reporting;
5662
import org.apache.maven.plugin.InvalidPluginException;
57-
import org.apache.maven.plugin.MojoExecutionException;
5863
import org.apache.maven.plugin.PluginManager;
5964
import org.apache.maven.plugin.PluginManagerException;
6065
import org.apache.maven.plugin.PluginNotFoundException;
@@ -72,6 +77,8 @@
7277
import org.eclipse.aether.resolution.ArtifactRequest;
7378
import org.eclipse.aether.resolution.ArtifactResolutionException;
7479

80+
import static java.util.Optional.ofNullable;
81+
7582
/**
7683
* This rule will enforce that all plugins specified in the poms have a version declared.
7784
*
@@ -127,6 +134,7 @@ public final class RequirePluginVersions extends AbstractStandardEnforcerRule {
127134
/**
128135
* Same as unCheckedPlugins but as a comma list to better support properties. Sample form:
129136
* <code>group:artifactId,group2:artifactId2</code>
137+
*
130138
* @since 1.0-beta-1
131139
*/
132140
private String unCheckedPluginList;
@@ -145,9 +153,6 @@ public final class RequirePluginVersions extends AbstractStandardEnforcerRule {
145153

146154
private final RepositorySystem repositorySystem;
147155

148-
/** The local. */
149-
private ArtifactRepository local;
150-
151156
/** The session. */
152157
private final MavenSession session;
153158

@@ -195,7 +200,6 @@ public void execute() throws EnforcerRuleException {
195200
// get the various expressions out of the helper.
196201

197202
lifecycles = defaultLifeCycles.getLifeCycles();
198-
local = session.getLocalRepository();
199203

200204
// get all the plugins that are bound to the specified lifecycles
201205
Set<Plugin> allPlugins = getBoundPlugins(project, phases);
@@ -236,18 +240,20 @@ public void execute() throws EnforcerRuleException {
236240
if (!failures.isEmpty()) {
237241
handleMessagesToTheUser(project, failures);
238242
}
239-
} catch (Exception e) {
243+
} catch (PluginNotFoundException | LifecycleExecutionException e) {
240244
throw new EnforcerRuleException(e.getLocalizedMessage(), e);
241245
}
242246
}
243247

244248
private void handleMessagesToTheUser(MavenProject project, List<Plugin> failures) throws EnforcerRuleException {
245249
StringBuilder newMsg = new StringBuilder();
246-
newMsg.append("Some plugins are missing valid versions or depend on Maven "
247-
+ runtimeInformation.getMavenVersion() + " defaults:");
250+
newMsg.append("Some plugins are missing valid versions or depend on Maven ");
251+
newMsg.append(runtimeInformation.getMavenVersion());
252+
newMsg.append(" defaults");
248253
handleBanMessages(newMsg);
249-
newMsg.append("\n");
254+
newMsg.append(System.lineSeparator());
250255
for (Plugin plugin : failures) {
256+
newMsg.append(" ");
251257
newMsg.append(plugin.getGroupId());
252258
newMsg.append(":");
253259
newMsg.append(plugin.getArtifactId());
@@ -280,7 +286,7 @@ private void handleMessagesToTheUser(MavenProject project, List<Plugin> failures
280286
getLog().debug("Exception while determining plugin Version " + e.getMessage());
281287
newMsg.append(". Unable to determine the plugin version.");
282288
}
283-
newMsg.append("\n");
289+
newMsg.append(System.lineSeparator());
284290
}
285291
String message = getMessage();
286292
if (StringUtils.isNotEmpty(message)) {
@@ -292,17 +298,24 @@ private void handleMessagesToTheUser(MavenProject project, List<Plugin> failures
292298

293299
private void handleBanMessages(StringBuilder newMsg) {
294300
if (banLatest || banRelease || banSnapshots || banTimestamps) {
295-
newMsg.append(" (");
301+
List<String> banList = new ArrayList<>();
296302
if (banLatest) {
297-
newMsg.append("LATEST ");
303+
banList.add("LATEST");
298304
}
299305
if (banRelease) {
300-
newMsg.append("RELEASE ");
306+
banList.add("RELEASE");
307+
}
308+
if (banSnapshots) {
309+
banList.add("SNAPSHOT");
310+
if (banTimestamps) {
311+
banList.add("TIMESTAMP SNAPSHOT");
312+
}
301313
}
302-
if (banSnapshots || banTimestamps) {
303-
newMsg.append("SNAPSHOT ");
314+
if (!banList.isEmpty()) {
315+
newMsg.append(" (");
316+
newMsg.append(String.join(", ", banList));
317+
newMsg.append(" as plugin version are not allowed)");
304318
}
305-
newMsg.append("are not allowed)");
306319
}
307320
}
308321

@@ -312,10 +325,9 @@ private void handleBanMessages(StringBuilder newMsg) {
312325
* @param uncheckedPlugins
313326
* @param plugins
314327
* @return The plugins which have been removed.
315-
* @throws MojoExecutionException
316328
*/
317329
Set<Plugin> removeUncheckedPlugins(Collection<String> uncheckedPlugins, Set<Plugin> plugins)
318-
throws MojoExecutionException {
330+
throws EnforcerRuleError {
319331
if (uncheckedPlugins != null && !uncheckedPlugins.isEmpty()) {
320332
for (String pluginKey : uncheckedPlugins) {
321333
Plugin plugin = parsePluginString(pluginKey, "UncheckedPlugins");
@@ -328,7 +340,7 @@ Set<Plugin> removeUncheckedPlugins(Collection<String> uncheckedPlugins, Set<Plug
328340
/**
329341
* Combines the old Collection with the new comma separated list.
330342
*
331-
* @param uncheckedPlugins a new collections
343+
* @param uncheckedPlugins a new collections
332344
* @param uncheckedPluginsList a list to merge
333345
* @return List of unchecked plugins.
334346
*/
@@ -354,10 +366,9 @@ public Collection<String> combineUncheckedPlugins(
354366
* @param existing the existing
355367
* @param additional the additional
356368
* @return the sets the
357-
* @throws MojoExecutionException the mojo execution exception
369+
* @throws EnforcerRuleError the enforcer error
358370
*/
359-
public Set<Plugin> addAdditionalPlugins(Set<Plugin> existing, List<String> additional)
360-
throws MojoExecutionException {
371+
public Set<Plugin> addAdditionalPlugins(Set<Plugin> existing, List<String> additional) throws EnforcerRuleError {
361372
if (additional != null) {
362373
for (String pluginString : additional) {
363374
Plugin plugin = parsePluginString(pluginString, "AdditionalPlugins");
@@ -377,11 +388,10 @@ public Set<Plugin> addAdditionalPlugins(Set<Plugin> existing, List<String> addit
377388
* Helper method to parse and inject a Plugin.
378389
*
379390
* @param pluginString a plugin description to parse
380-
* @param field a source of pluginString
391+
* @param field a source of pluginString
381392
* @return the prepared plugin
382-
* @throws MojoExecutionException in case of problems
383393
*/
384-
private Plugin parsePluginString(String pluginString, String field) throws MojoExecutionException {
394+
private Plugin parsePluginString(String pluginString, String field) throws EnforcerRuleError {
385395
if (pluginString != null) {
386396
String[] pluginStrings = pluginString.split(":");
387397
if (pluginStrings.length == 2) {
@@ -391,10 +401,10 @@ private Plugin parsePluginString(String pluginString, String field) throws MojoE
391401

392402
return plugin;
393403
} else {
394-
throw new MojoExecutionException("Invalid " + field + " string: " + pluginString);
404+
throw new EnforcerRuleError("Invalid " + field + " string: " + pluginString);
395405
}
396406
} else {
397-
throw new MojoExecutionException("Invalid " + field + " string: " + pluginString);
407+
throw new EnforcerRuleError("Invalid " + field + " string: " + pluginString);
398408
}
399409
}
400410

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

815825
private void addPluginsInProfiles(List<PluginWrapper> plugins, Model model) {
816-
List<Profile> profiles = model.getProfiles();
826+
List<Profile> profiles = ofNullable(model).map(Model::getProfiles).orElseGet(Collections::emptyList);
817827
for (Profile profile : profiles) {
818-
getProfilePlugins(plugins, model, profile);
819-
getProfileReportingPlugins(plugins, model, profile);
820-
getProfilePluginManagementPlugins(plugins, model, profile);
828+
getProfilePlugins(plugins, profile);
829+
getProfileReportingPlugins(plugins, profile);
830+
getProfilePluginManagementPlugins(plugins, profile);
821831
}
822832
}
823833

824-
private void getProfilePluginManagementPlugins(List<PluginWrapper> plugins, Model model, Profile profile) {
825-
List<Plugin> modelPlugins = profile.getBuild().getPluginManagement().getPlugins();
834+
private void getProfilePluginManagementPlugins(List<PluginWrapper> plugins, Profile profile) {
835+
List<Plugin> modelPlugins = ofNullable(profile)
836+
.map(Profile::getBuild)
837+
.map(PluginConfiguration::getPluginManagement)
838+
.map(PluginContainer::getPlugins)
839+
.orElseGet(Collections::emptyList);
826840
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins), banMavenDefaults));
827841
}
828842

829-
private void getProfileReportingPlugins(List<PluginWrapper> plugins, Model model, Profile profile) {
830-
List<ReportPlugin> modelReportPlugins = profile.getReporting().getPlugins();
843+
private void getProfileReportingPlugins(List<PluginWrapper> plugins, Profile profile) {
844+
List<ReportPlugin> modelReportPlugins = ofNullable(profile)
845+
.map(ModelBase::getReporting)
846+
.map(Reporting::getPlugins)
847+
.orElseGet(Collections::emptyList);
831848
// add the reporting plugins
832849
plugins.addAll(PluginWrapper.addAll(utils.resolveReportPlugins(modelReportPlugins), banMavenDefaults));
833850
}
834851

835-
private void getProfilePlugins(List<PluginWrapper> plugins, Model model, Profile profile) {
836-
List<Plugin> modelPlugins = profile.getBuild().getPlugins();
852+
private void getProfilePlugins(List<PluginWrapper> plugins, Profile profile) {
853+
List<Plugin> modelPlugins = ofNullable(profile)
854+
.map(Profile::getBuild)
855+
.map(PluginContainer::getPlugins)
856+
.orElseGet(Collections::emptyList);
837857
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins), banMavenDefaults));
838858
}
839859

840860
private void getPlugins(List<PluginWrapper> plugins, Model model) {
841-
List<Plugin> modelPlugins = model.getBuild().getPlugins();
861+
List<Plugin> modelPlugins = ofNullable(model)
862+
.map(Model::getBuild)
863+
.map(PluginContainer::getPlugins)
864+
.orElseGet(Collections::emptyList);
842865
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins), banMavenDefaults));
843866
}
844867

845868
private void getPluginManagementPlugins(List<PluginWrapper> plugins, Model model) {
846-
List<Plugin> modelPlugins = model.getBuild().getPluginManagement().getPlugins();
869+
List<Plugin> modelPlugins = ofNullable(model)
870+
.map(Model::getBuild)
871+
.map(PluginConfiguration::getPluginManagement)
872+
.map(PluginContainer::getPlugins)
873+
.orElseGet(Collections::emptyList);
847874
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins), banMavenDefaults));
848875
}
849876

850877
private void getReportingPlugins(List<PluginWrapper> plugins, Model model) {
851-
List<ReportPlugin> modelReportPlugins = model.getReporting().getPlugins();
878+
List<ReportPlugin> modelReportPlugins = ofNullable(model)
879+
.map(ModelBase::getReporting)
880+
.map(Reporting::getPlugins)
881+
.orElseGet(Collections::emptyList);
852882
// add the reporting plugins
853883
plugins.addAll(PluginWrapper.addAll(utils.resolveReportPlugins(modelReportPlugins), banMavenDefaults));
854884
}

‎enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/TestRequirePluginVersions.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import org.apache.maven.artifact.factory.ArtifactFactory;
2828
import org.apache.maven.enforcer.rule.api.EnforcerLogger;
29+
import org.apache.maven.enforcer.rule.api.EnforcerRuleError;
2930
import org.apache.maven.enforcer.rules.utils.EnforcerRuleUtils;
3031
import org.apache.maven.enforcer.rules.utils.ExpressionEvaluator;
3132
import org.apache.maven.enforcer.rules.utils.PluginWrapper;
@@ -249,7 +250,7 @@ void testHasVersionSpecifiedWithProperties() throws Exception {
249250
* @throws MojoExecutionException the mojo execution exception
250251
*/
251252
@Test
252-
void testGetAdditionalPluginsNull() throws MojoExecutionException {
253+
void testGetAdditionalPluginsNull() throws Exception {
253254
rule.addAdditionalPlugins(null, null);
254255
}
255256

@@ -268,7 +269,7 @@ void testGetAdditionalPluginsInvalidFormat() {
268269
try {
269270
rule.addAdditionalPlugins(plugins, additional);
270271
fail("Expected Exception because the format is invalid");
271-
} catch (MojoExecutionException e) {
272+
} catch (EnforcerRuleError e) {
272273
}
273274

274275
// invalid format (too many sections)
@@ -277,7 +278,7 @@ void testGetAdditionalPluginsInvalidFormat() {
277278
try {
278279
rule.addAdditionalPlugins(plugins, additional);
279280
fail("Expected Exception because the format is invalid");
280-
} catch (MojoExecutionException e) {
281+
} catch (EnforcerRuleError e) {
281282
}
282283
}
283284

@@ -287,7 +288,7 @@ void testGetAdditionalPluginsInvalidFormat() {
287288
* @throws MojoExecutionException the mojo execution exception
288289
*/
289290
@Test
290-
void testGetAdditionalPluginsEmptySet() throws MojoExecutionException {
291+
void testGetAdditionalPluginsEmptySet() throws Exception {
291292

292293
Set<Plugin> plugins = new HashSet<>();
293294
plugins.add(EnforcerTestUtils.newPlugin("group", "a-artifact", "1.0"));
@@ -312,7 +313,7 @@ void testGetAdditionalPluginsEmptySet() throws MojoExecutionException {
312313
* @throws MojoExecutionException the mojo execution exception
313314
*/
314315
@Test
315-
void testGetAdditionalPlugins() throws MojoExecutionException {
316+
void testGetAdditionalPlugins() throws Exception {
316317

317318
Set<Plugin> plugins = new HashSet<>();
318319
plugins.add(EnforcerTestUtils.newPlugin("group", "a-artifact", "1.0"));
@@ -338,7 +339,7 @@ void testGetAdditionalPlugins() throws MojoExecutionException {
338339
* @throws MojoExecutionException the mojo execution exception
339340
*/
340341
@Test
341-
void testGetUncheckedPlugins() throws MojoExecutionException {
342+
void testGetUncheckedPlugins() throws Exception {
342343

343344
Set<Plugin> plugins = new HashSet<>();
344345
plugins.add(EnforcerTestUtils.newPlugin("group", "a-artifact", "1.0"));

‎enforcer-rules/src/test/resources/requirePluginVersions/checkPluginVersionProfile/pom.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,12 @@
6464
<executions>
6565
<execution>
6666
<goals><goal>site</goal></goals>
67+
<phase>validate</phase>
6768
</execution>
68-
<phase>validate</phase>
6969
</executions>
7070
</plugin>
7171
</plugins>
7272
</build>
73-
</profile>
73+
</profile>
7474
</profiles>
7575
</project>

‎maven-enforcer-plugin/src/it/projects/require-plugin-versions-ci/verify.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,4 @@
1818
*/
1919
File buildLog = new File( basedir, 'build.log' )
2020
assert buildLog.text.contains( '[ERROR] Rule 0: org.apache.maven.enforcer.rules.RequirePluginVersions failed with message:' )
21-
assert buildLog.text.contains( "Some plugins are missing valid versions or depend on Maven ${mavenVersion} defaults: (LATEST RELEASE SNAPSHOT are not allowed)" )
21+
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)" )

‎maven-enforcer-plugin/src/it/projects/require-plugin-versions/pom.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ under the License.
3030
<description>
3131
</description>
3232

33+
<profiles>
34+
<profile>
35+
<id>test</id>
36+
</profile>
37+
</profiles>
38+
3339
<build>
3440
<plugins>
3541
<plugin>

‎maven-enforcer-plugin/src/it/projects/require-plugin-versions_failure/verify.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,4 @@
1818
*/
1919
File buildLog = new File( basedir, 'build.log' )
2020
assert buildLog.text.contains( '[ERROR] Rule 0: org.apache.maven.enforcer.rules.RequirePluginVersions failed with message:' )
21-
assert buildLog.text.contains( "Some plugins are missing valid versions or depend on Maven ${mavenVersion} defaults: (LATEST RELEASE SNAPSHOT are not allowed)" )
21+
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 commit comments

Comments
 (0)