Issue #3041: RequireThisCheck doesn't see outer classes for anonymous classes

This commit is contained in:
Vladislav Lisetskiy 2016-07-15 19:24:40 +03:00 committed by Roman Ivanov
parent 4aacddab59
commit 619e3b3dce
4 changed files with 81 additions and 4 deletions

View File

@ -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"/>

View File

@ -283,7 +283,7 @@ public class RequireThisCheck extends AbstractCheck {
if (frame.getFrameName().equals(getNearestClassFrameName())) {
log(ast, msgKey, ast.getText(), "");
}
else {
else if (!(frame instanceof AnonymousClassFrame)) {
log(ast, msgKey, ast.getText(), frame.getFrameName() + '.');
}
}
@ -361,6 +361,12 @@ public class RequireThisCheck extends AbstractCheck {
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
}
@ -405,11 +411,26 @@ public class RequireThisCheck extends AbstractCheck {
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.
@ -1062,7 +1083,7 @@ public class RequireThisCheck extends AbstractCheck {
}
/**
* 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
*/
@ -1248,6 +1269,30 @@ public class RequireThisCheck extends AbstractCheck {
}
}
/**
* 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

View File

@ -137,7 +137,11 @@ public class RequireThisCheckTest extends BaseCheckTestSupport {
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);

View File

@ -1,6 +1,9 @@
package com.puppycrawl.tools.checkstyle.checks.coding;
public class InputRequireThis3 {
private int bar;
interface AnonWithEmpty {
public void fooEmpty();
}
@ -17,7 +20,30 @@ public class InputRequireThis3 {
}
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 = 1;
@Override
public void fooEmpty() {
foobar++;
}
};
}