Solution to wrong variable reported because of name shadowing in FinalLocalVariableCheck. solves #1142

This commit is contained in:
Bhavik Patel 2015-06-14 20:04:25 +05:30 committed by Roman Ivanov
parent eb5896a4a1
commit 11ff590ba5
3 changed files with 83 additions and 9 deletions

View File

@ -159,8 +159,9 @@ public class FinalLocalVariableCheck extends Check {
}
case TokenTypes.VARIABLE_DEF:
if (ast.getParent().getType() != TokenTypes.OBJBLOCK
&& shouldCheckEnhancedForLoopVariable(ast)
&& isVariableInForInit(ast)) {
&& shouldCheckEnhancedForLoopVariable(ast)
&& isVariableInForInit(ast)
&& !ast.branchContains(TokenTypes.FINAL)) {
insertVariable(ast);
}
break;
@ -255,16 +256,48 @@ public class FinalLocalVariableCheck extends Check {
return paramDef.getParent().getParent().getType() == TokenTypes.LAMBDA;
}
/**
* Find the Class or Method in which it is defined.
* @param ast Variable for which we want to find the scope in which it is defined
* @return ast The Class or Method in which it is defined.
*/
private static DetailAST findClassOrMethodInWhichItIsDefined(DetailAST ast) {
DetailAST astTraverse = ast;
while (!(astTraverse.getType() == TokenTypes.METHOD_DEF
|| astTraverse.getType() == TokenTypes.CLASS_DEF)) {
astTraverse = astTraverse.getParent();
}
return astTraverse;
}
/**
* Check if both the Variable are same.
* @param ast1 Variable to compare
* @param ast2 Variable to compare
* @return true if both the variable are same, otherwise false
*/
private static boolean isSameVariables(DetailAST ast1, DetailAST ast2) {
final DetailAST classOrMethodOfAst1 =
findClassOrMethodInWhichItIsDefined(ast1);
final DetailAST classOrMethodOfAst2 =
findClassOrMethodInWhichItIsDefined(ast2);
final String identifierOfAst1 =
classOrMethodOfAst1.findFirstToken(TokenTypes.IDENT).getText();
final String identifierOfAst2 =
classOrMethodOfAst2.findFirstToken(TokenTypes.IDENT).getText();
return identifierOfAst1.equals(identifierOfAst2);
}
/**
* Inserts a variable at the topmost scope stack
* @param ast the variable to insert
*/
private void insertVariable(DetailAST ast) {
if (!ast.branchContains(TokenTypes.FINAL)) {
final Map<String, DetailAST> state = scopeStack.peek();
final DetailAST astNode = ast.findFirstToken(TokenTypes.IDENT);
state.put(astNode.getText(), astNode);
}
final Map<String, DetailAST> state = scopeStack.peek();
final DetailAST astNode = ast.findFirstToken(TokenTypes.IDENT);
state.put(astNode.getText(), astNode);
}
/**
@ -275,8 +308,9 @@ public class FinalLocalVariableCheck extends Check {
final Iterator<Map<String, DetailAST>> iterator = scopeStack.descendingIterator();
while (iterator.hasNext()) {
final Map<String, DetailAST> state = iterator.next();
final Object obj = state.remove(ast.getText());
if (obj != null) {
final DetailAST storedVariable = state.get(ast.getText());
if (storedVariable != null && isSameVariables(storedVariable, ast)) {
state.remove(ast.getText());
break;
}
}

View File

@ -131,4 +131,18 @@ public class FinalLocalVariableCheckTest
+ "tools/checkstyle/naming/InputFinalLocalVariableNameLambda.java")
.getCanonicalPath(), expected);
}
@Test
public void testVariableNameShadowing()
throws Exception {
final DefaultConfiguration checkConfig =
createCheckConfig(FinalLocalVariableCheck.class);
checkConfig.addAttribute("tokens", "PARAMETER_DEF,VARIABLE_DEF");
final String[] expected = {
"4:28: " + "Variable 'text' should be declared final.",
"17:13: " + "Variable 'x' should be declared final.",
};
verify(checkConfig, getPath("coding/InputFinalLocalVariableNameShadowing.java"), expected);
}
}

View File

@ -0,0 +1,26 @@
package com.puppycrawl.tools.checkstyle.coding;
class Foo1 {
public void foo(String text) {
System.out.println(text);
class Bar {
void bar (String text) {
text = "xxx";
}
}
}
}
class Foo2 {
public void foo() {
int x;
class Bar {
void bar () {
int x = 1;
x++;
x++;
}
}
}
}