Skip to content

Commit

Permalink
new Check UselessSuperCtorCallCheck implemeted. Fixes sevntu-checksty…
Browse files Browse the repository at this point in the history
  • Loading branch information
alex-zuy committed Nov 11, 2014
1 parent 7fe73be commit a5d94c4
Show file tree
Hide file tree
Showing 11 changed files with 322 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ UnnecessaryParenthesesExtended.ignoreCalculationOfBooleanVariables = C
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.
UselessSuperCtorCallCheck.name = Useless super constructor call
UselessSuperCtorCallCheck.desc = Checks for useless "super()" calls in c-tors.<p>"super()" call could be considered by Check as "useless" in two cases:<p>Case 1. Example:<br><pre>class Dummy {<br> Dummy() {<br> super();<br> }<br> }<br>"super()" call is redundant. This class does not extend anything.</pre><p>Case 2. Example:<br><pre>class Derived extends Base {<br> Derived() {<br> super();<br> }<br> }<br>"super()" is called without parameters. Java compiler automatically inserts a call to the no-argument constructor of the super class. Check has an option "ignoreSuperCtorCallWithoutArgs" to skip thiscase.</pre>
UselessSuperCtorCallCheck.ignoreSuperCtorCallWithoutArgs = Ignore super() calls without arguments even if this class extends some other class.
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".
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,16 @@
<message-key key="unnecessary.paren.string" />
</rule-metadata>

<rule-metadata name="%UselessSuperCtorCallCheck.name" internal-name="UselessSuperCtorCallCheck" parent="TreeWalker">
<alternative-name internal-name="com.github.sevntu.checkstyle.checks.coding.UselessSuperCtorCallCheck"/>
<description>%UselessSuperCtorCallCheck.desc</description>
<property-metadata name="ignoreSuperCtorCallWithoutArgs" datatype="Boolean" default-value="false">
<description>%UselessSuperCtorCallCheck.ignoreSuperCtorCallWithoutArgs</description>
</property-metadata>
<message-key key="useless.super.ctor.call.in.not.derived.class" />
<message-key key="useless.super.ctor.call.without.args" />
</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>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
////////////////////////////////////////////////////////////////////////////////
// checkstyle: Checks Java source code for adherence to a set of rules.
// Copyright (C) 2001-2014 Oliver Burn
//
// This library is free software; you can redistribute it and/or
// modify it under the terms of the GNU Lesser General Public
// License as published by the Free Software Foundation; either
// version 2.1 of the License, or (at your option) any later version.
//
// This library is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
// Lesser General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public
// License along with this library; if not, write to the Free Software
// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
////////////////////////////////////////////////////////////////////////////////

package com.github.sevntu.checkstyle.checks.coding;

import com.puppycrawl.tools.checkstyle.api.Check;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;

