diff --git a/src/main/java/org/sonar/plugins/findbugs/AnalysisResult.java b/src/main/java/org/sonar/plugins/findbugs/AnalysisResult.java new file mode 100644 index 00000000..28e4a378 --- /dev/null +++ b/src/main/java/org/sonar/plugins/findbugs/AnalysisResult.java @@ -0,0 +1,55 @@ +/* + * SonarQube Findbugs Plugin + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.plugins.findbugs; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; + +import edu.umd.cs.findbugs.AnalysisError; +import edu.umd.cs.findbugs.BugInstance; + +/** + * @author gtoison + * + */ +public class AnalysisResult { + private Collection reportedBugs; + private Collection analysisErrors; + + public AnalysisResult() { + this(new ArrayList<>(), new ArrayList<>()); + } + + public AnalysisResult(BugInstance bugInstance) { + this(Arrays.asList(new ReportedBug(bugInstance)), new ArrayList<>()); + } + + public AnalysisResult(Collection reportedBugs, Collection errors) { + this.reportedBugs = reportedBugs; + this.analysisErrors = new ArrayList<>(errors); + } + + public Collection getReportedBugs() { + return reportedBugs; + } + + public Collection getAnalysisErrors() { + return analysisErrors; + } +} diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java index 821be2ae..ee59c668 100644 --- a/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java @@ -22,6 +22,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Throwables; import com.google.common.collect.Lists; + +import edu.umd.cs.findbugs.AnalysisError; import edu.umd.cs.findbugs.BugCollection; import edu.umd.cs.findbugs.BugInstance; import edu.umd.cs.findbugs.DetectorFactoryCollection; @@ -31,6 +33,7 @@ import edu.umd.cs.findbugs.PluginException; import edu.umd.cs.findbugs.Priorities; import edu.umd.cs.findbugs.Project; +import edu.umd.cs.findbugs.SortedBugCollection; import edu.umd.cs.findbugs.XMLBugReporter; import edu.umd.cs.findbugs.config.UserPreferences; import edu.umd.cs.findbugs.plugins.DuplicatePluginIdException; @@ -91,15 +94,15 @@ public FindbugsExecutor(FindbugsConfiguration configuration, FileSystem fs, Conf } @VisibleForTesting - Collection execute() { + AnalysisResult execute() { return execute(true); } - public Collection execute(boolean useAllPlugin) { + public AnalysisResult execute(boolean useAllPlugin) { return execute(useAllPlugin,useAllPlugin); } - public Collection execute(boolean useFbContrib, boolean useFindSecBugs) { + public AnalysisResult execute(boolean useFbContrib, boolean useFindSecBugs) { ClassLoader initialClassLoader = Thread.currentThread().getContextClassLoader(); Thread.currentThread().setContextClassLoader(FindBugs2.class.getClassLoader()); @@ -115,7 +118,7 @@ public Collection execute(boolean useFbContrib, boolean useFindSecB if(project.getFileCount() == 0) { LOG.info("Findbugs analysis skipped for this project."); - return new ArrayList<>(); + return new AnalysisResult(); } customPlugins = loadFindbugsPlugins(useFbContrib,useFindSecBugs); @@ -178,7 +181,10 @@ public Collection execute(boolean useFbContrib, boolean useFindSecB if(!foundExistingReport) { //Avoid rescanning the project if FindBugs was run already executorService.submit(new FindbugsTask(engine)).get(configuration.getTimeout(), TimeUnit.MILLISECONDS); } - return toReportedBugs(xmlBugReporter.getBugCollection()); + Collection reportedBugs = toReportedBugs(xmlBugReporter.getBugCollection()); + Collection analysisErrors = ((SortedBugCollection) xmlBugReporter.getBugCollection()).getErrors(); + + return new AnalysisResult(reportedBugs, analysisErrors); } catch (TimeoutException e) { throw new IllegalStateException("Can not execute Findbugs with a timeout threshold value of " + configuration.getTimeout() + " milliseconds", e); } catch (Exception e) { diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java index 0fc2c1ca..81fb531e 100644 --- a/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java @@ -24,7 +24,6 @@ import java.io.FileOutputStream; import java.io.PrintWriter; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.SortedSet; @@ -38,6 +37,7 @@ 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.batch.sensor.error.NewAnalysisError; import org.sonar.api.batch.sensor.issue.NewIssue; import org.sonar.api.batch.sensor.issue.NewIssueLocation; import org.sonar.plugins.findbugs.language.Jsp; @@ -52,6 +52,8 @@ import org.sonar.plugins.findbugs.rules.FindbugsRulesDefinition; import org.sonar.plugins.java.api.JavaResourceLocator; +import edu.umd.cs.findbugs.AnalysisError; + public class FindbugsSensor implements Sensor { private static final Logger LOG = LoggerFactory.getLogger(FindbugsSensor.class); @@ -127,11 +129,11 @@ public void execute(SensorContext context) { return; } - Collection collection = executor.execute(hasActiveFbContribRules(), hasActiveFindSecBugsRules()); + AnalysisResult analysisResult = executor.execute(hasActiveFbContribRules(), hasActiveFindSecBugsRules()); try { - for (ReportedBug bugInstance : collection) { + for (ReportedBug bugInstance : analysisResult.getReportedBugs()) { try { ActiveRule rule = null; @@ -224,6 +226,9 @@ public void execute(SensorContext context) { } } + for (AnalysisError analysisError : analysisResult.getAnalysisErrors()) { + insertAnalysisError(context, analysisError); + } } finally { if(classMappingWriter != null) { @@ -233,6 +238,31 @@ public void execute(SensorContext context) { } } + public void insertAnalysisError(SensorContext context, AnalysisError analysisError) { + NewAnalysisError error = context.newAnalysisError(); + + StringBuilder message = buildAnalysisErrorMessage(analysisError); + error.message(message.toString()); + + error.save(); + } + + public StringBuilder buildAnalysisErrorMessage(AnalysisError analysisError) { + StringBuilder message = new StringBuilder(analysisError.getMessage()); + if (analysisError.getStackTrace() != null) { + for (String trace : analysisError.getStackTrace()) { + message.append('\n'); + message.append(trace); + } + } + if (analysisError.getNestedStackTrace() != null) { + for (String trace : analysisError.getNestedStackTrace()) { + message.append('\n'); + message.append(trace); + } + } + return message; + } protected void insertIssue(ActiveRule rule, InputFile resource, int line, String message, ReportedBug bugInstance) { NewIssue newIssue = sensorContext.newIssue().forRule(rule.ruleKey()); diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsExecutorTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsExecutorTest.java index 07694b9b..1cb4e245 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsExecutorTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsExecutorTest.java @@ -77,7 +77,7 @@ void canGenerateXMLReport() throws Exception { File reportFile = new File(temporaryFolder, "findbugs-report.xml"); when(conf.getTargetXMLReport()).thenReturn(reportFile); - new FindbugsExecutor(conf, fsEmpty, configEmpty).execute(); + AnalysisResult analysisResult = new FindbugsExecutor(conf, fsEmpty, configEmpty).execute(); assertThat(reportFile).exists(); String report = FileUtils.readFileToString(reportFile, StandardCharsets.UTF_8); @@ -87,6 +87,8 @@ void canGenerateXMLReport() throws Exception { .as("Report should be generated with messages").contains("") .contains("priority=\"1\"") .doesNotContain("priority=\"3\""); + + checkAnalysisResult(analysisResult); } @Test @@ -96,7 +98,7 @@ void canGenerateXMLReportWithCustomConfidence() throws Exception { when(conf.getTargetXMLReport()).thenReturn(reportFile); when(conf.getConfidenceLevel()).thenReturn("low"); - new FindbugsExecutor(conf, fsEmpty, configEmpty).execute(); + AnalysisResult analysisResult = new FindbugsExecutor(conf, fsEmpty, configEmpty).execute(); assertThat(reportFile).exists(); String report = FileUtils.readFileToString(reportFile, StandardCharsets.UTF_8); @@ -106,6 +108,8 @@ void canGenerateXMLReportWithCustomConfidence() throws Exception { .as("Report should be generated with messages").contains("") .contains("priority=\"1\"") .contains("priority=\"3\""); + + checkAnalysisResult(analysisResult); } public void shouldTerminateAfterTimeout() throws Exception { @@ -148,4 +152,13 @@ private FindbugsConfiguration mockConf() throws Exception { return conf; } + /** + * Smoke test checking that no errors where reported. + * This might happen when upgrading to a new version of SpotBugs or plugins and would most likely mean that there's a regression in the new version + * + * @param analysisResult + */ + public void checkAnalysisResult(AnalysisResult analysisResult) { + assertThat(analysisResult.getAnalysisErrors()).isEmpty(); + } } diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java index 2bae1f4e..1d5c15ca 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java @@ -20,6 +20,8 @@ package org.sonar.plugins.findbugs; import com.google.common.collect.Lists; + +import edu.umd.cs.findbugs.AnalysisError; import edu.umd.cs.findbugs.BugInstance; import edu.umd.cs.findbugs.ClassAnnotation; import edu.umd.cs.findbugs.MethodAnnotation; @@ -46,6 +48,7 @@ import org.sonar.api.batch.fs.TextRange; import org.sonar.api.batch.sensor.SensorContext; import org.sonar.api.batch.sensor.SensorDescriptor; +import org.sonar.api.batch.sensor.error.NewAnalysisError; import org.sonar.api.batch.sensor.issue.NewIssue; import org.sonar.api.batch.sensor.issue.NewIssueLocation; import org.sonar.api.rule.RuleKey; @@ -121,6 +124,9 @@ public void setUp() throws IOException { when(sensorContext.newIssue()).thenReturn(newIssue); + NewAnalysisError newAnalysisError = mock(NewAnalysisError.class); + when(sensorContext.newAnalysisError()).thenReturn(newAnalysisError); + pico.addComponent(executor); pico.addComponent(javaResourceLocator); } @@ -136,8 +142,7 @@ private static JavaResourceLocator mockJavaResourceLocator() { void should_execute_findbugs() throws Exception { BugInstance bugInstance = getBugInstance("AM_CREATES_EMPTY_ZIP_FILE_ENTRY", 6, true); - Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute(false, false)).thenReturn(collection); + when(executor.execute(false, false)).thenReturn(new AnalysisResult(bugInstance)); JavaResourceLocator javaResourceLocator = mockJavaResourceLocator(); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); @@ -153,8 +158,7 @@ void should_execute_findbugs() throws Exception { void should_not_add_issue_if_resource_not_found() throws Exception { BugInstance bugInstance = getBugInstance("AM_CREATES_EMPTY_ZIP_FILE_ENTRY", 13, false); - Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute(false, false)).thenReturn(collection); + when(executor.execute(false, false)).thenReturn(new AnalysisResult(bugInstance)); when(javaResourceLocator.findResourceByClassName(anyString())).thenReturn(null); when(fs.inputFiles(any(FilePredicate.class))).thenReturn(new ArrayList()); @@ -173,8 +177,7 @@ void should_not_add_issue_if_resource_not_found() throws Exception { void should_execute_findbugs_even_if_only_fbcontrib() throws Exception { BugInstance bugInstance = getBugInstance("ISB_INEFFICIENT_STRING_BUFFERING", 49, true); - Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute(true, false)).thenReturn(collection); + when(executor.execute(true, false)).thenReturn(new AnalysisResult(bugInstance)); JavaResourceLocator javaResourceLocator = mockJavaResourceLocator(); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); @@ -191,8 +194,7 @@ void should_execute_findbugs_even_if_only_fbcontrib() throws Exception { void should_execute_findbugs_even_if_only_findsecbug() throws Exception { BugInstance bugInstance = getBugInstance("PREDICTABLE_RANDOM", 0, true); - Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute(false, true)).thenReturn(collection); + when(executor.execute(false, true)).thenReturn(new AnalysisResult(bugInstance)); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); @@ -209,8 +211,7 @@ void should_execute_findbugs_even_if_only_findsecbug() throws Exception { void should_execute_findbugs_but_not_find_violation() throws Exception { BugInstance bugInstance = getBugInstance("THIS_RULE_DOES_NOT_EXIST", 107, true); - Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute(false, false)).thenReturn(collection); + when(executor.execute(false, false)).thenReturn(new AnalysisResult(bugInstance)); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); @@ -267,6 +268,7 @@ void should_execute_findbugs_if_only_jsp_rules_and_some_jsp_files() throws Excep when(fs.languages()).thenReturn(languages); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); + when(executor.execute(false, true)).thenReturn(new AnalysisResult()); pico.addComponent(FakeActiveRules.createWithOnlyFindSecBugsJspRules()); @@ -280,8 +282,7 @@ void should_execute_findbugs_if_only_jsp_rules_and_some_jsp_files() throws Excep @Test void should_execute_findbugs_with_missing_smap_and_source() throws Exception { BugInstance bugInstance = getBugInstance("AM_CREATES_EMPTY_ZIP_FILE_ENTRY", 6, true); - Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute(false, false)).thenReturn(collection); + when(executor.execute(false, false)).thenReturn(new AnalysisResult(bugInstance)); JavaResourceLocator javaResourceLocator = mockJavaResourceLocator(); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); @@ -301,8 +302,7 @@ void should_execute_findbugs_with_missing_smap_and_source() throws Exception { @Test void should_execute_findbugs_with_smap() throws Exception { BugInstance bugInstance = getBugInstance("AM_CREATES_EMPTY_ZIP_FILE_ENTRY", 6, true); - Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute(false, false)).thenReturn(collection); + when(executor.execute(false, false)).thenReturn(new AnalysisResult(bugInstance)); JavaResourceLocator javaResourceLocator = mockJavaResourceLocator(); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); @@ -345,16 +345,6 @@ private BugInstance getBugInstance(String name, int line, boolean mockFindSource return bugInstance; } - @Test - void should_not_execute_if_no_compiled_class_available() throws Exception { - when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Collections.emptyList()); - pico.addComponent(FakeActiveRules.createWithOnlyFindbugsRules()); - FindbugsSensor sensor = pico.getComponent(FindbugsSensor.class); - sensor.execute(sensorContext); - - verify(executor, never()).execute(); - } - @Test void shouldIgnoreNotActiveViolations() throws Exception { BugInstance bugInstance = new BugInstance("UNKNOWN", 2); @@ -362,10 +352,9 @@ void shouldIgnoreNotActiveViolations() throws Exception { String sourceFile = "org/sonar/commons/ZipUtils.java"; ClassAnnotation classAnnotation = new ClassAnnotation(className, sourceFile); bugInstance.add(classAnnotation); - Collection collection = Arrays.asList(new ReportedBug(bugInstance)); - when(executor.execute()).thenReturn(collection); pico.addComponent(FakeActiveRules.createWithOnlyFindbugsRules()); + when(executor.execute(false, false)).thenReturn(new AnalysisResult(bugInstance)); FindbugsSensor sensor = pico.getComponent(FindbugsSensor.class); sensor.execute(sensorContext); @@ -384,4 +373,22 @@ void describe() { verify(descriptor).name(argument.capture()); assertEquals("FindBugs Sensor", argument.getValue()); } + + @Test + void shouldReportAnalysisError() throws Exception { + Collection analysisErrors = Arrays.asList( + new AnalysisError("Only a message"), + new AnalysisError("A message and an exception", new UnsupportedOperationException("Unsupported")) + ); + + AnalysisResult analysisResult = new AnalysisResult(Collections.emptyList(), analysisErrors); + + pico.addComponent(FakeActiveRules.createWithOnlyFindbugsRules()); + when(executor.execute(false, false)).thenReturn(analysisResult); + FindbugsSensor sensor = pico.getComponent(FindbugsSensor.class); + sensor.execute(sensorContext); + + verify(sensorContext, never()).newIssue(); + verify(sensorContext, times(2)).newAnalysisError(); + } }