From 02cb30a7fd545e55f319c79cb70beb048dfa49ea Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Mon, 20 May 2013 20:53:29 +0200 Subject: [PATCH] SONAR-3755 exclude false-positives from issue counters --- .../org/sonar/plugins/core/CorePlugin.java | 3 +- .../issue/CountFalsePositivesDecorator.java | 81 ++++++++++++++++++ ...tor.java => CountOpenIssuesDecorator.java} | 24 ++---- .../CountFalsePositivesDecoratorTest.java | 79 +++++++++++++++++ ...java => CountOpenIssuesDecoratorTest.java} | 85 +++++++++---------- 5 files changed, 209 insertions(+), 63 deletions(-) create mode 100644 plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/CountFalsePositivesDecorator.java rename plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/{IssueCountersDecorator.java => CountOpenIssuesDecorator.java} (91%) create mode 100644 plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/CountFalsePositivesDecoratorTest.java rename plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/{IssueCountersDecoratorTest.java => CountOpenIssuesDecoratorTest.java} (87%) diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java index 2fa1725d52a..cc3b9e380d2 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java @@ -399,7 +399,8 @@ public final class CorePlugin extends SonarPlugin { // issues IssueHandlers.class, IssueFilters.class, - IssueCountersDecorator.class, + CountOpenIssuesDecorator.class, + CountFalsePositivesDecorator.class, WeightedIssuesDecorator.class, IssuesDensityDecorator.class, InitialOpenIssuesSensor.class, diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/CountFalsePositivesDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/CountFalsePositivesDecorator.java new file mode 100644 index 00000000000..6fab0f847de --- /dev/null +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/CountFalsePositivesDecorator.java @@ -0,0 +1,81 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube 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. + * + * SonarQube 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 02110-1301, USA. + */ +package org.sonar.plugins.core.issue; + +import org.sonar.api.batch.*; +import org.sonar.api.component.ResourcePerspectives; +import org.sonar.api.issue.Issuable; +import org.sonar.api.issue.Issue; +import org.sonar.api.measures.CoreMetrics; +import org.sonar.api.measures.MeasureUtils; +import org.sonar.api.measures.Metric; +import org.sonar.api.resources.Project; +import org.sonar.api.resources.Resource; +import org.sonar.api.resources.ResourceUtils; + +/** + * Computes the number of false-positives + * + * @since 3.6 + */ +@DependsUpon(DecoratorBarriers.END_OF_ISSUES_UPDATES) +public class CountFalsePositivesDecorator implements Decorator { + + private final ResourcePerspectives perspectives; + + public CountFalsePositivesDecorator(ResourcePerspectives perspectives) { + this.perspectives = perspectives; + } + + public boolean shouldExecuteOnProject(Project project) { + return true; + } + + @DependedUpon + public Metric generatesFalsePositiveMeasure() { + return CoreMetrics.FALSE_POSITIVE_ISSUES; + } + + public void decorate(Resource resource, DecoratorContext context) { + Issuable issuable = perspectives.as(Issuable.class, resource); + if (issuable != null) { + int falsePositives = 0; + for (Issue issue : issuable.issues()) { + if (Issue.RESOLUTION_FALSE_POSITIVE.equals(issue.resolution())) { + falsePositives++; + } + } + saveMeasure(context, CoreMetrics.FALSE_POSITIVE_ISSUES, falsePositives); + } + } + + private void saveMeasure(DecoratorContext context, Metric metric, int value) { + context.saveMeasure(metric, (double) (value + sumChildren(context, metric))); + } + + private int sumChildren(DecoratorContext context, Metric metric) { + return MeasureUtils.sum(true, context.getChildrenMeasures(metric)).intValue(); + } + + @Override + public String toString() { + return getClass().getSimpleName(); + } +} diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueCountersDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/CountOpenIssuesDecorator.java similarity index 91% rename from plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueCountersDecorator.java rename to plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/CountOpenIssuesDecorator.java index 3f82358a1db..4a4a51fa766 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueCountersDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/CountOpenIssuesDecorator.java @@ -48,13 +48,13 @@ import static com.google.common.collect.Lists.newArrayList; * @since 3.6 */ @DependsUpon(DecoratorBarriers.END_OF_ISSUES_UPDATES) -public class IssueCountersDecorator implements Decorator { +public class CountOpenIssuesDecorator implements Decorator { private final ResourcePerspectives perspectives; private final RuleFinder rulefinder; private final TimeMachineConfiguration timeMachineConfiguration; - public IssueCountersDecorator(ResourcePerspectives perspectives, RuleFinder rulefinder, TimeMachineConfiguration timeMachineConfiguration) { + public CountOpenIssuesDecorator(ResourcePerspectives perspectives, RuleFinder rulefinder, TimeMachineConfiguration timeMachineConfiguration) { this.perspectives = perspectives; this.rulefinder = rulefinder; this.timeMachineConfiguration = timeMachineConfiguration; @@ -79,7 +79,6 @@ public class IssueCountersDecorator implements Decorator { CoreMetrics.NEW_MAJOR_ISSUES, CoreMetrics.NEW_MINOR_ISSUES, CoreMetrics.NEW_INFO_ISSUES, - CoreMetrics.FALSE_POSITIVE_ISSUES, CoreMetrics.UNASSIGNED_ISSUES ); } @@ -90,30 +89,26 @@ public class IssueCountersDecorator implements Decorator { Collection issues = getOpenIssues(issuable.issues()); boolean shouldSaveNewMetrics = shouldSaveNewMetrics(context); - Multiset severitiesBag = HashMultiset.create(); + Multiset severityBag = HashMultiset.create(); Map> rulesPerSeverity = Maps.newHashMap(); - ListMultimap issuesPerSeverities = ArrayListMultimap.create(); + ListMultimap issuesPerSeverity = ArrayListMultimap.create(); int countUnassigned = 0; - int falsePositives = 0; for (Issue issue : issues) { - severitiesBag.add(RulePriority.valueOf(issue.severity())); + severityBag.add(RulePriority.valueOf(issue.severity())); Multiset rulesBag = initRules(rulesPerSeverity, RulePriority.valueOf(issue.severity())); rulesBag.add(rulefinder.findByKey(issue.ruleKey().repository(), issue.ruleKey().rule())); - issuesPerSeverities.put(RulePriority.valueOf(issue.severity()), issue); + issuesPerSeverity.put(RulePriority.valueOf(issue.severity()), issue); if (issue.assignee() == null) { countUnassigned++; } - if (Issue.RESOLUTION_FALSE_POSITIVE.equals(issue.resolution())) { - falsePositives++; - } } for (RulePriority ruleSeverity : RulePriority.values()) { - saveIssuesForSeverity(context, ruleSeverity, severitiesBag); + saveIssuesForSeverity(context, ruleSeverity, severityBag); saveIssuesPerRules(context, ruleSeverity, rulesPerSeverity); - saveNewIssuesForSeverity(context, ruleSeverity, issuesPerSeverities, shouldSaveNewMetrics); + saveNewIssuesForSeverity(context, ruleSeverity, issuesPerSeverity, shouldSaveNewMetrics); saveNewIssuesPerRule(context, ruleSeverity, issues, shouldSaveNewMetrics); } @@ -121,7 +116,6 @@ public class IssueCountersDecorator implements Decorator { saveNewIssues(context, issues, shouldSaveNewMetrics); saveMeasure(context, CoreMetrics.UNASSIGNED_ISSUES, countUnassigned); - saveMeasure(context, CoreMetrics.FALSE_POSITIVE_ISSUES, falsePositives); } } @@ -277,7 +271,7 @@ public class IssueCountersDecorator implements Decorator { return newArrayList(Iterables.filter(issues, new Predicate() { @Override public boolean apply(final Issue issue) { - return !Issue.STATUS_CLOSED.equals(issue.status()); + return issue.resolution()==null; } })); } diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/CountFalsePositivesDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/CountFalsePositivesDecoratorTest.java new file mode 100644 index 00000000000..714c8aced26 --- /dev/null +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/CountFalsePositivesDecoratorTest.java @@ -0,0 +1,79 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube 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. + * + * SonarQube 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 02110-1301, USA. + */ +package org.sonar.plugins.core.issue; + +import org.junit.Test; +import org.sonar.api.batch.DecoratorContext; +import org.sonar.api.component.ResourcePerspectives; +import org.sonar.api.issue.Issuable; +import org.sonar.api.issue.Issue; +import org.sonar.api.measures.CoreMetrics; +import org.sonar.api.resources.File; +import org.sonar.api.resources.Project; +import org.sonar.api.rule.RuleKey; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.java.api.JavaClass; + +import java.util.Arrays; + +import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Mockito.*; + +public class CountFalsePositivesDecoratorTest { + + ResourcePerspectives perspectives = mock(ResourcePerspectives.class); + CountFalsePositivesDecorator decorator = new CountFalsePositivesDecorator(perspectives); + + @Test + public void should_count_false_positives() { + DefaultIssue falsePositive = new DefaultIssue().setRuleKey(RuleKey.parse("squid:AvoidCycles")) + .setResolution(Issue.RESOLUTION_FALSE_POSITIVE).setStatus(Issue.STATUS_OPEN); + DefaultIssue open = new DefaultIssue().setRuleKey(RuleKey.parse("squid:AvoidCycles")) + .setResolution(null).setStatus(Issue.STATUS_OPEN); + + File file = new File("foo.c"); + Issuable issuable = mock(Issuable.class); + when(perspectives.as(Issuable.class, file)).thenReturn(issuable); + when(issuable.issues()).thenReturn(Arrays.asList(falsePositive, open)); + + DecoratorContext context = mock(DecoratorContext.class); + decorator.decorate(file, context); + + verify(context).saveMeasure(CoreMetrics.FALSE_POSITIVE_ISSUES, 1.0); + } + + @Test + public void should_declare_metadata() { + assertThat(decorator.shouldExecuteOnProject(new Project("foo"))).isTrue(); + assertThat(decorator.generatesFalsePositiveMeasure()).isEqualTo(CoreMetrics.FALSE_POSITIVE_ISSUES); + assertThat(decorator.toString()).isEqualTo("CountFalsePositivesDecorator"); + } + + @Test + public void should_ignore_classes_and_methods() { + JavaClass javaClass = JavaClass.create("Foo.java"); + when(perspectives.as(Issuable.class, javaClass)).thenReturn(null); + + DecoratorContext context = mock(DecoratorContext.class); + decorator.decorate(javaClass, context); + + verifyZeroInteractions(context); + } +} diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueCountersDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/CountOpenIssuesDecoratorTest.java similarity index 87% rename from plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueCountersDecoratorTest.java rename to plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/CountOpenIssuesDecoratorTest.java index 572f983298d..109aa467203 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueCountersDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/CountOpenIssuesDecoratorTest.java @@ -52,21 +52,21 @@ import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Mockito.*; -public class IssueCountersDecoratorTest { - - private IssueCountersDecorator decorator; - private TimeMachineConfiguration timeMachineConfiguration; - private RuleFinder rulefinder; - private Issuable issuable; - private DecoratorContext context; - private Resource resource; - private Project project; - private Rule ruleA1; - private Rule ruleA2; - private Rule ruleB1; - private Date rightNow; - private Date tenDaysAgo; - private Date fiveDaysAgo; +public class CountOpenIssuesDecoratorTest { + + CountOpenIssuesDecorator decorator; + TimeMachineConfiguration timeMachineConfiguration; + RuleFinder ruleFinder; + Issuable issuable; + DecoratorContext context; + Resource resource; + Project project; + Rule ruleA1; + Rule ruleA2; + Rule ruleB1; + Date rightNow; + Date tenDaysAgo; + Date fiveDaysAgo; @Before public void before() { @@ -74,10 +74,10 @@ public class IssueCountersDecoratorTest { ruleA2 = Rule.create().setRepositoryKey("ruleA2").setKey("ruleA2").setName("nameA2"); ruleB1 = Rule.create().setRepositoryKey("ruleB1").setKey("ruleB1").setName("nameB1"); - rulefinder = mock(RuleFinder.class); - when(rulefinder.findByKey(ruleA1.getRepositoryKey(), ruleA1.getKey())).thenReturn(ruleA1); - when(rulefinder.findByKey(ruleA2.getRepositoryKey(), ruleA2.getKey())).thenReturn(ruleA2); - when(rulefinder.findByKey(ruleB1.getRepositoryKey(), ruleB1.getKey())).thenReturn(ruleB1); + ruleFinder = mock(RuleFinder.class); + when(ruleFinder.findByKey(ruleA1.getRepositoryKey(), ruleA1.getKey())).thenReturn(ruleA1); + when(ruleFinder.findByKey(ruleA2.getRepositoryKey(), ruleA2.getKey())).thenReturn(ruleA2); + when(ruleFinder.findByKey(ruleB1.getRepositoryKey(), ruleB1.getKey())).thenReturn(ruleB1); rightNow = new Date(); tenDaysAgo = DateUtils.addDays(rightNow, -10); @@ -106,12 +106,12 @@ public class IssueCountersDecoratorTest { issuable = mock(Issuable.class); ResourcePerspectives perspectives = mock(ResourcePerspectives.class); when(perspectives.as(Issuable.class, resource)).thenReturn(issuable); - decorator = new IssueCountersDecorator(perspectives, rulefinder, timeMachineConfiguration); + decorator = new CountOpenIssuesDecorator(perspectives, ruleFinder, timeMachineConfiguration); } @Test public void should_be_depended_upon_metric() { - assertThat(decorator.generatesIssuesMetrics()).hasSize(14); + assertThat(decorator.generatesIssuesMetrics()).hasSize(13); } @Test @@ -129,7 +129,7 @@ public class IssueCountersDecoratorTest { public void should_do_nothing_when_issuable_is_null() { ResourcePerspectives perspectives = mock(ResourcePerspectives.class); when(perspectives.as(Issuable.class, resource)).thenReturn(null); - IssueCountersDecorator decorator = new IssueCountersDecorator(perspectives, rulefinder, timeMachineConfiguration); + CountOpenIssuesDecorator decorator = new CountOpenIssuesDecorator(perspectives, ruleFinder, timeMachineConfiguration); decorator.decorate(resource, context); @@ -219,18 +219,6 @@ public class IssueCountersDecoratorTest { verify(context).saveMeasure(CoreMetrics.UNASSIGNED_ISSUES, 2.0); } - @Test - public void should_save_false_positive_issues() { - List issues = newArrayList(); - issues.add(new DefaultIssue().setRuleKey(ruleA1.ruleKey()).setResolution(Issue.RESOLUTION_FALSE_POSITIVE).setStatus(Issue.STATUS_OPEN).setSeverity(RulePriority.CRITICAL.name())); - issues.add(new DefaultIssue().setRuleKey(ruleA1.ruleKey()).setResolution(Issue.RESOLUTION_FIXED).setStatus(Issue.STATUS_OPEN).setSeverity(RulePriority.CRITICAL.name())); - when(issuable.issues()).thenReturn(issues); - - decorator.decorate(resource, context); - - verify(context).saveMeasure(CoreMetrics.FALSE_POSITIVE_ISSUES, 1.0); - } - @Test public void same_rule_should_have_different_severities() { List issues = newArrayList(); @@ -308,16 +296,19 @@ public class IssueCountersDecoratorTest { verify(context, never()).saveMeasure(argThat(new IsMetricMeasure(CoreMetrics.NEW_CRITICAL_ISSUES))); } - private List createIssues() { + List createIssues() { List issues = newArrayList(); issues.add(new DefaultIssue().setRuleKey(ruleA1.ruleKey()).setSeverity(RulePriority.CRITICAL.name()).setStatus(Issue.STATUS_OPEN)); issues.add(new DefaultIssue().setRuleKey(ruleA1.ruleKey()).setSeverity(RulePriority.CRITICAL.name()).setStatus(Issue.STATUS_REOPENED)); issues.add(new DefaultIssue().setRuleKey(ruleA2.ruleKey()).setSeverity(RulePriority.MAJOR.name()).setStatus(Issue.STATUS_REOPENED)); issues.add(new DefaultIssue().setRuleKey(ruleB1.ruleKey()).setSeverity(RulePriority.MINOR.name()).setStatus(Issue.STATUS_OPEN)); + + // resolved + issues.add(new DefaultIssue().setRuleKey(ruleB1.ruleKey()).setResolution(Issue.RESOLUTION_FIXED).setStatus(Issue.STATUS_RESOLVED)); return issues; } - private List createIssuesForNewMetrics() { + List createIssuesForNewMetrics() { List issues = newArrayList(); issues.add(new DefaultIssue().setRuleKey(ruleA1.ruleKey()).setSeverity(RulePriority.CRITICAL.name()).setCreationDate(rightNow).setStatus(Issue.STATUS_OPEN)); issues.add(new DefaultIssue().setRuleKey(ruleA1.ruleKey()).setSeverity(RulePriority.CRITICAL.name()).setCreationDate(tenDaysAgo).setStatus(Issue.STATUS_OPEN)); @@ -328,11 +319,11 @@ public class IssueCountersDecoratorTest { return issues; } - private class IsVariationRuleMeasure extends ArgumentMatcher { - private Metric metric = null; - private Rule rule = null; - private Double var1 = null; - private Double var2 = null; + class IsVariationRuleMeasure extends ArgumentMatcher { + Metric metric = null; + Rule rule = null; + Double var1 = null; + Double var2 = null; public IsVariationRuleMeasure(Metric metric, Rule rule, Double var1, Double var2) { this.metric = metric; @@ -353,10 +344,10 @@ public class IssueCountersDecoratorTest { } } - private class IsVariationMeasure extends ArgumentMatcher { - private Metric metric = null; - private Double var1 = null; - private Double var2 = null; + class IsVariationMeasure extends ArgumentMatcher { + Metric metric = null; + Double var1 = null; + Double var2 = null; public IsVariationMeasure(Metric metric, Double var1, Double var2) { this.metric = metric; @@ -376,8 +367,8 @@ public class IssueCountersDecoratorTest { } } - private class IsMetricMeasure extends ArgumentMatcher { - private Metric metric = null; + class IsMetricMeasure extends ArgumentMatcher { + Metric metric = null; public IsMetricMeasure(Metric metric) { this.metric = metric; -- 2.39.5