checkstyle / checkstyle/checkstyle
SuppressWithNearbyCommentFilter: odd interplay between checkFormat and messageFormat
- Dominant language
- Java
- Stars
- 9.6k
- Forks
- 4.2k
- Avg merge
- 22h 23m
- Merged PRs (30d)
- 232
Description
I'm not sure the SuppressWithNearbyCommentFilter check is working correctly. What I'm trying to do is suppress warnings for a specific check _and_ with a specific message. The implementation of Tag.isMatch() in SuppressWithNearbyCommentFilter seems to indicate that I can only do one or the other. That is to say, I'd prefer if checkFormat and messageFormat are both set on the module, both must match. Leaving checkFormat off defaults to a regex of ".*" and so messageFormat is not respected. I needed to set checkFormat to a pattern that will never match a check name for messageFormat to get used. (This took me a couple hours of frustruction to figure out.)
For example, I would like to suppress a shared ImportControl check, but only when the message relates to com.sun.javadoc. The other imports that are prohibited should still be enforced.
``` xml
```
All my files start with "some comment text" in the header, so this should disable import warnings for com.sun.javadoc in this one project. However, the code for isMatch from SuppressWithNearbyCommentFilter below matches (filters) an event if only the check name matches (tagCheckRegexp).
I believe the "else if" should actually be moved up.
``` java
public boolean isMatch(AuditEvent event)
{
int line = event.getLine();
boolean match = false;
if ((line >= this.firstLine) && (line <= this.lastLine))
{
Matcher tagMatcher = this.tagCheckRegexp.matcher(event.getSourceName());
if (tagMatcher.find())
{
match = true;
}
else if (this.tagMessageRegexp != null)
{
Matcher messageMatcher = this.tagMessageRegexp.matcher(event.getMessage());
match = messageMatcher.find();
}
}
return match;
}
```
Proposed new implementation:
``` java
public boolean isMatch(AuditEvent event)
{
boolean match = false;
int line = event.getLine();
if ((line >= this.firstLine) && (line <= this.lastLine))
{
Matcher tagMatcher = this.tagCheckRegexp.matcher(event.getSourceName());
if (tagMatcher.find())
{
if (this.tagMessageRegexp == null)
{
match = true;
}
else
{
Matcher messageMatcher = this.tagMessageRegexp.matcher(event.getMessage());
match = messageMatcher.find();
}
}
return match;
}
}
```
This way, if checkFormat is not set, it defaults to pervasive match and messageFormat will be checked. If checkFormat is set and messageFormat is not, behavior is as before. Otherwise, both checkFormat and messageFormat must match to filter a warning.
##
Contributor guide
Research direction
Start with SuppressWithNearbyCommentFilter and the Tag.isMatch() entry point shown in the issue. Review how checkFormat and messageFormat are currently evaluated, then verify that both are required when configured while each still works alone. Done means a warning is filtered only when its line, check pattern, and configured message pattern match.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100