Groovy: reassigned vars without closures

This commit is contained in:
Max Medvedev
2013-12-23 19:53:53 +04:00
parent 89e472e454
commit e90c6785e9
5 changed files with 26 additions and 59 deletions
@@ -37,7 +37,6 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariable;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrCodeBlock;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrOpenBlock;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrAssignmentExpression;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression;
import org.jetbrains.plugins.groovy.lang.psi.dataFlow.types.TypeInferenceHelper;
@@ -57,7 +56,7 @@ public class GrReassignedLocalVarsChecker {
private static final Key<CachedValue<Boolean>> REASSIGNED_VAR = Key.create("least upper bound type");
@Nullable
public static Boolean isReassignedVar(final GrReferenceExpression refExpr) {
public static Boolean isReassignedVar(@NotNull final GrReferenceExpression refExpr) {
if (!PsiUtil.isCompileStatic(refExpr)) {
return false;
}
@@ -86,7 +85,7 @@ public class GrReassignedLocalVarsChecker {
return data.getValue();
}
private static boolean isReassignedVarImpl(final GrVariable resolved) {
private static boolean isReassignedVarImpl(@NotNull final GrVariable resolved) {
final GrControlFlowOwner variableScope = PsiTreeUtil.getParentOfType(resolved, GrCodeBlock.class, GroovyFile.class);
if (variableScope == null) return false;
@@ -97,7 +96,7 @@ public class GrReassignedLocalVarsChecker {
((GroovyPsiElement)scope).accept(new GroovyRecursiveElementVisitor() {
@Override
public void visitClosure(GrClosableBlock closure) {
if (getAssignedVarsInsideBlock(closure).contains(name)) {
if (getUsedVarsInsideBlock(closure).contains(name)) {
isReassigned.set(true);
}
}
@@ -133,7 +132,7 @@ public class GrReassignedLocalVarsChecker {
}
@Nullable
private static PsiType getLeastUpperBoundByVar(final GrVariable resolved) {
private static PsiType getLeastUpperBoundByVar(@NotNull final GrVariable resolved) {
CachedValue<PsiType> data = resolved.getUserData(LEAST_UPPER_BOUND_TYPE);
if (data == null) {
data = CachedValuesManager.getManager(resolved.getProject()).createCachedValue(new CachedValueProvider<PsiType>() {
@@ -148,7 +147,7 @@ public class GrReassignedLocalVarsChecker {
}
@Nullable
private static PsiType getLeastUpperBoundByVarImpl(final GrVariable resolved) {
private static PsiType getLeastUpperBoundByVarImpl(@NotNull final GrVariable resolved) {
return RecursionManager.doPreventingRecursion(resolved, false, new NullableComputable<PsiType>() {
@Override
public PsiType compute() {
@@ -170,7 +169,7 @@ public class GrReassignedLocalVarsChecker {
}
@NotNull
private static Set<String> getAssignedVarsInsideBlock(@NotNull final GrCodeBlock block) {
private static Set<String> getUsedVarsInsideBlock(@NotNull final GrCodeBlock block) {
CachedValue<Set<String>> data = block.getUserData(ASSIGNED_VARS);
if (data == null) {
@@ -181,27 +180,24 @@ public class GrReassignedLocalVarsChecker {
final Set<String> result = ContainerUtil.newHashSet();
block.acceptChildren(new GroovyRecursiveElementVisitor() {
@Override
public void visitAssignmentExpression(GrAssignmentExpression expression) {
super.visitAssignmentExpression(expression);
GrExpression lValue = expression.getLValue();
if (lValue instanceof GrReferenceExpression && !((GrReferenceExpression)lValue).isQualified()) {
result.add(((GrReferenceExpression)lValue).getReferenceName());
}
}
@Override
public void visitOpenBlock(GrOpenBlock openBlock) {
result.addAll(getAssignedVarsInsideBlock(openBlock));
result.addAll(getUsedVarsInsideBlock(openBlock));
}
@Override
public void visitClosure(GrClosableBlock closure) {
result.addAll(getAssignedVarsInsideBlock(closure));
result.addAll(getUsedVarsInsideBlock(closure));
}
@Override
public void visitReferenceExpression(GrReferenceExpression referenceExpression) {
if (referenceExpression.getQualifier() == null && referenceExpression.getReferenceName() != null) {
result.add(referenceExpression.getReferenceName());
}
}
});
return Result.create(result, block);
}
}, false);
@@ -85,7 +85,7 @@ replace.postfix.0.with.prefix.0=Replace postfix {0} with prefix {0}
replace.0.with.1=Replace {0} with binary {1}
gr.deprecated.api.usage=Deprecated API inspection
category.method.0.cannot.be.applied.to.1=Category method ''{0}'' cannot be applied to ''{1}''
local.var.0.is.reassigned.in.closure=Local variable {0} is reassigned in {1} with other type
local.var.0.is.reassigned=Local variable ''{0}'' is reassigned
anonymous.class=anonymous class
closure=closure
other.scope=Other scope
@@ -21,16 +21,12 @@ import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiType;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.plugins.groovy.codeInsight.GrReassignedLocalVarsChecker;
import org.jetbrains.plugins.groovy.codeInspection.BaseInspection;
import org.jetbrains.plugins.groovy.codeInspection.BaseInspectionVisitor;
import org.jetbrains.plugins.groovy.codeInspection.utils.ControlFlowUtils;
import org.jetbrains.plugins.groovy.lang.psi.GrControlFlowOwner;
import org.jetbrains.plugins.groovy.lang.psi.GrNamedElement;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrVariable;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression;
import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrAnonymousClassDefinition;
import org.jetbrains.plugins.groovy.lang.psi.impl.statements.expressions.TypesUtil;
import org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil;
import org.jetbrains.plugins.groovy.refactoring.GroovyRefactoringUtil;
@@ -70,38 +66,18 @@ public class GrReassignedInClosureLocalVarInspection extends BaseInspection {
final PsiElement resolved = referenceExpression.resolve();
if (!GroovyRefactoringUtil.isLocalVariable(resolved)) return;
final PsiType checked = GrReassignedLocalVarsChecker.getReassignedVarType(referenceExpression, false);
if (checked == null) return;
final GrControlFlowOwner varFlowOwner = ControlFlowUtils.findControlFlowOwner(resolved);
final GrControlFlowOwner refFlorOwner = ControlFlowUtils.findControlFlowOwner(referenceExpression);
if (isOtherScopeAndType(referenceExpression, checked, varFlowOwner, refFlorOwner)) {
String flowDescription = getFlowDescription(refFlorOwner);
final String message = message("local.var.0.is.reassigned.in.closure", ((GrNamedElement)resolved).getName(), flowDescription);
if (isOtherTypeOrDifferent(referenceExpression, (GrVariable)resolved) ) {
final String message = message("local.var.0.is.reassigned", ((GrNamedElement)resolved).getName());
registerError(referenceExpression, message, LocalQuickFix.EMPTY_ARRAY, ProblemHighlightType.GENERIC_ERROR_OR_WARNING);
}
}
};
}
private static boolean isOtherScopeAndType(GrReferenceExpression referenceExpression,
PsiType checked,
GrControlFlowOwner varFlowOwner,
GrControlFlowOwner refFlorOwner) {
return varFlowOwner != refFlorOwner && !TypesUtil.isAssignable(referenceExpression.getType(), checked, referenceExpression);
}
private static boolean isOtherTypeOrDifferent(@NotNull GrReferenceExpression referenceExpression, GrVariable resolved) {
if (ControlFlowUtils.findControlFlowOwner(referenceExpression) != ControlFlowUtils.findControlFlowOwner(resolved)) return true;
private static String getFlowDescription(GrControlFlowOwner refFlorOwner) {
String flowDescription;
if (refFlorOwner instanceof GrClosableBlock) {
flowDescription = message("closure");
}
else if (refFlorOwner instanceof GrAnonymousClassDefinition) {
flowDescription = message("anonymous.class");
}
else {
flowDescription = message("other.scope");
}
return flowDescription;
final PsiType currentType = referenceExpression.getType();
return currentType != null && currentType != PsiType.NULL && !ControlFlowUtils.findAccess(resolved, referenceExpression, false, true).isEmpty();
}
}
@@ -709,15 +709,10 @@ public class ControlFlowUtils {
* @param ahead if true search for next write. if false searches for previous write
* @return all write instructions leading to (or preceding) the place
*/
public static ReadWriteVariableInstruction[] findWriteAccess(GrVariable local, final PsiElement place, boolean ahead) {
List<ReadWriteVariableInstruction> res = findAccess(local, place, ahead, true);
return res.toArray(new ReadWriteVariableInstruction[res.size()]);
}
public static List<ReadWriteVariableInstruction> findAccess(GrVariable local, final PsiElement place, boolean ahead, boolean writeAccessOnly) {
LOG.assertTrue(!(local instanceof GrField), local.getClass());
final GrControlFlowOwner owner = findControlFlowOwner(local);
final GrControlFlowOwner owner = findControlFlowOwner(place);
assert owner != null;
final Instruction cur = findInstruction(place, owner.getControlFlow());
@@ -149,7 +149,7 @@ boolean bar(def list) {
test() {
def var = "abc"
def cl = {
<warning descr="Local variable var is reassigned in closure with other type">var</warning> = new Date()
<warning descr="Local variable 'var' is reassigned">var</warning> = new Date()
}
cl()
var.toUpperCase()
@@ -158,7 +158,7 @@ test() {
test2() {
def var = "abc"
def cl = {
var = 'cde'
<warning descr="Local variable 'var' is reassigned">var</warning> = 'cde'
}
cl()
var.toUpperCase()