Issue #373: upgrade to checkstyle 8.45.1

[muhlba91]

fix #373

depends on #367 to be pushed to SQ marketplace (unless skipped) merge after #379

[muhlba91]

@romani please verify, i cannot trigger the new check NoWhitespaceBeforeCaseDefaultColon with the provided example:

class Test {
    public void test() {
      switch(1) {
        case 1 : // violation, whitespace before ':' is not allowed here
            break;
        case 2: // ok
            break;
        default : // violation, whitespace before ':' is not allowed here
            break;
    }

    switch(2) {
        case 2: // ok
            break;
        case 3, 4
                 : break; // violation, whitespace before ':' is not allowed here
        case 4,
              5: break; // ok
        default
              : // violation, whitespace before ':' is not allowed here
            break;
    }
  }
}

(example taken from https://checkstyle.org/config_whitespace.html#NoWhitespaceBeforeCaseDefaultColon)

did i miss something? the check shows up in SQ:

[romani]

Pay attention to conflict meanwhile.

I need to take a look, to reproduce by cli first then do build of Sonar to play.

[romani]

Check work by CLI:

/var/tmp$ cat config.xml 
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

    <module name="TreeWalker">
<module name="NoWhitespaceBeforeCaseDefaultColon"/>
    </module>
</module>

/var/tmp$ cat Test.java 
class Test {
    public void test() {
      switch(1) {
        case 1 : // violation, whitespace before ':' is not allowed here
            break;
        case 2: // ok
            break;
        default : // violation, whitespace before ':' is not allowed here
            break;
    }

    switch(2) {
        case 2: // ok
            break;
        case 3, 4
                 : break; // violation, whitespace before ':' is not allowed here
        case 4,
              5: break; // ok
        default
              : // violation, whitespace before ':' is not allowed here
            break;
    }
  }
}

$ java -jar checkstyle-8.45.1-all.jar -c config.xml Test.java
Starting audit...
[ERROR] /var/tmp/Test.java:4:16: ':' is preceded with whitespace. [NoWhitespaceBeforeCaseDefaultColon]
[ERROR] /var/tmp/Test.java:8:17: ':' is preceded with whitespace. [NoWhitespaceBeforeCaseDefaultColon]
[ERROR] /var/tmp/Test.java:16:18: ':' is preceded with whitespace. [NoWhitespaceBeforeCaseDefaultColon]
[ERROR] /var/tmp/Test.java:20:15: ':' is preceded with whitespace. [NoWhitespaceBeforeCaseDefaultColon]
Audit done.
Checkstyle ends with 4 errors.

so issue is somewhere in plugin, or there was mistake in testing steps

[romani]

I confirmed the issue in plugin. ~I tried to activate violations from LineLength (max=40), no violations at all.~

@muhlba91 , lets upgrade version by version to catch where stuff become broken, it make it clear is problems came from library or plugin itself or together. version from 8.42 till 8.45.1 does not introduce any new Checks, so old Checks/violations should work.

[muhlba91]

i can also confirm that SQ creates a correct checkstyle.xml file for analysis and a checkstyle-result.xml is created with all content in it as necessary (all violations are shown). the config xml looks like:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE module PUBLIC "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN" "https://checkstyle.org/dtds/configuration_1_3.dtd">
<!-- Generated by Sonar -->
<module name="Checker">
   <module name="SuppressWarningsFilter" />
   <module name="TreeWalker">
      <module name="SuppressWarningsHolder" />
      <module name="NoWhitespaceBeforeCaseDefaultColon">
         <property name="severity" value="info" />
      </module>
      <module name="SuppressionCommentFilter" />
   </module>
</module>

[romani]

I made LineLength to work, looks like it was not activated in profile. So old Checks work fine, but not a new Check.

I see not ERRORs and exceptions in sonar logs.

[muhlba91]

i found the issue and we had this kind of regression since changing to metadata integration. i believe it did not show up due to most rules being templates (a.k.a. configurable multiple times). we appended "template" to every rule (name) but set the template flag in SQ only if it actually is a template. hence, SQ wasn't able to map the id back correctly and lost this rule's result.

can you please double check as well?

[romani]

GOOD - definition of template is not is checkstyle metadata https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/whitespace/NoWhitespaceBeforeCaseDefaultColonCheck.xml as it is nuance of Sonar plugin.

GOOD - Sonar plugin definition of Template is https://github.com/checkstyle/sonar-checkstyle/blob/a0e401d96e0b7cfb7a79bb0ab690d2aa79366796/src/main/java/org/sonar/plugins/checkstyle/metadata/CheckstyleMetadata.java#L164-L166

[romani]

@muhlba91 , how did you find root problem ? is it a bug in Sonar ? to skip rule silently. I would expect some ERROR somewhere in logs at least.

[muhlba91]

@romani can you restart wercker? it seems to have just stopped and when i press "retry" nothing happens.

requested changed made - i wasn't aware of the source formatting (probably my IntelliJ settings applied by default).

unfortunately there wasn't an error log entry in sonar. i don't know if this is by design or bug but both makes sense to me. you don't want lots of error lines if one tenant/project has unknown rules enabled; still it's not entirely user-friendly...

tbh, i checked the enabled rule again in sonar and saw it had "template" suffixed which looked odd to me. this one was probably coincidence as i wanted to fix the naming anyways until i saw the code where we determined when template was suffixed.

[romani]

Restarted

[sonarcloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication