Custom Lint Checks, part 3

If you didn’t use lint on your project before, most likely after enabling it you’ll see dozens (or hundreds) of warnings and errors. It can be intimidating and your first instinct would likely be to disable the lint and erase the last 15 minutes of your short-term memory. On one hand you probably can’t afford to fix all issues in one go, on the other hand you don’t want to ignore bunch of .

What works for us is gradually fixing all issues of a single type and increasing the severity of this issue to fatal, so it breaks the build. You can do this in xml file specified in build.gradle (as shown in part 1) or directly in build.gradle:

android {
  lintOptions {
    fatal 'UnusedResource' // Oops, it should be 'UnusedResources'
  }
}

But what happens if you mess up your lint configuration? How to detect errors in error detection tools?

We need to go deeper.

Lintception

We’ll write a lint check that checks the lint configuration!


public static final Issue ISSUE = Issue.create(
    "InvalidLintId",
    "Referencing non-existing lint issue id in your config",
    "Configuration references lint rule which doesn't exist, probably because of a typo",
    Category.CORRECTNESS,
    10,
    Severity.FATAL,
    new Implementation(
        InvalidLintIdDetector.class,
        Scope.GRADLE_SCOPE
    )
);
public class InvalidLintIdDetector extends Detector implements GradleScanner {
  //…

  void visitBuildScript(@NonNull Context context, Map<String, Object> sharedData) { 
    // TODO 
  } 
}

All other scanner classes are well documented, but the only information for GradleScanner is a cryptic Specialized interface for detectors that scan Gradle files JavaDoc comment. To make things worse, the only implementation of GradleScanner from lint-checks doesn’t even implement the only method of GradleScanner. If you dig deep enough, you’ll find out that the implementation of GradleDetector is replaced by GroovyGradleDetector as a part of a lint gradle task. We’ll base our work on it.

Here’s the rough plan for our lint check:

private static abstract class IssueReference {
  abstract String getIssueId();
  abstract Location getLocation();
}

private static ImmutableSet<String> getValidIssuesIds(Context context) {
  // …
}

private static Iterable<IssueReference> extractIssuesReferences(final Context context) {
  // …
}

@Override
public void visitBuildScript(@NonNull Context context, Map<String, Object> sharedData) {
  ImmutableSet<String> validIssuesIds = getValidIssuesIds(context);

  for (IssueReference issueReference : extractIssuesReferences(context)) {
    if (!validIssuesIds.contains(issueReference.getIssueId())) {
      context.report(
          ISSUE,
          issueReference.getLocation(),
          String.format(
              "Unknown lint issue id: %1$s",
              issueReference.getIssueId()
          )
      );
    }
  }
}

The first part is simple: we can get the list of valid issues’ ids from IssueRegistry:

private static ImmutableSet<String> getValidIssuesIds(Context context) {
  return FluentIterable
      .from(context.getDriver().getRegistry().getIssues())
      .transform(new Function<Issue, String>() {
        @Override
        public String apply(Issue input) {
          return input.getId();
        }
      })
      .toSet();
}

Extracting referenced issues’ ids is much trickier. We can get the build.gradle sources using Context.getContents() method:

private static Iterable extractIssuesReferences(final Context context) {
  String buildScriptSource = checkNotNull(context.getContents());
  
  // … 
}

The sources can be parsed to Groovy abstract syntact tree node with AstBuilder.buildFromString(), and then processed with a GroovyCodeVisitor. AstBuilder return multiple nodes, but since we process the whole build script we should get the single BlockStatement:

dependencies {
  compile gradleApi()
}
private static Iterable extractIssuesReferences(final Context context) {
  String buildScriptSource = checkNotNull(context.getContents());

  BuildScriptVisitor visitor = new BuildScriptVisitor();
  Iterables
      .getOnlyElement(new AstBuilder().buildFromString(buildScriptSource))
      .visit(visitor);

  // …
}

Now comes the hard part: processing Groovy AST. One thing you need to be aware of is what really happens when you write the code like this:

android {
  lintOptions {
    fatal 'Boom'
  }
}

