How can I create custom java rule for log.error method calls?

SonarQube Server / Community Build Writing rules
, ,
1

I am trying to create a java rule for my java repositories.

My objective is to create this rule:
Any object derived from Exception should not be passed as argument in log.error method.

I have followed the https://github.com/SonarSource/sonar-java/blob/master/docs/CUSTOM_RULES_101 and created the rule as such:

@Rule(key = "AvoidLogErrorWithExceptionRule")
public class AvoidLogErrorWithExceptionCheck extends IssuableSubscriptionVisitor {

  @Override
  public List<Tree.Kind> nodesToVisit() {
    return Collections.singletonList(Tree.Kind.METHOD_INVOCATION);
  }

  @Override
  public void visitNode(Tree tree) {
    MethodInvocationTree methodInvocation = (MethodInvocationTree) tree;
    if (methodInvocation.methodSymbol().name().equals("error") &&
      methodInvocation.methodSymbol().owner().name().equalsIgnoreCase("log")) {
      // Check arguments for types or subtypes of Exception
      for (ExpressionTree argument : methodInvocation.arguments()) {
        Type argumentType = argument.symbolType();
        if (argumentType.isSubtypeOf("java.lang.Exception")) {
          reportIssue(argument, "Avoid passing Exception or subclasses to log.error directly.");
        }
      }
    }
  }
}

For log.error statements in testfile, I see that it shows unknownMethod and unknownTypeSymbol etc.

Example testfile:

@Slf4j
public class TrialClass {
    int doSomething(int a) {
        try{
            //do something
        } catch (Exception e){
            log.error("Caught exception",e);
        }
        return 0;
    }
}

Also I have gone through Java rule for redundant log actions, but all the repos usually implement annotation based loggers. Does that mean there is no way of implementing this rule?

4

Hi @Rishabh_Kumar_Singh,

Welcome to the community!

The rule is not impossible to implement, but the main question is what level of accuracy you are looking for. To implement a rule, you don’t necessarily need the symbols; you can match on the AST and use the identifiers. Symbols help to provide more accuracy than just using the AST.

At Sonar when developing a rule, we optimize for accuracy and generally avoid raising errors when symbols are not available. However, this approach may not necessarily apply to your case. In your case, it is likely that the symbols will be unknown most of the time. You can try without and see if the result is satisfying.

Below is an example of the first part of your rule that doesn’t use the node symbols. I hope it helps. When developing a rule, a good tip is to use the debugger: put a breakpoint in the visitNode method and then use the debugger to explore the tree. You can then use this information to match exactly on the node you are expecting.

  @Override
  public void visitNode(Tree tree) {

    MethodInvocationTree mit = (MethodInvocationTree) tree;

    if(mit.methodSelect() instanceof  MemberSelectExpressionTree select
      // we are only interested in the case where select.expression() is an Identifier
      && select.expression() instanceof IdentifierTree ident
      && ident.name().equalsIgnoreCase("log")
      && select.identifier().name().equals("error")
    ){
      // rest of the code
    }

  }

Best,
Erwan

1 Like
5

Although the rule’s accuracy decreases, I think this is the best I can have right now.
Thank you.