From 9b1955062c118f0ae7d32a53d7c173dc5615f823 Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov Date: Fri, 6 Jul 2012 11:17:42 +0600 Subject: [PATCH] SONAR-3572 FindBugs plugin should log violation only on the primary location of a bug instance --- .../findbugs/FindbugsConfiguration.java | 26 +++-- .../plugins/findbugs/FindbugsSensor.java | 41 +++++--- .../findbugs/FindbugsXmlReportParser.java | 96 ++++++++++++------- .../findbugs/FindbugsAntConverterTest.java | 9 +- .../findbugs/FindbugsConfigurationTest.java | 31 +++--- .../findbugs/FindbugsExecutorTest.java | 14 ++- .../FindbugsMavenInitializerTest.java | 17 ++-- .../plugins/findbugs/FindbugsPluginTest.java | 7 +- .../findbugs/FindbugsProfileExporterTest.java | 31 +++--- .../findbugs/FindbugsProfileImporterTest.java | 46 +++++---- .../findbugs/FindbugsRuleRepositoryTest.java | 21 ++-- .../plugins/findbugs/FindbugsSensorTest.java | 16 ++-- .../plugins/findbugs/FindbugsVersionTest.java | 7 +- .../findbugs/FindbugsXmlReportParserTest.java | 54 ++++++----- .../SonarWayWithFindbugsProfileTest.java | 8 +- 15 files changed, 234 insertions(+), 190 deletions(-) diff --git a/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java b/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java index 2ef4d2b90eb..70eb010cc2b 100644 --- a/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java +++ b/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java @@ -25,6 +25,7 @@ import org.apache.commons.lang.StringUtils; import org.sonar.api.BatchExtension; import org.sonar.api.CoreProperties; import org.sonar.api.batch.ProjectClasspath; +import org.sonar.api.config.Settings; import org.sonar.api.profiles.RulesProfile; import org.sonar.api.resources.Project; import org.sonar.api.utils.SonarException; @@ -32,6 +33,8 @@ import org.sonar.plugins.findbugs.xml.ClassFilter; import org.sonar.plugins.findbugs.xml.FindBugsFilter; import org.sonar.plugins.findbugs.xml.Match; +import javax.annotation.CheckForNull; + import java.io.*; import java.util.ArrayList; import java.util.List; @@ -41,20 +44,23 @@ import java.util.List; */ public class FindbugsConfiguration implements BatchExtension { - private Project project; - private RulesProfile profile; - private FindbugsProfileExporter exporter; - private ProjectClasspath projectClasspath; + private final Project project; + private final Settings settings; + private final RulesProfile profile; + private final FindbugsProfileExporter exporter; + private final ProjectClasspath projectClasspath; - public FindbugsConfiguration(Project project, RulesProfile profile, FindbugsProfileExporter exporter, ProjectClasspath classpath) { + public FindbugsConfiguration(Project project, Settings settings, RulesProfile profile, FindbugsProfileExporter exporter, ProjectClasspath classpath) { this.project = project; + this.settings = settings; this.profile = profile; this.exporter = exporter; this.projectClasspath = classpath; } + @CheckForNull public File getTargetXMLReport() { - if (project.getConfiguration().getBoolean(FindbugsConstants.GENERATE_XML_KEY, FindbugsConstants.GENERATE_XML_DEFAULT_VALUE)) { + if (settings.getBoolean(FindbugsConstants.GENERATE_XML_KEY)) { return new File(project.getFileSystem().getSonarWorkingDirectory(), "findbugs-result.xml"); } return null; @@ -102,7 +108,7 @@ public class FindbugsConfiguration implements BatchExtension { public List getExcludesFilters() { List result = new ArrayList(); - String[] filters = project.getConfiguration().getStringArray(FindbugsConstants.EXCLUDES_FILTERS_PROPERTY); + String[] filters = settings.getStringArray(FindbugsConstants.EXCLUDES_FILTERS_PROPERTY); for (String excludesFilterPath : filters) { excludesFilterPath = StringUtils.trim(excludesFilterPath); if (StringUtils.isNotBlank(excludesFilterPath)) { @@ -113,12 +119,11 @@ public class FindbugsConfiguration implements BatchExtension { } public String getEffort() { - return StringUtils.lowerCase(project.getConfiguration().getString(CoreProperties.FINDBUGS_EFFORT_PROPERTY, - CoreProperties.FINDBUGS_EFFORT_DEFAULT_VALUE)); + return StringUtils.lowerCase(settings.getString(CoreProperties.FINDBUGS_EFFORT_PROPERTY)); } public long getTimeout() { - return project.getConfiguration().getLong(CoreProperties.FINDBUGS_TIMEOUT_PROPERTY, CoreProperties.FINDBUGS_TIMEOUT_DEFAULT_VALUE); + return settings.getLong(CoreProperties.FINDBUGS_TIMEOUT_PROPERTY); } private File jsr305Lib; @@ -151,4 +156,5 @@ public class FindbugsConfiguration implements BatchExtension { throw new SonarException(e); } } + } 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 ebe062c4e04..ca3d55b4990 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,6 +19,8 @@ */ package org.sonar.plugins.findbugs; +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; @@ -28,12 +30,14 @@ import org.sonar.api.resources.Project; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleFinder; import org.sonar.api.rules.Violation; -import org.sonar.api.utils.Logs; import java.io.File; import java.util.List; public class FindbugsSensor implements Sensor { + + private static final Logger LOG = LoggerFactory.getLogger(FindbugsSensor.class); + private RulesProfile profile; private RuleFinder ruleFinder; private FindbugsExecutor executor; @@ -51,24 +55,35 @@ public class FindbugsSensor implements Sensor { public void analyse(Project project, SensorContext context) { if (project.getReuseExistingRulesConfig()) { - Logs.INFO.warn("Reusing existing Findbugs configuration not supported any more."); + 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 fbViolations = reportParser.getViolations(); - for (FindbugsXmlReportParser.Violation fbViolation : fbViolations) { - Rule rule = ruleFinder.findByKey(FindbugsConstants.REPOSITORY_KEY, fbViolation.getType()); - if (rule != null) { // ignore violations from report, if rule not activated in Sonar - JavaFile resource = new JavaFile(fbViolation.getSonarJavaFileKey()); - if (context.getResource(resource) != null) { - Violation violation = Violation.create(rule, resource).setLineId(fbViolation.getStart()).setMessage(fbViolation.getLongMessage()); - context.saveViolation(violation); - } - } else { - Logs.INFO.warn("Findbugs rule '{}' not active in Sonar.", fbViolation.getType()); + List bugInstances = reportParser.getBugInstances(); + + for (FindbugsXmlReportParser.XmlBugInstance bugInstance : bugInstances) { + FindbugsXmlReportParser.XmlSourceLineAnnotation sourceLine = bugInstance.getPrimarySourceLine(); + if (sourceLine == null) { + LOG.warn("No source line for " + bugInstance.getType()); + continue; + } + + Rule rule = ruleFinder.findByKey(FindbugsConstants.REPOSITORY_KEY, bugInstance.getType()); + if (rule == null) { + // ignore violations from report, if rule not activated in Sonar + LOG.warn("Findbugs rule '{}' not active in Sonar.", bugInstance.getType()); + continue; + } + + JavaFile resource = new JavaFile(sourceLine.getSonarJavaFileKey()); + if (context.getResource(resource) != null) { + Violation violation = Violation.create(rule, resource) + .setLineId(sourceLine.getStart()) + .setMessage(bugInstance.getLongMessage()); + context.saveViolation(violation); } } } diff --git a/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsXmlReportParser.java b/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsXmlReportParser.java index 167efedc1a8..2a330959fa4 100644 --- a/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsXmlReportParser.java +++ b/plugins/sonar-findbugs-plugin/src/main/java/org/sonar/plugins/findbugs/FindbugsXmlReportParser.java @@ -19,18 +19,21 @@ */ package org.sonar.plugins.findbugs; -import java.io.File; -import java.util.ArrayList; -import java.util.List; - -import javax.xml.stream.XMLInputFactory; -import javax.xml.stream.XMLStreamException; - +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; import org.codehaus.staxmate.SMInputFactory; import org.codehaus.staxmate.in.SMInputCursor; import org.sonar.api.utils.SonarException; import org.sonar.api.utils.XmlParserException; +import javax.annotation.CheckForNull; +import javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLStreamException; + +import java.io.File; +import java.util.List; + class FindbugsXmlReportParser { private final File findbugsXmlReport; @@ -39,59 +42,83 @@ class FindbugsXmlReportParser { public FindbugsXmlReportParser(File findbugsXmlReport) { this.findbugsXmlReport = findbugsXmlReport; findbugsXmlReportPath = findbugsXmlReport.getAbsolutePath(); - if ( !findbugsXmlReport.exists()) { + if (!findbugsXmlReport.exists()) { throw new SonarException("The findbugs XML report can't be found at '" + findbugsXmlReportPath + "'"); } } - public List getViolations() { - List violations = new ArrayList(); + public List getBugInstances() { + List result = Lists.newArrayList(); try { SMInputFactory inf = new SMInputFactory(XMLInputFactory.newInstance()); SMInputCursor cursor = inf.rootElementCursor(findbugsXmlReport).advance(); SMInputCursor bugInstanceCursor = cursor.childElementCursor("BugInstance").advance(); while (bugInstanceCursor.asEvent() != null) { - String type = bugInstanceCursor.getAttrValue("type"); - String longMessage = ""; + XmlBugInstance xmlBugInstance = new XmlBugInstance(); + xmlBugInstance.type = bugInstanceCursor.getAttrValue("type"); + xmlBugInstance.longMessage = ""; + result.add(xmlBugInstance); + + ImmutableList.Builder lines = ImmutableList.builder(); SMInputCursor bugInstanceChildCursor = bugInstanceCursor.childElementCursor().advance(); while (bugInstanceChildCursor.asEvent() != null) { String nodeName = bugInstanceChildCursor.getLocalName(); if ("LongMessage".equals(nodeName)) { - longMessage = bugInstanceChildCursor.collectDescendantText(); + xmlBugInstance.longMessage = bugInstanceChildCursor.collectDescendantText(); } else if ("SourceLine".equals(nodeName)) { - Violation fbViolation = new Violation(); - fbViolation.type = type; - fbViolation.longMessage = longMessage; - fbViolation.parseStart(bugInstanceChildCursor.getAttrValue("start")); - fbViolation.parseEnd(bugInstanceChildCursor.getAttrValue("end")); - fbViolation.className = bugInstanceChildCursor.getAttrValue("classname"); - fbViolation.sourcePath = bugInstanceChildCursor.getAttrValue("sourcepath"); - violations.add(fbViolation); + XmlSourceLineAnnotation xmlSourceLineAnnotation = new XmlSourceLineAnnotation(); + xmlSourceLineAnnotation.parseStart(bugInstanceChildCursor.getAttrValue("start")); + xmlSourceLineAnnotation.parseEnd(bugInstanceChildCursor.getAttrValue("end")); + xmlSourceLineAnnotation.parsePrimary(bugInstanceChildCursor.getAttrValue("primary")); + xmlSourceLineAnnotation.className = bugInstanceChildCursor.getAttrValue("classname"); + lines.add(xmlSourceLineAnnotation); } bugInstanceChildCursor.advance(); } + xmlBugInstance.sourceLines = lines.build(); bugInstanceCursor.advance(); } cursor.getStreamReader().closeCompletely(); } catch (XMLStreamException e) { throw new XmlParserException("Unable to parse the Findbugs XML Report '" + findbugsXmlReportPath + "'", e); } - return violations; + return result; } - public static class Violation { - + public static class XmlBugInstance { private String type; private String longMessage; - private Integer start; - private Integer end; - protected String className; - protected String sourcePath; + private List sourceLines; public String getType() { return type; } + public String getLongMessage() { + return longMessage; + } + + @CheckForNull + public XmlSourceLineAnnotation getPrimarySourceLine() { + for (XmlSourceLineAnnotation sourceLine : sourceLines) { + if (sourceLine.isPrimary()) { + // According to source code of Findbugs 2.0 - should be exactly one primary + return sourceLine; + } + } + // As a last resort - return first line + return sourceLines.isEmpty() ? null : sourceLines.get(0); + } + + } + + public static class XmlSourceLineAnnotation { + private boolean primary; + private Integer start; + private Integer end; + @VisibleForTesting + protected String className; + public void parseStart(String attrValue) { try { start = Integer.parseInt(attrValue); @@ -108,8 +135,12 @@ class FindbugsXmlReportParser { } } - public String getLongMessage() { - return longMessage; + public void parsePrimary(String attrValue) { + primary = Boolean.parseBoolean(attrValue); + } + + public boolean isPrimary() { + return primary; } public Integer getStart() { @@ -124,16 +155,13 @@ class FindbugsXmlReportParser { return className; } - public String getSourcePath() { - return sourcePath; - } - public String getSonarJavaFileKey() { if (className.indexOf('$') > -1) { return className.substring(0, className.indexOf('$')); } return className; } + } } diff --git a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsAntConverterTest.java b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsAntConverterTest.java index 58863cf70b1..a850ea2214e 100644 --- a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsAntConverterTest.java +++ b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsAntConverterTest.java @@ -19,14 +19,13 @@ */ package org.sonar.plugins.findbugs; -import static org.junit.Assert.assertThat; - -import org.hamcrest.core.Is; import org.junit.Test; import java.util.regex.Matcher; import java.util.regex.Pattern; +import static org.fest.assertions.Assertions.assertThat; + public class FindbugsAntConverterTest { @Test @@ -60,7 +59,7 @@ public class FindbugsAntConverterTest { } private void assertAntPatternEqualsToFindBugsRegExp(String antPattern, String regExp, String example) { - assertThat(FindbugsAntConverter.antToJavaRegexpConvertor(antPattern), Is.is(regExp)); + assertThat(FindbugsAntConverter.antToJavaRegexpConvertor(antPattern)).isEqualTo(regExp); String javaRegexp = regExp.substring(1, regExp.length()); assertJavaRegexpResult(javaRegexp, example, true); } @@ -68,7 +67,7 @@ public class FindbugsAntConverterTest { private void assertJavaRegexpResult(String javaRegexp, String example, boolean expectedResult) { Pattern pattern = Pattern.compile(javaRegexp); Matcher matcher = pattern.matcher(example); - assertThat(example + " tested with pattern " + javaRegexp, matcher.matches(), Is.is(expectedResult)); + assertThat(matcher.matches()).as(example + " tested with pattern " + javaRegexp).isEqualTo(expectedResult); } } diff --git a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsConfigurationTest.java b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsConfigurationTest.java index c8d1f938655..0465c58174d 100644 --- a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsConfigurationTest.java +++ b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsConfigurationTest.java @@ -19,59 +19,58 @@ */ package org.sonar.plugins.findbugs; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import org.apache.commons.configuration.BaseConfiguration; -import org.apache.commons.configuration.Configuration; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.sonar.api.config.Settings; import org.sonar.api.profiles.RulesProfile; import org.sonar.api.resources.Project; import org.sonar.api.test.SimpleProjectFileSystem; import java.io.File; +import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + public class FindbugsConfigurationTest { @Rule public TemporaryFolder tempFolder = new TemporaryFolder(); private Project project; + private Settings settings; private File findbugsTempDir; @Before public void setup() { project = mock(Project.class); + settings = new Settings(); findbugsTempDir = tempFolder.newFolder("findbugs"); when(project.getFileSystem()).thenReturn(new SimpleProjectFileSystem(findbugsTempDir)); } @Test public void shouldSaveConfigFiles() throws Exception { - FindbugsConfiguration conf = new FindbugsConfiguration(project, RulesProfile.create(), new FindbugsProfileExporter(), null); + FindbugsConfiguration conf = new FindbugsConfiguration(project, settings, RulesProfile.create(), new FindbugsProfileExporter(), null); conf.saveIncludeConfigXml(); conf.saveExcludeConfigXml(); File findbugsIncludeFile = new File(findbugsTempDir + "/target/sonar/findbugs-include.xml"); File findbugsExcludeFile = new File(findbugsTempDir + "/target/sonar/findbugs-exclude.xml"); - assertThat(findbugsIncludeFile.exists(), is(true)); - assertThat(findbugsExcludeFile.exists(), is(true)); + assertThat(findbugsIncludeFile.exists()).isTrue(); + assertThat(findbugsExcludeFile.exists()).isTrue(); } @Test public void shouldReturnExcludesFilters() { - Configuration projectConfiguration = new BaseConfiguration(); - when(project.getConfiguration()).thenReturn(projectConfiguration); - FindbugsConfiguration conf = new FindbugsConfiguration(project, RulesProfile.create(), new FindbugsProfileExporter(), null); + FindbugsConfiguration conf = new FindbugsConfiguration(project, settings, RulesProfile.create(), new FindbugsProfileExporter(), null); - assertThat(conf.getExcludesFilters().size(), is(0)); - projectConfiguration.setProperty(FindbugsConstants.EXCLUDES_FILTERS_PROPERTY, " foo.xml , bar.xml,"); - assertThat(conf.getExcludesFilters().size(), is(2)); + assertThat(conf.getExcludesFilters()).isEmpty(); + settings.setProperty(FindbugsConstants.EXCLUDES_FILTERS_PROPERTY, " foo.xml , bar.xml,"); + assertThat(conf.getExcludesFilters()).hasSize(2); } + } diff --git a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsExecutorTest.java b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsExecutorTest.java index adc231ac529..afdf5797d96 100644 --- a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsExecutorTest.java +++ b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsExecutorTest.java @@ -29,9 +29,7 @@ import org.sonar.api.utils.SonarException; import java.io.File; -import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertThat; -import static org.junit.internal.matchers.StringContains.containsString; +import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -45,11 +43,11 @@ public class FindbugsExecutorTest { new FindbugsExecutor(conf).execute(); - assertThat(reportFile.exists(), is(true)); + assertThat(reportFile.exists()).isTrue(); String report = FileUtils.readFileToString(reportFile); - assertThat("Report should contain bug instance", report, containsString("")); - assertThat(report, containsString("synthetic=\"true\"")); + assertThat(report).as("Report should contain bug instance").contains(""); + assertThat(report).contains("synthetic=\"true\""); } @Test(expected = SonarException.class) @@ -65,7 +63,7 @@ public class FindbugsExecutorTest { Project project = mock(Project.class); ProjectFileSystem fs = mock(ProjectFileSystem.class); when(project.getFileSystem()).thenReturn(fs); - FindbugsConfiguration conf = new FindbugsConfiguration(project, null, null, null); + FindbugsConfiguration conf = new FindbugsConfiguration(project, null, null, null, null); new FindbugsExecutor(conf).execute(); } diff --git a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsMavenInitializerTest.java b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsMavenInitializerTest.java index a2870eeb554..a5faba2481d 100644 --- a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsMavenInitializerTest.java +++ b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsMavenInitializerTest.java @@ -19,8 +19,13 @@ */ package org.sonar.plugins.findbugs; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; +import org.apache.commons.configuration.Configuration; +import org.junit.Before; +import org.junit.Test; +import org.sonar.api.resources.Project; +import org.sonar.api.test.MavenTestUtils; + +import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; @@ -28,12 +33,6 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import org.apache.commons.configuration.Configuration; -import org.junit.Before; -import org.junit.Test; -import org.sonar.api.resources.Project; -import org.sonar.api.test.MavenTestUtils; - public class FindbugsMavenInitializerTest { private Project project; @@ -58,7 +57,7 @@ public class FindbugsMavenInitializerTest { public void shouldGetExcludesFiltersFromPom() { Project project = MavenTestUtils.loadProjectFromPom(getClass(), "pom.xml"); initializer.execute(project); - assertThat(project.getConfiguration().getString(FindbugsConstants.EXCLUDES_FILTERS_PROPERTY), is("foo.xml")); + assertThat(project.getConfiguration().getString(FindbugsConstants.EXCLUDES_FILTERS_PROPERTY)).isEqualTo("foo.xml"); } } diff --git a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsPluginTest.java b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsPluginTest.java index 58bd0cd1185..906688d886a 100644 --- a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsPluginTest.java +++ b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsPluginTest.java @@ -19,16 +19,15 @@ */ package org.sonar.plugins.findbugs; -import static org.hamcrest.Matchers.greaterThan; -import static org.junit.Assert.assertThat; - import org.junit.Test; +import static org.fest.assertions.Assertions.assertThat; + public class FindbugsPluginTest { @Test public void testGetExtensions() { - assertThat(new FindbugsPlugin().getExtensions().size(), greaterThan(1)); + assertThat(new FindbugsPlugin().getExtensions().size()).isGreaterThan(1); } } diff --git a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileExporterTest.java b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileExporterTest.java index d7d82df9ac9..27f3545dfd4 100644 --- a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileExporterTest.java +++ b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileExporterTest.java @@ -19,15 +19,6 @@ */ package org.sonar.plugins.findbugs; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; - -import java.io.IOException; -import java.io.StringWriter; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; - import org.junit.Test; import org.sonar.api.profiles.RulesProfile; import org.sonar.api.rules.ActiveRule; @@ -38,6 +29,14 @@ import org.sonar.plugins.findbugs.xml.FindBugsFilter; import org.sonar.plugins.findbugs.xml.Match; import org.xml.sax.SAXException; +import java.io.IOException; +import java.io.StringWriter; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.fest.assertions.Assertions.assertThat; + public class FindbugsProfileExporterTest extends FindbugsTests { private FindbugsProfileExporter exporter = new FindbugsProfileExporter(); @@ -66,7 +65,7 @@ public class FindbugsProfileExporterTest extends FindbugsTests { @Test public void shouldBuildOnlyOneModuleWhenNoActiveRules() { FindBugsFilter filter = FindbugsProfileExporter.buildFindbugsFilter(Collections. emptyList()); - assertThat(filter.getMatchs().size(), is(0)); + assertThat(filter.getMatchs()).hasSize(0); } @Test @@ -76,10 +75,10 @@ public class FindbugsProfileExporterTest extends FindbugsTests { FindBugsFilter filter = FindbugsProfileExporter.buildFindbugsFilter(Arrays.asList(activeRule1, activeRule2)); List matches = filter.getMatchs(); - assertThat(matches.size(), is(2)); + assertThat(matches).hasSize(2); - assertThat(matches.get(0).getBug().getPattern(), is("DLS_DEAD_LOCAL_STORE")); - assertThat(matches.get(1).getBug().getPattern(), is("SS_SHOULD_BE_STATIC")); + assertThat(matches.get(0).getBug().getPattern()).isEqualTo("DLS_DEAD_LOCAL_STORE"); + assertThat(matches.get(1).getBug().getPattern()).isEqualTo("SS_SHOULD_BE_STATIC"); } @Test @@ -88,7 +87,7 @@ public class FindbugsProfileExporterTest extends FindbugsTests { ActiveRule activeRule2 = anActiveRuleFromAnotherPlugin(); FindBugsFilter filter = FindbugsProfileExporter.buildFindbugsFilter(Arrays.asList(activeRule1, activeRule2)); - assertThat(filter.getMatchs().size(), is(0)); + assertThat(filter.getMatchs()).hasSize(0); } @Test @@ -96,8 +95,8 @@ public class FindbugsProfileExporterTest extends FindbugsTests { ActiveRule activeRule = anActiveRule(DLS_DEAD_LOCAL_STORE); FindBugsFilter filter = FindbugsProfileExporter.buildFindbugsFilter(Arrays.asList(activeRule)); - assertThat(filter.getMatchs().size(), is(1)); - assertThat(filter.getMatchs().get(0).getBug().getPattern(), is("DLS_DEAD_LOCAL_STORE")); + assertThat(filter.getMatchs()).hasSize(1); + assertThat(filter.getMatchs().get(0).getBug().getPattern()).isEqualTo("DLS_DEAD_LOCAL_STORE"); } @Test diff --git a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileImporterTest.java b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileImporterTest.java index 84d233f1e16..23251154ab4 100644 --- a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileImporterTest.java +++ b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileImporterTest.java @@ -35,9 +35,7 @@ import java.io.InputStreamReader; import java.io.StringReader; import java.util.List; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.CoreMatchers.notNullValue; -import static org.junit.Assert.assertThat; +import static org.fest.assertions.Assertions.assertThat; public class FindbugsProfileImporterTest { @@ -48,9 +46,9 @@ public class FindbugsProfileImporterTest { String findbugsConf = TestUtils.getResourceContent("/org/sonar/plugins/findbugs/shouldImportPatterns.xml"); RulesProfile profile = importer.importProfile(new StringReader(findbugsConf), ValidationMessages.create()); - assertThat(profile.getActiveRules().size(), is(2)); - assertThat(profile.getActiveRule(FindbugsConstants.REPOSITORY_KEY, "NP_CLOSING_NULL"), is(notNullValue())); - assertThat(profile.getActiveRule(FindbugsConstants.REPOSITORY_KEY, "RC_REF_COMPARISON_BAD_PRACTICE"), is(notNullValue())); + assertThat(profile.getActiveRules()).hasSize(2); + assertThat(profile.getActiveRule(FindbugsConstants.REPOSITORY_KEY, "NP_CLOSING_NULL")).isNotNull(); + assertThat(profile.getActiveRule(FindbugsConstants.REPOSITORY_KEY, "RC_REF_COMPARISON_BAD_PRACTICE")).isNotNull(); } @Test @@ -59,9 +57,9 @@ public class FindbugsProfileImporterTest { RulesProfile profile = importer.importProfile(new InputStreamReader(input), ValidationMessages.create()); List results = profile.getActiveRules(); - assertThat(results.size(), is(18)); - assertThat(profile.getActiveRule(FindbugsConstants.REPOSITORY_KEY, "EC_INCOMPATIBLE_ARRAY_COMPARE"), is(notNullValue())); - assertThat(profile.getActiveRule(FindbugsConstants.REPOSITORY_KEY, "BC_IMPOSSIBLE_DOWNCAST_OF_TOARRAY"), is(notNullValue())); + assertThat(results).hasSize(18); + assertThat(profile.getActiveRule(FindbugsConstants.REPOSITORY_KEY, "EC_INCOMPATIBLE_ARRAY_COMPARE")).isNotNull(); + assertThat(profile.getActiveRule(FindbugsConstants.REPOSITORY_KEY, "BC_IMPOSSIBLE_DOWNCAST_OF_TOARRAY")).isNotNull(); } @Test @@ -70,8 +68,8 @@ public class FindbugsProfileImporterTest { RulesProfile profile = importer.importProfile(new InputStreamReader(input), ValidationMessages.create()); List results = profile.getActiveRules(); - assertThat(results.size(), is(182)); - assertThat(profile.getActiveRule(FindbugsConstants.REPOSITORY_KEY, "LG_LOST_LOGGER_DUE_TO_WEAK_REFERENCE"), is(notNullValue())); + assertThat(results).hasSize(182); + assertThat(profile.getActiveRule(FindbugsConstants.REPOSITORY_KEY, "LG_LOST_LOGGER_DUE_TO_WEAK_REFERENCE")).isNotNull(); } @Test @@ -80,8 +78,8 @@ public class FindbugsProfileImporterTest { RulesProfile profile = importer.importProfile(new InputStreamReader(input), ValidationMessages.create()); List results = profile.getActiveRules(); - assertThat(results.size(), is(11)); - assertThat(profile.getActiveRule(FindbugsConstants.REPOSITORY_KEY, "RC_REF_COMPARISON_BAD_PRACTICE"), is(notNullValue())); + assertThat(results).hasSize(11); + assertThat(profile.getActiveRule(FindbugsConstants.REPOSITORY_KEY, "RC_REF_COMPARISON_BAD_PRACTICE")).isNotNull(); } @Test @@ -92,9 +90,9 @@ public class FindbugsProfileImporterTest { FindBugsFilter filter = (FindBugsFilter) xStream.fromXML(IOUtils.toString(input)); List matches = filter.getMatchs(); - assertThat(matches.size(), is(2)); - assertThat(matches.get(0).getBug().getPattern(), is("DLS_DEAD_LOCAL_STORE")); - assertThat(matches.get(1).getBug().getPattern(), is("URF_UNREAD_FIELD")); + assertThat(matches).hasSize(2); + assertThat(matches.get(0).getBug().getPattern()).isEqualTo("DLS_DEAD_LOCAL_STORE"); + assertThat(matches.get(1).getBug().getPattern()).isEqualTo("URF_UNREAD_FIELD"); } @Test @@ -104,8 +102,8 @@ public class FindbugsProfileImporterTest { RulesProfile profile = importer.importProfile(new StringReader(uncorrectFindbugsXml), messages); List results = profile.getActiveRules(); - assertThat(results.size(), is(0)); - assertThat(messages.getErrors().size(), is(1)); + assertThat(results).hasSize(0); + assertThat(messages.getErrors()).hasSize(1); } @Test @@ -115,8 +113,8 @@ public class FindbugsProfileImporterTest { RulesProfile profile = importer.importProfile(new StringReader(uncorrectFindbugsXml), messages); List results = profile.getActiveRules(); - assertThat(results.size(), is(1)); - assertThat(messages.getWarnings().size(), is(1)); + assertThat(results).hasSize(1); + assertThat(messages.getWarnings()).hasSize(1); } @Test @@ -126,8 +124,8 @@ public class FindbugsProfileImporterTest { RulesProfile profile = importer.importProfile(new StringReader(uncorrectFindbugsXml), messages); List results = profile.getActiveRules(); - assertThat(results.size(), is(141)); - assertThat(messages.getWarnings().size(), is(1)); + assertThat(results).hasSize(141); + assertThat(messages.getWarnings()).hasSize(1); } @Test @@ -137,7 +135,7 @@ public class FindbugsProfileImporterTest { RulesProfile profile = importer.importProfile(new StringReader(uncorrectFindbugsXml), messages); List results = profile.getActiveRules(); - assertThat(results.size(), is(9)); - assertThat(messages.getWarnings().size(), is(1)); + assertThat(results).hasSize(9); + assertThat(messages.getWarnings()).hasSize(1); } } diff --git a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsRuleRepositoryTest.java b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsRuleRepositoryTest.java index ace3fecfefe..b95dbf58f48 100644 --- a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsRuleRepositoryTest.java +++ b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsRuleRepositoryTest.java @@ -19,18 +19,16 @@ */ package org.sonar.plugins.findbugs; -import static org.hamcrest.Matchers.greaterThan; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; -import static org.mockito.Mockito.mock; - -import java.util.List; - import org.junit.Test; import org.sonar.api.platform.ServerFileSystem; import org.sonar.api.rules.Rule; import org.sonar.api.rules.XMLRuleParser; +import java.util.List; + +import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Mockito.mock; + public class FindbugsRuleRepositoryTest { @Test @@ -38,11 +36,12 @@ public class FindbugsRuleRepositoryTest { ServerFileSystem fileSystem = mock(ServerFileSystem.class); FindbugsRuleRepository repository = new FindbugsRuleRepository(fileSystem, new XMLRuleParser()); List rules = repository.createRules(); - assertThat(rules.size(), greaterThan(300)); + assertThat(rules.size()).isGreaterThan(300); for (Rule rule : rules) { - assertNotNull(rule.getKey()); - assertNotNull(rule.getConfigKey()); - assertNotNull(rule.getName()); + assertThat(rule.getKey()).isNotNull(); + assertThat(rule.getConfigKey()).isNotNull(); + assertThat(rule.getName()).isNotNull(); } } + } 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 72b478a5611..f1932c410d3 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 @@ -24,9 +24,9 @@ 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.ProjectFileSystem; import org.sonar.api.resources.JavaFile; 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; @@ -38,7 +38,11 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.argThat; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class FindbugsSensorTest extends FindbugsTests { @@ -72,16 +76,14 @@ public class FindbugsSensorTest extends FindbugsTests { analyser.analyse(project, context); verify(executor).execute(); - verify(context, times(3)).saveViolation(any(Violation.class)); + 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))); } @@ -100,16 +102,14 @@ public class FindbugsSensorTest extends FindbugsTests { analyser.analyse(project, context); verify(executor, never()).execute(); - verify(context, times(3)).saveViolation(any(Violation.class)); + 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))); } diff --git a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsVersionTest.java b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsVersionTest.java index a9bd61885c2..b9de4f88957 100644 --- a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsVersionTest.java +++ b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsVersionTest.java @@ -19,16 +19,15 @@ */ package org.sonar.plugins.findbugs; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.greaterThan; - import org.junit.Test; +import static org.fest.assertions.Assertions.assertThat; + public class FindbugsVersionTest { @Test public void getFindbugsVersion() { - assertThat(FindbugsVersion.getVersion().length(), greaterThan(1)); + assertThat(FindbugsVersion.getVersion().length()).isGreaterThan(1); } } diff --git a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsXmlReportParserTest.java b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsXmlReportParserTest.java index edd15b3660d..f163d6e814a 100644 --- a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsXmlReportParserTest.java +++ b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/FindbugsXmlReportParserTest.java @@ -19,57 +19,63 @@ */ package org.sonar.plugins.findbugs; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.api.utils.SonarException; import java.io.File; import java.net.URISyntaxException; import java.util.List; -import org.junit.Before; -import org.junit.Test; -import org.sonar.api.utils.SonarException; +import static org.fest.assertions.Assertions.assertThat; public class FindbugsXmlReportParserTest { - private List violations; + @Rule + public ExpectedException thrown = ExpectedException.none(); + + private List violations; @Before public void init() { File findbugsXmlReport = getFile("/org/sonar/plugins/findbugs/findbugsReport.xml"); FindbugsXmlReportParser xmlParser = new FindbugsXmlReportParser(findbugsXmlReport); - violations = xmlParser.getViolations(); + violations = xmlParser.getBugInstances(); } - @Test(expected = SonarException.class) + @Test public void createFindbugsXmlReportParserWithUnexistedReportFile() { File xmlReport = new File("doesntExist.xml"); + thrown.expect(SonarException.class); + thrown.expectMessage("The findbugs XML report can't be found at '" + xmlReport.getAbsolutePath() + "'"); new FindbugsXmlReportParser(xmlReport); } @Test public void testGetViolations() { - assertThat(violations.size(), is(3)); + assertThat(violations.size()).isEqualTo(2); + + FindbugsXmlReportParser.XmlBugInstance fbViolation = violations.get(0); + assertThat(fbViolation.getType()).isEqualTo("AM_CREATES_EMPTY_ZIP_FILE_ENTRY"); + assertThat(fbViolation.getLongMessage()).isEqualTo("Empty zip file entry created in org.sonar.commons.ZipUtils._zip(String, File, ZipOutputStream)"); - FindbugsXmlReportParser.Violation fbViolation = violations.get(0); - assertThat(fbViolation.getType(), is("AM_CREATES_EMPTY_ZIP_FILE_ENTRY")); - assertThat(fbViolation.getLongMessage(), - is("Empty zip file entry created in org.sonar.commons.ZipUtils._zip(String, File, ZipOutputStream)")); - assertThat(fbViolation.getStart(), is(107)); - assertThat(fbViolation.getEnd(), is(107)); - assertThat(fbViolation.getClassName(), is("org.sonar.commons.ZipUtils")); - assertThat(fbViolation.getSourcePath(), is("org/sonar/commons/ZipUtils.java")); + FindbugsXmlReportParser.XmlSourceLineAnnotation sourceLine = fbViolation.getPrimarySourceLine(); + assertThat(sourceLine.getStart()).isEqualTo(107); + assertThat(sourceLine.getEnd()).isEqualTo(107); + assertThat(sourceLine.getClassName()).isEqualTo("org.sonar.commons.ZipUtils"); } @Test public void testGetSonarJavaFileKey() { - FindbugsXmlReportParser.Violation violation = new FindbugsXmlReportParser.Violation(); - violation.className = "org.sonar.batch.Sensor"; - assertThat(violation.getSonarJavaFileKey(), is("org.sonar.batch.Sensor")); - violation.className = "Sensor"; - assertThat(violation.getSonarJavaFileKey(), is("Sensor")); - violation.className = "org.sonar.batch.Sensor$1"; - assertThat(violation.getSonarJavaFileKey(), is("org.sonar.batch.Sensor")); + FindbugsXmlReportParser.XmlSourceLineAnnotation sourceLine = new FindbugsXmlReportParser.XmlSourceLineAnnotation(); + sourceLine.className = "org.sonar.batch.Sensor"; + assertThat(sourceLine.getSonarJavaFileKey()).isEqualTo("org.sonar.batch.Sensor"); + sourceLine.className = "Sensor"; + assertThat(sourceLine.getSonarJavaFileKey()).isEqualTo("Sensor"); + sourceLine.className = "org.sonar.batch.Sensor$1"; + assertThat(sourceLine.getSonarJavaFileKey()).isEqualTo("org.sonar.batch.Sensor"); } private final File getFile(String filename) { diff --git a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/SonarWayWithFindbugsProfileTest.java b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/SonarWayWithFindbugsProfileTest.java index 0186ca90b37..6b516576fbb 100644 --- a/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/SonarWayWithFindbugsProfileTest.java +++ b/plugins/sonar-findbugs-plugin/src/test/java/org/sonar/plugins/findbugs/SonarWayWithFindbugsProfileTest.java @@ -23,8 +23,7 @@ import org.junit.Test; import org.sonar.api.profiles.RulesProfile; import org.sonar.api.utils.ValidationMessages; -import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertThat; +import static org.fest.assertions.Assertions.assertThat; public class SonarWayWithFindbugsProfileTest { @@ -34,7 +33,8 @@ public class SonarWayWithFindbugsProfileTest { SonarWayWithFindbugsProfile sonarWayWithFindbugs = new SonarWayWithFindbugsProfile(importer); ValidationMessages validation = ValidationMessages.create(); RulesProfile profile = sonarWayWithFindbugs.createProfile(validation); - assertThat(profile.getActiveRulesByRepository(FindbugsConstants.REPOSITORY_KEY).size(), is(399)); - assertThat(validation.hasErrors(), is(false)); + assertThat(profile.getActiveRulesByRepository(FindbugsConstants.REPOSITORY_KEY)).hasSize(399); + assertThat(validation.hasErrors()).isFalse(); } + } -- 2.39.5