Multi-module Lint Rules Follow Up: Suppressions ☠️

It has been a hot minute since I posted about writing multi-module Lint rules so it’s time for a follow up. Today’s topic: suppressions! A quick recap of where we are:

We have written a Lint rule that checks for usages of deprecated colours (including selectors) in XML files. The rule goes through all modules in the project looking for colours that are contained in any file with the _deprecated suffix in the filename. We then report usages of those colours as errors. We have also written tests for our Lint rule that cover most (all?) scenarios.

Suppression Checks 🛡️

A key mechanism we employ in our Lint rule is calling getPartialResults in the afterCheckEachProject callback. We use the returned PartialResults to store:

the list of deprecated colours, and
the list of all colour usages

in each module (If it’s a bit confusing, I highly recommend reading through the OG post and maybe things will make more sense).

The KDoc for getPartialResults point out that suppressions are not checked at this point:

Note that in this case, the lint infrastructure will not automatically look up the error location (since there isn’t one yet) to see if the issue has been suppressed (via annotations, lint.xml and other mechanisms), so you should do this yourself, via the various LintDriver.isSuppressed methods.

This presents us with a great opportunity to improve our DeprecatedColorInXml Lint rule. We don’t even want to consider reporting a colour usage if our Lint rule is suppressed. Since we are parsing an XML file, we can use the isSuppressed() variant that takes in an XmlContext:

override fun visitAttribute(context: XmlContext, attribute: Attr) {
    // The issue is suppressed for this attribute, skip it
    val isIssueSuppressed = context.driver.isSuppressed(context, ISSUE, attribute)
    if (isIssueSuppressed) return

    // ...
}

override fun visitElement(context: XmlContext, element: Element) {
  // The issue is suppressed for this element, skip it
  val isIssueSuppressed = context.driver.isSuppressed(context, ISSUE, element)
  if (isIssueSuppressed) return

  // ...
}

I assume that the suppression checks can also be done in afterCheckEachProject but why delay when we can bail out early?

Tests 🔬

With these updates, suppressed Lint issues in XML files will not be reported even if they are missing from the baseline file. We can leverage our existing tests to come up with new ones.

Let’s write a test for an example layout file using a deprecated colour. We provide the test with two files: one for deprecated colours and another for the layout file. When we suppress the DeprecatedColorInXml rule in a widget in the layout file, there should not be any reported issues.

@Test
fun testSuppressedDeprecatedColorInWidget() {
    lint().files(
        xml(
            "res/values/colors_deprecated.xml",
            """
            <resources>
                <color name="some_colour">#d6163e</color>
            </resources>
        """
        ).indented(),
        xml(
            "res/layout/layout.xml",
            """
            <View xmlns:android="http://schemas.android.com/apk
                xmlns:tools="http://schemas.android.com/tools"
                android:layout_width="wrap_content"
                android:layout_height="wrap_content">
                <TextView android:layout_width="wrap_content"
                    android:layout_height="wrap_content"
                    android:textColor="@color/some_colour" 
                    tools:ignore="DeprecatedColorInXml" />
            </View>
        """
        ).indented()
    )
        .testModes(TestMode.PARTIAL)
        .run()
        .expectClean()
}

For completeness, we can also add a test where the suppression is declared in the root element of the layout file and a deprecated colour is used in a widget (i.e. move tools:ignore="DeprecatedColorInXml" to the View).

As always, the rule updates and the new test cases are in Github.

Tags

Suppression Checks 🛡️

Tests 🔬