SonarQube

Upgrade to Checkstyle 5.5

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 2.13
  • Component/s: Checkstyle, Squid
  • Labels:
    None
  • Number of attachments :
    1

Description

Before upgrading to Checkstyle 5.5, check that it does indeed integrate the patches SonarSource provided (see related Jira tickets).

Issue Links

is depended upon by

Improvement - An improvement or enhancement to an existing feature or task. SONAR-2938 Remove declarations of third-party repositories

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.
is related to

Sub-task - The sub-task of the issue SONAR-2585 Checkstyle - grammar update for Java 7 support

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Bug - A problem which impairs or prevents the functions of the product. SONAR-1653 'enum' can't be used as an identifier in java source code

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Bug - A problem which impairs or prevents the functions of the product. SONAR-1908 AnalysisException parsing error on escaped unicode

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Bug - A problem which impairs or prevents the functions of the product. SONAR-3032 When the "Redundant Throws" Checkstyle check can't manage to get a class information, a runtime exception is thrown and the analysis of this file is stopped

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Julien HENRY added a comment -

Here is a patch to upgrade checkstyle to 5.3. I have added/modified rule set according to checkstyle release notes, but I don't have the time to validate that each new rule/parameter is working...

Show
Julien HENRY added a comment - Here is a patch to upgrade checkstyle to 5.3. I have added/modified rule set according to checkstyle release notes, but I don't have the time to validate that each new rule/parameter is working...
Hide
Permalink
Evgeny Mandrikov added a comment -

Hi Julien,

