SONAR-3755 fix NoSonarFilter to accept issues on rules "NoSonar" !

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 8421f36b4b8ceeebfb64ce77f8d1e89f71468abc..e6b107c67a0c00795b1e982c30e1b6dd2a5a01d6 100644 (file)
--- 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
@@ -402,6 +402,7 @@ public final class CorePlugin extends SonarPlugin {
       ActionPlansWidget.class,
       UnresolvedIssuesPerAssigneeWidget.class,
       UnresolvedIssuesStatusesWidget.class,
+      org.sonar.api.issue.NoSonarFilter.class,
 
       // issue notifications
       SendIssueNotificationsPostJob.class,

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/checks/NoSonarFilter.java b/sonar-plugin-api/src/main/java/org/sonar/api/checks/NoSonarFilter.java
index 66c201acae7bfeabb6caa8ed46b2ec00d3eb134b..eb014513054911c2508fd576288dfd68256223a6 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/checks/NoSonarFilter.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/checks/NoSonarFilter.java
@@ -20,6 +20,7 @@
 package org.sonar.api.checks;
 
 import com.google.common.collect.Maps;
+import org.apache.commons.lang.StringUtils;
 import org.sonar.api.resources.Resource;
 import org.sonar.api.rules.Violation;
 import org.sonar.api.rules.ViolationFilter;
@@ -29,7 +30,9 @@ import java.util.Set;
 
 /**
  * @since 2.1
+ * @deprecated in 3.6. Replaced by {@link org.sonar.api.issue.NoSonarFilter}
  */
+@Deprecated
 public class NoSonarFilter implements ViolationFilter {
 
   private final Map<Resource, Set<Integer>> noSonarLinesByResource = Maps.newHashMap();
@@ -41,10 +44,14 @@ public class NoSonarFilter implements ViolationFilter {
   }
 
   public boolean isIgnored(Violation violation) {
+    boolean ignored = false;
     if (violation.getResource() != null && violation.getLineId() != null) {
       Set<Integer> noSonarLines = noSonarLinesByResource.get(violation.getResource());
-      return (noSonarLines != null && noSonarLines.contains(violation.getLineId()));
+      ignored = noSonarLines != null && noSonarLines.contains(violation.getLineId());
+      if (ignored && violation.getRule() != null && StringUtils.containsIgnoreCase(violation.getRule().getKey(), "nosonar")) {
+        ignored = false;
+      }
     }
-    return false;
+    return ignored;
   }
 }

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/NoSonarFilter.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/NoSonarFilter.java
new file mode 100644 (file)
index 0000000..939ef86
--- /dev/null
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/NoSonarFilter.java
@@ -0,0 +1,52 @@
+/*
+ * 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.api.issue;
+
+import com.google.common.collect.Maps;
+import org.apache.commons.lang.StringUtils;
+
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * @since 3.6
+ */
+public class NoSonarFilter implements IssueFilter {
+
+  private final Map<String, Set<Integer>> noSonarLinesByResource = Maps.newHashMap();
+
+  public NoSonarFilter addComponent(String componentKey, Set<Integer> noSonarLines) {
+    noSonarLinesByResource.put(componentKey, noSonarLines);
+    return this;
+  }
+
+  @Override
+  public boolean accept(Issue issue) {
+    boolean accepted = true;
+    if (issue.line() != null) {
+      Set<Integer> noSonarLines = noSonarLinesByResource.get(issue.componentKey());
+      accepted = noSonarLines == null || !noSonarLines.contains(issue.line());
+      if (!accepted && StringUtils.containsIgnoreCase(issue.ruleKey().rule(), "nosonar")) {
+        accepted = true;
+      }
+    }
+    return accepted;
+  }
+}

diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/checks/NoSonarFilterTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/checks/NoSonarFilterTest.java
index 0370dacc0ce02e748831ce622a5e49c0b89e746d..a8940dcd39b6ada36beb443a303255a3424df3ea 100644 (file)
--- a/sonar-plugin-api/src/test/java/org/sonar/api/checks/NoSonarFilterTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/checks/NoSonarFilterTest.java
@@ -21,18 +21,20 @@ package org.sonar.api.checks;
 
 import org.junit.Test;
 import org.sonar.api.resources.JavaFile;
+import org.sonar.api.rules.Rule;
 import org.sonar.api.rules.Violation;
 
 import java.util.HashSet;
 import java.util.Set;
 
-import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertThat;
+import static org.fest.assertions.Assertions.assertThat;
 
 public class NoSonarFilterTest {
+
+  NoSonarFilter filter = new NoSonarFilter();
+
   @Test
   public void ignoreLinesCommentedWithNoSonar() {
-    NoSonarFilter filter = new NoSonarFilter();
     JavaFile javaFile = new JavaFile("org.foo.Bar");
 
     Set<Integer> noSonarLines = new HashSet<Integer>();
@@ -41,19 +43,31 @@ public class NoSonarFilterTest {
     filter.addResource(javaFile, noSonarLines);
 
     // violation on class
-    assertThat(filter.isIgnored(new Violation(null, javaFile)), is(false));
+    assertThat(filter.isIgnored(new Violation(null, javaFile))).isFalse();
 
     // violation on lines
-    assertThat(filter.isIgnored(new Violation(null, javaFile).setLineId(30)), is(false));
-    assertThat(filter.isIgnored(new Violation(null, javaFile).setLineId(31)), is(true));
+    assertThat(filter.isIgnored(new Violation(null, javaFile).setLineId(30))).isFalse();
+    assertThat(filter.isIgnored(new Violation(null, javaFile).setLineId(31))).isTrue();
   }
 
 
   @Test
   public void doNotIgnoreWhenNotFoundInSquid() {
-    NoSonarFilter filter = new NoSonarFilter();
     JavaFile javaFile = new JavaFile("org.foo.Bar");
+    assertThat(filter.isIgnored(new Violation(null, javaFile).setLineId(30))).isFalse();
+  }
+
+  @Test
+  public void should_accept_violations_from_no_sonar_rules() throws Exception {
+    // The "No Sonar" rule logs violations on the lines that are flagged with "NOSONAR" !!
+    JavaFile javaFile = new JavaFile("org.foo.Bar");
+
+    Set<Integer> noSonarLines = new HashSet<Integer>();
+    noSonarLines.add(31);
+    filter.addResource(javaFile, noSonarLines);
+
+    Rule noSonarRule = new Rule("squid", "NoSonarCheck");
+    assertThat(filter.isIgnored(new Violation(noSonarRule, javaFile).setLineId(31))).isFalse();
 
-    assertThat(filter.isIgnored(new Violation(null, javaFile).setLineId(30)), is(false));
   }
 }

diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/issue/NoSonarFilterTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/issue/NoSonarFilterTest.java
new file mode 100644 (file)
index 0000000..63c506f
--- /dev/null
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/issue/NoSonarFilterTest.java
@@ -0,0 +1,72 @@
+/*
+ * 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.api.issue;
+
+import com.google.common.collect.ImmutableSet;
+import org.junit.Test;
+import org.sonar.api.rule.RuleKey;
+
+import java.util.Set;
+
+import static org.fest.assertions.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class NoSonarFilterTest {
+
+  NoSonarFilter filter = new NoSonarFilter();
+
+  @Test
+  public void should_ignore_lines_commented_with_nosonar() {
+    Issue issue = mock(Issue.class);
+    when(issue.componentKey()).thenReturn("struts:org.apache.Action");
+    when(issue.ruleKey()).thenReturn(RuleKey.of("squid", "AvoidCycles"));
+
+    Set<Integer> noSonarLines = ImmutableSet.of(31, 55);
+    filter.addComponent("struts:org.apache.Action", noSonarLines);
+
+    // issue on file
+    assertThat(filter.accept(issue)).isTrue();
+
+    // issue on lines
+    when(issue.line()).thenReturn(31);
+    assertThat(filter.accept(issue)).isFalse();
+
+    when(issue.line()).thenReturn(222);
+    assertThat(filter.accept(issue)).isTrue();
+  }
+
+  @Test
+  public void should_accept_issues_on_no_sonar_rules() throws Exception {
+    // The "No Sonar" rule logs violations on the lines that are flagged with "NOSONAR" !!
+    Issue issue = mock(Issue.class);
+    when(issue.componentKey()).thenReturn("struts:org.apache.Action");
+    when(issue.ruleKey()).thenReturn(RuleKey.of("squid", "NoSonarCheck"));
+
+    Set<Integer> noSonarLines = ImmutableSet.of(31, 55);
+    filter.addComponent("struts:org.apache.Action", noSonarLines);
+
+    when(issue.line()).thenReturn(31);
+    assertThat(filter.accept(issue)).isTrue();
+
+    when(issue.line()).thenReturn(222);
+    assertThat(filter.accept(issue)).isTrue();
+  }
+}