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

Merge pull request #42 from isopov/issue-41 

fix for #41 RequireThisCheck doesn't check methods
This commit is contained in:
Roman Ivanov 2013-10-31 16:32:32 -07:00
parent f010eb4786 53d1722bdb
commit 96eca118f5
4 changed files with 167 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -19,11 +19,15 @@
package com.puppycrawl.tools.checkstyle.checks;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
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.LinkedList;
import java.util.Deque;
import java.util.Map;
import java.util.Queue;
import java.util.Set;
/**
@ -34,19 +38,67 @@ import java.util.Set;
*/
public abstract class DeclarationCollector extends Check
{
/** Stack of variable declaration frames. */
private FrameStack mFrames;
/**
* Tree of all the parsed frames
*/
private Map<DetailAST, LexicalFrame> mFrames;
/**
* Frame for the currently processed AST
*/
private LexicalFrame mCurrent;
@Override
public void beginTree(DetailAST aRootAST)
{
mFrames = new FrameStack();
final Deque<LexicalFrame> aFrameStack = Lists.newLinkedList();
aFrameStack.add(new GlobalFrame());
mFrames = Maps.newHashMap();
DetailAST curNode = aRootAST;
while (curNode != null) {
collectDeclarations(aFrameStack, curNode);
DetailAST toVisit = curNode.getFirstChild();
while (curNode != null && toVisit == null) {
endCollectingDeclarations(aFrameStack, curNode);
toVisit = curNode.getNextSibling();
if (toVisit == null) {
curNode = curNode.getParent();
}
}
curNode = toVisit;
}
}
@Override
public void visitToken(DetailAST aAST)
{
final LexicalFrame frame = this.mFrames.current();
switch (aAST.getType()) {
case TokenTypes.CLASS_DEF :
case TokenTypes.INTERFACE_DEF :
case TokenTypes.ENUM_DEF :
case TokenTypes.ANNOTATION_DEF :
case TokenTypes.SLIST :
case TokenTypes.METHOD_DEF :
case TokenTypes.CTOR_DEF :
this.mCurrent = this.mFrames.get(aAST);
break;
default :
// do nothing
}
} // end visitToken
/**
* Parse the next AST for declarations
*
* @param aFrameStack Stack containing the FrameTree being built
* @param aAST AST to parse
*/
private void collectDeclarations(Deque<LexicalFrame> aFrameStack,
DetailAST aAST)
{
final LexicalFrame frame = aFrameStack.peek();
switch (aAST.getType()) {
case TokenTypes.VARIABLE_DEF : {
final String name =
@ -77,11 +129,11 @@ public abstract class DeclarationCollector extends Check
case TokenTypes.ANNOTATION_DEF : {
final DetailAST nameAST = aAST.findFirstToken(TokenTypes.IDENT);
frame.addName(nameAST.getText());
this.mFrames.enter(new ClassFrame());
aFrameStack.addFirst(new ClassFrame(frame));
break;
}
case TokenTypes.SLIST :
this.mFrames.enter(new BlockFrame());
aFrameStack.addFirst(new BlockFrame(frame));
break;
case TokenTypes.METHOD_DEF : {
final String name = aAST.findFirstToken(TokenTypes.IDENT).getText();
@ -97,7 +149,7 @@ public abstract class DeclarationCollector extends Check
}
}
case TokenTypes.CTOR_DEF :
this.mFrames.enter(new MethodFrame());
aFrameStack.addFirst(new MethodFrame(frame));
break;
default:
// do nothing
@ -105,8 +157,14 @@ public abstract class DeclarationCollector extends Check
}
@Override
public void leaveToken(DetailAST aAST)
/**
* End parsing of the AST for declarations.
*
* @param aFrameStack Stack containing the FrameTree being built
* @param aAST AST that was parsed
*/
private void endCollectingDeclarations(Queue<LexicalFrame> aFrameStack,
DetailAST aAST)
{
switch (aAST.getType()) {
case TokenTypes.CLASS_DEF :
@ -116,7 +174,7 @@ public abstract class DeclarationCollector extends Check
case TokenTypes.SLIST :
case TokenTypes.METHOD_DEF :
case TokenTypes.CTOR_DEF :
this.mFrames.leave();
this.mFrames.put(aAST, aFrameStack.poll());
break;
default :
// do nothing
@ -130,11 +188,26 @@ public abstract class DeclarationCollector extends Check
*/
protected final boolean isClassField(String aName)
{
final LexicalFrame frame = mFrames.findFrame(aName);
final LexicalFrame frame = findFrame(aName);
return (frame instanceof ClassFrame)
&& ((ClassFrame) frame).hasInstanceMember(aName);
}
/**
* Find frame containing declaration
* @param aName name of the declaration to find
* @return LexicalFrame containing declaration or null
*/
private LexicalFrame findFrame(String aName)
{
if (mCurrent != null) {
return mCurrent.getIfContains(aName);
}
else {
return null;
}
}
/**
* A declaration frame.
* @author Stephen Bloch
@ -144,10 +217,19 @@ public abstract class DeclarationCollector extends Check
{
/** Set of name of variables declared in this frame. */
private final Set<String> mVarNames;
/**
* Parent frame.
*/
private final LexicalFrame mParent;
/** constructor -- invokable only via super() from subclasses */
protected LexicalFrame()
/**
* constructor -- invokable only via super() from subclasses
*
* @param aParent parent frame
*/
protected LexicalFrame(LexicalFrame aParent)
{
mParent = aParent;
mVarNames = Sets.newHashSet();
}
@ -167,6 +249,23 @@ public abstract class DeclarationCollector extends Check
{
return mVarNames.contains(aNameToFind);
}
/** check whether the frame contains a given name.
* @param aNameToFind the name we're looking for
* @return whether it was found
*/
LexicalFrame getIfContains(String aNameToFind)
{
if (contains(aNameToFind)) {
return this;
}
else if (mParent != null) {
return mParent.getIfContains(aNameToFind);
}
else {
return null;
}
}
}
/**
@ -175,6 +274,14 @@ public abstract class DeclarationCollector extends Check
*/
private static class GlobalFrame extends LexicalFrame
{
/**
* Constructor for the root of the FrameTree
*/
protected GlobalFrame()
{
super(null);
}
}
/**
@ -183,6 +290,13 @@ public abstract class DeclarationCollector extends Check
*/
private static class MethodFrame extends LexicalFrame
{
/**
* @param aParent parent frame
*/
protected MethodFrame(LexicalFrame aParent)
{
super(aParent);
}
}
/**
@ -200,10 +314,11 @@ public abstract class DeclarationCollector extends Check
/**
* Creates new instance of ClassFrame
* @param aParent parent frame
*/
public ClassFrame()
public ClassFrame(LexicalFrame aParent)
{
super();
super(aParent);
mInstanceMembers = Sets.newHashSet();
mStaticMembers = Sets.newHashSet();
}
@ -254,61 +369,13 @@ public abstract class DeclarationCollector extends Check
*/
private static class BlockFrame extends LexicalFrame
{
}
/**
* A stack of LexicalFrames. Standard issue....
* @author Stephen Bloch
*/
private static class FrameStack
{
/** List of lexical frames. */
private final LinkedList<LexicalFrame> mFrameList;
/** Creates an empty FrameStack. */
FrameStack()
{
mFrameList = Lists.newLinkedList();
this.enter(new GlobalFrame());
}
/**
* Enter a scope, i.e. push a frame on the stack.
* @param aNewFrame the already-created frame to push
* @param aParent parent frame
*/
void enter(LexicalFrame aNewFrame)
protected BlockFrame(LexicalFrame aParent)
{
mFrameList.addFirst(aNewFrame);
}
/** Leave a scope, i.e. pop a frame from the stack. */
void leave()
{
mFrameList.removeFirst();
}
/**
* Get current scope, i.e. top frame on the stack.
* @return the current frame
*/
LexicalFrame current()
{
return mFrameList.getFirst();
}
/**
* Search this and containing frames for a given name.
* @param aNameToFind the name we're looking for
* @return the frame in which it was found, or null if not found
*/
LexicalFrame findFrame(String aNameToFind)
{
for (LexicalFrame thisFrame : mFrameList) {
if (thisFrame.contains(aNameToFind)) {
return thisFrame;
}
}
return null;
super(aParent);
}
}
}

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

@ -185,7 +185,8 @@ public class RequireThisCheck extends DeclarationCollector
|| (parentType == TokenTypes.CLASS_DEF)
|| (parentType == TokenTypes.ENUM_DEF)
|| (parentType == TokenTypes.INTERFACE_DEF)
|| (parentType == TokenTypes.PARAMETER_DEF))
|| (parentType == TokenTypes.PARAMETER_DEF)
|| (parentType == TokenTypes.TYPE_ARGUMENT))
{
// it's being declared; no problem
return;

15
src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputRequireThis2.java Normal file
View File

@ -0,0 +1,15 @@
package com.puppycrawl.tools.checkstyle.coding;
public class InputRequireThis2 {
private final int number = 1;
public int check() {
int sum = number;
sum += other();
return sum;
}
private int other() {
return 0;
}
}

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

@ -34,6 +34,7 @@ public class RequireThisCheckTest extends BaseCheckTestSupport
"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.\".",
"46:13: Reference to instance variable 'z' needs \"this.\".",
"53:9: Reference to instance variable 'z' needs \"this.\".",
// "13:9: Unable find where 'j' is declared.",
};
@ -65,6 +66,7 @@ public class RequireThisCheckTest extends BaseCheckTestSupport
final String[] expected = {
"8:9: Reference to instance variable 'i' needs \"this.\".",
"28:9: Reference to instance variable 'i' needs \"this.\".",
"46:13: Reference to instance variable 'z' needs \"this.\".",
"53:9: Reference to instance variable 'z' needs \"this.\".",
// "13:9: Unable find where 'j' is declared.",
};
@ -81,4 +83,18 @@ public class RequireThisCheckTest extends BaseCheckTestSupport
final String[] expected = {};
verify(checkConfig, getPath("Input15Extensions.java"), expected);
}
@Test
public void testGithubIssue41() throws Exception
{
final DefaultConfiguration checkConfig =
createCheckConfig(RequireThisCheck.class);
final String[] expected = {
"7:19: Reference to instance variable 'number' needs \"this.\".",
"8:16: Method call to 'other' needs \"this.\".",
};
verify(checkConfig,
getPath("coding" + File.separator + "InputRequireThis2.java"),
expected);
}
}