Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.13
    • Component/s: None
    • 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

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Evgeny Mandrikov added a comment -

          Upgraded in 3243252e10.

          Show
          Evgeny Mandrikov added a comment - Upgraded in 3243252e10 .
          Hide
          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
          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
          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
          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
          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
          Freddy Mallet added a comment -

          Thanks Evgeny, Manually tested

          Show
          Freddy Mallet added a comment - Thanks Evgeny, Manually tested
          Hide
          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
            • Votes:
              13 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: