Implementation of Prefer comparison to zero

Hi everyone! first of all sorry if i make any dumb mistakes, i’m new to writing sonarqube rules! i’m trying to write a java rule that checks if the condition of a for cycle is a comparison to zero and if it isn’t, it reports the problem. I am using this rule as a base sonar-java/ForLoopUsedAsWhileLoopCheck.java at master · SonarSource/sonar-java · GitHub since it does something similar in the fact that it checks the condition of the for loop. i’ve modified the visitNode method like this:

		@Override
		  public void visitNode(Tree tree) {
		    ForStatementTree forStatementTree = (ForStatementTree) tree;
		    ExpressionTree condition = forStatementTree.condition();
		    if (condition == null) {
		      context.reportIssue(this, forStatementTree.forKeyword(), "Condition is null");
		    } else if(condition != null) {
		    	BinaryExpressionTree binaryExp = (BinaryExpressionTree) condition;
		    	if(!binaryExp.rightOperand().equals(0)) {
		    		context.reportIssue(this, forStatementTree.forKeyword(), "Prefer Comparison To Zero");
		    	}
		    }
		  }	 

and i’ve been using this test file:

class A {

  private void noncompliant() {
    
    for (int i = 0; i < 5; i++) { // Noncompliant
      // Do Something;
    }
  }

  private void compliant() {
    
    for (int j = 5; j > 0; j--) { // Compliant
      // Do Something
    }
  }

}

but when i run the junit test i still get both for cycles as failures. is there something i’m missing?

Hey there Giuseppe,
it’s always nice to see people trying to implement their own rules and I’m glad to help :slight_smile: Our analyzer API can be tricky to navigate the first times, as it has a lot of different components.

Your issue is that you are comparing the wrong thing here:
if(!binaryExp.rightOperand().equals(0)) {

When you access binaryExp.rightOperand(), what you get is an ExpressionTree which can represent any type of expression, but it’s not inherently a number or a string.

What you are looking for at this point is ExpressionUtils.resolveAsConstant(), which is a utility method that takes an expression and returns an Object if it was possible to retrieve its constant value. (This will not work with non constant variables)

So your visit node should look something like this:

@Override
  public void visitNode(Tree tree) {
    ForStatementTree forStatementTree = (ForStatementTree) tree;
    ExpressionTree condition = forStatementTree.condition();
    if (condition == null) {
      context.reportIssue(this, forStatementTree.forKeyword(), "Condition is null");
    } else {
      BinaryExpressionTree binaryExp = (BinaryExpressionTree) condition;
      Integer constant = (Integer) ExpressionUtils.resolveAsConstant(binaryExp.rightOperand());
      if (constant != 0) {
        context.reportIssue(this, forStatementTree.forKeyword(), "Prefer Comparison To Zero");
      }
    }
  }

Now, you should be aware that if the rightOperand of the binaryExpression is not an integer, the (Integer) cast will fail, so I let you deal with those cases :slight_smile:

P.s. Are you using SonarLint in your IDE? It would have marked your if(!binaryExp.rightOperand().equals(0)) { as an issue saying:
“Remove this call to “equals”; comparisons between unrelated types always return false.”
which would have maybe helped you understand what the problem was!

1 Like

Ciao Leonardo! First of all thank you for your response! In the end, i was able to find a solution by using a piece of code (the isZero function) taken from here: https://github.com/SonarSource/sonar-java/blob/81734f9851dcb1059c00b6bbc7522a515cef874f/java-frontend/src/main/java/org/sonar/java/model/LiteralUtils.java
Now it detects comparison to anything different than zero in While-Loops aswell! Here is the updated code:

package org.sonar.samples.java.checks;

import java.util.List;
import java.util.Arrays;

import org.sonar.check.Priority;
import org.sonar.check.Rule;
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
import org.sonar.plugins.java.api.tree.ForStatementTree;
import org.sonar.plugins.java.api.tree.LiteralTree;
import org.sonar.plugins.java.api.tree.Tree;
import org.sonar.plugins.java.api.tree.WhileStatementTree;
import org.sonar.plugins.java.api.tree.Tree.Kind;
import org.sonarsource.analyzer.commons.annotations.DeprecatedRuleKey;
import org.sonar.plugins.java.api.tree.ExpressionTree;
import org.sonar.plugins.java.api.tree.BinaryExpressionTree;

@Rule(key = "PreferComparisonToZero",
		name = "Developement",
		description = PreferComparisonToZero.MESSAGERULE,
		priority = Priority.MINOR,
		tags = {"performance", "eco-design", "ecocode"})
@DeprecatedRuleKey(repositoryKey = "Giuseppe-CustomRule", ruleKey = "PCTZ")
public class PreferComparisonToZero extends IssuableSubscriptionVisitor {
	
	protected static final String MESSAGERULE = "In Loops Prefer Comparison To Zero";
	
	public static boolean isZero(ExpressionTree tree) {
		return tree.is(Kind.INT_LITERAL) && "0".equals(((LiteralTree) tree).value());
	}
	
	@Override
	public List<Tree.Kind> nodesToVisit() {
		return Arrays.asList(Tree.Kind.FOR_STATEMENT, Kind.WHILE_STATEMENT);
	}
	  
	@Override
	public void visitNode(Tree tree) {
		
		if(tree.is(Kind.FOR_STATEMENT)) {
			ForStatementTree forStatementTree = (ForStatementTree) tree;
			ExpressionTree condition = forStatementTree.condition();
			if (condition != null) {
				BinaryExpressionTree binaryExp = (BinaryExpressionTree) condition;
				if(binaryExp.rightOperand().symbolType().isNumerical()) {
			    		
					if(!isZero(binaryExp.rightOperand())) {
						context.reportIssue(this, forStatementTree.forKeyword(), MESSAGERULE);
					}	
				}
			} 
		} else if (tree.is(Kind.WHILE_STATEMENT)) {
			WhileStatementTree whileStatementTree = (WhileStatementTree) tree;
			ExpressionTree condition = whileStatementTree.condition();
			BinaryExpressionTree binaryExp = (BinaryExpressionTree) condition;
			if(binaryExp.rightOperand().symbolType().isNumerical()) {			    		
				if(!isZero(binaryExp.rightOperand())) {
					context.reportIssue(this, whileStatementTree.whileKeyword(), MESSAGERULE);
				}	
			} 
		} else {
            throw new UnsupportedOperationException("Kind of statement NOT supported - real kind : " + tree.kind().getAssociatedInterface());
        }
	}	 
}

Everything works fine now, i was able to create my own plugin and use it on Sonarqube!

2 Likes