These are method calls, all the way down: on the top level we have a call to android method with a ClosureExpression as a parameter, which contains a lintOptions method call with a ClosureExpression parameter, which finally contains the fatal method call we’re interested in.

The same thing can be written as:

android { 
  lintOptions.fatal 'Boom' 
}

The difference in the AST is the ObjectExpression on which the fatal method is called: in previous case it was this from lintOptions closure, in this case it’s the lintOptions VariableExpression from android closure.

The last syntax form is:

android.lintOptions.fatal 'Boom'

In this case we’re calling fatal on lintOptions PropertyExpression of android PropertyExpression of the top level block of build.gradle.

The simplest way of handling all syntax forms is treating all “leaf” expressions in Groovy AST as scope of the call and detect when our scope adds up to android.lintOptions.fatal or another method call which configures lint issues by id:

private static class BuildScriptVisitor extends CodeVisitorSupport {
  private final Stack<String> mScopeStack = new Stack<>();

  public final List<Expression> mIssueIdsExpressions = new ArrayList<>();

  @Override
  public void visitMethodCallExpression(MethodCallExpression call) {
    int stackSize = mScopeStack.size();

    call.getObjectExpression().visit(this);
    call.getMethod().visit(this);

    if (parseLintIssuesId()) {
      // TODO append issue ids to mIssueIdsExpressions
    } else {
      call.getArguments().visit(this);
    }

    mScopeStack.setSize(stackSize);
  }

  private static final ImmutableList<String> LINT_OPTIONS_SCOPE = ImmutableList.of("android", "lintOptions");
  private static final ImmutableSet<String> ISSUE_CONFIG_METHODS = ImmutableSet.of(
      "check",
      "disable",
      "enable",
      "error",
      "fatal",
      "ignore",
      "warning"
  );

  private boolean parseLintIssuesId() {
    return ISSUE_CONFIG_METHODS.contains(mScopeStack.peek()) && scopeStackInitMatches(LINT_OPTIONS_SCOPE);
  }

  private boolean scopeStackInitMatches(Iterable<String> expectedMatch) {
    Iterable<String> init = Iterables.limit(mScopeStack, mScopeStack.size() - 1);
    return Iterables.elementsEqual(init, expectedMatch);
  }

  @Override
  public void visitVariableExpression(VariableExpression expression) {
    updateScope(expression);
  }

  @Override
  public void visitConstantExpression(ConstantExpression expression) {
    updateScope(expression);
  }

  private void updateScope(Expression expression) {
    String text = expression.getText();
    if (!text.equals("this")) {
      mScopeStack.push(text);
    }
  }
}

The most basic form of AST for fatal method call is a bunch of ConstantExpression wrapped into ArgumentListExpression:

@Override
public void visitMethodCallExpression(MethodCallExpression call) {
  // …

  if (call.getArguments() instanceof ArgumentListExpression) {
    FluentIterable
        .from(((ArgumentListExpression) call.getArguments()).getExpressions())
        .filter(Predicates.instanceOf(ConstantExpression.class))
        .copyInto(mIssueIdsExpressions);
  }

  // …
}

What about the other syntax forms? What if someone calls another method to get the list of issues to be passed to fatal method? How do you handle it?

The answer is simple: you don’t. You should realize that there are infinite ways of writing the code that does exactly the same thing and you won’t be able to handle them all. What’s more important is making sure that there are no false positives, i.e. your lint check should never report an error in a valid code.

The last part to do is converting the AST Expression into our IssueReference. The issue id is just a result of Expression.getText() call and the code to get the Location object from a Groovy AST node can be copied from GradleDetector implementation.

Gotcha

If you put together the code from the snippets above, it won’t detect a thing. The visitBuildScript method on your Detector won’t be called unless you also return true from appliesTo method:

@Override
public boolean appliesTo(@NonNull Context context, @NonNull File file) {
  return true;
}

Recap

Good lint checks have no false positives, even at the cost of not covering some exotic cases. The lint checks API is still not in final stage. Don’t be afraid to dive into lint sources when something doesn’t work.

The full sources of the code above are available on github.

Posted by

Jerzy Chałupski

Share this article