Call Detector.UastScanner visitClass without declaring concrete applicableSuperClasses

[Tommy Wi]

Hi everyone,

I'm about to write a lint library to add to our company's projects. I already wrote some rules for Android specific classes like custom Fragments and Activities. That was pretty straightforward since I was able to list the superclasses from com/android/SdkConstants in my applicableSuperClasses() method.

But I'm facing difficulties while writing tests for the Presenter/Interactor classes. I like to check if those extend from defined BasePresenter/BaseInteractor classes. But since those are all "plain objects" I have no specific superclass I can add to applicableSuperClasses() and the UastScanner won't visit anything.

Are there any best practice for my usecase? I am wondering if there is a way by using UElementHandler?

Thomas

[Tor Norbye]

On Friday, November 10, 2017 at 8:46:02 AM UTC-8, Tommy Wi wrote:

Hi everyone,

I'm about to write a lint library to add to our company's projects. I already wrote some rules for Android specific classes like custom Fragments and Activities. That was pretty straightforward since I was able to list the superclasses from com/android/SdkConstants in my applicableSuperClasses() method.

But I'm facing difficulties while writing tests for the Presenter/Interactor classes. I like to check if those extend from defined BasePresenter/BaseInteractor classes. But since those are all "plain objects" I have no specific superclass I can add to applicableSuperClasses() and the UastScanner won't visit anything.

Note that applicableSuperClasses, despite the name, also includes interfaces -- that's for example how the ParcelDetector works:

    @Nullable

    @Override

    public List<String> applicableSuperClasses() {

        return Collections.singletonList("android.os.Parcelable");

    }

Does that answer your question? (Sorry, I'm not very familiar with your use case; if you can include some code fragments showing the pattern you want to detect, I might be able to say something more useful.)

-- Tor

[Tommy Wi]

On Friday, 10. November 2017 18:23:34 UTC+1 Tor Norbye wrote:

    @Nullable

    @Override

    public List<String> applicableSuperClasses() {

        return Collections.singletonList("android.os.Parcelable");

    }

Does that answer your question? (Sorry, I'm not very familiar with your use case; if you can include some code fragments showing the pattern you want to detect, I might be able to say something more useful.)

-- Tor 

Hi Tor, appreciate you took the time to answer my question! I wasn't aware that I can also use Interfaces there.

But unfortunately I can't utilize this in my case.

This is how one implementation of my tests looks like:

public class InteractorNotExtendingBaseInteractor extends Detector implements UastScanner, Detector.ClassScanner {

  public static final Issue ISSUE =
      Issue.create("InteractorNotExtendingBaseInteractor", "Interactor should extend from BaseInteractor",
          "Interactor implementations should extend from BaseInteractor.", Category.MESSAGES, 5, Severity.WARNING,
          new Implementation(InteractorNotExtendingBaseInteractor.class, Scope.JAVA_FILE_SCOPE));

  @Nullable @Override public List<String> applicableSuperClasses() {

    // Not sure what to add in this case
    return null;
  }

  @Override public void visitClass(@NonNull JavaContext context, @NonNull UClass declaration) {
    if (declaration.getName() != null
        && !declaration.getName().equals("BaseInteractor")
        && !declaration.isInterface()
        && declaration.getName().endsWith("Interactor")
        && (declaration.getSuperClass() == null || !inheritsFromBaseInteractor(declaration))) {

      context.report(ISSUE, declaration.getPsi(), context.getLocation(declaration.getPsi()),
          "Interactor should to extend from BaseInteractor");
    }
  }

  private boolean inheritsFromBaseInteractor(UClass declaration) {
    boolean inherits = false;

    while ((declaration != null ? declaration.getSuperClass() : null) != null) {
      if (declaration.getSuperClass().getName() != null && declaration.getSuperClass()
          .getName()
          .equals("BaseInteractor")) {
        inherits = true;
        break;
      }
    }
    return inherits;
  }

  @Override public void checkClass(@NonNull final ClassContext context, @NonNull ClassNode classNode) {

  }
}

 

So all I want to check if, when a class has "Interactor" in it's name (at the end), then this class should inherit/extend from a class called "BaseInteractor". The same logic is working fine for Fragments, since in this case my overwritten applicableSuperClasses method looks like this:

@Override public List<String> applicableSuperClasses() {

  return Arrays.asList(CLASS_FRAGMENT, CLASS_V4_FRAGMENT);
}

So I'm am not sure if there is a way to achieve my goal. Basically I'd like to check classes which are plain java objects and may missing an superclass or interface.

[Tor Norbye]

I see. In that case you don't want to use the applicableSuperClasses+visitClass mechanism; instead you'd do something like this:

    @Override

    public List<Class<? extends UElement>> getApplicableUastTypes() {

        return Collections.singletonList(UClass.class);

    }

    @Override

    public UElementHandler createUastHandler(@NonNull JavaContext context) {

        return new UElementHandler() {

            @Override

            public void visitClass(@NotNull UClass node) {

                // Your logic here

            }

        };

    }

"// Your logic here" is where you put the code you had in the visitClass() method above when using the applicableSuperClass mechanism.

-- Tor

[Tommy Wi]

Thank you very much! Works like charm.

But if I get it right, this method is the "slowest" approach to detect lint issues, since we have to travel through the whole syntax tree?

[Tor Norbye]

On Friday, November 24, 2017 at 2:59:38 AM UTC-8, Tommy Wi wrote:

Thank you very much! Works like charm.

But if I get it right, this method is the "slowest" approach to detect lint issues, since we have to travel through the whole syntax tree?

No, the reason this API exists is precisely to avoid that problem. Take the lint check which looks to see if any of your strings start with "/sdcard":

        override fun visitLiteralExpression(node: ULiteralExpression) {

            val s = node.getValueIfStringLiteral()

            if (s.startsWith("/sdcard/") { ... }

If this lint check used its own visitor to check the file, then it would have to walk through the entire AST to find any string references in the file. 

There are > 300 built in lint checks; this would mean lint would walk through the tree 300 times. For each source file. That would obviously be slow.

So instead, lint asks each lint check that is doing something like the above to tell it which kinds of nodes it's interested in. 

The SdCardDetector says (in the getApplicableUastTypes method) "I care about ULiteralExpression nodes".

Lint asks all the detectors up front which lists of node types they care about, and then builds up a map, from node type to lists of interested detectors.

Then it performs a *single* AST walk through the tree, and for each node type, dispatches to the detectors interested in that node type.

-- Tor