Issue #1008: Add possibility to suppress checks by id

This commit is contained in:
Andrei Selkin 2015-12-31 16:11:19 +03:00 committed by Roman Ivanov
parent 425fd5a27c
commit 6438bb246b
12 changed files with 311 additions and 22 deletions

View File

@ -29,6 +29,7 @@ import org.apache.commons.beanutils.ConversionException;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.puppycrawl.tools.checkstyle.api.AuditEvent;
import com.puppycrawl.tools.checkstyle.api.Check;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
@ -142,16 +143,17 @@ public class SuppressWarningsHolder
/**
* Checks for a suppression of a check with the given source name and
* location in the last file processed.
* @param sourceName the source name of the check
* @param line the line number of the check
* @param column the column number of the check
* @param event audit event.
* @return whether the check with the given name is suppressed at the given
* source location
*/
public static boolean isSuppressed(String sourceName, int line,
int column) {
public static boolean isSuppressed(AuditEvent event) {
final List<Entry> entries = ENTRIES.get();
final String sourceName = event.getSourceName();
final String checkAlias = getAlias(sourceName);
final int line = event.getLine();
final int column = event.getColumn();
boolean suppressed = false;
for (Entry entry : entries) {
final boolean afterStart =
entry.getFirstLine() < line
@ -164,11 +166,13 @@ public class SuppressWarningsHolder
final boolean nameMatches =
ALL_WARNING_MATCHING_ID.equals(entry.getCheckName())
|| entry.getCheckName().equalsIgnoreCase(checkAlias);
if (afterStart && beforeEnd && nameMatches) {
return true;
final boolean idMatches = event.getModuleId() != null
&& event.getModuleId().equals(entry.getCheckName());
if (afterStart && beforeEnd && (nameMatches || idMatches)) {
suppressed = true;
}
}
return false;
return suppressed;
}
@Override

View File

@ -34,7 +34,6 @@ public class SuppressWarningsFilter
implements Filter {
@Override
public boolean accept(AuditEvent event) {
return !SuppressWarningsHolder.isSuppressed(event.getSourceName(),
event.getLine(), event.getColumn());
return !SuppressWarningsHolder.isSuppressed(event);
}
}

View File

@ -415,7 +415,13 @@ public class SuppressWithNearbyCommentFilter
if (tagMatcher.find()) {
match = true;
}
else if (tagMessageRegexp != null) {
else if (tagMessageRegexp == null) {
if (event.getModuleId() != null) {
final Matcher idMatcher = tagCheckRegexp.matcher(event.getModuleId());
match = idMatcher.find();
}
}
else {
final Matcher messageMatcher = tagMessageRegexp.matcher(event.getMessage());
match = messageMatcher.find();
}

View File

@ -451,6 +451,10 @@ public class SuppressionCommentFilter
match = messageMatcher.find();
}
}
else if (event.getModuleId() != null) {
final Matcher idMatcher = tagCheckRegexp.matcher(event.getModuleId());
match = idMatcher.find();
}
return match;
}

View File

