Skip to content

Commit

Permalink
Fixed sevntu-checkstyle#72. FinalizeImplementationCheck was introduced.
Browse files Browse the repository at this point in the history
  • Loading branch information
maxvetrenko committed Mar 5, 2014
1 parent 0196a73 commit 581511c
Show file tree
Hide file tree
Showing 7 changed files with 394 additions and 0 deletions.
Expand Up @@ -29,6 +29,9 @@ ConfusingConditionCheck.ignoreSequentialIf = Disable warnings for all sequential
ConfusingConditionCheck.ignoreNullCaseInIf = Disable warnings for "if" if it expression contains "null".
ConfusingConditionCheck.ignoreThrowInElse = Disable warnings for "if" if "else" block contain "throw".

FinalizeImplementationCheck.name = Finalize Implementation Check
FinalizeImplementationCheck.desc = <p>This Check prevents you from incorrect finalize() implementation below:</p><ol><li>// negates effect of superclass finalizer<br/> protected void finalize() { }<br/></li> <li>// fails to call superclass finalize method<br/> protected void finalize() { doSomething(); }</li><li> // useless (or worse) finalizer<br/>protected void finalize() { super.finalize(); }<br/></li><li> // public finalizer<br/> public void finalize() { try { doSomething(); } finally { super.finalize() } }</li><br/></ol>

ForbidCertainImports.name = Forbid Certain Imports
ForbidCertainImports.desc = Forbids certain imports usage in certain packages. <br/><br/>You can configure this check using following parameters:<ol><li>Package qualified name regexp;</li><li>Forbidden imports regexp;</li><li>Forbidden imports excludes regexp.</li></ol>This check reads packages and imports qualified names without words "package","import" and semicolons, so, please, do NOT include "package" or "import" words or semicolons into your regular expressions when configuring.<br/><br/>Real-life example of usage: forbid to use all "*.ui.*" packages in "*.dao.*" packages, but ignore all Exception imports (such as <b>org.springframework.dao.InvalidDataAccessResourceUsageException</b>). For doing that, you should use the following check parameters: <br/><br/><dl><li>Package name regexp = ".*ui.*"</li><li>Forbidden imports regexp = ".*dao.*"</li><li>Forbidden imports excludes regexp = "^.+Exception$"</li></dl><br/>By means of few instances of this check will be possible to cover any rules.<br/><br/>Author: <a href="https://github.com/daniilyar"> Daniil Yaroslavtsev</a>
ForbidCertainImports.packageNameRegexp = Package name regexp.
Expand Down
Expand Up @@ -59,6 +59,16 @@
<message-key key="avoid.modifiers.for.types"/>
</rule-metadata>

<rule-metadata name="%FinalizeImplementationCheck.name" internal-name="FinalizeImplementationCheck" parent="TreeWalker">
<alternative-name internal-name="com.puppycrawl.tools.checkstyle.checks.coding.FinalizeImplementationCheck"/>
<description>%FinalizeImplementationCheck.desc</description>
<message-key key="finalize.implementation.empty"/>
<message-key key="finalize.implementation.missed.try.finally"/>
<message-key key="finalize.implementation.public"/>
<message-key key="finalize.implementation.useless"/>
<message-key key="finalize.implementation.missed.super.finalize"/>
</rule-metadata>

<rule-metadata name="%CustomDeclarationOrder.name" internal-name="CustomDeclarationOrder" parent="TreeWalker">
<alternative-name internal-name="com.puppycrawl.tools.checkstyle.checks.coding.CustomDeclarationOrderCheck"/>
<description>%CustomDeclarationOrder.desc</description>
Expand Down
@@ -0,0 +1,252 @@
////////////////////////////////////////////////////////////////////////////////
// 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;


