PSA: Lint check for lint checks

[Tor Norbye]

Hi everyone,

As you may already be aware, lint itself isn't specific to Android, and it has a number of general checks you can run on (for example) Gradle projects that apply just the kotlin or java module.

This itself includes implementations of lint checks, which normally are just applying kotlin or java plugins (and then you have a wrapper Android lint project using the lintPublish configuration to bundle it as an AAR).

This setup is already shown in the lint sample project on github:

https://github.com/googlesamples/android-custom-lint-rules

Anyway -- you might find it interesting that as of Studio 4.1, lint includes a number (~10) of checks for implementations of lint checks! Here's the list from the checkinP:

* Checks for many UAST pitfalls -- such as calling equals on PSI

  elements, or calling getMethodBody on a PsiMethod or getInitializer on

  a PsiField. Similarly, if the receiver is a UElement, don't call

  getParent, getContainingClass, PsiTreeUtil.getParentOfType, and so on.

* Many String-related checks for strings passed as error messages or

  used in issue registrations:

  * If it sees something which looks like a symbol, such as a <tag>

    or a methodCall() or a ClassName, it will suggest surrounding it

    with `` such that it will be formatted as a symbol in the IDE and in

     HTML Reports.

  * Any URL encountered is checked to make sure it's using https, not

    http, and is from an expected domain (such as .android.com or

    .google.com), and that it is well formed. Internal links are

    forbidden (such as b.corp.google.com).

  * Links to bugs are validated (should be using issuetracker.google

    .com, not code.google.com/android, and it makes sure there's the

    expected number of digits

  * Calling .trimIndent() or .trimMargin() on raw strings is unnecessary

    in lint because lint will do it internally anyway prior to

    presentation, and doing it in the source code breaks nested syntax

    highlighting.

  * Error messages that are a single sentence should not end with a dot.

  * Issue id's should be at most 40 characters and capitalized camel

    case

  * Issue summaries should be capitalized and not too long

  * It also looks for typos in error messages and suggests replacements

* New lint checks (e.g. detectors with a copyright year of 2020 or

  higher) should be implemented in Kotlin

* Use existing constants. If it sees custom construction of EnumSets

  of Scopes, it compares it against the set of builtin scope sets

  and suggests using one of those.

For example, if I check out the above sample project, and then add problems to the lint checks, and then run the following lint target, it spits out the below warnings. Give it a try -- hopefully it's useful (and let us know about false positives. And yes, many of those checks are opinionated, such as the Kotlin one -- feel free to disable the ones you don't find useful.)

-- Tor

$ cd android-studio-4; ./gradlew :checks:lint

> Task :checks:lint FAILED

src/main/java/com/example/lint/checks/SampleCodeDetector.kt:61: Error: Lint issue IDs should not contain spaces, such as MyIssueId [LintImplIdFormat]

                id = "My Issue Id",

                      ~~~~~~~~~~~

   Explanation for issues of type "LintImplIdFormat":

   This check looks at lint issue id registrations and makes sure the id

   follows the expected conventions: capitalized, camel case, no spaces, and

   not too long.

   Note: You shouldn't change id's for lint checks that are already widely

   used, since the id can already appear in @SuppressLint annotations,

   tools:ignore= attributes, lint baselines, Gradle lintOptions blocks,

   lint.xml files, and so on. In these cases, just explicitly suppress this

   warning instead using something like

       @JvmField

       val ISSUE = Issue.create(

           // ID string is too long, but we can't change this now since this

           // id is already used in user suppress configurations

           //noinspection LintImplIdFormat

           id = "IncompatibleMediaBrowserServiceCompatVersion",

           ...

src/main/java/com/example/lint/checks/SampleCodeDetector.kt:50: Warning: Single sentence error messages should not end with a period [LintImplTextFormat]

                            "This code mentions lint: Congratulations.")

                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

   Explanation for issues of type "LintImplTextFormat":

   Lint supports various markdown like formatting directives in all of its

   strings (issue explanations, reported error messages, etc).

   This lint check looks for strings that look like they may benefit from

   additional formatting. For example, if a snippet looks like code it should

   be surrounded with backticks.

   Note: Be careful changing existing strings; this may stop baseline file

   matching from working, so consider suppressing existing violations of this

   check if this is an error many users may be filtering in baselines. (This

   is only an issue for strings used in report calls; for issue registration

   strings like summaries and explanations there's no risk changing the text

   contents.)