@ -44,9 +44,13 @@ import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
import com.puppycrawl.tools.checkstyle.BaseCheckTestSupport;
import com.puppycrawl.tools.checkstyle.Checker;
import com.puppycrawl.tools.checkstyle.api.AuditEvent;
import com.puppycrawl.tools.checkstyle.api.Configuration;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.LocalizedMessage;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.checks.naming.MemberNameCheck;
@RunWith(PowerMockRunner.class)
@PrepareForTest({ SuppressWarningsHolder.class, SuppressWarningsHolderTest.class })
@ -140,7 +144,12 @@ public class SuppressWarningsHolderTest extends BaseCheckTestSupport {
entries.setAccessible(true);
entries.set(holder, threadLocal);
assertFalse(SuppressWarningsHolder.isSuppressed("SourceName", 100, 10));
final Checker source = new Checker();
final LocalizedMessage message =
new LocalizedMessage(100, 10, null, null, null, "id", MemberNameCheck.class, "message");
final AuditEvent event = new AuditEvent(source, "fileName", message);
assertFalse(SuppressWarningsHolder.isSuppressed(event));
}
@Test
@ -164,11 +173,24 @@ public class SuppressWarningsHolderTest extends BaseCheckTestSupport {
entries.setAccessible(true);
entries.set(holder, threadLocal);
assertFalse(SuppressWarningsHolder.isSuppressed("SourceName", 100, 10));
final Checker source = new Checker();
final LocalizedMessage firstMessageForTest =
new LocalizedMessage(100, 10, null, null, null, "id", MemberNameCheck.class, "msg");
final AuditEvent firstEventForTest =
new AuditEvent(source, "fileName", firstMessageForTest);
assertFalse(SuppressWarningsHolder.isSuppressed(firstEventForTest));
assertTrue(SuppressWarningsHolder.isSuppressed("SourceName", 100, 150));
final LocalizedMessage secondMessageForTest =
new LocalizedMessage(100, 150, null, null, null, "id", MemberNameCheck.class, "msg");
final AuditEvent secondEventForTest =
new AuditEvent(source, "fileName", secondMessageForTest);
assertTrue(SuppressWarningsHolder.isSuppressed(secondEventForTest));
assertTrue(SuppressWarningsHolder.isSuppressed("SourceName", 200, 1));
final LocalizedMessage thirdMessageForTest =
new LocalizedMessage(200, 1, null, null, null, "id", MemberNameCheck.class, "msg");
final AuditEvent thirdEventForTest =
new AuditEvent(source, "fileName", thirdMessageForTest);
assertTrue(SuppressWarningsHolder.isSuppressed(thirdEventForTest));
}
@Test

View File

@ -125,11 +125,19 @@ public class SuppressWarningsFilterTest
"com.puppycrawl.tools.checkstyle.checks.sizes."
+ "ParameterNumberCheck=paramnum");
checksConfig.addChild(holderConfig);
checksConfig.addChild(createCheckConfig(MemberNameCheck.class));
checksConfig.addChild(createCheckConfig(ConstantNameCheck.class));
final DefaultConfiguration memberNameCheckConfig = createCheckConfig(MemberNameCheck.class);
memberNameCheckConfig.addAttribute("id", "ignore");
checksConfig.addChild(memberNameCheckConfig);
final DefaultConfiguration constantNameCheckConfig =
createCheckConfig(ConstantNameCheck.class);
constantNameCheckConfig.addAttribute("id", "");
checksConfig.addChild(constantNameCheckConfig);
checksConfig.addChild(createCheckConfig(ParameterNumberCheck.class));
checksConfig.addChild(createCheckConfig(IllegalCatchCheck.class));
checksConfig.addChild(createCheckConfig(UncommentedMainCheck.class));
final DefaultConfiguration uncommentedMainCheckConfig =
createCheckConfig(UncommentedMainCheck.class);
uncommentedMainCheckConfig.addAttribute("id", "ignore");
checksConfig.addChild(uncommentedMainCheckConfig);
checksConfig.addChild(createCheckConfig(JavadocTypeCheck.class));
checkerConfig.addChild(checksConfig);
if (checkConfig != null) {
@ -152,4 +160,23 @@ public class SuppressWarningsFilterTest
coll.removeAll(Arrays.asList(remove));
return coll.toArray(new String[coll.size()]);
}
@Test
public void testSuppressById() throws Exception {
final DefaultConfiguration filterConfig =
createFilterConfig(SuppressWarningsFilter.class);
final String[] suppressedViolationMessages = {
"6:17: Name 'A1' must match pattern '^[a-z][a-zA-Z0-9]*$'.",
"8: Uncommented main method found.",
};
final String[] expectedViolationMessages = {
"3: Missing a Javadoc comment.",
"6:17: Name 'A1' must match pattern '^[a-z][a-zA-Z0-9]*$'.",
"8: Uncommented main method found.",
};
verify(createChecker(filterConfig),
getPath("InputSuppressByIdWithWarningsFilter.java"),
removeSuppressed(expectedViolationMessages, suppressedViolationMessages));
}
}

View File