I'm able to apply your patch with some modifications. I didn't make a lot of tests, but I definitely can reproduce one bug ( http://sourceforge.net/tracker/?func=detail&aid=3132645&group_id=29721&atid=397078 ). I think that we must wait with upgrade to the next version of checkstyle, even if workaround can be straightforward - just add an empty line to the end of file.

Show
Evgeny Mandrikov added a comment - Hi Julien, I'm able to apply your patch with some modifications. I didn't make a lot of tests, but I definitely can reproduce one bug ( http://sourceforge.net/tracker/?func=detail&aid=3132645&group_id=29721&atid=397078 ). I think that we must wait with upgrade to the next version of checkstyle, even if workaround can be straightforward - just add an empty line to the end of file.
Hide
Permalink
Julien HENRY added a comment -

I agree this can be a major regression for people that don't have an empty line at the end of their files.
So let's wait for a fix of checkstyle.

Show
Julien HENRY added a comment - I agree this can be a major regression for people that don't have an empty line at the end of their files. So let's wait for a fix of checkstyle.
Hide
Permalink
Simon Brandhof added a comment -

Do you know if the bug has been introduced in checkstyle 5.2 or 5.3 ?

Show
Simon Brandhof added a comment - Do you know if the bug has been introduced in checkstyle 5.2 or 5.3 ?
Hide
Permalink
Evgeny Mandrikov added a comment -

I can reproduce it with 5.2 too.

Show
Evgeny Mandrikov added a comment - I can reproduce it with 5.2 too.
Hide
Permalink
Simon Brandhof added a comment -

Ok, so I temporarily remove this issue from release 2.6.

Show
Simon Brandhof added a comment - Ok, so I temporarily remove this issue from release 2.6.
Hide
Permalink
Julien HENRY added a comment -

I reproduce the issue with checkstyle 5.2 so it was certainly introduced in a previous version.

>java -jar checkstyle-5.2-all.jar -c sun_checks.xml Test.java
Starting audit...
[...]\Test.java:0: Il manque un caractÞre NewLine Ó la fin du fichier
[...]\Test.java:0: Missing package-info.java file.
[...]\Test.java:1:17: Exception levÚe : unexpected char: 0xFFFF
Audit done.
Show
Julien HENRY added a comment - I reproduce the issue with checkstyle 5.2 so it was certainly introduced in a previous version. >java -jar checkstyle-5.2-all.jar -c sun_checks.xml Test.java Starting audit... [...]\Test.java:0: Il manque un caractÞre NewLine Ó la fin du fichier [...]\Test.java:0: Missing package -info.java file. [...]\Test.java:1:17: Exception levÚe : unexpected char : 0xFFFF Audit done.
Hide
Permalink
Evgeny Mandrikov added a comment -

Also note that Checkstyle from 5.1 to 5.3 has a memory leak (see SONAR-2511).

Show
Evgeny Mandrikov added a comment - Also note that Checkstyle from 5.1 to 5.3 has a memory leak (see SONAR-2511 ).
Hide
Permalink
Evgeny Mandrikov added a comment - - edited

For information : memory leak was fixed in checkstyle 5.3 - cad2022770b8

Show
Evgeny Mandrikov added a comment - - edited For information : memory leak was fixed in checkstyle 5.3 - cad2022770b8
Hide
Permalink
Evgeny Mandrikov added a comment -

For information : parsing exception (unexpected char: 0xFFFF) was fixed in checkstyle 5.4-SNAPSHOT - 90eadf9cf0b2

Show
Evgeny Mandrikov added a comment - For information : parsing exception (unexpected char: 0xFFFF) was fixed in checkstyle 5.4-SNAPSHOT - 90eadf9cf0b2
Hide
Permalink
Leonard Brünings added a comment -

Could we directly upgrade to Checkstyle 5.4 I need this change

  • Enhanced MagicNumber to support ignoring numbers in annotation declarations.
Show
Leonard Brünings added a comment - Could we directly upgrade to Checkstyle 5.4 I need this change Enhanced MagicNumber to support ignoring numbers in annotation declarations.
Hide
Permalink
Simon Brandhof added a comment -

We were waiting for the availability in maven central repository. It should be possible now. We have to check that the memory leak and the parsing exception are fixed.

Show
Simon Brandhof added a comment - We were waiting for the availability in maven central repository. It should be possible now. We have to check that the memory leak and the parsing exception are fixed.
Hide
Permalink
Julien HENRY added a comment -

Checkstyle available on central: http://repo1.maven.org/maven2/com/puppycrawl/tools/checkstyle/5.4/

Is there anybody working on it? Would you like help?

Show
Julien HENRY added a comment - Checkstyle available on central: http://repo1.maven.org/maven2/com/puppycrawl/tools/checkstyle/5.4/ Is there anybody working on it? Would you like help?
Hide
Permalink
Freddy Mallet added a comment -

Any help is welcome Julien . The most important thing is too list all rule changes between version 5.1 and version 5.4.

Show
Freddy Mallet added a comment - Any help is welcome Julien . The most important thing is too list all rule changes between version 5.1 and version 5.4.
Hide
Permalink
Julien HENRY added a comment -

I already did the work for 5.1 to 5.3. Just need to merge my old patch with i18n. I'm working on a new patch.

Show
Julien HENRY added a comment - I already did the work for 5.1 to 5.3. Just need to merge my old patch with i18n. I'm working on a new patch.
Hide
Permalink
Julien HENRY added a comment -

See my pull request. Warning: not very well tested so may be typos. I'll work on french plugin update.

Show
Julien HENRY added a comment - See my pull request. Warning: not very well tested so may be typos. I'll work on french plugin update.
Hide
Permalink
Julien HENRY added a comment -

Nothing to do in french plugin. Translation of checkstyle is not done yet

Show
Julien HENRY added a comment - Nothing to do in french plugin. Translation of checkstyle is not done yet
Hide
Permalink
Evgeny Mandrikov added a comment -

Guys, looks like there was a typo in my previous comment - parsing exception (unexpected char: 0xFFFF) was not fixed in 5.4, but in 5.5, which is not yet released. Thus and having in consideration SONAR-2585 it might make sense to wait a bit more before upgrade of checkstyle.

Show
Evgeny Mandrikov added a comment - Guys, looks like there was a typo in my previous comment - parsing exception (unexpected char: 0xFFFF) was not fixed in 5.4, but in 5.5, which is not yet released. Thus and having in consideration SONAR-2585 it might make sense to wait a bit more before upgrade of checkstyle.
Hide
Permalink
Thomas added a comment -

Checkstyle 5.5 is out, any updates on this issue?

Show
Thomas added a comment - Checkstyle 5.5 is out, any updates on this issue?
Hide
Permalink
Evgeny Mandrikov added a comment -

Thomas, please be just a bit patient - we know about release as we participated in it. This issue is scheduled for 2.13 and we work on it.

Show
Evgeny Mandrikov added a comment - Thomas, please be just a bit patient - we know about release as we participated in it. This issue is scheduled for 2.13 and we work on it.
Hide
Permalink
Evgeny Mandrikov added a comment -

Upgraded in 3243252e10.

Show
Evgeny Mandrikov added a comment - Upgraded in 3243252e10 .
Hide
Permalink
Mark Mielke added a comment -

Looks like we're waiting for 2.13 - but just to note as I don't see it mentioned in the notes and somebody else might benefit from this:

Using try-with-resources in Java 7 seems to trigger Checkstyle "Inner Assignment" on every single use.

For the time being, I've disabled this check. I'm hoping that Checkstyle 5.5 with official Java 7 support won't have this problem?

Show
Mark Mielke added a comment - Looks like we're waiting for 2.13 - but just to note as I don't see it mentioned in the notes and somebody else might benefit from this: Using try-with-resources in Java 7 seems to trigger Checkstyle "Inner Assignment" on every single use. For the time being, I've disabled this check. I'm hoping that Checkstyle 5.5 with official Java 7 support won't have this problem?
Hide
Permalink
Evgeny Mandrikov added a comment -

Hi Mark,

Thanks for this info. However I'm not sure that Checkstyle 5.5 solves this issue, so it would be better to check this.

Show
Evgeny Mandrikov added a comment - Hi Mark, Thanks for this info. However I'm not sure that Checkstyle 5.5 solves this issue, so it would be better to check this.
Hide
Permalink
Mark Mielke added a comment -

Hi Evgeny:

You are correct. Checkstyle 5.5 continues to give these warnings. I guess I will take this up with them...

Show
Mark Mielke added a comment - Hi Evgeny: You are correct. Checkstyle 5.5 continues to give these warnings. I guess I will take this up with them...
Hide
Permalink
Freddy Mallet added a comment -

Manually tested Evgeny, but could you list in this JIRA ticket all the changes relating to this migration (mainly new rules)

Show
Freddy Mallet added a comment - Manually tested Evgeny, but could you list in this JIRA ticket all the changes relating to this migration (mainly new rules)
Hide
Permalink
Evgeny Mandrikov added a comment -

Freddy, I'm not sure that this is required since Checkstyle provides nice list of changes (in comparison with PMD) - http://checkstyle.sourceforge.net/releasenotes.html
However here is a list of changed rules :

  • New rules from 5.2:
    • InnerTypeLast
  • Modified rules from 5.2:
    • DeclarationOrder
    • ImportOrder
  • New rules from 5.3:
    • OuterTypeFilename
    • NestedForDepth
    • MethodCount
    • OneStatementPerLine
  • Modified rules from 5.3:
    • PackageDeclaration
    • MagicNumber
    • AbstractClassName
    • AvoidStarImport
  • Modified rules from 5.4:
    • UnusedImports
    • MagicNumber
    • EqualsAvoidNull
    • IllegalThrows
    • RedundantModifier
Show
Evgeny Mandrikov added a comment - Freddy, I'm not sure that this is required since Checkstyle provides nice list of changes (in comparison with PMD) - http://checkstyle.sourceforge.net/releasenotes.html However here is a list of changed rules : New rules from 5.2: InnerTypeLast Modified rules from 5.2: DeclarationOrder ImportOrder New rules from 5.3: OuterTypeFilename NestedForDepth MethodCount OneStatementPerLine Modified rules from 5.3: PackageDeclaration MagicNumber AbstractClassName AvoidStarImport Modified rules from 5.4: UnusedImports MagicNumber EqualsAvoidNull IllegalThrows RedundantModifier
Hide
Permalink
Freddy Mallet added a comment -

Thanks Evgeny, Manually tested

Show
Freddy Mallet added a comment - Thanks Evgeny, Manually tested
Hide
Permalink
Freddy Mallet added a comment -

Upcoming IT with a checkstyle-ruling

Show
Freddy Mallet added a comment - Upcoming IT with a checkstyle-ruling

People

  • Assignee:
    Evgeny Mandrikov
    Reporter:
    Simon Brandhof
Vote (13)
Watch (11)

Dates

  • Created:
    Updated:
    Resolved: