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 146 forks source link

Refactoring of checks messages #139

Closed daniilyar closed 10 years ago

daniilyar commented 11 years ago

Please fix common rules for all messages:

A. Check messages should not include redundant words/phrases such as 'please, ...'. If check generates warnings or errors, there should be no 'please' word, but should be something like 'avoid' , 'fix', 'do' or smth else with short problem description.

Example: message of Confusing condition check ('confusing.condition.check=Please Avoid negation within an "if" expression with an "else" clause.')

B. Check messages should not confuse people with different terminology. Example: incorrect.getter.name=Unexpected getter name. incorrect.setter.name=Unexpected setter name. 'Unexpected' for this case should be changed to 'Incorrect' as message key contains 'incorrect' word already. Second example: mutable.exception=The field ''{0}'' must be declared final. instantiation.avoid=Instantiation of {0} should be avoided.

either 'must' or 'should' have to be used in all messages. Must is a bad variant for most cases so I propose to use 'should' where possible.

Please remove all redundant messages in all messages.properties files. For example, message 'avoid.declare.constants=Please avoid to declare constant(s) in the interface.' is not used at all and should be removed.

Also, please, update some check messages to be both more correct and pithy:

  1. avoid.declare.constants=Please avoid to declare constant(s) in the interface. Constants declaration inside interfaces should be avoided.
  2. avoid.finalizer.method=Avoid using finalizer method. Usage of finalizer method should be avoided
  3. avoid.clone.method=Avoid using clone method. Usage of 'clone' method should be avoided
  4. avoid.return.in.finally=Finally block contains return statement. Finally block should not contain return statements
  5. avoid.not.short.circuit.operators.for.boolean=Not short-circuit Operator ''{0}'' used. Short-circuit operator should be used instead of ''{0}''.
  6. covariant.equals=covariant equals without overriding equals(java.lang.Object). Covariant equals without overriding equals(java.lang.Object).
  7. default.comes.last=Default should be last label in the switch. Default should be the last switch label.
  8. illegal.token=Using ''{0}'' is not allowed. Usage of ''{0}'' is not allowed.
  9. illegal.token.text=Token text matches the illegal pattern ''{0}''. illegal.token.text=Token text matches an illegal pattern ''{0}''.
  10. inline.conditional.avoid=Avoid inline conditionals. Inline conditionals should be avoided.
  11. no.null.for.collections=Method return null instead empty collection. should be: 'Method returns null instead of empty collection.' or 'Method should return empty collection instead of null'
  12. forbid.wildcard.as.return.type=Do not use wildcard types as return types. Wildcard as return type should be avoided.

Propose your variants if you dislike something above

VadimPanasiuk commented 11 years ago

confusing.condition.check and avoid.return.in.finally fixes.

https://github.com/VadimPanasiuk/sevntu.checkstyle/commit/d26bfd3db20ff9f5473e3d40425f3a106e4997a8

romani commented 11 years ago

@VadimPanasiuk, please do pull request.

daniilyar commented 10 years ago

One more suggestion: MaximumLineLengthExtended should print the current line length in warnings.

romani commented 10 years ago

last request was moved to separate issue - https://github.com/sevntu-checkstyle/sevntu.checkstyle/issues/181.

Issue is closed.