@ -217,8 +217,13 @@ public class SuppressWithNearbyCommentFilterTest
new DefaultConfiguration("configuration");
final DefaultConfiguration checksConfig = createCheckConfig(TreeWalker.class);
checksConfig.addChild(createCheckConfig(FileContentsHolder.class));
checksConfig.addChild(createCheckConfig(MemberNameCheck.class));
checksConfig.addChild(createCheckConfig(ConstantNameCheck.class));
final DefaultConfiguration memberNameCheckConfig = createCheckConfig(MemberNameCheck.class);
memberNameCheckConfig.addAttribute("id", "ignore");
checksConfig.addChild(memberNameCheckConfig);
final DefaultConfiguration constantNameCheckConfig =
createCheckConfig(ConstantNameCheck.class);
constantNameCheckConfig.addAttribute("id", null);
checksConfig.addChild(constantNameCheckConfig);
checksConfig.addChild(createCheckConfig(IllegalCatchCheck.class));
checkerConfig.addChild(checksConfig);
if (checkConfig != null) {
@ -311,4 +316,27 @@ public class SuppressWithNearbyCommentFilterTest
final String[] suppressed = ArrayUtils.EMPTY_STRING_ARRAY;
verifySuppressed(filterConfig, suppressed);
}
@Test
public void testSuppressById() throws Exception {
final DefaultConfiguration filterConfig =
createFilterConfig(SuppressWithNearbyCommentFilter.class);
filterConfig.addAttribute("commentFormat", "@cs-: (\\w+) \\(\\w+\\)");
filterConfig.addAttribute("checkFormat", "$1");
filterConfig.addAttribute("influenceFormat", "0");
final String[] suppressedViolationMessages = {
"5:17: Name 'A1' must match pattern '^[a-z][a-zA-Z0-9]*$'.",
"9:9: Name 'line_length' must match pattern '^[a-z][a-zA-Z0-9]*$'.",
};
final String[] expectedViolationMessages = {
"5:17: Name 'A1' must match pattern '^[a-z][a-zA-Z0-9]*$'.",
"7:30: Name 'abc' must match pattern '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'.",
"9:9: Name 'line_length' must match pattern '^[a-z][a-zA-Z0-9]*$'.",
"11:18: Name 'ID' must match pattern '^[a-z][a-zA-Z0-9]*$'.",
};
verify(createChecker(filterConfig),
getPath("InputSuppressByIdWithNearbyCommentFilter.java"),
removeSuppressed(expectedViolationMessages, suppressedViolationMessages));
}
}

View File

@ -217,8 +217,13 @@ public class SuppressionCommentFilterTest
new DefaultConfiguration("configuration");
final DefaultConfiguration checksConfig = createCheckConfig(TreeWalker.class);
checksConfig.addChild(createCheckConfig(FileContentsHolder.class));
checksConfig.addChild(createCheckConfig(MemberNameCheck.class));
checksConfig.addChild(createCheckConfig(ConstantNameCheck.class));
final DefaultConfiguration memberNameCheckConfig = createCheckConfig(MemberNameCheck.class);
memberNameCheckConfig.addAttribute("id", "ignore");
checksConfig.addChild(memberNameCheckConfig);
final DefaultConfiguration constantNameCheckConfig =
createCheckConfig(ConstantNameCheck.class);
constantNameCheckConfig.addAttribute("id", null);
checksConfig.addChild(constantNameCheckConfig);
checksConfig.addChild(createCheckConfig(IllegalCatchCheck.class));
checkerConfig.addChild(checksConfig);
if (checkConfig != null) {
@ -307,4 +312,27 @@ public class SuppressionCommentFilterTest
final SuppressionCommentFilter filter = new SuppressionCommentFilter();
Assert.assertTrue(filter.accept(auditEvent));
}
@Test
public void testSuppressById() throws Exception {
final DefaultConfiguration filterConfig =
createFilterConfig(SuppressionCommentFilter.class);
filterConfig.addAttribute("offCommentFormat", "CSOFF (\\w+) \\(\\w+\\)");
filterConfig.addAttribute("onCommentFormat", "CSON (\\w+)");
filterConfig.addAttribute("checkFormat", "$1");
final String[] suppressedViolationMessages = {
"6:17: Name 'A1' must match pattern '^[a-z][a-zA-Z0-9]*$'.",
"12:9: Name 'line_length' must match pattern '^[a-z][a-zA-Z0-9]*$'.",
};
final String[] expectedViolationMessages = {
"6:17: Name 'A1' must match pattern '^[a-z][a-zA-Z0-9]*$'.",
"9:30: Name 'abc' must match pattern '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'.",
"12:9: Name 'line_length' must match pattern '^[a-z][a-zA-Z0-9]*$'.",
"15:18: Name 'ID' must match pattern '^[a-z][a-zA-Z0-9]*$'.",
};
verify(createChecker(filterConfig),
getPath("InputSuppressByIdWithCommentFilter.java"),
removeSuppressed(expectedViolationMessages, suppressedViolationMessages));
}
}

