API changes in 3.1

[Tor Norbye]

As of 3.0, you can use the ServiceLoader mechanism to register your lint check; you no longer need to insert a special manifest key entry into the lint.jar; instead you just register your IssueRegistry by listing its fully qualified name in src/main/resources/META-INF/services/com.android.tools.lint.client.api.IssueRegistry. 

However, in the past, with the manifest approach to loading lint checks, I had a simple way of discarding older, potentially incompatible checks: Changing the key name. That doesn't work in the service loader world.

I've thought of various ways of fixing this, and I would really appreciate your suggestions on better way to fix this. But for now, for canary 4, I've added this mechanism:

IssueRegistry has a new "api" property; a simple int.

When you write your custom IssueRegistry implementations, you'll write them like this:

import com.android.tools.lint.detector.api.CURRENT_API

import com.android.tools.lint.detector.api.Issue

class MyIssueRegistry : IssueRegistry() {

    override val issues: List<Issue> = listOf(MY_ISSUE)

    override val api: Int = CURRENT_API

}

The new part is the "override val api" part. Basically, your registry is stating which API level it was compiled with. This is a constant, so the value (from the lint api jar) gets inlined into your issue registry at compilation time. When your custom check is loaded, lint will see if it matches the hosting lint environment, and if not, lint will warn that the lint check may not work correctly. 

Lint then tries to instantiate the checks, and if that throws any exceptions, lint tells you that the checks definitely don't work and it will skip them completely. This will be the case for checks that use the older JavaScanner and JavaPsiScanner (e.g. Lombok and non-UAST PSI checks), since I've just removed all the support for that in 3.1; it removed a lot of deprecated code.

Lint may also encounter a check that reports a higher compilation API than the current lint check -- e.g. you may compile your custom lint checks against 3.1, but somebody on the team then runs the check on an older lint, let's say 3.0. In that case lint will again warn that the versions don't match. But, if you've tested your check and you've made sure it also works on 3.0, you call tell lint that by overriding a second attribute:

    override val minApi: Int = 2

Here you're saying that lint also works with all versions from 2 and up the value listed for the api property. (The various version levels are found in the ApiKt class; there's a describeApi(level: Int) function you can call).

Anyway, this seemed like a pretty simple way to support this usecase. I considered trying to solve this a different way, e.g. updating the packaging steps for lint checks such that we instead insert metadata into your lint jar's META-INF folder recording the various API information. But lint jars are packaged in many different ways, including custom mechanisms and via other build systems, so that seemed like it could be brittle. This now makes it pretty explicit.

Thoughts, suggestions? I have a few days left before the next canary cut-off before this goes out so I can still make changes!

-- Tor

P.S. Here's how it would look from a Java implementation of an issue registry:

import com.android.tools.lint.detector.api.ApiKt;

import com.android.tools.lint.detector.api.Issue;

import java.util.Collections;

import java.util.List;

public class MyIssueRegistry extends IssueRegistry {

    @Override

    public List<Issue> getIssues() {

        return Collections.singletonList(MY_ISSUE);

    }

    @Override

    public int getApi() {

        return ApiKt.CURRENT_API;

    }

}

P.S.2: Here are some examples of the kinds of warning messages you now get for lint checks that don't supply api == CURRENT_API:

lint3.jar: Warning: Lint found an issue registry (com.example.google.lint.MyIssueRegistry) which did not specify the Lint API version it was compiled with.

This means that the lint checks are likely not compatible.

To fix this, make your lint IssueRegistry class contain

  override val api: Int = com.android.tools.lint.detector.api.CURRENT_API

or from Java,

  @Override public int getApi() { return com.android.tools.lint.detector.api.ApiKt.CURRENT_API; } [ObsoleteLintCustomCheck]

lint3.jar: Warning: Lint found one or more custom checks that could not be loaded. The most likely reason for this is that it is using an older, incompatible or unsupported API in lint. Make sure these lint checks are updated to the new APIs. The issue registry class is com.example.google.lint.MyIssueRegistry. The class loading issue is com/android/tools/lint/detector/api/Detector$JavaPsiScanner: ClassLoader.defineClass1(ClassLoader.java:-2)←ClassLoader.defineClass(ClassLoader.java:763)←ClassLoader.defineClass(ClassLoader.java:642)←UrlClassLoader._defineClass(UrlClassLoader.java:272)←UrlClassLoader.defineClass(UrlClassLoader.java:268)←UrlClassLoader.findClass(UrlClassLoader.java:222)←ClassLoader.loadClass(ClassLoader.java:424)←ClassLoader.loadClass(ClassLoader.java:357) [ObsoleteLintCustomCheck]

0 errors, 2 warnings

[César Puerta]

Ah, I missed this post. The proposal looks good and makes sense. Just one question: did you consider using an annotation instead of methods in the base class? It strikes me as a more compact solution (since you can bundle all configuration parameters in the single annotation), and it allows you to check compatibility without instantiating the class. It looks easy enough either way though.

[Zac Sweers]

Looks good to me too! Would be nice if it could default to something like a static `getLatestApi()`, similar to annotation processors can with source version.

On Friday, November 10, 2017 at 9:44:14 AM UTC-8, Tor Norbye wrote:

[Tor Norbye]

On Sunday, December 3, 2017 at 5:23:01 PM UTC-8, Zac Sweers wrote:

Looks good to me too! Would be nice if it could default to something like a static `getLatestApi()`, similar to annotation processors can with source version.

Can you point me to an example? 

[Zac Sweers]

https://github.com/rharter/auto-value-gson/blob/master/auto-value-gson/src/main/java/com/ryanharter/auto/value/gson/AutoValueGsonAdapterFactoryProcessor.java#L63-L65

[Michael Bailey]

Kotlin style guide suggests using  @file:JvmName("Api") in Api.kt

https://android.github.io/kotlin-guides/interop.html#file-name

When a file contains top-level functions or properties, always annotate it with @file:JvmName("Foo") to provide a nice name.
By default, top-level members in a file Foo.kt will end up in a class called FooKt which is unappealing and leaks the language as an implementation detail.

[Tor Norbye]

On Thursday, December 7, 2017 at 5:05:11 PM UTC-8, Zac Sweers wrote:

https://github.com/rharter/auto-value-gson/blob/master/auto-value-gson/src/main/java/com/ryanharter/auto/value/gson/AutoValueGsonAdapterFactoryProcessor.java#L63-L65

Thanks -- and sorry for being dense, but I'm still not quite following how this would work.

My goal here is to somehow record (into your custom lint jar) which version of the lint APIs you were compiling against.

And I'd like this to be automatic -- such that if you happen to build against a different lint API, possibly using a different mechanism than the "lintChecks" packaging mechanism in the Gradle plugin, it still works. (That rules out the most obvious solution, which would be to record some extra stuff in the META-INF folder in the jar when it's packaging the lint jar).

The way this works with the current mechanism, where your lint issue registry includes this:

  override val api: Int = CURRENT_API

is that the constant referred to above in a constant value, so the compiler *inlines* the value.

When your lint rule is loaded in some future version of lint, it doesn't look up the current value of CURRENT_API, instead it looks at the hardcoded number that was inserted into your lint jar at the time of compilation.

This only works because the above is a constant expression which the compiler inlines. The part I'm missing from the above example is that it looks like what is being returned is a non-constant expression. If I did that, then when loading your lint rule, it would just return the *current* (e.g. hosting-lint) API version instead of the one the lint rules were compiled against.

But I'm assuming there's something more subtle going on here since you suggested it -- I'm only slightly familiar with auto-value, but I think it's a compiler plugin that generates stuff at compile time. So is a potential solution where the user doesn't have to do anything and by applying an annotation processor the annotation processor would go and insert the right stuff into the class file without the user having to do anything?

I'm a bit conflicted about adding additional hurdles to the build process for everyone -- and how does this work with Kotlin? As will be apparent from my next reply on this thread to Michael, I'm optimizing the APIs for convenience from Kotlin...

-- Tor

[Tor Norbye]

On Thursday, December 14, 2017 at 2:18:23 PM UTC-8, Michael Bailey wrote:

Kotlin style guide suggests using  @file:JvmName("Api") in Api.kt

https://android.github.io/kotlin-guides/interop.html#file-name

When a file contains top-level functions or properties, always annotate it with @file:JvmName("Foo") to provide a nice name.
By default, top-level members in a file Foo.kt will end up in a class called FooKt which is unappealing and leaks the language as an implementation detail.

Yes -- I can change that here. But..

I realize that I haven't discussed this anywhere else other than at my recent lint talk at KotlinConf, and this is as good as a time as any ...

As part of the lint 2.0 API overhaul I'm seriously considering optimizing the APIs for access from Kotlin. There are several reasons for this:

(1) The lint APIs already depend on some libraries which themselves are much better from Kotlin, such as UAST. There are a lot of extension methods in UAST that are really important, and are hard to discover from Java because they're sitting in random utility classes scattered above).  Similarly, lint itself has a number of utility methods which work better as extension methods.

(2) Kotlin has a nice deprecation mechanism, along with replacement suggestions and quickfixes, and even the ability to remove methods that continue to live in the .class files but are are invisible/compiler errors when recompiling.

(3) Kotlin has the ability to add parameters with default values, which is source compatible.

Obviously you can still access all these APIs from Java, but I don't have a way to migrate APIs in Java with migration quickfixes etc the way I can for Kotlin, so you're better off using Kotlin.

-- Tor