Skip to content

Commit

Permalink
Merge 03c1937 into 2f65280
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexkravin
alexkravin committed Sep 8, 2014
2 parents 2f65280 + 03c1937 commit ddfc460
Show file tree
Hide file tree
Showing 7 changed files with 288 additions and 88 deletions.
2 changes: 1 addition & 1 deletion ...vntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ ReturnNullInsteadOfBoolean.name = Returning Null Instead of Boolean
ReturnNullInsteadOfBoolean.desc = Method declares to return Boolean, but returns null
RedundantReturnCheck.name=Redundant Return Check
RedundantReturnCheck.desc=Check code for presence of redundant return
RedundantReturnCheck.desc=<p>Highlight usage redundant returns inside constructors and methods with void result.</p><p>For example:</p><p>1. Non empty constructor</p><code><pre>public HelloWorld(){<br/> doStuff();<br/> return;<br/>}</pre></code><p>2. Method with void result</p><code><pre>public void testMethod1(){<br/> doStuff();<br/> return;<br/>}</pre></code><p>However, if your IDE does not support breakpoints on the method entry, you can allow the use of redundant returns in constructors and methods with void result without code except for 'return;'.</p><p>For example:</p><p>1. Empty constructor</p><code><pre>public HelloWorld(){<br/> return;<br/>}</pre></code><p>2. Method with void result and empty body</p><code><pre>public void testMethod1(){<br/> return;<br/>}</pre></code>@author <a href="mailto:fishh1991@gmail.com">Troshin Sergey</a><br>@author <a href="mailto:maxvetrenko2241@gmail.com">Max Vetrenko</a><br>@author <a href="mailto:nesterenko-aleksey@list.ru">Alexey Nesterenko</a>
RedundantReturnCheck.allowReturnInEmptyMethodsAndConstructors=If True, allow 'return' in empty constructors and methods that return void.

SimpleAccessorNameNotationCheck.name=Simple Accessor Name Notation
Expand Down
2 changes: 1 addition & 1 deletion ...secs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@
<alternative-name internal-name="com.github.sevntu.checkstyle.checks.coding.NoMainMethodInAbstractClass"/>
<description>%NoMainMethodInAbstractClass.desc</description>
</rule-metadata>
-->

<rule-metadata name="%RedundantReturnCheck.name" internal-name="RedundantReturn" parent="TreeWalker">
<alternative-name internal-name="com.github.sevntu.checkstyle.checks.coding.RedundantReturnCheck" />
Expand All @@ -346,7 +347,6 @@
</property-metadata>
<message-key key="redundant.return" />
</rule-metadata>
-->

<rule-metadata name="%SimpleAccessorNameNotationCheck.name" internal-name="SimpleAccessorNameNotation" parent="TreeWalker">
<alternative-name internal-name="com.github.sevntu.checkstyle.checks.coding.SimpleAccessorNameNotationCheck" />
Expand Down
174 changes: 141 additions & 33 deletions ...checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/RedundantReturnCheck.java
View file
Original file line number Diff line number Diff line change
@@ -1,20 +1,84 @@
/*!! DOES NOT WORK PROPERLY !!*/
////////////////////////////////////////////////////////////////////////////////
// checkstyle: Checks Java source code for adherence to a set of rules.
// Copyright (C) 2001-2012 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;

/**
* Highlight the redundant returns in constructors and void methods, 'return' in
* empty void method and constructors may be allowed
*
* <p>
* Highlight usage redundant returns inside constructors and methods with void
* result.
* </p>
* <p>
* For example:
* </p>
* <p>
* 1. Non empty constructor
* </p>
* <code><pre>
* public HelloWorld(){
* doStuff();
* return;
* }</pre></code>
* <p>
* 2. Method with void result
* </p>
* <code><pre>
* public void testMethod1(){
* doStuff();
* return;
* }</pre></code>
* <p>
* However, if your IDE does not support breakpoints on the method entry, you
* can allow the use of redundant returns in constructors and methods with void
* result without code except for 'return;'.
* </p>
* <p>
* For example:
* </p>
* <p>
* 1. Empty constructor
* </p>
* <code><pre>
* public HelloWorld(){
* return;
* }</pre></code>
* <p>
* 2. Method with void result and empty body
* </p>
* <code><pre>
* public void testMethod1(){
* return;
* }</pre></code>
* @author <a href="mailto:fishh1991@gmail.com">Troshin Sergey</a>
* @author <a href="mailto:maxvetrenko2241@gmail.com">Max Vetrenko</a>
* @author <a href="mailto:nesterenko-aleksey@list.ru">Alexey Nesterenko</a>
*/
public class RedundantReturnCheck extends Check {

public static final String MSG_KEY = "redundant.return.check";

// If True, allow 'return' in empty constructors and methods that return void.
private boolean mAllowReturnInEmptyMethodsAndConstructors = true;
private boolean mAllowReturnInEmptyMethodsAndConstructors = false;

public void setAllowReturnInEmptyMethodsAndConstructors(boolean aAllowEmptyBlocks) {
mAllowReturnInEmptyMethodsAndConstructors = aAllowEmptyBlocks;
Expand Down Expand Up @@ -52,34 +116,33 @@ public void visitToken(DetailAST aAst) {
/**
* Return is redundant if he is on the end of objectBlock and the
* objectBlock of the method divided into several tokens.
*
* @param aMethodObjectBlock
* @param aMethodObjectBlockAst
* - a method or constructor object block
*/
private void checkForRedundantReturn(DetailAST aMethodObjectBlock) {
private void checkForRedundantReturn(DetailAST aMethodObjectBlockAst) {

final int methodChildCount = aMethodObjectBlock.getChildCount();
final int methodChildCount = aMethodObjectBlockAst.getChildCount();

if (methodChildCount != 1) {

final int placeForRedundantReturn = aMethodObjectBlock
final int placeForRedundantReturn = aMethodObjectBlockAst
.getLastChild().getPreviousSibling().getType();

final int methodWithSingleChild = 2;

if (methodChildCount > methodWithSingleChild) {

handlePlacesForRedundantReturn(placeForRedundantReturn,
aMethodObjectBlock);
aMethodObjectBlockAst);
} else {

if (!mAllowReturnInEmptyMethodsAndConstructors) {
handlePlacesForRedundantReturn(placeForRedundantReturn,
aMethodObjectBlock);
aMethodObjectBlockAst);
}

if (placeForRedundantReturn == TokenTypes.LITERAL_TRY) {
submitRedundantReturnInTryCatch(aMethodObjectBlock
submitRedundantReturnInTryCatch(aMethodObjectBlockAst
.getFirstChild());
}
}
Expand All @@ -89,20 +152,20 @@ private void checkForRedundantReturn(DetailAST aMethodObjectBlock) {
/**
* @param aType
* - Type of token, where redundant return is expected.
* @param aMethodObjectBlock
* @param aMethodObjectBlockAst
* - A method or constructor object block.
*/
private void handlePlacesForRedundantReturn(int aType,
DetailAST aMethodObjectBlock) {
DetailAST aMethodObjectBlockAst) {

if (aType == TokenTypes.LITERAL_RETURN) {

final DetailAST aLastChild = aMethodObjectBlock.getLastChild();
final DetailAST lastChildAst = aMethodObjectBlockAst.getLastChild();

log(aLastChild.getPreviousSibling().getLineNo());
log(lastChildAst.getPreviousSibling().getLineNo(), MSG_KEY);
} else if (aType == TokenTypes.LITERAL_TRY) {

submitRedundantReturnInTryCatch(aMethodObjectBlock.getFirstChild());
submitRedundantReturnInTryCatch(aMethodObjectBlockAst
.findFirstToken(TokenTypes.LITERAL_TRY));
}
}

Expand All @@ -114,24 +177,24 @@ private void handlePlacesForRedundantReturn(int aType,
*/
private void submitRedundantReturnInTryCatch(DetailAST aTryAst) {

DetailAST astBlockTry = aTryAst.getFirstChild();
DetailAST tryBlockAst = aTryAst.getFirstChild();

handleBlocksTryCatchFinally(astBlockTry.getLastChild()
handleBlocksTryCatchFinally(tryBlockAst.getLastChild()
.getPreviousSibling());

final int catchBlocksAmount = aTryAst
.getChildCount(TokenTypes.LITERAL_CATCH);

for (int i = 0; i < catchBlocksAmount; i++) {

astBlockTry = astBlockTry.getNextSibling();
handleBlocksTryCatchFinally(astBlockTry.getLastChild()
tryBlockAst = tryBlockAst.getNextSibling();
handleBlocksTryCatchFinally(tryBlockAst.getLastChild()
.getLastChild().getPreviousSibling());
}

if (astBlockTry.getNextSibling() != null) {
if (tryBlockAst.getNextSibling() != null) {

handleBlocksTryCatchFinally(astBlockTry.getNextSibling()
handleBlocksTryCatchFinally(tryBlockAst.getNextSibling()
.getLastChild().getLastChild().getPreviousSibling());

}
Expand All @@ -140,22 +203,67 @@ private void submitRedundantReturnInTryCatch(DetailAST aTryAst) {
/**
* Submit a mistake if the try or catch or finally blocks have redundant
* return.
*
* @param aAstReturn
* @param aLastStatementInCatchBlockAst
* - a place where the redundantReturn is expected.
*/
private void handleBlocksTryCatchFinally(DetailAST aAstReturn) {
private void handleBlocksTryCatchFinally(DetailAST aLastStatementInCatchBlockAst) {

if (aAstReturn != null) {
if (aLastStatementInCatchBlockAst != null) {

if (aAstReturn.getType() == TokenTypes.LITERAL_RETURN) {
if (aLastStatementInCatchBlockAst.getType() == TokenTypes.LITERAL_RETURN) {
log(aLastStatementInCatchBlockAst.getLineNo(), MSG_KEY);
}
else {
if (aLastStatementInCatchBlockAst.getFirstChild() != null
&& findLiteralReturn(aLastStatementInCatchBlockAst) != null) {
log(findLiteralReturn(aLastStatementInCatchBlockAst).getLineNo(),
MSG_KEY);
}
}
}
}

log(aAstReturn.getLineNo());
/**
* Looks for literal return in the last statement of a catch block
* @param aLastStatementInCatchBlockAst
* @return
*/
private static DetailAST findLiteralReturn(DetailAST aLastStatementInCatchBlockAst) {
DetailAST literalReturnAst = null;
DetailAST currentNode = aLastStatementInCatchBlockAst;
DetailAST toVisit = currentNode;
while (toVisit != null) {
toVisit = getNextSubTreeNode(toVisit, currentNode);
if (toVisit != null
&& toVisit.getType() == TokenTypes.LITERAL_RETURN) {
literalReturnAst = toVisit;
}
}
currentNode = getNextSubTreeNode(currentNode, aLastStatementInCatchBlockAst);
return literalReturnAst;
}

private void log(int lineNumber) {
log(lineNumber, "redundant.return");
/**
* Gets the next node of a syntactical tree (child of a current node or
* sibling of a current node, or sibling of a parent of a current node)
* @param aCurrentNode
* Current node in considering
* @return Current node after bypassing
*/
private static DetailAST
getNextSubTreeNode(DetailAST aCurrentNode, DetailAST aSubTreeRoot) {
DetailAST toVisit = aCurrentNode.getFirstChild();
while (toVisit == null) {
toVisit = aCurrentNode.getNextSibling();
if (toVisit == null) {
if (aCurrentNode.getParent().equals(aSubTreeRoot)) {
break;
}
aCurrentNode = aCurrentNode.getParent();
}
}
aCurrentNode = toVisit;
return aCurrentNode;
}

}
1 change: 1 addition & 0 deletions ...-checks/src/main/resources/com/github/sevntu/checkstyle/checks/coding/messages.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ no.null.for.collections=Method should return empty collection instead of null.
overridable.method=Overridable method ''{0}'' is called in {1} body.
overridable.method.leads=Calling the method ''{0}'' in {1} body leads to the call of the overridable method ''{2}''.
parameter.assignment=Assignment of parameter ''{0}'' is not allowed.
redundant.return.check=Redundant return.
redundant.throws.classInfo=Unable to get class information for {0}.
redundant.throws.duplicate=Redundant throws: ''{0}'' listed more then one time.
redundant.throws.subclass=Redundant throws: ''{0}'' is subclass of ''{1}''.
Expand Down
Loading

0 comments on commit ddfc460

Please sign in to comment.