Skip to content

Commit

Permalink
Fixes sevntu-checkstyle#82. EitherLogOrThrowCheck was introduced.
Browse files Browse the repository at this point in the history
  • Loading branch information
baratali committed Sep 13, 2013
1 parent 5669e87 commit da3c698
Show file tree
Hide file tree
Showing 6 changed files with 1,019 additions and 0 deletions.
Expand Up @@ -108,3 +108,8 @@ UnnecessaryParenthesesExtended.name = Unnecessary Parentheses Extended
UnnecessaryParenthesesExtended.ignoreCalculationOfBooleanVariables = Cancel validation setups of unnecessary parentheses in Boolean computations.
UnnecessaryParenthesesExtended.ignoreCalculationOfBooleanVariablesWithReturn = Cancel validation setups of unnecessary parentheses in Boolean computations with return state.
UnnecessaryParenthesesExtended.ignoreCalculationOfBooleanVariablesWithAssert = Cancel validation setups of unnecessary parentheses in Boolean computations with assert state.
EitherLogOrThrowCheck.desc = <p>Either log the exception, or throw it, but never do both. Logging and throwing results in multiple log messages for a single problem in the code, and makes problems for the support engineer who is trying to dig through the logs. This is one of the most annoying error-handling antipatterns. All of these examples are equally wrong.</p><p><b>Examples:</b><pre>catch (NoSuchMethodException e) {\t\nLOG.error("Message", e);\t\nthrow e;\n}</pre><b>or</b><pre>catch (NoSuchMethodException e) {\t\nLOG.error("Message", e);\t\nthrow new MyServiceException("AnotherMessage", e);\n}</pre><b>or</b><pre>catch (NoSuchMethodException e) {\t\ne.printStackTrace();\t\nthrow new MyServiceException("Message", e);\n}</pre></p><p><b>What check can detect:</b> <br><b>Loggers</b><ul><li>logger is declared as class field</li><li>logger is declared as method's local variable</li><li>logger is declared as local variable in <code>catch</code> block</li><li>logger is passed through method's parameters</li></ul><b>Exceptions</b><ul><li>logger logs <code>catch</code> parameter exception or it's message</li><li>throw <code>catch</code> parameter exception</li><li>throw another exception which is based on <code>catch</code> parameter exception</li><li>printStackTrace was called on <code>catch</code> parameter exception</li></ul></p><b>What check can not detect:</b> <br><ul><li>loggers that is used like method's return value. Example:<pre>getLogger().error(&quot;message&quot;, e)</pre></li> <li>loggers that is used like static fields from another classes:<pre>MyAnotherClass.LOGGER.error("message", e);<pre></li></ul></p><p>Default parameters are:<ul><li><b>loggerFullyQualifiedClassName</b> - fully qualified class name of logger type. Default value is <i>"org.slf4j.Logger"</i>.</li><li><b>loggingMethodNames</b> - comma separated names of logging methods. Default value is <i>"error, warn, info, debug"</i>.</li></ul></p><p>Note that check works with only one logger type. If you have multiple different loggers, then create another instance of this check.</p>
EitherLogOrThrowCheck.name = Either log exception or throw exception.
EitherLogOrThrowCheck.loggerFullyQualifiedClassName = Logger fully qualified class name. Example: "org.slf4j.Logger".
EitherLogOrThrowCheck.loggingMethodNames = Logging method names separated with commas. Example: "error,warn".
11 changes: 11 additions & 0 deletions ...secs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.xml 100755 → 100644
Expand Up @@ -262,6 +262,17 @@
<message-key key="unnecessary.paren.return" />
<message-key key="unnecessary.paren.string" />
</rule-metadata>

<rule-metadata name="%EitherLogOrThrowCheck.name" internal-name="EitherLogOrThrowCheck" parent="TreeWalker">
<alternative-name internal-name="com.github.sevntu.checkstyle.checks.coding.EitherLogOrThrowCheck" />
<description>%EitherLogOrThrowCheck.desc</description>
<property-metadata name="loggerFullyQualifiedClassName" datatype="String" default-value="org.slf4j.Logger">
<description>%EitherLogOrThrowCheck.loggerFullyQualifiedClassName</description>
</property-metadata>
<property-metadata name="loggingMethodNames" datatype="String" default-value="error, warn, info, debug">
<description>%EitherLogOrThrowCheck.loggerMethodNames</description>
</property-metadata>
</rule-metadata>

<!-- DOES NOT WORK PROPERLY
<rule-metadata name="%NoMainMethodInAbstractClass.name" internal-name="NoMainMethodInAbstractClass" parent="TreeWalker">
Expand Down

0 comments on commit da3c698

Please sign in to comment.