Getting Lint Errors to Display in IDE for a Complex Lint Rule

[Fuad Balashov]

Hello, I am working on a lint rule + annotation (Module Enforcer) and I had a couple questions around making the lint errors show up in the IDE.

Background:

I'm working on a lint library that helps developers modularize their classes safely by enforcing apis with lint. Developers can use the lint rule by annotating their classes with the `@Module` annotation and then annotate required methods using `@RequiredMethod`. The lint rule will identify any instances of that `@Module` where all `@RequiredMethod`s are not called. 

Module Enforcer performs 2 passes over your project files. In the first, it identifies classes with the `@Module` annotation and saves all the methods from it that use the `@RequiredMethod` annotation. In the second pass it will look for classes that hold a `@Module` instance and warn you if not all `@RequiredMethod`s are invoked in that class.

The tool works from both the command line and "analyze code" tool in Android studio. It also has a good number of unit tests. However, it does not show any of my lint errors in Android Studio, which I think would add a lot of utility to the project. My understanding, based on the slides by Tor from KotlinConf is that the IDE inspections aren't shown because the rule relies on multiple files in its scope.

Questions:

1. Is the cause of the problem that I identified correct? (eg, is my lint rule not highlighted in the IDE because it needs to process multiple files?)

2. Any suggestions for getting the lint errors to show up in the IDE?

2b. Based on Tor's slides I was thinking of using `visitAnnotationUsage` to find instances of `@Module` classes and validate that `@RequiredMethod`s are called, would that be a fruitful strategy?

I also appreciate any feedback people have on the tool and its code.

Thanks,

Fuad

[Tor Norbye]

Correct; lint will only run checks on the fly in the editor if it's the check indicates that it can run on a single file scope.

The method in lint which computes whether an issue's implementation is capable of analysis on the current scope is

Implementation#isAdequate. 

Note that a lint check can supply *multiple* analysis scopes, which is what that file looks up.

For example, this is the implementation registration for the API check:

                    new Implementation(

                            ApiDetector.class,

                            EnumSet.of(Scope.JAVA_FILE, Scope.RESOURCE_FILE, Scope.MANIFEST),

                            Scope.JAVA_FILE_SCOPE,

                            Scope.RESOURCE_FILE_SCOPE,

                            Scope.MANIFEST_SCOPE));

The last three lines above tells lint that this lint check is capable of doing its work when editing java/kotlin files, when editing xml files, and when editing manifest files.

I didn't quite follow what your analysis is supposed to do, but note that there is direct support for writing annotation handling now (as of 3.1.)

We pushed the source code for 3.1 to AOSP recently, so try checking out the gradle_3.1.2 tag (see http://tools.android.com/build) and take a look at many of the existing annotation-based checks that use it, such as RestrictToDetector, ResourceTypeDetector, ThreadDetector, etc etc - basically any subclass of AbstractAnnotationDetector 

-- Tor

[Fuad Balashov]

Thanks Tor,

I'll look over the lint rules you mentioned and see if I can leverage what they are doing.

I'll take a stab at clarifying what I am trying to achieve for those who are interested:

The lint analysis I am writing is supposed to validate that WHEN you have a field whose class has an @Module annotation THEN all methods with a @RequiredMethod annotation are called on that field.

The goal is to validate that developers don't miss any method calls when they modularize their code.

Fuad

[Zac Sweers]

We noticed in https://github.com/uber/AutoDispose/pull/282#issuecomment-441159272 that marking a lint as applicable for test sources causes it to not show in the IDE. Is this intended behavior?

[Tor Norbye]

On Friday, November 23, 2018 at 6:55:12 PM UTC-8, Zac Sweers wrote:

We noticed in https://github.com/uber/AutoDispose/pull/282#issuecomment-441159272 that marking a lint as applicable for test sources causes it to not show in the IDE. Is this intended behavior?

That's what the "analysis scopes" are for:

    /**

     * Creates a new implementation for analyzing one or more issues

     *

     * @param detectorClass the class of the detector to find this issue

     * @param scope the scope of files required to analyze this issue

     * @param analysisScopes optional set of extra scopes the detector is capable of working in

     */

    @SafeVarargs

    public Implementation(

            @NonNull Class<? extends Detector> detectorClass,

            @NonNull EnumSet<Scope> scope,

            @NonNull EnumSet<Scope>... analysisScopes) {

Basically, the "scope" parameter to Implementation says "ALL of the following scopes must be present in order for this detector to kick in".

However, there are cases where a detector can *still* do some useful work when looking at a subset of the files. In that case, you can pass in one more more subsets of the scope as subsequent arguments to the Implementation constructor. That means your check will also be called if any of those analysis scopes are matched -- and your detector will have to be more careful to make sure that it's doing the right thing (for example, it can look up the full scope via the driver, which it can access via the scope).

TEST_SCOPE is a bit of a "meta" scope; it overlaps others. So in 3.4 I recently made some changes such that I think you don't need to specify analysis scopes with it; if you pass in EnumSet.of(Scope.JAVA_FILE, Scope.TEST) I think it would also treat this as a single-file scope that would work in the IDE. (Let me know if that doesn't actually work.) But for 3.3 you'll still need to specify both.

-- Tor

[Shaishav Gandhi]

Thanks Tor for the detailed explanation. Worked splendidly! 

[Zac Sweers]

Hey Tor - we've tried this out in newer studio versions, and it appears to still be required even as late as 3.6-alpha10. Did this maybe regress? Happy to file a bug if need be

[Zac Sweers]

To clarify - the original workaround still works. The issue is that the workaround is still required :)