How to teach Sonar to find new bug?
Background
A few days ago my Jenkins instance gave me
the regurlar hint for updates. So I checked the changelog for changes which are
interesting to me. One of them hit my eye: Jenkins 2.53 – „GC Performance:
Avoid using FileInputStream and FileOutputStream in the core codebase.“ I
read the two tickets (for Jenkins and the JDK itself) and was
surprised. I hadn’t knew that.
Some days later I regognized a longer
blog about that by CloudBees on DZone.
Also interesting.
During thinking about changing the „new FIS/FOS“
to something better in the open source projects I am working on (Apache Ant, Apache Commons) – Stefan
was faster. 😉
Using „search and (manually) replace“ on my
office-codebase I thought about how to ensure that nobody intoduced these
instantiations again?
The tool
At work we use a Sonar instance for detecting propable
issues. Sonar uses static code analysis for detecting errors – unclosed
streams, possible NullPointers, etc. But this (new) „bug pattern“ was not
implemented. Sonar itself could scan Java files, but also other languages.
The documentation of Sonar contains a
chapter of writing
custom Java rules.
I don’t want to copy that tutorial with my
own word, but here are some highlights:
·
Finally you end a with a plain JAR you‘ll have
to copy to the Sonar extensions/plugins
directory, restart Sonar and activate your new rule.
·
The Tutorial gives you a pointer to Github
repository containing the sample sources, so you start with them. I decided
to start from scratch and copied the parts I could need instead of cloning the
whole repo and extend that.
·
The entry class is an implementation of org.sonar.api.Plugin
and its declaration in the manifest file (key=Plugin-Class).
·
This PlugIn registers two classes: one defining
your „repository“ with all the help files and metadata. And another which
registers the rules for production code or test code. They have to implement
the interfaces org.sonar.api.server.rule.RulesDefinition respective org.sonar.plugins.java.api.CheckRegistrar.
·
The rule itself could extend several entry
points, here the easiest is extending org.sonar.plugins.java.api.IssuableSubscriptionVisitor, so you could benefit from the Java knowledge and the access to the
Java AST.
The rule
Implementing the IssuableSubscriptionVisitor itself is fairly easy:
1.
Register which types of AST nodes you are
interested in – here instantiation of objects:
@Override
public
List nodesToVisit() {
return
ImmutableList.of(Tree.Kind.NEW_CLASS);
}
2. Implement the logic for handling this. Here we are interested in
instantiation of java.io.FileInputStream
or java.io.FileOutputStream:
private
List forbiddenClasses = ImmutableList.of(
„java.io.FileInputStream“,
„java.io.FileOutputStream“
);
@Override
public void
visitNode(Tree tree) {
NewClassTree nct =
(NewClassTree)tree;
for(String forbiddenClass : forbiddenClasses) {
if (nct.symbolType().is(forbiddenClass)) {
reportIssue(tree, „Avoid using „ + forbiddenClass);
}
}
}
3. The typecast in the visitNode
method could be done safely because we registered only (that) one type in the nodesToVisit method. First I intended to use a list.contains(nct.symbolType()) but the is() method also checks
for subclasses.
When reporting
the issue the method registers the exact location via the give node.
4. Provide metadata via a @org.sonar.check.Rule annotation:
@Rule(
key=„AvoidClassRule“,
name=„Avoid usage of FileInputStream or FileOutputStream“,
description=„FileInputStream and FileOutputStream can cause memory
leaks
and should be replaced by NIO-functionality“,
priority=Priority.INFO,
tags={„bug“}
)
public class AvoidClassRule
extends IssuableSubscriptionVisitor {
Sadly we have
to duplicate the key. Also the other information we have duplicated in metadata
files. I haven’t dived that deep to check if we could get rid of some
duplicatations here.
In addition to these Java files we supply
two resources: a JSON file with metadata and a HTML file with documentation.
The most interesting part of the JSON file
is (in my opinion) the remediation key: here you specify the amount of work to
fix the problem.
{
„title“: „Avoid
usage of special Classes“,
„status“:
„ready“,
„remediation„: {
„func„: „Constant\/Issue“,
„constantCost“:
„5min“
},
„tags“: [
„example“
],
„defaultSeverity“:
„Minor“
}
The HTML file provides that part of the
which is shown as explanation of the rule.
Tests
Of course we have to test our code. The
example shows how to do that: provide classes which contains the error and use org.sonar.java.checks.verifier.JavaCheckVerifier to verify that an issue was reported or not reported.
public class InValidFISInstance {
public void x() {
try (InputStream is = new
FileInputStream(„test“)) {
is.available();
} catch (IOException e) {
}
}
}
public class CompliantClass {
public void x() {
try (InputStream in = Files.newInputStream(Paths.get(„file.txt“))) {
in.available();
} catch (IOException e) {
}
try (OutputStream out = Files.newOutputStream(Paths.get(„file.txt“))) {
out.flush();
} catch (IOException e) {
}
}
}
public class AvoidClassRuleTest {
@Rule
public TestName testname = new
TestName();
@Test
public void compliantClass() {
verify( (file,rule) ->
JavaCheckVerifier.verifyNoIssue(file, rule) );
}
@Test(expected=AssertionError.class)
public void invalidFISInstance()
{
verify( (file,rule) ->
JavaCheckVerifier.verify(file, rule) );
}
private void
verify(BiConsumer consumer) {
String filename = String.format(„src/test/files/%s.java“,
StringUtils.capitalize(testname.getMethodName()));
consumer.accept(filename, new
AvoidClassRule());
}
}
Some notes
The JSON and HTML resource have to be in /org/sonar/l10n/java/rules/squid/. Of course your RulesDefinition class could load them from wherever
you want, but the used JavaCheckVerifier expect them to be there.
The file names of the resources have to be ${ruleKey}.html and ${ruleKey}_${language}.json.
In constrast to the example I tend to also
have a positive test. I am not only interested in that the rule reports the
issue when required, it should do nothing if the code is compliant.
Rules could be parameterized via the web
ui. For that you annotate the field with @RuleProperty. You could access the values via:
@Before
public void init() {
JavaCustomRulesDefinition
rulesDefinition = new
JavaCustomRulesDefinition();
RulesDefinition.Context context = new
RulesDefinition.Context();
rulesDefinition.define(context);
repository = context.repository((„RepositoryKey“);
}
@Test
public void
assertParameterProperties() {
Param
param = repository.rule(„RuleKey“).param(„paramName“);
// Not
available:
assertThat(param).isNull();
// If
available:
// assertThat(param).isNotNull();
// assertThat(param.defaultValue()).isEqualTo(„Inject“);
// assertThat(param.description()).isEqualTo(„…“);
// assertThat(param.type()).isEqualTo(RuleParamType.STRING);
Jan's Blog 