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 c04468d  [MENFORCER-459] Avoid NullPointerException where there is no 
plugins (#193)
c04468d is described below

commit c04468d8521db5b0f9aa1fb532f3d9a7324e74e7
Author: Andrey Turbanov <[email protected]>
AuthorDate: Wed Jan 11 00:04:06 2023 +0300

    [MENFORCER-459] Avoid NullPointerException where there is no plugins (#193)
    
    Exceptions shouldn't be used for non-exceptional flow.
    Also it's considered bad practice to return null from a method which 
returns collection.
---
 .../maven/enforcer/rules/utils/PluginWrapper.java  | 35 ++++++++-------
 .../plugins/enforcer/RequirePluginVersions.java    | 52 ++++++----------------
 2 files changed, 32 insertions(+), 55 deletions(-)

diff --git 
a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/utils/PluginWrapper.java
 
b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/utils/PluginWrapper.java
index 18103fd..5fd5e41 100644
--- 
a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/utils/PluginWrapper.java
+++ 
b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/utils/PluginWrapper.java
@@ -19,6 +19,7 @@
 package org.apache.maven.enforcer.rules.utils;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
@@ -41,24 +42,24 @@ public class PluginWrapper {
     private final InputLocationTracker locationTracker;
 
     public static List<PluginWrapper> addAll(List<? extends 
InputLocationTracker> plugins, boolean banMavenDefaults) {
-        List<PluginWrapper> results = null;
-
-        if (!plugins.isEmpty()) {
-            results = new ArrayList<>(plugins.size());
-            for (InputLocationTracker o : plugins) {
-                // null or true means it is most assumed a Maven default
-                if (banMavenDefaults
-                        && 
(isVersionFromDefaultLifecycleBindings(o).orElse(true)
-                                || isVersionFromSuperpom(o).orElse(true))) {
-                    continue;
-                }
+        if (plugins.isEmpty()) {
+            return Collections.emptyList();
+        }
+
+        List<PluginWrapper> results = new ArrayList<>(plugins.size());
+        for (InputLocationTracker o : plugins) {
+            // null or true means it is most assumed a Maven default
+            if (banMavenDefaults
+                    && (isVersionFromDefaultLifecycleBindings(o).orElse(true)
+                            || isVersionFromSuperpom(o).orElse(true))) {
+                continue;
+            }
 
-                if (o instanceof Plugin) {
-                    results.add(new PluginWrapper((Plugin) o));
-                } else {
-                    if (o instanceof ReportPlugin) {
-                        results.add(new PluginWrapper((ReportPlugin) o));
-                    }
+            if (o instanceof Plugin) {
+                results.add(new PluginWrapper((Plugin) o));
+            } else {
+                if (o instanceof ReportPlugin) {
+                    results.add(new PluginWrapper((ReportPlugin) o));
                 }
             }
         }
diff --git 
a/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequirePluginVersions.java
 
b/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequirePluginVersions.java
index eb75161..a552f03 100644
--- 
a/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequirePluginVersions.java
+++ 
b/enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/RequirePluginVersions.java
@@ -834,59 +834,35 @@ public class RequirePluginVersions extends 
AbstractNonCacheableEnforcerRule {
     }
 
     private void getProfilePluginManagementPlugins(List<PluginWrapper> 
plugins, Model model, Profile profile) {
-        try {
-            List<Plugin> modelPlugins = 
profile.getBuild().getPluginManagement().getPlugins();
-            
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins), 
banMavenDefaults));
-        } catch (NullPointerException e) {
-            // guess there are no plugins here.
-        }
+        List<Plugin> modelPlugins = 
profile.getBuild().getPluginManagement().getPlugins();
+        
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins), 
banMavenDefaults));
     }
 
     private void getProfileReportingPlugins(List<PluginWrapper> plugins, Model 
model, Profile profile) {
-        try {
-            List<ReportPlugin> modelReportPlugins = 
profile.getReporting().getPlugins();
-            // add the reporting plugins
-            
plugins.addAll(PluginWrapper.addAll(utils.resolveReportPlugins(modelReportPlugins),
 banMavenDefaults));
-        } catch (NullPointerException e) {
-            // guess there are no plugins here.
-        }
+        List<ReportPlugin> modelReportPlugins = 
profile.getReporting().getPlugins();
+        // add the reporting plugins
+        
plugins.addAll(PluginWrapper.addAll(utils.resolveReportPlugins(modelReportPlugins),
 banMavenDefaults));
     }
 
     private void getProfilePlugins(List<PluginWrapper> plugins, Model model, 
Profile profile) {
-        try {
-            List<Plugin> modelPlugins = profile.getBuild().getPlugins();
-            
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins), 
banMavenDefaults));
-        } catch (NullPointerException e) {
-            // guess there are no plugins here.
-        }
+        List<Plugin> modelPlugins = profile.getBuild().getPlugins();
+        
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins), 
banMavenDefaults));
     }
 
     private void getPlugins(List<PluginWrapper> plugins, Model model) {
-        try {
-            List<Plugin> modelPlugins = model.getBuild().getPlugins();
-            
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins), 
banMavenDefaults));
-        } catch (NullPointerException e) {
-            // guess there are no plugins here.
-        }
+        List<Plugin> modelPlugins = model.getBuild().getPlugins();
+        
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins), 
banMavenDefaults));
     }
 
     private void getPluginManagementPlugins(List<PluginWrapper> plugins, Model 
model) {
-        try {
-            List<Plugin> modelPlugins = 
model.getBuild().getPluginManagement().getPlugins();
-            
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins), 
banMavenDefaults));
-        } catch (NullPointerException e) {
-            // guess there are no plugins here.
-        }
+        List<Plugin> modelPlugins = 
model.getBuild().getPluginManagement().getPlugins();
+        
plugins.addAll(PluginWrapper.addAll(utils.resolvePlugins(modelPlugins), 
banMavenDefaults));
     }
 
     private void getReportingPlugins(List<PluginWrapper> plugins, Model model) 
{
-        try {
-            List<ReportPlugin> modelReportPlugins = 
model.getReporting().getPlugins();
-            // add the reporting plugins
-            
plugins.addAll(PluginWrapper.addAll(utils.resolveReportPlugins(modelReportPlugins),
 banMavenDefaults));
-        } catch (NullPointerException e) {
-            // guess there are no plugins here.
-        }
+        List<ReportPlugin> modelReportPlugins = 
model.getReporting().getPlugins();
+        // add the reporting plugins
+        
plugins.addAll(PluginWrapper.addAll(utils.resolveReportPlugins(modelReportPlugins),
 banMavenDefaults));
     }
 
     /**

Reply via email to