Upgrading SonarJava from 4.2.1.6971 to 6.3.2.22818 gives problems our own developed rules

[eraonel (Mikael Petterson)]

Hi,

We upgraded sonar to 7.9.3 and use SonarJava 6.3.2.22818.

We have a set of rules created and used with sonar. When I compile my project with these settings in pom.xml

<java.plugin.version>6.3.2.22818</java.plugin.version>
  <!-- minimal version of SonarQube to support.-->
  <sonar.version>7.9.3</sonar.version>

I get the following error in my project:

11:32:59.299 [main] ERROR org.sonar.java.ast.JavaAstScanner - Unable to parse source file : ‘src/test/files/MemberVarsInitializedCheck.java’
11:32:59.299 [main] ERROR org.sonar.java.ast.JavaAstScanner - Parse error at line 277 column 8: Constructor call must be the first statement in a constructor

The error prone line is:

this(-1, new Object()); // This constructor never initializes the field b

Here is the code in the source file:

public class Test20 {
    private String a;
    private Object b;
    private int c;
    
    // Noncompliant@+1 -- Two noncompliances here; one for each of a and b. @+1 signals that we specify the row below
    public Test20(int c, Object o) { // Noncompliant
        this.c = c;
    }
    
    public Test20(String a) { // Noncompliant
        this.a = a;
        this(-1, new Object()); // This constructor never initializes the field b
    }
    
    public Test20() {
        b = new Object();
        this("");
    }
      
}

What has changed?

If I change to use this as test case ( unsure if it is correct syntax and will do same as verify)

CheckVerifier newVerifier = JavaCheckVerifier.newVerifier();
        newVerifier.withCheck(new MemberVarsInitializedCheck());

instead of this:

JavaCheckVerifier.verify("src/test/files/MemberVarsInitializedCheck.java", new MemberVarsInitializedCheck());

Strange thing is that have the above line for most of my testcases and there it works!

A piece of advice would be greatly appreciated.

//mike

[Michael (Michael Gumowski)]

Hello @eraonel,

Thanks for reaching out, and thanks for updating to a much recent version of the Java Analyzer!

@eraonel

What has changed?

Well, from a 4.2.1 to a 6.3.2, a world has changed. However, for you, the most significant change is that we completely changed the frontend of the Java Analyzer. Instead of relying on our own in-house faulty parsing, semantic and bytecode engine, we decided to move for the eclipse foundation frontend (ECJ).

In that regard, ECJ is much more strict in terms of Java correctness it requires when reading a java file. More particularly, it now requires the file to compile.

Looking at your test code, the two latest constructors are not valid java code, as the call to the this(...) constructors have to be on the first line of the constructor.

    public Test20(String a) {
        this.a = a;
        this(-1, new Object()); // This constructor should be placed on first line of the body, prior to "this.a = a;"
    }
    
    public Test20() {
        b = new Object();
        this("");  // This constructor should be placed on first line of the body, prior to "b = new Object();"
    }

As a consequence, it is most likely that some of your test code will not contain valid java code, and therefore will need to be fixed first. We had to do the same on our side for our 600+ rules, but we strongly believe it’s a win-win situation. At least we are sure that what we read is a valid java code.

Regarding the new verifier, we made sure to keep the same syntax, so everything should work as usual, transparently. Behind the curtain, the various flavor of the verifiers are now much more consistent in terms of verification (there were some loop-holes previously, depending on which method you would have called). Regarding how you should use it, I would recommend to inline everything, like in the following example:

  @Test
  void test() {
    BadFieldNameCheck check = new BadFieldNameCheck();
    check.format = "^[a-zA-Z0-9_]*$";
    JavaCheckVerifier.newVerifier()
      .onFile(testSourcesPath("checks/naming/BadFieldName2.java"))
      .withCheck(check)
      .verifyIssues();
  }

Don’t hesitate to have a look at how we use it in the analyzer, here:

I hope this helps,
Michael

[eraonel (Mikael Petterson)]

Hi Michael

Yes it sounds reasonable that it is more strict and we need to use compilable java code.
We should definitely take a look at the inline. Looks nice!

Thanks for quick support.

//mike