4. Kibit support (#17)

* Remove testing extensions by number

Extensios adding is easier if there is not a test which requires certain number of extensions to be present. Easier is to add tests one by one

* Load property definition in first extension

Current test assumes propertydefinition to be in certain position. If position is changed to first this test cannot break when adding new extensions

* Move rules checking to sensors test

A sensor test should not assume certain numbers of rules to be present to avoid collisions with other sensors. Instead test should assert that rules that are used by sensor are in the rules not that rules are expected to contain only sensors rules.

* Add instructions to run SonarQube locally in docker

The script is added which builds the plugin and starts SonarQube with plugin installed in local environment. This is useful fo locally testing the components

* Change readme to have sensors in their own chapters

* Add support for Kibit

Kibit is run via Leiningen. Suggestions are mapped against correct lines. Kibit suggestion is by default minor code smell

* Add Kibit to readme

* Add Kibit support

Author: hjhamala

Dockerfile

@@ -1,4 +1,3 @@
 FROM sonarqube:7.1
 
-COPY target/sonar-clojure-plugin-*-SNAPSHOT.jar $SONARQUBE_HOME/extensions/plugins/
-
+COPY target/sonar-clojure-plugin-*-SNAPSHOT.jar $SONARQUBE_HOME/extensions/plugins/

README.md

@@ -17,6 +17,17 @@ that uses [Eastwood](https://github.com/jonase/eastwood) lint tool to analyze Cl
 [Eastwood](https://github.com/jonase/eastwood) is a lintter for Clojure (no CLJS support) which detects for example misplaced docstrings
  , def in defs and tests which always returns true.
 
+### Kibit
+[Kibit](https://github.com/jonase/eastwood) is a static code analyzer which detects code that could be rewritten with a more idiomatic 
+function or macro. For example: 
+```clojure 
+(> x 0) 
+; more idiomatically
+(pos? x)
+```
+
+Kibit is most useful for beginning Clojure programmers. Kibit supports also Clojurescript.
+
 ### ancient-clj
 
 [ancient-clj](https://github.com/xsc/lein-ancient) is a Clojure library with Leiningen plugin which checks your project for outdated dependencies and plugins and  suggest updates. The Sonarqube plugin
@@ -45,6 +56,7 @@ Set the ```sonar.clojure.lein-nvd.json-output-location``` property in sonar-proj
 this moment is not under development anymore and doesn't support SonarQube 6.7. Since the changes to port
 the old plugin were very extensive, I decided to start from scratch and use the old plugin as inspiration.
 
+
 ## Installation
 
 In order to install SonarClojure:
@@ -77,18 +89,18 @@ In order to install SonarClojure:
 Sensors can be disabled by setting ```sonar.clojure.sensorname.disabled=true```  or
 by using command line switch ```-Dsonar.clojure.sensorname.disabled``` when running ```sonar-scanner```.
 
-Sensor names are ```eastwood```, ```ancient-clj```, ``lein-nvd``` and ```cloverage```.
+Sensor names are ```eastwood```, ```kibit```, ```ancient-clj```, ``lein-nvd``` and ```cloverage```.
 
-3. Run [sonnar-scanner](https://docs.sonarqube.org/display/SCAN/Analyzing+with+SonarQube+Scanner) on your project.
+3. Run [sonar-scanner](https://docs.sonarqube.org/display/SCAN/Analyzing+with+SonarQube+Scanner) on your project.
 
 ## Building from source
-
+◊
 ```sh
 ./mvnw clean package
 ```
 
 Maven will generate an SNAPSHOT under the folder ***target***.
-
+``
 ## Testing the plugin with locally running dockerized Sonarqube 
 
 ```sh

src/main/java/org/sonar/plugins/clojure/ClojurePlugin.java

@@ -8,6 +8,7 @@
 import org.sonar.plugins.clojure.sensors.CommandRunner;
 import org.sonar.plugins.clojure.sensors.ancient.AncientSensor;
 import org.sonar.plugins.clojure.sensors.eastwood.EastwoodSensor;
+import org.sonar.plugins.clojure.sensors.kibit.KibitSensor;
 import org.sonar.plugins.clojure.sensors.leinnvd.LeinNvdSensor;
 import org.sonar.plugins.clojure.settings.ClojureProperties;
 
@@ -20,6 +21,7 @@ public void define(Context context) {
         context.addExtension(ClojureLintRulesDefinition.class);
         context.addExtension(CommandRunner.class);
         context.addExtension(EastwoodSensor.class);
+        context.addExtension(KibitSensor.class);
         context.addExtension(AncientSensor.class);
         context.addExtension(CloverageSensor.class);
         context.addExtension(LeinNvdSensor.class);

src/main/java/org/sonar/plugins/clojure/sensors/AbstractSensor.java

@@ -3,8 +3,12 @@
 import org.sonar.api.batch.fs.FileSystem;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.sensor.SensorContext;
+import org.sonar.api.batch.sensor.issue.NewIssue;
+import org.sonar.api.batch.sensor.issue.NewIssueLocation;
+import org.sonar.api.rule.RuleKey;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
+import org.sonar.plugins.clojure.rules.ClojureLintRulesDefinition;
 
 import java.io.IOException;
 import java.nio.file.Files;
@@ -71,6 +75,36 @@ protected Optional<InputFile> getFile(String filePath, FileSystem fileSystem) {
                         fileSystem.predicates().hasType(InputFile.Type.MAIN))));
     }
 
+    private InputFile getFile(Issue issue, FileSystem fileSystem) {
+        return fileSystem.inputFile(
+                fileSystem.predicates().and(
+                        fileSystem.predicates().hasRelativePath(issue.getFilePath()),
+                        fileSystem.predicates().hasType(InputFile.Type.MAIN)));
+    }
+
+    protected void saveIssue(Issue issue, SensorContext context) {
+        InputFile file = getFile(issue, context.fileSystem());
+
+        if (file == null) {
+            LOG.warn("Not able to find a file with path '{}'", issue.getFilePath());
+            return;
+        }
+
+        RuleKey ruleKey = RuleKey.of(ClojureLintRulesDefinition.REPOSITORY_KEY, issue.getExternalRuleId().trim());
+
+        NewIssue newIssue = context.newIssue().forRule(ruleKey);
+
+        NewIssueLocation primaryLocation = newIssue
+                .newLocation()
+                .on(file)
+                .message(issue.getDescription().trim());
+
+        primaryLocation.at(file.selectLine(issue.getLine()));
+
+        newIssue.at(primaryLocation);
+        newIssue.save();
+    }
+
     /**
      * Gets the file directly from filesystem. This is useful when the file is needed to be read which is not wanted to
      * be part of SonarQube scanning

src/main/java/org/sonar/plugins/clojure/sensors/eastwood/EastwoodIssue.java renamed to src/main/java/org/sonar/plugins/clojure/sensors/Issue.java

@@ -1,13 +1,13 @@
-package org.sonar.plugins.clojure.sensors.eastwood;
+package org.sonar.plugins.clojure.sensors;
 
-public class EastwoodIssue {
+public class Issue {
 
     private String externalRuleId;
     private String issueMessage;
     private String filePath;
     private int line;
 
-    public EastwoodIssue(String externalRuleId, String description, String filePath, int line) {
+    public Issue(String externalRuleId, String description, String filePath, int line) {
         this.externalRuleId = externalRuleId;
         this.issueMessage = description;
         this.filePath = filePath;

src/main/java/org/sonar/plugins/clojure/sensors/eastwood/EastwoodIssueParser.java

@@ -1,6 +1,7 @@
 package org.sonar.plugins.clojure.sensors.eastwood;
 
 import org.sonar.plugins.clojure.sensors.CommandStreamConsumer;
+import org.sonar.plugins.clojure.sensors.Issue;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -23,8 +24,8 @@ public static String parseRuntimeInfo(CommandStreamConsumer output) {
         return null;
     }
 
-    public static List<EastwoodIssue> parse(CommandStreamConsumer output) {
-        List<EastwoodIssue> eastwoodIssues = new ArrayList<>();
+    public static List<Issue> parse(CommandStreamConsumer output) {
+        List<Issue> issues = new ArrayList<>();
 
         if (output != null) {
             for (String line : output.getData()) {
@@ -36,11 +37,11 @@ public static List<EastwoodIssue> parse(CommandStreamConsumer output) {
                     String filePath = matcher.group(1);
                     int lineNumber = Integer.parseInt(matcher.group(2));
 
-                    eastwoodIssues.add(new EastwoodIssue(externalRuleId, description, filePath, lineNumber));
+                    issues.add(new Issue(externalRuleId, description, filePath, lineNumber));
                 }
             }
         }
 
-        return eastwoodIssues;
+        return issues;
     }
 }

src/main/java/org/sonar/plugins/clojure/sensors/eastwood/EastwoodSensor.java

@@ -16,6 +16,7 @@
 import org.sonar.plugins.clojure.sensors.AbstractSensor;
 import org.sonar.plugins.clojure.sensors.CommandStreamConsumer;
 import org.sonar.plugins.clojure.sensors.CommandRunner;
+import org.sonar.plugins.clojure.sensors.Issue;
 import org.sonar.plugins.clojure.settings.ClojureProperties;
 
 import java.util.List;
@@ -30,36 +31,7 @@ public EastwoodSensor(CommandRunner commandRunner) {
         super(commandRunner);
     }
 
-    private void saveIssue(EastwoodIssue eastwoodIssue, SensorContext context) {
-        InputFile file = getFile(eastwoodIssue, context.fileSystem());
 
-        if (file == null) {
-            LOG.warn("Not able to find a file with path '{}'", eastwoodIssue.getFilePath());
-            return;
-        }
-
-        RuleKey ruleKey = RuleKey.of(ClojureLintRulesDefinition.REPOSITORY_KEY, eastwoodIssue.getExternalRuleId().trim());
-
-        NewIssue newIssue = context.newIssue().forRule(ruleKey);
-
-        NewIssueLocation primaryLocation = newIssue
-                .newLocation()
-                .on(file)
-                .message(eastwoodIssue.getDescription().trim());
-
-        primaryLocation.at(file.selectLine(eastwoodIssue.getLine()));
-
-        newIssue.at(primaryLocation);
-
-        newIssue.save();
-    }
-
-    private InputFile getFile(EastwoodIssue eastwoodIssue, FileSystem fileSystem) {
-        return fileSystem.inputFile(
-                fileSystem.predicates().and(
-                        fileSystem.predicates().hasRelativePath(eastwoodIssue.getFilePath()),
-                        fileSystem.predicates().hasType(InputFile.Type.MAIN)));
-    }
 
 
     @Override
@@ -84,10 +56,10 @@ public void execute(SensorContext context) {
                     LOG.warn("Eastwood resulted in empty output");
                 }
 
-                List<EastwoodIssue> eastwoodIssues = EastwoodIssueParser.parse(stdOut);
-                LOG.info("Saving eastwoodIssues");
-                for (EastwoodIssue eastwoodIssue : eastwoodIssues) {
-                    saveIssue(eastwoodIssue, context);
+                List<Issue> issues = EastwoodIssueParser.parse(stdOut);
+                LOG.info("Saving issues");
+                for (Issue issue : issues) {
+                    saveIssue(issue, context);
                 }
             }
         } else {

src/main/java/org/sonar/plugins/clojure/sensors/kibit/KibitIssueParser.java

@@ -0,0 +1,51 @@
+package org.sonar.plugins.clojure.sensors.kibit;
+
+import org.sonar.api.utils.log.Logger;
+import org.sonar.api.utils.log.Loggers;
+import org.sonar.plugins.clojure.sensors.CommandStreamConsumer;
+import org.sonar.plugins.clojure.sensors.Issue;
+
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+public class KibitIssueParser {
+    private static final Pattern KIBIT_START = Pattern.compile("----");
+    private static final Pattern KIBIT_ENTRY = Pattern.compile("At\\s([^:]+):([^`]+):");
+    private KibitIssueParser() {
+    }
+
+    private static boolean isEntryStart(String row) {
+        return KIBIT_START.matcher(row).find();
+    }
+
+    public static List<Issue> parse(CommandStreamConsumer output) {
+        List<Issue> issues = new ArrayList<>();
+        if (output != null) {
+            List<String> kibitReport = output.getData();
+            ArrayDeque<String> filteredReport = kibitReport.stream()
+                    .filter(line -> !line.isEmpty())
+                    .filter(line -> !isEntryStart(line))
+                    .collect(Collectors.toCollection(ArrayDeque::new));
+
+            while (!filteredReport.isEmpty()) {
+                String path = filteredReport.pop();
+                Matcher matcher = KIBIT_ENTRY.matcher(path);
+                if (matcher.find()) {
+                    String filename = matcher.group(1);
+                    // Kibit may return line number sometimes as string "null"
+                    int lineNumber = !matcher.group(2).equals("null") ? Integer.parseInt(matcher.group(2)) : 1;
+                    StringBuilder description = new StringBuilder();
+                    while (!filteredReport.isEmpty() && !KIBIT_ENTRY.matcher(filteredReport.peek()).find()) {
+                        description.append(filteredReport.pop()).append("\n");
+                    }
+                    issues.add(new Issue("kibit", description.toString(), filename, lineNumber));
+                }
+            }
+        }
+        return issues;
+    }
+}

src/main/java/org/sonar/plugins/clojure/sensors/kibit/KibitSensor.java

@@ -0,0 +1,58 @@
+package org.sonar.plugins.clojure.sensors.kibit;
+
+
+import org.sonar.api.batch.fs.FileSystem;
+import org.sonar.api.batch.fs.InputFile;
+
+import org.sonar.api.batch.sensor.Sensor;
+import org.sonar.api.batch.sensor.SensorContext;
+import org.sonar.api.batch.sensor.SensorDescriptor;
+
+import org.sonar.api.utils.log.Logger;
+import org.sonar.api.utils.log.Loggers;
+import org.sonar.plugins.clojure.language.ClojureLanguage;
+
+import org.sonar.plugins.clojure.sensors.AbstractSensor;
+import org.sonar.plugins.clojure.sensors.CommandRunner;
+import org.sonar.plugins.clojure.sensors.CommandStreamConsumer;
+import org.sonar.plugins.clojure.sensors.Issue;
+import org.sonar.plugins.clojure.settings.ClojureProperties;
+
+import java.util.List;
+
+public class KibitSensor extends AbstractSensor implements Sensor {
+
+    private static final Logger LOG = Loggers.get(KibitSensor.class);
+
+    private static final String KIBIT_COMMAND = "kibit";
+
+    public KibitSensor(CommandRunner commandRunner) { super(commandRunner); }
+
+    @Override
+    public void describe(SensorDescriptor descriptor) {
+        descriptor.name("SonarClojureKibit")
+                .onlyOnLanguage(ClojureLanguage.KEY)
+                .global();
+    }
+
+    @Override
+    public void execute(SensorContext context) {
+        LOG.info("Running Kibit");
+        CommandStreamConsumer stdOut = this.commandRunner.run(LEIN_COMMAND, KIBIT_COMMAND);
+        if (!checkIfPluginIsDisabled(context, ClojureProperties.KIBIT_DISABLED)) {
+            if (isLeinInstalled(stdOut.getData()) && isPluginInstalled(stdOut.getData(), KIBIT_COMMAND)){
+
+                List<Issue> issues = KibitIssueParser.parse(stdOut);
+                LOG.info("Saving issues");
+                for (Issue issue : issues) {
+                    saveIssue(issue, context);
+                }
+            } else {
+                LOG.warn("Parsing skipped because Leiningen or Kibit are not installed");
+            }
+        } else {
+            LOG.info ("Kibit sensor is disabled");
+        }
+    }
+
+}

src/main/java/org/sonar/plugins/clojure/settings/ClojureProperties.java

@@ -16,6 +16,7 @@ public class ClojureProperties {
     public static final String LEIN_NVD_JSON_OUTPUT_LOCATION = "sonar.clojure.lein-nvd.json-output-location";
     public static final String MAIN_CATEGORY = "ClojureLanguage";
     public static final String CLOVERAGE_DISABLED = "sonar.clojure.cloverage.disabled";
+    public static final String KIBIT_DISABLED = "sonar.clojure.kibit.disabled";
     public static final String CLOVERAGE_JSON_OUTPUT_LOCATION = "sonar.clojure.cloverage.json-output-location";
 
     private ClojureProperties() {}
@@ -27,7 +28,8 @@ public static List<PropertyDefinition> getProperties() {
                 getCloverageDisabledProperty(),
                 getCloverageJsonOutputLocation(),
                 getLeinNvdDisabledProperty(),
-                getLeinNVdXMLOutputLocation());
+                getLeinNVdXMLOutputLocation(),
+                getKibitDisabledProperty());
     }
 
     public static PropertyDefinition getFileSuffixProperty() {
@@ -99,4 +101,14 @@ public static PropertyDefinition getLeinNVdXMLOutputLocation() {
                 .description("Set this to path where Lein NVD generates the result xml file.")
                 .build();
     }
+
+    public static PropertyDefinition getKibitDisabledProperty() {
+        return PropertyDefinition.builder(KIBIT_DISABLED)
+                .category("ClojureLanguage")
+                .subCategory("Sensors")
+                .defaultValue("false")
+                .name("Kibit sensor disabling")
+                .description("Set true to disable the sensor.")
+                .build();
+    }
 }