View File

@ -0,0 +1,16 @@
package com.puppycrawl.tools.checkstyle.filters;
public class InputSuppressByIdWithCommentFilter {
//CSOFF ignore (reason)
private int A1;
// @cs-: ignore (No NPE here)
private static final int abc = 5;
int line_length = 100;
//CSON ignore
private long ID = 1;
}

View File

@ -0,0 +1,12 @@
package com.puppycrawl.tools.checkstyle.filters;
public class InputSuppressByIdWithNearbyCommentFilter {
private int A1; // @cs-: ignore (reason)
private static final int abc = 5; // @cs-: violation (No NPE here)
int line_length = 100; // Suppression @cs-: ignore (reason)
private long ID = 1; // Suppression @cs-:
}

View File

@ -0,0 +1,13 @@
package com.puppycrawl.tools.checkstyle.filters;
public class InputSuppressByIdWithWarningsFilter {
@SuppressWarnings("checkstyle:ignore")
private int A1 = 1;
@SuppressWarnings("checkstyle:ignore")
public static void main(String[] args) {
}
}

View File

@ -317,6 +317,53 @@ HashSet hashSet; // Warning here: Declaring variables, return values or paramete
//resume
...
</source>
<p>
It is possible to specify an ID of checks, so that it can be leveraged by the
SuppressionCommentFilter to skip validations. The following examples show how to skip
validations near code that is surrounded with <code>// CSOFF &lt;ID> (reason)</code>
and <code>// CSON &lt;ID></code>, where ID is the ID of checks you want to suppress.
</p>
<p>
Examples of Checkstyle checks configuration:
</p>
<source>
&lt;module name=&quot;RegexpSinglelineJava&quot;&gt;
&lt;property name=&quot;id&quot; value=&quot;ignore&quot;/&gt;
&lt;property name="format" value="^.*@Ignore\s*$"/&gt;
&lt;property name="message" value="@Ignore should have a reason."/&gt;
&lt;/module&gt;
&lt;module name=&quot;RegexpSinglelineJava&quot;&gt;
&lt;property name=&quot;id&quot; value=&quot;systemout&quot;/&gt;
&lt;property name="format" value="^.*System\.(out|err).*$"/&gt;
&lt;property name="message" value="Don't use System.out/err, use SLF4J instead."/&gt;
&lt;/module&gt;
</source>
<p>
Example of SuppressionCommentFilter configuration (checkFormat which is set to '$1'
points that ID of the checks is in the first group of offCommentFormat and
onCommentFormat regular expressions):
</p>
<source>
&lt;module name="SuppressionCommentFilter&quot;&gt;
&lt;property name="offCommentFormat" value="CSOFF (\w+) \(\w+\)"/&gt;
&lt;property name="onCommentFormat" value="CSON (\w+)"/&gt;
&lt;property name="checkFormat" value="$1"/&gt;
&lt;/module&gt;
</source>
<source>
// CSOFF ignore (test has not been emplemented yet)
@Ignore // should NOT fail RegexpSinglelineJava
@Test
public void testMethod() { }
// CSON ignore
// CSOFF systemout (debug)
public static void foo() {
System.out.println("Debug info."); // should NOT fail RegexpSinglelineJava
}
// CSON systemout
</source>
</subsection>
<subsection name="Example of Usage">
<ul>
@ -599,6 +646,45 @@ private int J; // should NOT fail MemberNameCheck
@SuppressWarnings({"NoWhitespaceAfter"})
private int [] ARRAY; // should NOT fail MemberNameCheck and NoWhitespaceAfterCheck
</source>
<p>
It is possible to specify an ID of checks, so that it can be leveraged by the
SuppressWarningsFilter to skip validations. The following examples show how to skip
validations near code that has <code>@SuppressWarnings("checkstyle:&lt;ID>")</code> or
just <code>@SuppressWarnings("&lt;ID>")</code> annotation, where ID is the ID of checks
you want to suppress.
</p>
<p>
Example of Checkstyle check configuration:
</p>
<source>
&lt;module name="RegexpSinglelineJava&quot;&gt;
&lt;property name="id" value="systemout"/&gt;
&lt;property name="format" value=&quot;^.*System\.(out|err).*$&quot;/&gt;
&lt;property name="message" value="Don't use System.out/err, use SLF4J instead."/&gt;
&lt;/module&gt;
</source>
<p>
To make the annotations available to the filter.
</p>
<source>
&lt;module name=&quot;TreeWalker&quot;&gt;
...
&lt;module name=&quot;SuppressWarningsHolder&quot; /&gt;
...
&lt;/module&gt;
</source>
<p>
To configure filter to suppress audit events for annotations add:
</p>
<source>
&lt;module name=&quot;SuppressWarningsFilter&quot; /&gt;
</source>
<source>
@SuppressWarnings("checkstyle:systemout")
public static void foo() {
System.out.println("Debug info."); // should NOT fail RegexpSinglelineJava
}
</source>
</subsection>
<subsection name="Example of Usage">
<ul>
@ -790,6 +876,50 @@ private int D2;
<source>
public static final int [] array; // @cs.suppress ConstantName | NoWhitespaceAfter
</source>
<p>
It is possible to specify an ID of checks, so that it can be leveraged by the
SuppressWithNearbyCommentFilter to skip validations. The following examples
show how to skip validations near code that has comment like
<code>// @cs-: &lt;ID/> (reason)</code>, where ID is the ID of checks you want to
suppress.
</p>
<p>
Examples of Checkstyle checks configuration:
</p>
<source>
&lt;module name=&quot;RegexpSinglelineJava&quot;&gt;
&lt;property name=&quot;id&quot; value=&quot;ignore&quot;/&gt;
&lt;property name="format" value="^.*@Ignore\s*$"/&gt;
&lt;property name="message" value="@Ignore should have a reason."/&gt;
&lt;/module&gt;
&lt;module name=&quot;RegexpSinglelineJava&quot;&gt;
&lt;property name=&quot;id&quot; value=&quot;systemout&quot;/&gt;
&lt;property name="format" value="^.*System\.(out|err).*$"/&gt;
&lt;property name="message" value="Don't use System.out/err, use SLF4J instead."/&gt;
&lt;/module&gt;
</source>
<p>
Example of SuppressWithNearbyCommentFilter configuration (checkFormat which
is set to '$1' points that ID of the checks is in the first group of
commentFormat regular expressions):
</p>
<source>
&lt;module name=&quot;SuppressWithNearbyCommentFilter&quot;&gt;
&lt;property name=&quot;commentFormat&quot; value=&quot;@cs-: (\w+) \(\w+\)"/&gt;
&lt;property name=&quot;checkFormat&quot; value=&quot;$1&quot;/&gt;
&lt;property name=&quot;influenceFormat&quot; value=&quot;0&quot;/&gt;
&lt;/module&gt;
</source>
<source>
@Ignore // @cs-: ignore (test has not been implemented yet)
@Test
public void testMethod() { }
public static void foo() {
System.out.println("Debug info."); // @cs-: systemout (should not fail RegexpSinglelineJava)
}
</source>
</subsection>
<subsection name="Example of Usage">
<ul>