MethodTreeImpl cannot be cast to MethodTreeImpl

simonsirak · July 10, 2018, 2:46pm

Hello,

I created a rule that checks whether each non-primitive field in a class is initialized in the constructor/in declaration. I have created several tests that exhaust most possible code variations, compliant and noncompliant, and these all pass.

However, when I try to run the rule on the actual codebase, I get an error saying:

SonarQube is unable to analyze file : '<my-java-file>': org.sonar.java.model.declaration.MethodTreeImpl cannot be cast to org.sonar.java.model.declaration.MethodTreeImpl -> [Help 1]

The nature of the class in which the analyzer fails is that it has no fields and no constructors. It has one static method that uses generic types. However, my rule only ever traverses past MethodTree nodes that have Kind "CONSTRUCTOR" (as you will see below).

When I copy the java class into my file of noncompliant and compliant tests, it analyzes and passes with no problem. It is only when the rule is run directly on the codebase that the analysis fails.

I have extended BaseTreeVisitor in order to implement this rule (and am therefore using visitXXX() to express the rule). I have made sure to override all other visit-methods with empty implementations besides the ones I actually need, in order to not get unexpected behaviour due to the base implementation of these methods from BaseTreeVisitor. All of the relevant code that I wrote and that is actually related to the casting of trees to MethodTree is shown below (“constructors” is a list of MethodTree:s):

In visitClass(ClassTree tree):

    for (Tree t : tree.members()) {
        if (t.is(Kind.CONSTRUCTOR)) {
            constructors.add((MethodTree) t);
        }
    }

In visitMethodInvocation(MethodInvocationTree tree):

    public void visitMethodInvocation(MethodInvocationTree tree) {
        if (tree.symbol().declaration().is(Kind.CONSTRUCTOR)) { // if the method invocation is a constructor invocation
            ...
            scan(tree.symbol().declaration()); // declaration() should be a MethodTree representing the invoked constructor
            ...
        }
    }

In visitMethod(MethodTree tree):

    public void visitMethod(MethodTree tree) {
        if (tree.is(Kind.CONSTRUCTOR)) {
            scan(tree.block());
        }
    }

As you can see in the code, I only ever work with Kind.CONSTRUCTOR, and I always check it before casting (which I only do in 1 place). I honestly don’t understand how this error could occur; any help is greatly appreciated.

Best Regards,
Simon Sirak

PS: I use SonarQube version 5.6.6.

Michael · July 10, 2018, 3:13pm

Hey @simonsirak,

To answer your question: the analysis is failing because you are using an implementation class, which is not allowed for custom rules plugins. You can only rely and cast to classes which are available in the SonarJava syntax tree API: org/sonar/plugins/java/api

Therefore, you can not cast to MethodTreeImpl, as this class won’t be available at runtime. The CheckVerifier that you probably uses to unit test your rule is however executed in a different context, and rely on the totality of the sources of SonarJava… which won’t be the case once your plugin is deployed in a SonarQube instance.

In order to fix the issue, you should cast your constructors into MethodTree. If you need a method from the MethodTreeImpl which is not available in the MethodTree interface, then this is a different story.

A few remark now regarding your implementation:

Every time your override a visitXXX() method, be careful to call somewhere in its body super.visitXXX(), or you will interrupt the visit of the tree, and therefore truncate the exploration.
I’m not sure why you need to visit method invocations, but be careful here, tree.symbol().declaration() can return null when the called method/constructor is physically in a different file than the file currently analyzed. So there is probably NPEs around the corner.
You probably do not need to force the scan of the declaration, it should occur naturally.
If you really only need to visit Constructors, then it’s probably simpler to only override the visitMethod() method. With such approach, you can be sure that the all the constructors will be visited.

  @Override
  public void visitMethod(MethodTree tree) {
    if (tree.is(Tree.Kind.CONSTRUCTOR)) {
      // do some stuff
    }
    super.visitMethod(tree);
  }

If your rule is only targeting field not initialized, then it should be simpler to visit classes, then loop on the members which are VARIABLE, and for each of them check if there is a initializer.

Hope this helps,
Michael

simonsirak · July 10, 2018, 4:00pm

Hi @Michael,

Thank you so much for the reply and the insightful explanations of how to use the API!

I am certain that I have not cast directly to MethodTreeImpl, only to MethodTree as shown in the above code, but thanks for clarifying that only the API is to be used (so that I don’t make that mistake in the future)!

I will now reply to each of your remarks in order, hopefully giving more information about my implementation:

Yes, I will keep this in mind! I actually sort of do “super” by calling scan() on the nodes I deemed were appropriate (similar to the BaseTreeVisitor class which I extend), and this passes all of my tests. But I will keep this in mind, since it is easy to forget some possible subtree.
I visit method invocations to cover the case where constructors are invoked through this(), to provide a more complete scan and ensure that every variable is truly initialized. However I do not check for null, so thanks for catching that! I will make that change!
(similar to 2.)
I will keep that in mind! As of now, I visit each constructor through the visitClass-method, after I’ve collected them into the list constructors in the above code. I do not have my full implementation in front of me at the moment, so I’m unsure if I use visitMethod() directly or if I use scan() to visit each constructor. This might potentially be what is causing the issue (there might be some implicit casting done depending on which of these that I chose).
Yes I do that – the rule checks what you said, and then goes on to check the constructors for any remaining variables!

Hopefully, this new information may give you further clues towards what might be wrong. Unfortunately, I am still somewhat lost on what might cause this error (knowing that I cast only to official interfaces). MethodTreeImpl cannot be cast to MethodTreeImpl is such an odd error. If you (or anyone else) have any further ideas, please share them – they are greatly appreciated!

Best Regards,
Simon Sirak

Michael · July 11, 2018, 6:58am

Hey,

Okay, if there is no explicit cast, then the error is probably somewhere else. Maybe there is an incompatibility between the binaries that your custom plugin is using, and the binaries of SonarJava which are installed on your SonarQube instance.

In the pom.xml file, check that the version of SonarJava and SonarQube API are aligned with the one installed in your SonarQube instance.

  ...

  <dependencies>
    <dependency>
      <groupId>org.sonarsource.sonarqube</groupId>
      <artifactId>sonar-plugin-api</artifactId>
      <!-- Should be your version of SonarQube -->
      <version>5.6.6</version>
      <scope>provided</scope>
    </dependency>

    <dependency>
      <groupId>org.sonarsource.java</groupId>
      <artifactId>sonar-java-plugin</artifactId>
      <type>sonar-plugin</type>
      <!-- Should be the version of the SonarJava plugin -->
      <version>4.15</version>
      <scope>provided</scope>
    </dependency>

   ...

Finally, please note that your version of SonarQube (5.6.6) is quite old, and not supported anymore. The new LTS is version 6.7.x. It also probably implies more recent version of the SonarJava plugin. The same goes for the java plugin. Compatibility with older versions is not guaranteed.

Cheers,
Michael

simonsirak · July 11, 2018, 1:40pm

This may very well be the issue! I will try this and get back to you Thank you for the help, it is greatly appreciated!

Best Regards,
Simon Sirak

simonsirak · July 12, 2018, 8:45am

Hi again,

It turns out there were indeed a bunch of previously implemented rules that used non-api classes. This along with the updating of the POM seems to have fixed the issue! Thanks a ton

Best Regards,
Simon Sirak

Michael · July 12, 2018, 12:54pm

Thanks for coming back with some news, and happy to know that you solved your issue!

Cheers,
Michael