/**
* <p>
* This Check detects incorrect finalize() implementation below:
* </p>
* <ol><li>
* // negates effect of superclass finalize
* protected void finalize() { } </li>
* <li>
* // fails to call superclass finalize method
* protected void finalize() { doSomething(); }</li>
* <li>
* // useless (or worse) finalize
* protected void finalize() { super.finalize(); }</li>
* <li>
* // public finalize
* public void finalize() { try { doSomething(); } finally { super.finalize() } }</li></ol>
*
* @author <a href="mailto:maxvetrenko2241@gmail.com">Max Vetrenko</a>
*
*/
public class FinalizeImplementationCheck extends Check
{

/**
* The key is pointing to the warning message text in "messages.properties"
* file.
*/
public static final String MSG_KEY_EMPTY_FINALIZE = "finalize.implementation.empty";

/**
* The key is pointing to the warning message text in "messages.properties"
* file.
*/
public static final String MSG_KEY_MISSED_TRY_FINALLY =
"finalize.implementation.missed.try.finally";

/**
* The key is pointing to the warning message text in "messages.properties"
* file.
*/
public static final String MSG_KEY_PUBLIC_FINALIZE = "finalize.implementation.public";

/**
* The key is pointing to the warning message text in "messages.properties"
* file.
*/
public static final String MSG_KEY_USELESS_FINALIZE = "finalize.implementation.useless";

/**
* The key is pointing to the warning message text in "messages.properties"
* file.
*/
public static final String MSG_KEY_MISSED_SUPER_FINALIZE_CALL =
"finalize.implementation.missed.super.finalize";

/**
* The name of finalize() method.
*/
public static final String FINALIZE_METHOD_NAME = "finalize";

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

@Override
public void visitToken(DetailAST aMethodDefToken)
{
if (isFinalizeMethodSignature(aMethodDefToken)) {

DetailAST finalizeMethodToken = aMethodDefToken;
String warningMessage = validateFinalizeMethod(finalizeMethodToken);

if (warningMessage != null) {
log(finalizeMethodToken.getLineNo(), warningMessage);
}
}
}

/**
* Checks, if finalize implementation is correct. If implementation is bad,
* this method will call log() with suitable warning message.
* @param aFinalizeMethodToken
* current finalize() token
* @return warning message or null, if all is well.
*/
private String validateFinalizeMethod(DetailAST aFinalizeMethodToken)
{
String warningMessage = null;
if (hasModifier(TokenTypes.LITERAL_PROTECTED,
aFinalizeMethodToken)) {
if (isMethodEmpty(aFinalizeMethodToken.getLastChild())) {
warningMessage = MSG_KEY_EMPTY_FINALIZE;

} else {
DetailAST methodOpeningBrace = aFinalizeMethodToken.getLastChild();
DetailAST literalTry = methodOpeningBrace.findFirstToken(TokenTypes.LITERAL_TRY);

if (literalTry == null) {
if (containsSuperFinalizeCall(methodOpeningBrace)) {
warningMessage = MSG_KEY_USELESS_FINALIZE;
} else {
warningMessage = MSG_KEY_MISSED_TRY_FINALLY;
}

} else {
DetailAST literalFinally = literalTry.findFirstToken(TokenTypes.LITERAL_FINALLY);

if (literalFinally != null &&
!containsSuperFinalizeCall(literalFinally.getLastChild())) {
warningMessage = MSG_KEY_MISSED_SUPER_FINALIZE_CALL;
}
}
}

} else {
warningMessage = MSG_KEY_PUBLIC_FINALIZE;
}
return warningMessage;
}

/**
* Checks, if current method is finalize().
* @param aMethodDefToken
* current method definition.
* @return true, if method is finalize() method.
*/
public static boolean isFinalizeMethodSignature(DetailAST aMethodDefToken)
{
return !hasModifier(TokenTypes.LITERAL_STATIC ,aMethodDefToken)
&& isFinalizeMethodName(aMethodDefToken) && isVoid(aMethodDefToken)
&& getParamsCount(aMethodDefToken) == 0;
}

/**
* Checks, if finalize() has "static" access modifier.
* @param aModifireType
* modifier type.
* @param aMethodNToken
* MODIFIRES Token.
* @return true, if finalize() has "protected" access modifier.
*/
public static boolean hasModifier(int aModifireType, DetailAST aMethodToken)
{
DetailAST modifiersToken = aMethodToken.getFirstChild();
return modifiersToken.findFirstToken(aModifireType) != null;
}

/**
* Checks, if current method name is "finalize".
* @param aMethodDefToken
* method definition Token.
* @return true, if current method name is "finalize".
*/
private static boolean isFinalizeMethodName(DetailAST aMethodDefToken)
{
DetailAST identToken = aMethodDefToken.findFirstToken(TokenTypes.IDENT);
return FINALIZE_METHOD_NAME.equals(identToken.getText());
}


/**
* Checks, if method is void.
* @param aMethodDefToken
* method definition Token.
* @return true, if method is void.
*/
private static boolean isVoid(DetailAST aMethodDefToken)
{
DetailAST typeToken = aMethodDefToken.findFirstToken(TokenTypes.TYPE);
return typeToken.findFirstToken(TokenTypes.LITERAL_VOID) != null;
}

/**
* Counts number of parameters.
* @param aMethodDefToken
* method definition Token.
* @return number of parameters.
*/
private static int getParamsCount(DetailAST aMethodDefToken)
{
return aMethodDefToken.findFirstToken(TokenTypes.PARAMETERS).getChildCount();
}

/**
* Checks, if finalize() is empty.
* @param aMethodOpeningBraceToken
* method opening brace.
* @return true, if finalize() is empty.
*/
public static boolean isMethodEmpty(DetailAST aMethodOpeningBraceToken)
{
return aMethodOpeningBraceToken.getFirstChild().getType() == TokenTypes.RCURLY;
}

/**
* Checks, if current method has super.finalize() call.
* @param aOpeningBrace
* current method definition.
* @return true, if method has super.finalize() call.
*/
public static boolean containsSuperFinalizeCall(DetailAST aOpeningBrace)
{
DetailAST methodCallToken = aOpeningBrace.getFirstChild().getFirstChild();
if (methodCallToken != null) {
DetailAST dotToken = methodCallToken.getFirstChild();
if (dotToken.findFirstToken(TokenTypes.LITERAL_SUPER) != null) {
return true;
}
}
return false;
}

/**
* Checks, if current method has try-finally block.
* @param aMethodOpeningBrace
* Method opening brace.
* @return
*/
public static boolean hasTryFinallyBlock(DetailAST aMethodOpeningBrace)
{
DetailAST tryToken = aMethodOpeningBrace.findFirstToken(TokenTypes.LITERAL_TRY);
return tryToken != null && tryToken.getLastChild().getType() == TokenTypes.LITERAL_FINALLY;
}
}
Expand Up @@ -29,6 +29,11 @@ explicit.init=Variable ''{0}'' explicitly initialized to ''{1}'' (default value
fall.through=Fall through from previous branch of the switch statement.
fall.through.last=Fall through from the last branch of the switch statement.
final.variable=Variable ''{0}'' should be declared final.
finalize.implementation.empty = Empty finalize() method (it negates effect of superclass finalizer).
finalize.implementation.missed.try.finally = finalize() method should contain try-finally block with super.finalize() call inside finally block.
finalize.implementation.public = finalize() method should have protected visibility.
finalize.implementation.useless = finalize() method is useless: it does nothing except for calling super.finalize().
finalize.implementation.missed.super.finalize = You have to call super.finalize() right after finally opening brace.
forbid.certain.imports=This import should not match ''{0}'' pattern, it is forbidden in {1}.
forbid.c.comments.in.the.method.body=C-style comments (/*...*/) inside method body are not allowed.
forbid.return.in.final.block=Finally block should not contain return statements.
Expand Down
@@ -0,0 +1,34 @@
package com.github.sevntu.checkstyle.checks.coding;

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

import org.junit.Test;

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

public class FinalizeImplementationCheckTest extends BaseCheckTestSupport
{

/**
* Default check configuration
*/
private final DefaultConfiguration mCheckConfig =
createCheckConfig(FinalizeImplementationCheck.class);

@Test
public final void basicTest() throws Exception
{

final String[] expected = {
"21: " + getCheckMessage(MSG_KEY_EMPTY_FINALIZE),
"34: " + getCheckMessage(MSG_KEY_MISSED_TRY_FINALLY),
"47: " + getCheckMessage(MSG_KEY_PUBLIC_FINALIZE),
"61: " + getCheckMessage(MSG_KEY_USELESS_FINALIZE),
"74: " + getCheckMessage(MSG_KEY_MISSED_SUPER_FINALIZE_CALL),
};

verify(mCheckConfig,
getPath("InputFinalizeImplementationCheck.java"), expected);
}
}

0 comments on commit 581511c

Please sign in to comment.