Navigation Menu

Skip to content

Commit

Permalink
Issue checkstyle#3041: RequireThisCheck doesn't see outer classes for…
Browse files Browse the repository at this point in the history
… anonymous classes
  • Loading branch information
Vladlis committed Jan 25, 2017
1 parent 8df31ca commit cfaecff
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 4 deletions.
2 changes: 2 additions & 0 deletions config/suppressions.xml
Expand Up @@ -68,6 +68,8 @@
<suppress checks="MethodCount" files="[\\/]CommentsIndentationCheck.java$"/>
<!--VisibilityModifierCheck has 7 options which require 7 additional methods (setters)-->
<suppress checks="MethodCount" files="[\\/]VisibilityModifierCheck.java$"/>
<!--RequireThisCheck has a hierarchy of nested classes which contains a lot of methods. -->
<suppress checks="MethodCount" files="[\\/]RequireThisCheck.java$"/>

<!-- we need that set of converters -->
<suppress checks="ClassDataAbstractionCoupling" files="AutomaticBean\.java"/>
Expand Down
Expand Up @@ -283,7 +283,7 @@ private void logViolation(String msgKey, DetailAST ast, AbstractFrame frame) {
if (frame.getFrameName().equals(getNearestClassFrameName())) {
log(ast, msgKey, ast.getText(), "");
}
else {
else if (!(frame instanceof AnonymousClassFrame)) {
log(ast, msgKey, ast.getText(), frame.getFrameName() + '.');
}
}
Expand Down Expand Up @@ -361,6 +361,12 @@ private static void collectDeclarations(Deque<AbstractFrame> frameStack, DetailA
final DetailAST ctorFrameNameIdent = ast.findFirstToken(TokenTypes.IDENT);
frameStack.addFirst(new ConstructorFrame(frame, ctorFrameNameIdent));
break;
case TokenTypes.LITERAL_NEW:
if (isAnonymousClassDef(ast)) {
frameStack.addFirst(new AnonymousClassFrame(frame,
ast.getFirstChild().toString()));
}
break;
default:
// do nothing
}
Expand Down Expand Up @@ -405,11 +411,26 @@ private void endCollectingDeclarations(Queue<AbstractFrame> frameStack, DetailAS
case TokenTypes.CTOR_DEF :
frames.put(ast, frameStack.poll());
break;
case TokenTypes.LITERAL_NEW :
if (isAnonymousClassDef(ast)) {
frames.put(ast, frameStack.poll());
}
break;
default :
// do nothing
}
}

/**
* Whether the AST is a definition of an anonymous class.
* @param ast the AST to process.
* @return true if the AST is a definition of an anonymous class.
*/
private static boolean isAnonymousClassDef(DetailAST ast) {
final DetailAST lastChild = ast.getLastChild();
return lastChild != null && lastChild.getType() == TokenTypes.OBJBLOCK;
}

/**
* Returns the class frame where violation is found (where the field is used without 'this')
* or null otherwise.
Expand Down Expand Up @@ -1062,7 +1083,7 @@ protected FrameType getType() {
}

/**
* A frame initiated at class< enum or interface definition; holds instance variable names.
* A frame initiated at class, enum or interface definition; holds instance variable names.
* @author Stephen Bloch
* @author Andrei Selkin
*/
Expand Down Expand Up @@ -1248,6 +1269,30 @@ private static boolean isSimilarSignature(DetailAST ident, DetailAST ast) {
}
}

/**
* An anonymous class frame; holds instance variable names.
*/
private static class AnonymousClassFrame extends ClassFrame {

/** The name of the frame. */
private final String frameName;

/**
* Creates anonymous class frame.
* @param parent parent frame.
* @param frameName name of the frame.
*/
protected AnonymousClassFrame(AbstractFrame parent, String frameName) {
super(parent, null);
this.frameName = frameName;
}

@Override
protected String getFrameName() {
return frameName;
}
}

/**
* A frame initiated on entering a statement list; holds local variable names.
* @author Stephen Bloch
Expand Down
Expand Up @@ -137,7 +137,11 @@ public void testTokensNotNull() {
public void testWithAnonymousClass() throws Exception {
final DefaultConfiguration checkConfig = createCheckConfig(RequireThisCheck.class);
checkConfig.addAttribute("validateOnlyOverlapping", "false");
final String[] expected = CommonUtils.EMPTY_STRING_ARRAY;
final String[] expected = {
"19:25: " + getCheckMessage(MSG_METHOD, "doSideEffect", ""),
"23:24: " + getCheckMessage(MSG_VARIABLE, "bar", "InputRequireThis3."),
"46:17: " + getCheckMessage(MSG_VARIABLE, "foobar", ""),
};
verify(checkConfig,
getPath("InputRequireThis3.java"),
expected);
Expand Down
@@ -1,6 +1,9 @@
package com.puppycrawl.tools.checkstyle.checks.coding;

public class InputRequireThis3 {

private int bar;

interface AnonWithEmpty {
public void fooEmpty();
}
Expand All @@ -17,7 +20,30 @@ public void fooEmpty() {
}

public int doSideEffect() {
return 1;
return bar;
}
};

new AnonWithEmpty() {
int anonMember = 0;

@Override
public void fooEmpty() {
new AnonWithEmpty() {

@Override
public void fooEmpty() {
anonMember++;
}
};
}
};

new AnonWithEmpty() {
int foobar = 0;
@Override
public void fooEmpty() {
foobar++;
}
};
}
Expand Down

0 comments on commit cfaecff

Please sign in to comment.