TouchInstinct
/
checkstyle
307
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Simple fix for bug #1155921. 

RequireThis check now ignores static fields and methods.
This commit is contained in:
Oleg Sukhodolsky 2011-10-13 00:08:33 +04:00
parent 8787efc822
commit 2fe9f501d8
4 changed files with 122 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

114
src/checkstyle/com/puppycrawl/tools/checkstyle/checks/DeclarationCollector.java
View File

@ -23,11 +23,11 @@ import com.google.common.collect.Sets;
import com.puppycrawl.tools.checkstyle.api.Check;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.Set;
/**
* Abstract class for chekcs which need to collect information about
* Abstract class for checks which need to collect information about
* declared members/parameters/variables.
*
* @author o_sukhodolsky
@ -46,11 +46,29 @@ public abstract class DeclarationCollector extends Check
@Override
public void visitToken(DetailAST aAST)
{
final LexicalFrame frame = this.mFrames.current();
switch (aAST.getType()) {
case TokenTypes.PARAMETER_DEF :
case TokenTypes.VARIABLE_DEF : {
case TokenTypes.VARIABLE_DEF : {
final String name =
aAST.findFirstToken(TokenTypes.IDENT).getText();
if (frame instanceof ClassFrame) {
final DetailAST mods =
aAST.findFirstToken(TokenTypes.MODIFIERS);
if (mods.branchContains(TokenTypes.LITERAL_STATIC)) {
((ClassFrame) frame).addStaticMember(name);
}
else {
((ClassFrame) frame).addInstanceMember(name);
}
}
else {
frame.addName(name);
}
break;
}
case TokenTypes.PARAMETER_DEF : {
final DetailAST nameAST = aAST.findFirstToken(TokenTypes.IDENT);
this.mFrames.current().addName(nameAST.getText());
frame.addName(nameAST.getText());
break;
}
case TokenTypes.CLASS_DEF :
@ -58,14 +76,26 @@ public abstract class DeclarationCollector extends Check
case TokenTypes.ENUM_DEF :
case TokenTypes.ANNOTATION_DEF : {
final DetailAST nameAST = aAST.findFirstToken(TokenTypes.IDENT);
this.mFrames.current().addName(nameAST.getText());
frame.addName(nameAST.getText());
this.mFrames.enter(new ClassFrame());
break;
}
case TokenTypes.SLIST :
this.mFrames.enter(new BlockFrame());
break;
case TokenTypes.METHOD_DEF :
case TokenTypes.METHOD_DEF : {
final String name = aAST.findFirstToken(TokenTypes.IDENT).getText();
if (frame instanceof ClassFrame) {
final DetailAST mods =
aAST.findFirstToken(TokenTypes.MODIFIERS);
if (mods.branchContains(TokenTypes.LITERAL_STATIC)) {
((ClassFrame) frame).addStaticMember(name);
}
else {
((ClassFrame) frame).addInstanceMember(name);
}
}
}
case TokenTypes.CTOR_DEF :
this.mFrames.enter(new MethodFrame());
break;
@ -93,17 +123,6 @@ public abstract class DeclarationCollector extends Check
}
}
/**
* Check if given name is a name for declafred variable/parameter/member in
* current environment.
* @param aName a name to check
* @return true is the given name is declare one.
*/
protected final boolean isDeclared(String aName)
{
return (null != mFrames.findFrame(aName));
}
/**
* Check if given name is a name for class field in current environment.
* @param aName a name to check
@ -111,7 +130,9 @@ public abstract class DeclarationCollector extends Check
*/
protected final boolean isClassField(String aName)
{
return (mFrames.findFrame(aName) instanceof ClassFrame);
final LexicalFrame frame = mFrames.findFrame(aName);
return (frame instanceof ClassFrame)
&& ((ClassFrame) frame).hasInstanceMember(aName);
}
/**
@ -122,9 +143,9 @@ public abstract class DeclarationCollector extends Check
private abstract static class LexicalFrame
{
/** Set of name of variables declared in this frame. */
private final HashSet<String> mVarNames;
private final Set<String> mVarNames;
/** constructor -- invocable only via super() from subclasses */
/** constructor -- invokable only via super() from subclasses */
protected LexicalFrame()
{
mVarNames = Sets.newHashSet();
@ -172,6 +193,57 @@ public abstract class DeclarationCollector extends Check
*/
private static class ClassFrame extends LexicalFrame
{
/** Set of name of instance members declared in this frame. */
private final Set<String> mInstanceMembers;
/** Set of name of variables declared in this frame. */
private final Set<String> mStaticMembers;
/**
* Creates new instance of ClassFrame
*/
public ClassFrame()
{
super();
mInstanceMembers = Sets.newHashSet();
mStaticMembers = Sets.newHashSet();
}
/**
* Adds static member's name.
* @param aName a name of static member of the class
*/
public void addStaticMember(final String aName)
{
mStaticMembers.add(aName);
}
/**
* Adds instance member's name.
* @param aName a name of instance member of the class
*/
public void addInstanceMember(final String aName)
{
mInstanceMembers.add(aName);
}
/**
* Checks if a given name is a known instance member of the class.
* @param aName a name to check
* @return true is the given name is a name of a known
* instance member of the class
*/
public boolean hasInstanceMember(final String aName)
{
return mInstanceMembers.contains(aName);
}
@Override
boolean contains(String aNameToFind)
{
return super.contains(aNameToFind)
|| mInstanceMembers.contains(aNameToFind)
|| mStaticMembers.contains(aNameToFind);
}
}
/**

8
src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheck.java
View File

@ -69,7 +69,7 @@ public class RequireThisCheck extends DeclarationCollector
mCheckFields = aCheckFields;
}
/**
* @return true if we should check fields usage false overwise.
* @return true if we should check fields usage false otherwise.
*/
public boolean getCheckFields()
{
@ -85,7 +85,7 @@ public class RequireThisCheck extends DeclarationCollector
mCheckMethods = aCheckMethods;
}
/**
* @return true if we should check methods usage false overwise.
* @return true if we should check methods usage false otherwise.
*/
public boolean getCheckMethods()
{
@ -148,7 +148,7 @@ public class RequireThisCheck extends DeclarationCollector
// let's check method calls
if (parentType == TokenTypes.METHOD_CALL) {
if (mCheckMethods) {
if (mCheckMethods && isClassField(aAST.getText())) {
log(aAST, "require.this.method", aAST.getText());
}
return;
@ -161,7 +161,7 @@ public class RequireThisCheck extends DeclarationCollector
}
if (ScopeUtils.getSurroundingScope(aAST) == null) {
// it is not a class or inteface it's
// it is not a class or interface it's
// either import or package
// we shouldn't checks this
return;

17
src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputRequireThis.java
View File

@ -1,3 +1,5 @@
package com.puppycrawl.tools.checkstyle.coding;
import java.awt.Toolkit;
public class InputRequireThis {
@ -69,4 +71,19 @@ class Bug2123003 {
@interface Rock {
String[] band() default "Metallica";
}
}
class Bug1155921 {
private static int CONST = 1;
private static int static_method() {
return 1;
}
private int method1() {
return CONST;
}
private int method2() {
return static_method();
}
}

16
src/tests/com/puppycrawl/tools/checkstyle/checks/coding/RequireThisCheckTest.java
View File

@ -31,10 +31,10 @@ public class RequireThisCheckTest extends BaseCheckTestSupport
final DefaultConfiguration checkConfig =
createCheckConfig(RequireThisCheck.class);
final String[] expected = {
"6:9: Reference to instance variable 'i' needs \"this.\".",
"12:9: Method call to 'method1' needs \"this.\".",
"26:9: Reference to instance variable 'i' needs \"this.\".",
"51:9: Reference to instance variable 'z' needs \"this.\".",
"8:9: Reference to instance variable 'i' needs \"this.\".",
"14:9: Method call to 'method1' needs \"this.\".",
"28:9: Reference to instance variable 'i' needs \"this.\".",
"53:9: Reference to instance variable 'z' needs \"this.\".",
// "13:9: Unable find where 'j' is declared.",
};
verify(checkConfig,
@ -49,7 +49,7 @@ public class RequireThisCheckTest extends BaseCheckTestSupport
createCheckConfig(RequireThisCheck.class);
checkConfig.addAttribute("checkFields", "false");
final String[] expected = {
"12:9: Method call to 'method1' needs \"this.\".",
"14:9: Method call to 'method1' needs \"this.\".",
};
verify(checkConfig,
getPath("coding" + File.separator + "InputRequireThis.java"),
@ -63,9 +63,9 @@ public class RequireThisCheckTest extends BaseCheckTestSupport
createCheckConfig(RequireThisCheck.class);
checkConfig.addAttribute("checkMethods", "false");
final String[] expected = {
"6:9: Reference to instance variable 'i' needs \"this.\".",
"26:9: Reference to instance variable 'i' needs \"this.\".",
"51:9: Reference to instance variable 'z' needs \"this.\".",
"8:9: Reference to instance variable 'i' needs \"this.\".",
"28:9: Reference to instance variable 'i' needs \"this.\".",
"53:9: Reference to instance variable 'z' needs \"this.\".",
// "13:9: Unable find where 'j' is declared.",
};
verify(checkConfig,