SONARJAVA-14,SONARJAVA-18 Grab violations directly from FindBugs

3 files changed, 50 insertions, 84 deletions

diff --git a/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java b/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java
index 101ff5587fc..578a99cd7ce 100644
--- a/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java
+++ b/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java
@@ -20,14 +20,7 @@
 package org.sonar.plugins.findbugs;
 
 import com.google.common.collect.Lists;
-import edu.umd.cs.findbugs.DetectorFactoryCollection;
-import edu.umd.cs.findbugs.FindBugs;
-import edu.umd.cs.findbugs.FindBugs2;
-import edu.umd.cs.findbugs.Plugin;
-import edu.umd.cs.findbugs.PluginException;
-import edu.umd.cs.findbugs.Priorities;
-import edu.umd.cs.findbugs.Project;
-import edu.umd.cs.findbugs.XMLBugReporter;
+import edu.umd.cs.findbugs.*;
 import edu.umd.cs.findbugs.config.UserPreferences;
 import edu.umd.cs.findbugs.plugins.DuplicatePluginIdException;
 import org.apache.commons.io.FileUtils;
@@ -44,12 +37,7 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.io.PrintStream;
 import java.net.URL;
-import java.util.Collection;
-import java.util.Enumeration;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
+import java.util.*;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -80,7 +68,7 @@ public class FindbugsExecutor implements BatchExtension {
     this.configuration = configuration;
   }
 
-  public File execute() {
+  public BugCollection execute() {
     TimeProfiler profiler = new TimeProfiler().start("Execute Findbugs " + FindbugsVersion.getVersion());
     // We keep a handle on the current security manager because FB plays with it and we need to restore it before shutting down the executor
     // service
@@ -140,7 +128,7 @@ public class FindbugsExecutor implements BatchExtension {
 
       profiler.stop();
 
-      return xmlReport;
+      return xmlBugReporter.getBugCollection();
     } catch (Exception e) {
       throw new SonarException("Can not execute Findbugs", e);
     } finally {
diff --git a/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java b/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java
index d9ca6cecfe3..d56a470359a 100644
--- a/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java
+++ b/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java
@@ -19,9 +19,11 @@
  */
 package org.sonar.plugins.findbugs;
 
+import edu.umd.cs.findbugs.BugCollection;
+import edu.umd.cs.findbugs.BugInstance;
+import edu.umd.cs.findbugs.SourceLineAnnotation;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.sonar.api.CoreProperties;
 import org.sonar.api.batch.Sensor;
 import org.sonar.api.batch.SensorContext;
 import org.sonar.api.profiles.RulesProfile;
@@ -32,9 +34,6 @@ import org.sonar.api.rules.Rule;
 import org.sonar.api.rules.RuleFinder;
 import org.sonar.api.rules.Violation;
 
-import java.io.File;
-import java.util.List;
-
 public class FindbugsSensor implements Sensor {
 
   private static final Logger LOG = LoggerFactory.getLogger(FindbugsSensor.class);
@@ -51,23 +50,19 @@ public class FindbugsSensor implements Sensor {
 
   public boolean shouldExecuteOnProject(Project project) {
     return Java.KEY.equals(project.getLanguageKey())
-      && !project.getFileSystem().mainFiles(Java.KEY).isEmpty()
-      && !profile.getActiveRulesByRepository(FindbugsConstants.REPOSITORY_KEY).isEmpty();
+        && !project.getFileSystem().mainFiles(Java.KEY).isEmpty()
+        && !profile.getActiveRulesByRepository(FindbugsConstants.REPOSITORY_KEY).isEmpty();
   }
 
   public void analyse(Project project, SensorContext context) {
     if (project.getReuseExistingRulesConfig()) {
       LOG.warn("Reusing existing Findbugs configuration not supported any more.");
     }
-    File report = getFindbugsReportFile(project);
-    if (report == null) {
-      report = executor.execute();
-    }
-    FindbugsXmlReportParser reportParser = new FindbugsXmlReportParser(report);
-    List<FindbugsXmlReportParser.XmlBugInstance> bugInstances = reportParser.getBugInstances();
 
-    for (FindbugsXmlReportParser.XmlBugInstance bugInstance : bugInstances) {
-      FindbugsXmlReportParser.XmlSourceLineAnnotation sourceLine = bugInstance.getPrimarySourceLine();
+    BugCollection collection = executor.execute();
+
+    for (BugInstance bugInstance : collection) {
+      SourceLineAnnotation sourceLine = bugInstance.getPrimarySourceLineAnnotation();
       if (sourceLine == null) {
         LOG.warn("No source line for " + bugInstance.getType());
         continue;
@@ -80,25 +75,30 @@ public class FindbugsSensor implements Sensor {
         continue;
       }
 
-      JavaFile resource = new JavaFile(sourceLine.getSonarJavaFileKey());
+      String longMessage = bugInstance.getMessageWithoutPrefix();
+      String className = bugInstance.getPrimarySourceLineAnnotation().getClassName();
+      int start = bugInstance.getPrimarySourceLineAnnotation().getStartLine();
+
+      JavaFile resource = new JavaFile(getSonarJavaFileKey(className));
       if (context.getResource(resource) != null) {
         Violation violation = Violation.create(rule, resource)
-            .setLineId(sourceLine.getStart())
-            .setMessage(bugInstance.getLongMessage());
+            .setLineId(start)
+            .setMessage(longMessage);
         context.saveViolation(violation);
       }
     }
   }
 
-  protected final File getFindbugsReportFile(Project project) {
-    if (project.getConfiguration().getString(CoreProperties.FINDBUGS_REPORT_PATH) != null) {
-      return new File(project.getConfiguration().getString(CoreProperties.FINDBUGS_REPORT_PATH));
+  private static String getSonarJavaFileKey(String className) {
+    if (className.indexOf('$') > -1) {
+      return className.substring(0, className.indexOf('$'));
     }
-    return null;
+    return className;
   }
 
   @Override
   public String toString() {
     return getClass().getSimpleName();
   }
+
 }
diff --git a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java
index 317b72e46d6..82379f3b1a7 100644
--- a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java
+++ b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java
@@ -20,22 +20,16 @@
 package org.sonar.plugins.findbugs;
 
 import com.google.common.collect.Lists;
-import org.apache.commons.configuration.Configuration;
+import edu.umd.cs.findbugs.*;
 import org.junit.Test;
-import org.sonar.api.CoreProperties;
 import org.sonar.api.batch.SensorContext;
 import org.sonar.api.profiles.RulesProfile;
-import org.sonar.api.resources.InputFile;
-import org.sonar.api.resources.InputFileUtils;
-import org.sonar.api.resources.JavaFile;
+import org.sonar.api.resources.*;
 import org.sonar.api.resources.Project;
-import org.sonar.api.resources.ProjectFileSystem;
-import org.sonar.api.resources.Resource;
 import org.sonar.api.rules.Rule;
 import org.sonar.api.rules.Violation;
 import org.sonar.api.test.IsViolation;
 
-import java.io.File;
 import java.util.ArrayList;
 
 import static org.fest.assertions.Assertions.assertThat;
@@ -99,52 +93,31 @@ public class FindbugsSensorTest extends FindbugsTests {
     Project project = createProject();
     FindbugsExecutor executor = mock(FindbugsExecutor.class);
     SensorContext context = mock(SensorContext.class);
-    Configuration conf = mock(Configuration.class);
-    // We assume that this report was generated during findbugs execution
-    File xmlFile = new File(getClass().getResource("/org/sonar/plugins/findbugs/findbugsReport.xml").toURI());
-    when(project.getConfiguration()).thenReturn(conf);
-    when(executor.execute()).thenReturn(xmlFile);
-    when(context.getResource(any(Resource.class))).thenReturn(new JavaFile("org.sonar.MyClass"));
-
-    FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(), new FakeRuleFinder(), executor);
-    analyser.analyse(project, context);
-
-    verify(executor).execute();
-    verify(context, times(2)).saveViolation(any(Violation.class));
-
-    Violation wanted = Violation.create((Rule) null, new JavaFile("org.sonar.commons.ZipUtils")).setMessage(
-        "Empty zip file entry created in org.sonar.commons.ZipUtils._zip(String, File, ZipOutputStream)").setLineId(107);
-    verify(context).saveViolation(argThat(new IsViolation(wanted)));
 
-    wanted = Violation.create((Rule) null, new JavaFile("org.sonar.commons.resources.MeasuresDao")).setMessage(
-        "The class org.sonar.commons.resources.MeasuresDao$1 could be refactored into a named _static_ inner class").setLineId(56);
-    verify(context).saveViolation(argThat(new IsViolation(wanted)));
-  }
+    BugCollection collection = new SortedBugCollection();
+    BugInstance bugInstance = new BugInstance("AM_CREATES_EMPTY_ZIP_FILE_ENTRY", 2);
+    String className = "org.sonar.commons.ZipUtils";
+    String sourceFile = "org/sonar/commons/ZipUtils.java";
+    int startLine = 107;
+    ClassAnnotation classAnnotation = new ClassAnnotation(className, sourceFile);
+    bugInstance.add(classAnnotation);
+    MethodAnnotation methodAnnotation = new MethodAnnotation(className, "_zip", "(Ljava/lang/String;Ljava/io/File;Ljava/util/zip/ZipOutputStream;)V", true);
+    methodAnnotation.setSourceLines(new SourceLineAnnotation(className, sourceFile, startLine, 0, 0, 0));
+    bugInstance.add(methodAnnotation);
+    collection.add(bugInstance);
+    when(executor.execute()).thenReturn(collection);
 
-  @Test
-  public void shouldReuseReport() throws Exception {
-    Project project = createProject();
-    FindbugsExecutor executor = mock(FindbugsExecutor.class);
-    SensorContext context = mock(SensorContext.class);
-    Configuration conf = mock(Configuration.class);
-    File xmlFile = new File(getClass().getResource("/org/sonar/plugins/findbugs/findbugsReport.xml").toURI());
-    when(conf.getString(CoreProperties.FINDBUGS_REPORT_PATH)).thenReturn(xmlFile.getAbsolutePath());
-    when(project.getConfiguration()).thenReturn(conf);
     when(context.getResource(any(Resource.class))).thenReturn(new JavaFile("org.sonar.MyClass"));
 
     FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(), new FakeRuleFinder(), executor);
     analyser.analyse(project, context);
 
-    verify(executor, never()).execute();
-    verify(context, times(2)).saveViolation(any(Violation.class));
+    verify(executor).execute();
+    verify(context, times(1)).saveViolation(any(Violation.class));
 
     Violation wanted = Violation.create((Rule) null, new JavaFile("org.sonar.commons.ZipUtils")).setMessage(
         "Empty zip file entry created in org.sonar.commons.ZipUtils._zip(String, File, ZipOutputStream)").setLineId(107);
     verify(context).saveViolation(argThat(new IsViolation(wanted)));
-
-    wanted = Violation.create((Rule) null, new JavaFile("org.sonar.commons.resources.MeasuresDao")).setMessage(
-        "The class org.sonar.commons.resources.MeasuresDao$1 could be refactored into a named _static_ inner class").setLineId(56);
-    verify(context).saveViolation(argThat(new IsViolation(wanted)));
   }
 
   @Test
@@ -152,12 +125,17 @@ public class FindbugsSensorTest extends FindbugsTests {
     Project project = createProject();
     FindbugsExecutor executor = mock(FindbugsExecutor.class);
     SensorContext context = mock(SensorContext.class);
-    Configuration conf = mock(Configuration.class);
-    File xmlFile = new File(getClass().getResource("/org/sonar/plugins/findbugs/findbugsReportWithUnknownRule.xml").toURI());
-    when(conf.getString(CoreProperties.FINDBUGS_REPORT_PATH)).thenReturn(xmlFile.getAbsolutePath());
-    when(project.getConfiguration()).thenReturn(conf);
     when(context.getResource(any(Resource.class))).thenReturn(new JavaFile("org.sonar.MyClass"));
 
+    BugCollection collection = new SortedBugCollection();
+    BugInstance bugInstance = new BugInstance("UNKNOWN", 2);
+    String className = "org.sonar.commons.ZipUtils";
+    String sourceFile = "org/sonar/commons/ZipUtils.java";
+    ClassAnnotation classAnnotation = new ClassAnnotation(className, sourceFile);
+    bugInstance.add(classAnnotation);
+    collection.add(bugInstance);
+    when(executor.execute()).thenReturn(collection);
+
     FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(), new FakeRuleFinder(), executor);
     analyser.analyse(project, context);