/**
* Checks for useless "super()" calls in c-tors.
* <p>
* "super()" call could be considered by Check as "useless" in two cases:
* <p>
* Case 1. Example:<br>
* <pre>
* class Dummy {<br> Dummy() {<br> super();<br> }<br> }<br>
* "super()" call is redundant. This class does not extend
* anything.
* </pre>
* <p>
* Case 2. Example:<br>
* <pre>
* class Derived extends Base {<br> Derived() {<br> super();<br> }<br> }<br>
* "super()" is called without parameters. Java compiler
* automatically inserts a call to the no-argument constructor of the
* superclass. Check has an option "ignoreSuperCtorCallWithoutArgs" to skip this
* case.
* </pre>
*/
public class UselessSuperCtorCallCheck extends Check
{
/**
* Warning message key.
*/
public static final String MSG_KEY_SUPER_CALL_IN_NOT_DERIVED_CLASS = "useless.super.ctor.call.in.not.derived.class";

/**
* Warning message key.
*/
public static final String MSG_KEY_SUPER_CALL_WITHOUT_ARGS = "useless.super.ctor.call.without.args";

/**
* Used to ignore "super()" calls without arguments in derived class. By
* default check will log this case.
*/
private boolean mIgnoreSuperCtorCallWithoutArgs = false;

/**
* Sets flag to IgnoreSuperCallWithoutArgs.
* @param aIgnoreSuperCtorCallWithoutArgs
* if true, check will ignore super() calls without arguments
*/
public void setIgnoreSuperCtorCallWithoutArgs(boolean aIgnoreSuperCtorCallWithoutArgs)
{
mIgnoreSuperCtorCallWithoutArgs = aIgnoreSuperCtorCallWithoutArgs;
}

@Override
public int[] getDefaultTokens()
{
return new int[] { TokenTypes.SUPER_CTOR_CALL };
}

@Override
public void visitToken(DetailAST aSuperCallNode)
{
if (isSuperArgListEmpty(aSuperCallNode))
{
DetailAST classDefNode = getClassDefinitionNode(aSuperCallNode);

if (isClassDerived(classDefNode))
{
if (!mIgnoreSuperCtorCallWithoutArgs)
{
log(aSuperCallNode.getLineNo(), MSG_KEY_SUPER_CALL_WITHOUT_ARGS);
}
}
else
{
String className = classDefNode.findFirstToken(TokenTypes.IDENT).getText();
log(aSuperCallNode.getLineNo(), MSG_KEY_SUPER_CALL_IN_NOT_DERIVED_CLASS, className);
}
}
}

/**
* Checks whether this super c-tor call has no arguments.
* @param aSuperCallNode
* node of type TokenTypes.SUPER_CTOR_CALL
* @return true, if super constructor call has no arguments
*/
private static boolean isSuperArgListEmpty(DetailAST aSuperCallNode)
{
DetailAST argsListNode = aSuperCallNode.findFirstToken(TokenTypes.ELIST);

return argsListNode.getChildCount() == 0;
}

/**
* Returns class definition node which contains c-tor with given super
* c-tor. call node
* @param aSuperCallNode
* node of type TokenTypes.SUPER_CTOR_CALL
* @return class definition node
*/
private static DetailAST getClassDefinitionNode(DetailAST aSuperCallNode)
{
DetailAST result = aSuperCallNode;

while (result.getType() != TokenTypes.CLASS_DEF)
{
result = result.getParent();
}

return result;
}

/**
* Checks whether this class is derived from other class.
* @param aClassDefNode
* class definition node
* @return true, if this class extends anything
*/
private static boolean isClassDerived(DetailAST aClassDefNode)
{
return aClassDefNode.findFirstToken(TokenTypes.EXTENDS_CLAUSE) != null;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,7 @@ unnecessary.paren.ident=Unnecessary parentheses around identifier ''{0}''.
unnecessary.paren.literal=Unnecessary parentheses around literal ''{0}''.
unnecessary.paren.return=Unnecessary parentheses around return value.
unnecessary.paren.string=Unnecessary parentheses around string {0}.
useless.super.ctor.call.in.not.derived.class= Super call could be removed: Class ''{0}'' does not extend anything.
useless.super.ctor.call.without.args= Redundant super constructor call could be removed. Java compiler automatically inserts a call to the no-argument constructor of the superclass.
return.null.Boolean=Method declares to return Boolean and returns null.
return.boolean.ternary=Returning explicit boolean from ternary operator.
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package com.github.sevntu.checkstyle.checks.coding;

import static com.github.sevntu.checkstyle.checks.coding.UselessSuperCtorCallCheck.*;

import org.junit.Test;

import com.github.sevntu.checkstyle.BaseCheckTestSupport;
import com.puppycrawl.tools.checkstyle.DefaultConfiguration;

public class UselessSuperCtorCallCheckTest extends BaseCheckTestSupport
{
private final String msgSuperWithoutArgs = getCheckMessage(MSG_KEY_SUPER_CALL_WITHOUT_ARGS);

private final DefaultConfiguration mDefaultConfig = createCheckConfig(UselessSuperCtorCallCheck.class);

@Test
public void testSingleCtorWithSuperWithinNotDerivedClass()
throws Exception
{
final String expected[] = {
"7: " + getCheckMessage(MSG_KEY_SUPER_CALL_IN_NOT_DERIVED_CLASS,
"InputUselessSuperCtorCall1"),
};

verify(mDefaultConfig, getPath("InputUselessSuperCtorCall1.java"), expected);
}

@Test
public void testSingleCtorWithSuperWithinDerivedClass()
throws Exception
{
final String expected[] = {
"7: " + msgSuperWithoutArgs,
};

verify(mDefaultConfig, getPath("InputUselessSuperCtorCall2.java"), expected);
}

@Test
public void testMultipleCtorsWithSuperWithinNonsubclass()
throws Exception
{
final String expected[] = {
"7: " + getCheckMessage(MSG_KEY_SUPER_CALL_IN_NOT_DERIVED_CLASS,
"InputUselessSuperCtorCall3"),
"12: " + getCheckMessage(MSG_KEY_SUPER_CALL_IN_NOT_DERIVED_CLASS,
"InputUselessSuperCtorCall3"),
};

verify(mDefaultConfig, getPath("InputUselessSuperCtorCall3.java"), expected);
}

@Test
public void testInnerClassWithCtorWithSuper()
throws Exception
{
final String expected[] = {
"9: " + getCheckMessage(MSG_KEY_SUPER_CALL_IN_NOT_DERIVED_CLASS,
"Inner"),
};

verify(mDefaultConfig, getPath("InputUselessSuperCtorCall4.java"), expected);
}

@Test
public void testClassWithSuperCtorWithArgs()
throws Exception
{
final String expected[] = {};

verify(mDefaultConfig, getPath("InputUselessSuperCtorCall5.java"), expected);
}

@Test
public void testClassWithSuperCtorWithoutArgsAndIgnoreSuperCtorCallOptionSetToTrue()
throws Exception
{
final DefaultConfiguration checkConfig = createCheckConfig(UselessSuperCtorCallCheck.class);
checkConfig.addAttribute("ignoreSuperCtorCallWithoutArgs", "true");

final String expected[] = {};

verify(checkConfig, getPath("InputUselessSuperCtorCall6.java"), expected);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.github.sevntu.checkstyle.checks.coding;

public class InputUselessSuperCtorCall1
{
public InputUselessSuperCtorCall1()
{
super();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.github.sevntu.checkstyle.checks.coding;

public class InputUselessSuperCtorCall2 extends java.lang.Object
{
public InputUselessSuperCtorCall2()
{
super();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.github.sevntu.checkstyle.checks.coding;

public class InputUselessSuperCtorCall3
{
public InputUselessSuperCtorCall3()
{
super();
}

public InputUselessSuperCtorCall3(int i)
{
super();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.github.sevntu.checkstyle.checks.coding;

public class InputUselessSuperCtorCall4 extends java.lang.Object
{
public class Inner
{
public Inner()
{
super();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package com.github.sevntu.checkstyle.checks.coding;

import java.io.File;
import java.io.PrintWriter;

public class InputUselessSuperCtorCall5
{
private static class Base
{
public Base(int i)
{

}
}

private static class Derived extends Base
{
public Derived()
{
super(2);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.github.sevntu.checkstyle.checks.coding;

public class InputUselessSuperCtorCall6 extends java.lang.Object
{
public InputUselessSuperCtorCall6() {
super();
}
}

0 comments on commit a5d94c4

Please sign in to comment.