Issue #382: Fix HiddenField false positive violations for anonymous classes

This commit is contained in:
Andrei Selkin 2015-09-11 19:22:58 +03:00 committed by Roman Ivanov
parent bd9efb5e66
commit dba6c944cd
4 changed files with 104 additions and 6 deletions

View File

@ -27,6 +27,7 @@ import java.util.regex.Pattern;
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.Scope;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
import com.puppycrawl.tools.checkstyle.utils.CommonUtils;
import com.puppycrawl.tools.checkstyle.utils.ScopeUtils;
@ -280,16 +281,38 @@ public class HiddenFieldCheck
final DetailAST nameAST = ast.findFirstToken(TokenTypes.IDENT);
final String name = nameAST.getText();
if (isStaticOrInstanceField(ast, name)
&& !isMatchingRegexp(name)
&& !isIgnoredSetterParam(ast, name)
&& !isIgnoredConstructorParam(ast)
&& !isIgnoredParamOfAbstractMethod(ast)) {
if ((isStaticFieldHiddenFromAnonymousClass(ast, name)
|| isStaticOrInstanceField(ast, name))
&& !isMatchingRegexp(name)
&& !isIgnoredParam(ast, name)) {
log(nameAST, MSG_KEY, name);
}
}
}
/**
* Checks whether a static field is hidden from closure.
* @param nameAST local variable or parameter.
* @param name field name.
* @return true if static field is hidden from closure.
*/
private boolean isStaticFieldHiddenFromAnonymousClass(DetailAST nameAST, String name) {
return isInStatic(nameAST)
&& frame.containsStaticField(name);
}
/**
* Checks whether method or constructor parameter is ignored.
* @param ast the parameter token.
* @param name the parameter name.
* @return true if parameter is ignored.
*/
private boolean isIgnoredParam(DetailAST ast, String name) {
return isIgnoredSetterParam(ast, name)
|| isIgnoredConstructorParam(ast)
|| isIgnoredParamOfAbstractMethod(ast);
}
/**
* Check for static or instance field.
* @param ast token
@ -324,7 +347,9 @@ public class HiddenFieldCheck
if (parent.getType() == TokenTypes.STATIC_INIT) {
inStatic = true;
}
else if (parent.getType() == TokenTypes.METHOD_DEF) {
else if (parent.getType() == TokenTypes.METHOD_DEF
&& !ScopeUtils.isInScope(parent, Scope.ANONINNER)
|| parent.getType() == TokenTypes.VARIABLE_DEF) {
final DetailAST mods =
parent.findFirstToken(TokenTypes.MODIFIERS);
inStatic = mods.branchContains(TokenTypes.LITERAL_STATIC);

View File

@ -265,4 +265,15 @@ public final class ScopeUtils {
}
return localVariableDef;
}
/**
* Checks whether ast node is in a specific scope.
* @param ast the node to check.
* @param scope a {@code Scope} value.
* @return true if the ast node is in the scope.
*/
public static boolean isInScope(DetailAST ast, Scope scope) {
final Scope surroundingScopeOfAstToken = getSurroundingScope(ast);
return surroundingScopeOfAstToken == scope;
}
}

View File

@ -29,6 +29,21 @@ import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
public class HiddenFieldCheckTest
extends BaseCheckTestSupport {
@Test
public void testStaticVisibilityFromAnonymousClasses() throws Exception {
final DefaultConfiguration checkConfig =
createCheckConfig(HiddenFieldCheck.class);
final String[] expected = {
"10:95: " + getCheckMessage(MSG_KEY, "other"),
"16:92: " + getCheckMessage(MSG_KEY, "other"),
"24:99: " + getCheckMessage(MSG_KEY, "other"),
"34:103: " + getCheckMessage(MSG_KEY, "other"),
"46:26: " + getCheckMessage(MSG_KEY, "someField"),
};
verify(checkConfig, getPath("InputHidenFieldStaticVisibility.java"), expected);
}
@Test
public void testNoParameters()
throws Exception {

View File

@ -0,0 +1,47 @@
package com.puppycrawl.tools.checkstyle;
import java.util.Comparator;
public class InputHidenFieldStaticVisibility {
static int someField;
static InputHidenFieldStaticVisibility other = null;
InputHidenFieldStaticVisibility field = null;
static void method(InputHidenFieldStaticVisibility field, InputHidenFieldStaticVisibility other) {
// field 'field' can not be referenced form a static context
// static field 'other' can be referenced from a static context
}
static class B {
void method(InputHidenFieldStaticVisibility field, InputHidenFieldStaticVisibility other) {
// field 'field' can not be referenced form a static context
// static field 'other' can be referenced from a static context
}
}
static Comparator<InputHidenFieldStaticVisibility> COMP = new Comparator<InputHidenFieldStaticVisibility>() {
@Override
public int compare(InputHidenFieldStaticVisibility field, InputHidenFieldStaticVisibility other) {
// field 'field' can not be referenced form a static context
// static field 'other' can be referenced from a static context
return 0;
}
};
static Comparator<InputHidenFieldStaticVisibility> createComp() {
return new Comparator<InputHidenFieldStaticVisibility>() {
@Override
public int compare(InputHidenFieldStaticVisibility field, InputHidenFieldStaticVisibility other) {
// field 'field' can not be referenced form a static context
// static field 'other' can be referenced from a static context
return 0;
}
};
}
static void foo1(int a) {}
void foo2(int a) {}
static void foo3(int someField) {}
}