src/main/java/com/example/lint/checks/SampleCodeDetector.kt:72: Error: No need to call .trimIndent() in issue registration strings; they are already trimmed by indent by lint when displaying to users [LintImplTrimIndent]

                    """.trimIndent(),

                        ~~~~~~~~~~~~

   Explanation for issues of type "LintImplTrimIndent":

   Lint implicitly calls .trimIndent() (lazily, at the last minute) in a

   number of places:

   * Issue explanations

   * Error messages

   * Lint test file descriptions

   * etc

   That means you don't need to put .trimIndent() in your source code to

   handle this.

   There are advantages to not putting .trimIndent() in the code. For test

   files, if you call for example kotlin(""\"source code"\""\") then

   IntelliJ/Android Studio will syntax highlight the source code as Kotlin.

   The second you add in a .trimIndent() on the string, the syntax

   highlighting goes away. For test files you can instead call ".indented()"

   on the test file builder to get it to indent the string.

2 errors, 1 warnings

Wrote HTML report to file:///Users/tnorbye/dev/github/tnorbye/android-custom-lint-rules/android-studio-4/checks/lint-report.html

Wrote XML report to file:///Users/tnorbye/dev/github/tnorbye/android-custom-lint-rules/android-studio-4/checks/build/test-results/lint-results.xml

FAILURE: Build failed with an exception.

* What went wrong:

Execution failed for task ':checks:lint'.

> Lint found errors in the project; aborting build.

  Fix the issues identified by lint, or add the following to your build script to proceed with errors:

  ...

  lintOptions {

      abortOnError false

  }

  ...

* Try:

Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 2s

1 actionable task: 1 executed

[Drew Hannay]

In case it's not obvious to anyone else, in order to use this you need to add 

apply plugin: 'com.android.lint'

to the build.gradle file for your lint check module. You also might need to configure lintOptions as seen here

I just tried this for our custom lint rules and got the following counts:

One of the LintImplBadUrl errors is because we're using an email address as a more info link (should that be allowed?) and the other is flagging this as a suspicious issue tracker link for some reason: https://issuetracker.google.com/issues/37077118

The LintImplIdFormat errors are all because we namespace our lint ids (e.g. MyCompany.FooDetector) to avoid potential conflicts with lint checks from other libraries (is this a silly thing to do?)

LintImplTextFormat are all just length issues, and the LintImplUseExistingConstants is catching a case where we could be using Scope.JAVA_FILE_SCOPE (yay!)

[Tor Norbye]

On Tue, Mar 10, 2020 at 8:32 PM Drew Hannay <drewh...@gmail.com> wrote:

In case it's not obvious to anyone else, in order to use this you need to add 

apply plugin: 'com.android.lint'

to the build.gradle file for your lint check module. You also might need to configure lintOptions as seen here

I just tried this for our custom lint rules and got the following counts:

One of the LintImplBadUrl errors is because we're using an email address as a more info link (should that be allowed?)

I was thinking of More Info as a context link you can open to read further information. For example, for some of the security related lint checks, it points to the vulnerability disclosures so you can see the whole backstory in case the condensed issue explanation isn't enough. I'm not sure what an e-mail address would do there -- is the thinking that you can e-mail the address to ask them to provide more information?

 

and the other is flagging this as a suspicious issue tracker link for some reason: https://issuetracker.google.com/issues/37077118

Ah yes. That one was a bit tricky. There have been many cases where people have cut & pasted issue id's but then missed a digit or two -- so I've had to trial&error add all the digits at the end to find which issue they meant. So here I basically made sure the issue id has 9 (or whatever it is) digits which is the case for almost all recent issues. But there are some really old bugs in there, especially for the older lint checks -- so in that case (I think there were 3-4 cases) I just suppressed the lint check locally instead:

                    //noinspection LintImplBadUrl -- old bug, fewer digits than usual
                    .addMoreInfo("https://issuetracker.google.com/36988969");

 

The LintImplIdFormat errors are all because we namespace our lint ids (e.g. MyCompany.FooDetector) to avoid potential conflicts with lint checks from other libraries (is this a silly thing to do?)

Hmmm, that's surprising; it should only complain if there are (a) spaces, or (b) not capitalized or (c) not a lowercase character later in the string. And "MyCompany.FooDetector" passes here. Is the issue that you really used lowercase, e.g. "com.google.MyId" would be a problem. I agree we should allow that; I'll tweak it to access initial lowercase letter if there's a dot somewhere in the id.

-- Tor

--
You received this message because you are subscribed to the Google Groups "lint-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to lint-dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/lint-dev/ec9f3574-f6af-4c09-a1de-972862d7e196%40googlegroups.com.

[Drew Hannay]

I'm not sure what an e-mail address would do there -- is the thinking that you can e-mail the address to ask them to provide more information?

Yes, we actually provide two More Info links for this check. One goes to an internal site that explains the rules the check is enforcing, and then the email address is to contact the team that owns the rule in case there are additional questions / concerns. 

Hmmm, that's surprising; it should only complain if there are (a) spaces, or (b) not capitalized or (c) not a lowercase character later in the string.

The description of the error is "Lint issue IDs should be reasonably short (< 40 chars); they're used in suppress annotations etc". The issue id "MyCompany.DoubleBraceInitializationDetector" is maybe a more realistic example of one of our failures.

To unsubscribe from this group and stop receiving emails from it, send an email to lint...@googlegroups.com.

[Tor Norbye]

These two issues should be addressed in 4.1 canary 4; it will now allow mailto: as a URL type, and if an id contains dots, it's treated as a qualified name and the capitalization and length rule is only applied to the last segment.

-- Tor

https://android.googlesource.com/platform/tools/base/+/f684fd6e8d65bd11afb4d7c93c68d2a34dc2a22d

[Drew Hannay]

It seems like this was broken at some point: https://issuetracker.google.com/issues/197146610