Java custom rule to track method calls / semantic analysis

[arjen.v...@gmail.com]

HI,

I'm trying to create a custom Java rule to track calls to certain methods. I have working code in my unittest, printing all method calls. 

However when I have my rule in SonarQube 4.5.5, SonarJava 3.4, I get an exception.

Caused by: java.lang.ClassNotFoundException: org.sonar.java.symexecengine.SymbolicExecutionCheck

Code fragment

public class CallGraphVisitor extends SymbolicExecutionCheck {

        // ... 

@Override

protected void onExecutableElementInvocation(ExecutionState executionState, Tree tree, List<ExpressionTree> arguments) {

        // code with instance of checks removed

MethodInvocationTreeImpl methodInvocationTree = (MethodInvocationTreeImpl) tree;

Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) methodInvocationTree.symbol();

String method = methodSymbol.name();

String fullNameClass = ((TypeJavaSymbol) methodSymbol.owner()).getFullyQualifiedName();

List<Type> parametersTypes = methodSymbol.parameterTypes();

System.out.println(callerFile + " " + callerMethod + " -> "+ fullNameClass + "." + method + parametersTypes);

                        

                        // TODO: some code to create issues for calls to X.y() (name/string based filtering, configured by property file)

What am I doing wrong ?

Kind Regards,

Arjen

[Nicolas Peru]

Hi Arjen, 

What is wrong is the class that you extend to implement your check. SymbolicExecutionCheck class is not part of the API for custom rules (and has nothing to do with SemanticAnalysis).  You should be extending IssuableSubscriptionVisitor as in this example : https://github.com/SonarSource/sonar-examples/blob/master/plugins/java-custom-rules/src/main/java/org/sonar/samples/java/AvoidSuperClassCheck.java

Cheers,

Nicolas PERU | SonarSource

Senior Developer
http://sonarsource.com

https://twitter.com/benzonico

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/7c975762-6ce0-4e4c-a30d-859bfc9f25a3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[arjen.v...@gmail.com]

Hi Nicolas,

My approach was based on the implementation of the lock/unlock rule that is part of sonar-java. However you are right, it can be written as a regular rule :-)

Although that still needed some other modifications (not using import org.sonar.java.model.expression.MethodInvocationTreeImpl and TypeJavaSymbol which are also not part of the api).

I do find it annoying that using non-api classes does work from within Unit-tests, but fails at runtime in SonarQube. Is there a way to see this while unit-testing ?

My usual work approach is: 

- write the example class that has some bug-pattern I want to track in Sonar, 

- then write the unit-test, 

- then create a rule based on the closest example from sonar-java,

- then work on this rule until it passes the test...

 it is quite annoying to lose that sense of victory of passing the test, when it turns out your code will 'never' work in actual sonar, only in the test-suite :-)

How does sonar prevent me from using those classes, and is it possible to do that for unit-tests, to detect the problem early ?

Also somewhat related to my fan-out rule question: is there any way to retrieve the fan in of a method (preferably multi-module). If it is not possible, is it on a roadmap ?

Kind Regards,

Arjen

[Michel Pawlak]

Hi, here is a suggestion :

Write a UT for your CheckRegistrar implementation in which you check that none of your checks depend on a sonarqube class if "api" is not in the fully qualified name. This should not be difficult to do using for instance ASM (directly as a UT). Or you could also use a tool such as forbidden-apis (for maven and ant builds) in order to fail your build.

Michel

[Nicolas Peru]

Hi Arjen, 

General rule of thumb is to rely only on interfaces for tree (so you shouldn't have to downcast to any *Impl class). 

There is a very good reason to provide you only an access to the api package (this is done on platform side by some class loader magic). Let imagine we let you use any *Impl class. At one point it would be impossible for us to change/improve implementation without breaking backward compatibility (and you would be one of the complainers about the fact that your 15 325 custom rules are now broken ;) ). 

We usually try to implement our rules with the same tools as we provide for custom rules with one big exception : when we are currently developing a tool (like MethodMatcher for instance). 

And the Lock/Unlock check you are referring to is actually using an approach that we still have not matured enough yet which is why it is not part of the API (and I would recommend to stay away from it as it will most probably be refactored). 

As for method fan : I don't know well this metric, nothing is planned (AFAIK) about it. And I don't exactly see why you would need to use the Lock/Unlock check here. 

AFAIU, computing the fan in would be quite hard with the current state of things : we have this strong limitation of analyzing file by file so computing cross files will require to hack around this (but still might be doable). 

And if I get it well, to put it very simply fan out is the number of return/throw statements so you have all the tools to compute this right away.

Cheers,

Nicolas PERU | SonarSource

Senior Developer
http://sonarsource.com

https://twitter.com/benzonico

[arjen.v...@gmail.com]

Hi, 

I understand the reasons for hiding the implementations, I just didn't notice they were for internal usage only.

The lack of comments in the sonar-java plugin make it difficult to see if things are only for internal usage, like the implementation of the lock/unlock rule.

It is more clear to me now what and why things work :)

Method Fan out is a metric based on the methods/functions called from within a certain function/method <- amount of calls from a method

Method Fan in is a metric based on the methods/functions that calll a certain method <- amount of calls to a method

Like all metrics there are variants for measuring on different levels: class/function. Or taking into account internal+external calls, or only external calls. Or only counting unique calls.

My custom rule is useful to get the fan out, which is relatively easy because it depends only on local code within one method. However Fan-in is much more complex, since it needs the inverse of the relation. You would need to scan all code first to know which methods call some method.

This is especially difficult in multi modules (since the dependency of poms is inverse to the call/usage relationship of methods/class). I think it could be done with a sensor that gathers all calls along the way, and dumps the information after it reaches the root-pom (of a multi module) but even then you would need to re-visit all methods to create violations, and i wouldnt know how that would fit into the Sonar-structure. 

Arjen

[arjen.v...@gmail.com]

Hi,

I just found that a call to org.sonar.plugins.java.api.tree.MethodTree.throwsClauses() results in:

     java.lang.NoSuchMethodError: org.sonar.plugins.java.api.tree.MethodTree.throwsClauses()Lorg/sonar/plugins/java/api/tree/ListTree;

This does not involve any casting to *Impl

SonarQube 4.5.4

Sonar-java 3.1

It does work with Sonar-java 3.4

Any ideas what is going wrong ?

Kind Regards,

Arjen

[Nicolas Peru]

Hi arjjen,  

API has changed in sonar-java 3.4. The reason for this change is to make available all the tokens (so for instance the commas in the throws clause of a method in your example). 

If you have a closer look you will find that the return type of the method is different.

Cheers,

Nicolas PERU | SonarSource

Senior Developer
http://sonarsource.com

https://twitter.com/benzonico

[arjen.v...@gmail.com]

My bad :)

I triple checked if the method did already exist and returned a collection, but didn't notice List vs ListTree :-)