Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable PMD rule UseUnderscoresInNumericLiterals #6579

Closed
pbludov opened this issue Mar 16, 2019 · 6 comments

Comments

Projects
None yet
3 participants
@pbludov
Copy link
Collaborator

commented Mar 16, 2019

https://pmd.github.io/latest/pmd_rules_java_codestyle.html#useunderscoresinnumericliterals

Part of #6536

[INFO] PMD Failure: com.puppycrawl.tools.checkstyle.api.JavadocTokenTypes:1225
Rule:UseUnderscoresInNumericLiterals Priority:3 
Number 10000 should separate every third digit with an underscore.
[INFO] PMD Failure: com.puppycrawl.tools.checkstyle.api.DetailASTTest:288
 Rule:UseUnderscoresInNumericLiterals Priority:3 
Number 30000 should separate every third digit with an underscore.
[INFO] PMD Failure: com.puppycrawl.tools.checkstyle.api.JavadocTokenTypesTest:126
 Rule:UseUnderscoresInNumericLiterals Priority:3 
Number 10000 should separate every third digit with an underscore.
[INFO] PMD Failure: com.puppycrawl.tools.checkstyle.api.JavadocTokenTypesTest:127 
Rule:UseUnderscoresInNumericLiterals Priority:3 
Number 10071 should separate every third digit with an underscore.
...

@pbludov pbludov referenced this issue Mar 16, 2019

Closed

Upgrade to PMD 6.12.0 #6536

0 of 2 tasks complete

@romani romani added the approved label Mar 16, 2019

@romani

This comment has been minimized.

@pbludov

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 16, 2019

We have following options:

  • Enable the rule with minLength of 7 digits
  • Fix all numbers and set NumericLiteralNeedsUnderscore to 5 digits also
  • Disable PMD rule in favor of our check

I vote for the last option.

@rnveach

This comment has been minimized.

Copy link
Member

commented Mar 16, 2019

It is very rare we have such big numbers in our repo. Violations all refer to token types, which the numbers are generated and out of our control. If someone wants to add _ so it is clear how many digits are in a number I am fine.

@romani

This comment has been minimized.

Copy link
Member

commented Mar 17, 2019

I am voting for "Enable the rule with minLength of 7 digits" to match our Check and keep both of them active.
We disable pmd in favor of our Check when rule is misbehaving or conflicts a Check.

@rnveach

This comment has been minimized.

Copy link
Member

commented Mar 17, 2019

Fix was merged

@rnveach rnveach closed this Mar 17, 2019

@rnveach rnveach added this to the 8.19 milestone Mar 17, 2019

Vantuz added a commit to Vantuz/checkstyle that referenced this issue Apr 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.