Refactoring of checks messages #139

Closed
daniilyar opened this Issue Sep 24, 2013 · 4 comments

Comments

Projects
None yet
5 participants
Owner

daniilyar commented Sep 24, 2013

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

Member

VadimPanasiuk commented Sep 25, 2013

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

VadimPanasiuk/sevntu.checkstyle@d26bfd3

Owner

romani commented Sep 26, 2013

@VadimPanasiuk, please do pull request.

Owner

daniilyar commented Nov 5, 2013

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

@ghost ghost assigned Alexey2701 Jan 28, 2014

@romani romani assigned maxvetrenko and unassigned Alexey2701 Mar 26, 2014

maxvetrenko added a commit to maxvetrenko/sevntu.checkstyle that referenced this issue Apr 10, 2014

Owner

romani commented Apr 10, 2014

last request was moved to separate issue - #181.

Issue is closed.

@romani romani closed this Apr 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment