Custom Lint Checks, part 2

In part 1 of this blog series I showed how to write a barebones custom Android lint check, which checks your project configuration. This time I’ll implement a check, which analyzes your Java code to warn you about the most important performance issue of 2015 – using enums.

JavaScanner

As usual, the first thing to do is to define an Issue instance:

public static final Issue ENUMS_ARE_BAD_ISSUE = Issue.create(
      "EnumsAreBad",
      "M'kay?",
      "" +
          "Every byte is sacred,\n" +
          "Every byte is great.\n" +
          "If a byte is wasted,\n" +
          "Colt gets quite irate.",
      Category.PERFORMANCE,
      1,
      Severity.INFORMATIONAL,
      new Implementation(EnumDetector.class, Scope.JAVA_FILE_SCOPE));

The important part is the Scope.JAVA_FILE_SCOPE which indicates our Detector will scan Java files in our project. The next step is making sure our Detector implements the appropriate scanner class. In our case that’s JavaScanner.

public class EnumDetector extends Detector implements JavaScanner {
  //…
}

The JavaScanner interface can be used in a couple ways. In the most general case you should implement a createJavaVisitor(JavaContext) method and return your AstVisitor implementation. Visitor is a common design pattern for cases, when you have a well defined data structure (in this case an Abstract Syntax Tree of your Java code) and you want to define multiple operations for this data structure without modifying the strucutre itself. If you’re not familiar with this pattern I recommend getting to know it before reading this post further.

Going back to JavaScanner – it also exposes a few helper methods for common use cases like checking only some method invocations or class declarations. All possible options are thoroughly described in JavaScanner‘s javadocs. It’s also a good practice to additionally implement a getApplicableNodeTypes() method to narrow down the analysis scope for better performance.

In our case we need a very simple AstVisitor which reports all encountered EnumDeclaration nodes in a syntax tree:

@Override
public List<Class<? extends Node>> getApplicableNodeTypes() {
  return Collections.<Class<? extends Node>>singletonList(EnumDeclaration.class);
}

@Override
public AstVisitor createJavaVisitor(final JavaContext context) {
  return new ForwardingAstVisitor() {
    @Override
    public boolean visitEnumDeclaration(EnumDeclaration node) {
      context.report(
          ENUMS_ARE_BAD_ISSUE,
          null,
          "Consider using int constants instead of enums"
      );

      return super.visitEnumDeclaration(node);
    }
  };
}

Location

This Detector implementation works, but the warning message is not very helpful:

Information: Consider using int constants instead of enums [EnumsAreBad]
0 errors, 1 warnings

To give more context, we should change the JavaContext.report() call to include the location:

      context.report(
          ENUMS_ARE_BAD_ISSUE,
          context.getLocation(node),
          "Consider using int constants instead of enums"
      );

Which yields much better results:

enum/TheEnum.java:3: Information: Consider using int constants instead of enums [EnumsAreBad]
public enum TheEnum {
^
0 errors, 1 warnings

Unless the enum class is documented:

enum_with_javadoc/TheEnum.java:4: Information: Consider using int constants instead of enums [EnumsAreBad]
// javadocs you don't want to see in your lint message
^
0 errors, 1 warnings

This happens, because we supplied the Location of an entire EnumDeclaration AST node. We can be more precise and point lint to the name of the enum:

      context.report(
          ENUMS_ARE_BAD_ISSUE,
          context.getLocation(node.astName()),
          "Consider using int constants instead of enums"
      );

To get this message:

enum_with_javadoc/TheEnum.java:4: Information: Consider using int constants instead of enums [EnumsAreBad]
public enum TheEnum {
            ~~~~~~~
0 errors, 1 warnings

Scope

The last thing to do is adding support for suppressing lint checks. You can do this by supplying an AST node as a scope parameter to JavaContext.report call. The lint will scan the code outwards from the supplied node and look for a @SuppressLint annotation or a //noinspect comment. In most cases you should just pass the same node you use for getting the Location:

      context.report(
          ENUMS_ARE_BAD_ISSUE,
          node.astName(),
          context.getLocation(node.astName()),
          "Consider using int constants instead of enums"
      );

Quis custodiet ipsos custodes?

If you can screw up so many things in your Detector class, it would be nice to have unit tests for it. Google provides a LintDetectorTest class which helps with that.

dependencies {
  testCompile 'com.android.tools.lint:lint-tests:24.3.1'
}
public class EnumDetectorTest extends LintDetectorTest {
  @Override
  protected Detector getDetector() {
    return new EnumDetector();
  }

  @Override
  protected List<Issue> getIssues() {
    return ImmutableList.of(EnumDetector.ENUMS_ARE_BAD_ISSUE);
  }

  //…
}

In your tests you can invoke a lintFiles(…), lintProject(…) or checkLint(…) method and compare the returned String with the expected error message or override the checkReportedError(…) method with your assertions. I prefer the former method, because it allows for writing multiple test cases in the same class. I also had to override the getTestResource(…) method to read my test’s resources. I ended up with the following base class:

public abstract class BaseLintDetectorTest extends LintDetectorTest {
  @Override
  protected InputStream getTestResource(String relativePath, boolean expectExists) {
    InputStream stream = getClass().getClassLoader().getResourceAsStream(relativePath);
    if (!expectExists && stream == null) {
      return null;
    }
    return stream;
  }

  protected String getExpectedError(String relativePath) throws IOException {
    URL resource = getClass().getClassLoader().getResource(relativePath);

    assertNotNull(resource);

    return Resources.toString(resource, Charsets.UTF_8);
  }
}

The detector test class:

public class EnumDetectorTest extends BaseLintDetectorTest {
  @Override
  protected Detector getDetector() {
    return new EnumDetector();
  }

  @Override
  protected List<Issue> getIssues() {
    return ImmutableList.of(EnumDetector.ENUMS_ARE_BAD_ISSUE);
  }

  public void testHappyPath() throws Exception {
    test("enum");
  }

  public void testCorrectLocation() throws Exception {
    test("enum_with_javadoc");
  }

  public void testSuppressionSupport() throws Exception {
    test("enum_with_suppression");
  }

  private void test(String resourceDirectory) throws Exception {
    assertEquals(getExpectedError(resourceDirectory + "/expected"), lintFiles(resourceDirectory + "/TheEnum.java"));
  }
}

And a bunch of resources with scanned Java classes and expected outputs. Unfortunately the LintDetectorTest is a JUnit3 test, so we cannot use parametrized tests.

Recap

The more I play with the custom lint rules, the more I like them. The sample Detectors from this blog series are trivial to make the explanations of lint concepts simpler, but if you take a step back and think about the possibilities, you’ll see what a powerful and versatile tool lint is. For example all support annotations introduced in v19.1 of the support library are backed by lint, and sometimes they feel like a Java language extensions.

The full sources of the code snippets above are available on github. In the next part I’ll explore other types of Scanners.

Posted by

Jerzy Chałupski

Share this article