Fix quality flaws

4 files changed, 117 insertions, 42 deletions

diff --git a/src/main/java/org/sonarsource/scanner/cli/Cli.java b/src/main/java/org/sonarsource/scanner/cli/Cli.java
index 74baa0e..df95b05 100644
--- a/src/main/java/org/sonarsource/scanner/cli/Cli.java
+++ b/src/main/java/org/sonarsource/scanner/cli/Cli.java
@@ -22,6 +22,8 @@ package org.sonarsource.scanner.cli;
 import java.util.Properties;
 import org.sonarsource.scanner.api.ScannerProperties;
 
+import static java.util.Arrays.asList;
+
 class Cli {
 
   private boolean debugEnabled = false;
@@ -50,46 +52,58 @@ class Cli {
   Cli parse(String[] args) {
     reset();
     props.putAll(System.getProperties());
-    for (int i = 0; i < args.length; i++) {
-      String arg = args[i];
-      if (i == 0 && arg.charAt(0) != '-') {
-        props.setProperty(ScannerProperties.TASK, arg);
-
-      } else if ("-h".equals(arg) || "--help".equals(arg)) {
-        printUsage();
-        exit.exit(Exit.SUCCESS);
-
-      } else if ("-v".equals(arg) || "--version".equals(arg)) {
-        displayVersionOnly = true;
-        
-      } else if ("-e".equals(arg) || "--errors".equals(arg)) {
-        logger.info("Option -e/--errors is no longer supported and will be ignored");
-
-      } else if ("-X".equals(arg) || "--debug".equals(arg)) {
-        props.setProperty("sonar.verbose", "true");
-        debugEnabled = true;
-        logger.setDebugEnabled(true);
-
-      } else if ("-D".equals(arg) || "--define".equals(arg)) {
-        i++;
-        if (i >= args.length) {
-          printError("Missing argument for option --define");
-        }
-        arg = args[i];
-        appendPropertyTo(arg, props);
-
-      } else if (arg.startsWith("-D")) {
-        arg = arg.substring(2);
-        appendPropertyTo(arg, props);
-
-      } else {
-        printError("Unrecognized option: " + arg);
-      }
+    if (args.length > 0) {
+      int pos = 0;
+      do {
+        pos = processNextArg(args, pos);
+      } while (pos < args.length);
     }
-
     return this;
   }
 
+  private int processNextArg(String[] args, int pos) {
+    String arg = args[pos];
+    if (pos == 0 && arg.charAt(0) != '-') {
+      props.setProperty(ScannerProperties.TASK, arg);
+
+    } else if (asList("-h", "--help").contains(arg)) {
+      printUsage();
+      exit.exit(Exit.SUCCESS);
+
+    } else if (asList("-v", "--version").contains(arg)) {
+      displayVersionOnly = true;
+
+    } else if (asList("-e", "--errors").contains(arg)) {
+      logger.info("Option -e/--errors is no longer supported and will be ignored");
+
+    } else if (asList("-X", "--debug").contains(arg)) {
+      props.setProperty("sonar.verbose", "true");
+      debugEnabled = true;
+      logger.setDebugEnabled(true);
+
+    } else if (asList("-D", "--define").contains(arg)) {
+      return processProp(args, pos);
+
+    } else if (arg.startsWith("-D")) {
+      arg = arg.substring(2);
+      appendPropertyTo(arg, props);
+
+    } else {
+      printErrorAndExit("Unrecognized option: " + arg);
+    }
+    return pos + 1;
+  }
+
+  private int processProp(String[] args, int pos) {
+    int valuePos = pos + 1;
+    if (valuePos >= args.length) {
+      printErrorAndExit("Missing argument for option -D/--define");
+    } else {
+      appendPropertyTo(args[valuePos], props);
+    }
+    return valuePos + 1;
+  }
+
   private void reset() {
     props.clear();
     debugEnabled = false;
@@ -110,7 +124,7 @@ class Cli {
     props.setProperty(key, value);
   }
 
-  private void printError(String message) {
+  private void printErrorAndExit(String message) {
     logger.error(message);
     printUsage();
     exit.exit(Exit.ERROR);
diff --git a/src/test/java/org/sonar/api/utils/MessageException.java b/src/test/java/org/sonar/api/utils/MessageException.java
index 0e72d3c..3769368 100644
--- a/src/test/java/org/sonar/api/utils/MessageException.java
+++ b/src/test/java/org/sonar/api/utils/MessageException.java
@@ -20,7 +20,7 @@
 package org.sonar.api.utils;
 
 public class MessageException extends RuntimeException {
-  public MessageException(String msg) {
-    super(msg);
+  public MessageException(String msg, Throwable cause) {
+    super(msg, cause);
   }
 }
diff --git a/src/test/java/org/sonarsource/scanner/cli/CliTest.java b/src/test/java/org/sonarsource/scanner/cli/CliTest.java
index 640ca8d..9fb5786 100644
--- a/src/test/java/org/sonarsource/scanner/cli/CliTest.java
+++ b/src/test/java/org/sonarsource/scanner/cli/CliTest.java
@@ -47,10 +47,19 @@ public class CliTest {
   }
 
   @Test
+  public void should_fail_on_missing_prop() {
+    logs = mock(Logs.class);
+    cli = new Cli(exit, logs);
+    cli.parse(new String[] {"-D"});
+    verify(logs).error("Missing argument for option -D/--define");
+    verify(exit).exit(Exit.ERROR);
+  }
+
+  @Test
   public void should_not_fail_with_errors_option() {
     cli.parse(new String[] {"-e"});
   }
-  
+
   @Test
   public void should_parse_optional_task() {
     cli.parse(new String[] {"-D", "foo=bar"});
@@ -68,6 +77,25 @@ public class CliTest {
   }
 
   @Test
+  public void should_enable_debug_mode_full() {
+    cli.parse(new String[] {"--debug"});
+    assertThat(cli.isDebugEnabled()).isTrue();
+    assertThat(cli.properties().get("sonar.verbose")).isEqualTo("true");
+  }
+
+  @Test
+  public void should_show_version() {
+    cli.parse(new String[] {"-v"});
+    assertThat(cli.isDisplayVersionOnly()).isTrue();
+  }
+
+  @Test
+  public void should_show_version_full() {
+    cli.parse(new String[] {"--version"});
+    assertThat(cli.isDisplayVersionOnly()).isTrue();
+  }
+
+  @Test
   public void should_enable_stacktrace_log() {
     cli.parse(new String[] {"-e"});
     assertThat(cli.isDebugEnabled()).isFalse();
@@ -75,6 +103,13 @@ public class CliTest {
   }
 
   @Test
+  public void should_enable_stacktrace_log_full() {
+    cli.parse(new String[] {"--errors"});
+    assertThat(cli.isDebugEnabled()).isFalse();
+    assertThat(cli.properties().get("sonar.verbose")).isNull();
+  }
+
+  @Test
   public void should_disable_debug_mode_and_stacktrace_log_by_default() {
     cli.parse(new String[0]);
     assertThat(cli.isDebugEnabled()).isFalse();
@@ -91,6 +126,15 @@ public class CliTest {
   }
 
   @Test
+  public void should_show_usage_full() {
+    logs = mock(Logs.class);
+    cli = new Cli(exit, logs);
+    cli.parse(new String[] {"--help"});
+    verify(logs).info("usage: sonar-scanner [options]");
+    verify(exit).exit(Exit.SUCCESS);
+  }
+
+  @Test
   public void should_show_usage_on_bad_syntax() {
     logs = mock(Logs.class);
     cli = new Cli(exit, logs);
diff --git a/src/test/java/org/sonarsource/scanner/cli/MainTest.java b/src/test/java/org/sonarsource/scanner/cli/MainTest.java
index 3484aef..b410cbd 100644
--- a/src/test/java/org/sonarsource/scanner/cli/MainTest.java
+++ b/src/test/java/org/sonarsource/scanner/cli/MainTest.java
@@ -95,7 +95,23 @@ public class MainTest {
   }
 
   @Test
-  public void show_error() {
+  public void should_exit_on_error() {
+    EmbeddedScanner runner = mock(EmbeddedScanner.class);
+    Exception e = new NullPointerException("NPE");
+    e = new IllegalStateException("Error", e);
+    doThrow(e).when(runner).stop();
+    when(runnerFactory.create(any(Properties.class))).thenReturn(runner);
+
+    Main main = new Main(exit, cli, conf, runnerFactory, logs);
+    main.execute();
+
+    verify(runner).stop();
+    verify(exit).exit(Exit.ERROR);
+    verify(logs).error("Unable to properly stop the scanner", e);
+  }
+
+  @Test
+  public void show_error_with_stacktrace() {
     Exception e = createException(false);
     testException(e, false);
 
@@ -109,6 +125,7 @@ public class MainTest {
     testException(e, false);
 
     verify(logs).error("Error during SonarQube Scanner execution");
+    verify(logs).error("Caused by: NPE");
     verify(logs).error("Re-run SonarQube Scanner using the -X switch to enable full debug logging.");
   }
 
@@ -138,7 +155,7 @@ public class MainTest {
   private Exception createException(boolean messageException) {
     Exception e;
     if (messageException) {
-      e = new MessageException("my message");
+      e = new MessageException("my message", new NullPointerException("NPE"));
     } else {
       e = new IllegalStateException("Error", new NullPointerException("NPE"));
     }