sevntu-checkstyle / sevntu.checkstyle

Additional Checkstyle checks, that could be added as extension to EclipseCS plugin and maven-checkstyle-plugin, Sonar checkstyle plugin, extension for CheckStyle IDEA plugin.
http://sevntu-checkstyle.github.io/sevntu.checkstyle/
190 stars 147 forks source link

NumericLiteralNeedsUnderscore's new option to skeep variables by name #425

Closed romani closed 8 years ago

romani commented 8 years ago

all details : https://travis-ci.org/checkstyle/checkstyle/jobs/103024859

code coverage should be 100%, so line https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/master/sevntu-checks/pom.xml#L134 should be removed.

romani commented 8 years ago

@cypai , please address this ASAP.

please generate reports (by chekcstyle-tester project) over other projects to make sure there are no other code that need to be ignored.

cypai commented 8 years ago

Will be done this week

cypai commented 8 years ago

@romani I think exactly which variables to be ignored merits some discussion. Like, should only variable declarations be ignored? For example:

public static final long serialVersionUID = 1234567890L;  // This is ignored

public void bar() {
  long foo = 1234567890L;  // This is ignored
  foo -= 1234567;  // What about this?
  foo = 1234567;  // What about this?
}

(By the way, you really need to fix your formatter for eclipse, I needed to fix indentation and other whitespace problems manually because it didn't behave correctly, and it wasn't fun when saving caused everything to mess up again)

romani commented 8 years ago

We need to ignore variable or field that could have generated value. For now we know only about fields that could have generated fields that people usually do not care but that used in massive amount of code.

For now, lets make ignore option only for field names, serialVetsionID is default value.

After update please run Check against some opensource code to make sure it works and nothing new need to be created. You can use contribution/checkstyle-tester project to generate html report.

cypai commented 8 years ago

From discussion: Ignore configuration should be called ignoreFieldNamePattern. It should be done as a Regexp. These should only care about class fields; if a number is used in a function then it is significant and should be checked. Reference the main checkstyle project for examples of getting fields.

Afterwards, run it on Spring, Guava, and Orekit to see what comes up.

romani commented 8 years ago

default value of ignoreFieldNamePattern is "^serialVersionUID$" .

Try to run your Check at your private projects.

romani commented 8 years ago

@cypai , please finish this issue.

romani commented 8 years ago

@cypai , please send PR checkstyle repo to start usage of this Check. I will do release of sevntu tomorrow.

FYI: All senvtu Checks have to be used in main repo - checkstyle. config - https://github.com/checkstyle/checkstyle/blob/master/config/checkstyle_sevntu_checks.xml

romani commented 8 years ago

@cypai ,

New option is introduces but eclipse-cs/sonar/default-config are not updated:

sevntu.checkstyle/sevntu-checkstyle-sonar-plugin/src/main/resources/com/github/sevntu/checkstyle/sonar/checkstyle-extensions.xml sevntu.checkstyle/eclipsecs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.xml

New option need to be added here too with its default value (as it is example of usage of all properties): sevntu.checkstyle/gh-pages/sevntu-checkstyle-default-configuration.xml

Without such update, release is useless. Please confirm that Elcipse-cs and Sonar works fine on new configurations, property change is working fine.

romani commented 8 years ago

fixes are merged.