Fix some quality flaws

7 files changed, 78 insertions, 68 deletions

diff --git a/plugins/sonar-maven-batch-plugin/src/main/java/org/sonar/plugins/maven/MavenProjectBootstrapper.java b/plugins/sonar-maven-batch-plugin/src/main/java/org/sonar/plugins/maven/MavenProjectBootstrapper.java
index 48fbdb91058..eb1c4f69ea8 100644
--- a/plugins/sonar-maven-batch-plugin/src/main/java/org/sonar/plugins/maven/MavenProjectBootstrapper.java
+++ b/plugins/sonar-maven-batch-plugin/src/main/java/org/sonar/plugins/maven/MavenProjectBootstrapper.java
@@ -49,7 +49,12 @@ public class MavenProjectBootstrapper extends ProjectBootstrapper {
         break;
       }
     }
-    return new ProjectReactor(mavenProjectConverter.configure(sortedProjects, topLevelProject));
+    if (topLevelProject != null && sortedProjects != null) {
+      return new ProjectReactor(mavenProjectConverter.configure(sortedProjects, topLevelProject));
+    }
+    else {
+      throw new IllegalStateException("Maven session is not in a good state. No top level project or empty reactor.");
+    }
   }
 
 }
diff --git a/plugins/sonar-maven-batch-plugin/src/main/java/org/sonar/plugins/maven/MavenProjectConverter.java b/plugins/sonar-maven-batch-plugin/src/main/java/org/sonar/plugins/maven/MavenProjectConverter.java
index c6ac61c728d..bc605474136 100644
--- a/plugins/sonar-maven-batch-plugin/src/main/java/org/sonar/plugins/maven/MavenProjectConverter.java
+++ b/plugins/sonar-maven-batch-plugin/src/main/java/org/sonar/plugins/maven/MavenProjectConverter.java
@@ -37,6 +37,8 @@ import org.sonar.api.utils.SonarException;
 import org.sonar.batch.scan.filesystem.DefaultModuleFileSystem;
 import org.sonar.java.api.JavaUtils;
 
+import javax.annotation.Nullable;
+
 import java.io.File;
 import java.io.IOException;
 import java.util.Arrays;
@@ -54,31 +56,9 @@ public class MavenProjectConverter implements TaskExtension {
     Map<MavenProject, ProjectDefinition> defs = Maps.newHashMap();
 
     try {
-      for (MavenProject pom : poms) {
-        paths.put(pom.getFile().getCanonicalPath(), pom);
-        ProjectDefinition def = ProjectDefinition.create();
-        merge(pom, def);
-        defs.put(pom, def);
-      }
+      configureModules(poms, paths, defs);
 
-      for (Map.Entry<String, MavenProject> entry : paths.entrySet()) {
-        MavenProject pom = entry.getValue();
-        for (Object m : pom.getModules()) {
-          String moduleId = (String) m;
-          File modulePath = new File(pom.getBasedir(), moduleId);
-          MavenProject module = findMavenProject(modulePath, paths);
-
-          ProjectDefinition parentProject = defs.get(pom);
-          if (parentProject == null) {
-            throw new IllegalStateException(UNABLE_TO_DETERMINE_PROJECT_STRUCTURE_EXCEPTION_MESSAGE);
-          }
-          ProjectDefinition subProject = defs.get(module);
-          if (subProject == null) {
-            throw new IllegalStateException(UNABLE_TO_DETERMINE_PROJECT_STRUCTURE_EXCEPTION_MESSAGE);
-          }
-          parentProject.addSubProject(subProject);
-        }
-      }
+      rebuildModuleHierarchy(paths, defs);
     } catch (IOException e) {
       throw new SonarException(e);
     }
@@ -90,6 +70,36 @@ public class MavenProjectConverter implements TaskExtension {
     return rootProject;
   }
 
+  private void rebuildModuleHierarchy(Map<String, MavenProject> paths, Map<MavenProject, ProjectDefinition> defs) throws IOException {
+    for (Map.Entry<String, MavenProject> entry : paths.entrySet()) {
+      MavenProject pom = entry.getValue();
+      for (Object m : pom.getModules()) {
+        String moduleId = (String) m;
+        File modulePath = new File(pom.getBasedir(), moduleId);
+        MavenProject module = findMavenProject(modulePath, paths);
+
+        ProjectDefinition parentProject = defs.get(pom);
+        if (parentProject == null) {
+          throw new IllegalStateException(UNABLE_TO_DETERMINE_PROJECT_STRUCTURE_EXCEPTION_MESSAGE);
+        }
+        ProjectDefinition subProject = defs.get(module);
+        if (subProject == null) {
+          throw new IllegalStateException(UNABLE_TO_DETERMINE_PROJECT_STRUCTURE_EXCEPTION_MESSAGE);
+        }
+        parentProject.addSubProject(subProject);
+      }
+    }
+  }
+
+  private void configureModules(List<MavenProject> poms, Map<String, MavenProject> paths, Map<MavenProject, ProjectDefinition> defs) throws IOException {
+    for (MavenProject pom : poms) {
+      paths.put(pom.getFile().getCanonicalPath(), pom);
+      ProjectDefinition def = ProjectDefinition.create();
+      merge(pom, def);
+      defs.put(pom, def);
+    }
+  }
+
   private static MavenProject findMavenProject(final File modulePath, Map<String, MavenProject> paths) throws IOException {
     if (modulePath.exists() && modulePath.isDirectory()) {
       for (Map.Entry<String, MavenProject> entry : paths.entrySet()) {
@@ -220,11 +230,10 @@ public class MavenProjectConverter implements TaskExtension {
         getBuildDir(pom),
         resolvePaths((List<String>) pom.getCompileSourceRoots(), pom.getBasedir()),
         resolvePaths((List<String>) pom.getTestCompileSourceRoots(), pom.getBasedir()),
-        Arrays.asList(resolvePath(pom.getBuild().getOutputDirectory(), pom.getBasedir()))
-        );
+        Arrays.asList(resolvePath(pom.getBuild().getOutputDirectory(), pom.getBasedir())));
   }
 
-  static File resolvePath(String path, File basedir) {
+  static File resolvePath(@Nullable String path, File basedir) {
     if (path != null) {
       File file = new File(path);
       if (!file.isAbsolute()) {
diff --git a/plugins/sonar-maven-batch-plugin/src/main/java/org/sonar/plugins/maven/RealMavenPluginExecutor.java b/plugins/sonar-maven-batch-plugin/src/main/java/org/sonar/plugins/maven/RealMavenPluginExecutor.java
index 69a0d6b36f5..58a4440b9d8 100644
--- a/plugins/sonar-maven-batch-plugin/src/main/java/org/sonar/plugins/maven/RealMavenPluginExecutor.java
+++ b/plugins/sonar-maven-batch-plugin/src/main/java/org/sonar/plugins/maven/RealMavenPluginExecutor.java
@@ -32,6 +32,8 @@ import org.sonar.api.utils.TimeProfiler;
 import org.sonar.batch.scan.filesystem.DefaultModuleFileSystem;
 import org.sonar.batch.scan.maven.MavenPluginExecutor;
 
+import javax.annotation.Nullable;
+
 import java.lang.reflect.Method;
 import java.util.Arrays;
 
@@ -49,6 +51,9 @@ public class RealMavenPluginExecutor implements MavenPluginExecutor {
   @Override
   public final MavenPluginHandler execute(Project project, DefaultModuleFileSystem fs, MavenPluginHandler handler) {
     for (String goal : handler.getGoals()) {
+      if (goal == null) {
+        throw new IllegalStateException("Maven goal can't be null");
+      }
       MavenPlugin plugin = MavenPlugin.getPlugin(project.getPom(), handler.getGroupId(), handler.getArtifactId());
       execute(project,
           fs,
@@ -76,7 +81,7 @@ public class RealMavenPluginExecutor implements MavenPluginExecutor {
     }
   }
 
-  static String getGoal(String groupId, String artifactId, String version, String goal) {
+  static String getGoal(String groupId, String artifactId, @Nullable String version, String goal) {
     String defaultVersion = (version == null ? "" : version);
     return new StringBuilder()
         .append(groupId).append(":")
@@ -87,10 +92,10 @@ public class RealMavenPluginExecutor implements MavenPluginExecutor {
         .toString();
   }
 
-  public void concreteExecute(MavenProject pom, String goal) throws Exception {
+  public void concreteExecute(MavenProject pom, String goal) throws SecurityException {
     Method executeMethod = null;
     for (Method m : lifecycleExecutor.getClass().getMethods()) {
-      if (m.getName().equals("execute")) {
+      if ("execute".equals(m.getName())) {
         executeMethod = m;
         break;
       }
@@ -123,18 +128,22 @@ public class RealMavenPluginExecutor implements MavenPluginExecutor {
     }
   }
 
-  public void concreteExecuteMaven2(Method executeMethod, MavenProject pom, String goal) throws Exception {
-    ReactorManager reactor = new ReactorManager(Arrays.asList(pom));
-    MavenSession clonedSession = new MavenSession(mavenSession.getContainer(),
-        mavenSession.getSettings(),
-        mavenSession.getLocalRepository(),
-        mavenSession.getEventDispatcher(),
-        reactor,
-        Arrays.asList(goal),
-        mavenSession.getExecutionRootDirectory(),
-        mavenSession.getExecutionProperties(),
-        mavenSession.getStartTime());
-    executeMethod.invoke(lifecycleExecutor, clonedSession, reactor, clonedSession.getEventDispatcher());
+  public void concreteExecuteMaven2(Method executeMethod, MavenProject pom, String goal) {
+    try {
+      ReactorManager reactor = new ReactorManager(Arrays.asList(pom));
+      MavenSession clonedSession = new MavenSession(mavenSession.getContainer(),
+          mavenSession.getSettings(),
+          mavenSession.getLocalRepository(),
+          mavenSession.getEventDispatcher(),
+          reactor,
+          Arrays.asList(goal),
+          mavenSession.getExecutionRootDirectory(),
+          mavenSession.getExecutionProperties(),
+          mavenSession.getStartTime());
+      executeMethod.invoke(lifecycleExecutor, clonedSession, reactor, clonedSession.getEventDispatcher());
+    } catch (Exception e) {
+      throw new SonarException("Unable to execute Maven 2 plugin", e);
+    }
   }
 
 }
diff --git a/sonar-batch/src/main/java/org/sonar/batch/profiling/PhasesSumUpTimeProfiler.java b/sonar-batch/src/main/java/org/sonar/batch/profiling/PhasesSumUpTimeProfiler.java
index e8402e2b7be..ad1a9d7bc6d 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/profiling/PhasesSumUpTimeProfiler.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/profiling/PhasesSumUpTimeProfiler.java
@@ -53,7 +53,7 @@ import static org.sonar.batch.profiling.AbstractTimeProfiling.truncate;
 public class PhasesSumUpTimeProfiler implements ProjectAnalysisHandler, SensorExecutionHandler, DecoratorExecutionHandler, PostJobExecutionHandler, DecoratorsPhaseHandler,
     SensorsPhaseHandler, PostJobsPhaseHandler, MavenPhaseHandler, InitializersPhaseHandler, InitializerExecutionHandler, BatchStepHandler {
 
-  static Logger LOG = LoggerFactory.getLogger(PhasesSumUpTimeProfiler.class);
+  final static Logger LOG = LoggerFactory.getLogger(PhasesSumUpTimeProfiler.class);
   private static final int TEXT_RIGHT_PAD = 60;
   private static final int TIME_LEFT_PAD = 10;
 
diff --git a/sonar-batch/src/test/java/org/sonar/batch/profiling/PhasesSumUpTimeProfilerTest.java b/sonar-batch/src/test/java/org/sonar/batch/profiling/PhasesSumUpTimeProfilerTest.java
index 9c504dbe35a..43cda3afbd3 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/profiling/PhasesSumUpTimeProfilerTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/profiling/PhasesSumUpTimeProfilerTest.java
@@ -42,7 +42,7 @@ import org.sonar.api.batch.events.SensorsPhaseHandler;
 import org.sonar.api.batch.events.SensorsPhaseHandler.SensorsPhaseEvent;
 import org.sonar.api.resources.Project;
 import org.sonar.api.resources.Resource;
-import org.sonar.batch.events.BatchStepHandler;
+import org.sonar.batch.events.BatchStepEvent;
 import org.sonar.batch.phases.Phases.Phase;
 
 import java.util.Arrays;
@@ -188,10 +188,10 @@ public class PhasesSumUpTimeProfilerTest {
 
   private void batchStep(PhasesSumUpTimeProfiler profiler) throws InterruptedException {
     // Start of batch step
-    profiler.onBatchStep(batchStepEvent(true, "Free memory"));
+    profiler.onBatchStep(new BatchStepEvent("Free memory", true));
     clock.sleep(9);
     // End of batch step
-    profiler.onBatchStep(batchStepEvent(false, "Free memory"));
+    profiler.onBatchStep(new BatchStepEvent("Free memory", false));
   }
 
   private void mavenPhase(PhasesSumUpTimeProfiler profiler) throws InterruptedException {
@@ -341,26 +341,6 @@ public class PhasesSumUpTimeProfilerTest {
     };
   }
 
-  private BatchStepHandler.BatchStepEvent batchStepEvent(final boolean start, final String stepName) {
-    return new BatchStepHandler.BatchStepEvent() {
-
-      @Override
-      public boolean isStart() {
-        return start;
-      }
-
-      @Override
-      public boolean isEnd() {
-        return !start;
-      }
-
-      @Override
-      public String stepName() {
-        return stepName;
-      }
-    };
-  }
-
   private MavenPhaseHandler.MavenPhaseEvent mavenEvent(final boolean start) {
     return new MavenPhaseHandler.MavenPhaseEvent() {
 
diff --git a/sonar-maven-plugin/src/main/java/org/sonar/maven/SonarMojo.java b/sonar-maven-plugin/src/main/java/org/sonar/maven/SonarMojo.java
index f5b18848593..0bb664d9ffc 100644
--- a/sonar-maven-plugin/src/main/java/org/sonar/maven/SonarMojo.java
+++ b/sonar-maven-plugin/src/main/java/org/sonar/maven/SonarMojo.java
@@ -197,7 +197,7 @@ public final class SonarMojo extends AbstractMojo {
         try {
           file = new File(basedir, path).getCanonicalFile();
         } catch (IOException e) {
-          throw new RuntimeException("Unable to resolve path '" + path + "'", e);
+          throw new IllegalStateException("Unable to resolve path '" + path + "'", e);
         }
       }
       return file;
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/maven/MavenPluginHandler.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/maven/MavenPluginHandler.java
index bac5aec6ca9..e2646af8f0f 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/maven/MavenPluginHandler.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/maven/MavenPluginHandler.java
@@ -22,6 +22,9 @@ package org.sonar.api.batch.maven;
 import org.sonar.api.BatchExtension;
 import org.sonar.api.resources.Project;
 
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
 /**
  * @since 1.10
  */
@@ -32,6 +35,7 @@ public interface MavenPluginHandler extends BatchExtension {
    *
    * @return the group id
    */
+  @Nonnull
   String getGroupId();
 
   /**
@@ -39,6 +43,7 @@ public interface MavenPluginHandler extends BatchExtension {
    *
    * @return artifact id
    */
+  @Nonnull
   String getArtifactId();
 
   /**
@@ -46,6 +51,7 @@ public interface MavenPluginHandler extends BatchExtension {
    *
    * @return the plugin version
    */
+  @Nullable
   String getVersion();
 
   /**
@@ -61,6 +67,7 @@ public interface MavenPluginHandler extends BatchExtension {
    *
    * @return an array of goals
    */
+  @Nonnull
   String[] getGoals();
 
   /**