diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInsight/GrReassignedLocalVarsChecker.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInsight/GrReassignedLocalVarsChecker.java index 5a155c18d640..580dd918bf9e 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInsight/GrReassignedLocalVarsChecker.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInsight/GrReassignedLocalVarsChecker.java @@ -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> 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 data = resolved.getUserData(LEAST_UPPER_BOUND_TYPE); if (data == null) { data = CachedValuesManager.getManager(resolved.getProject()).createCachedValue(new CachedValueProvider() { @@ -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() { @Override public PsiType compute() { @@ -170,7 +169,7 @@ public class GrReassignedLocalVarsChecker { } @NotNull - private static Set getAssignedVarsInsideBlock(@NotNull final GrCodeBlock block) { + private static Set getUsedVarsInsideBlock(@NotNull final GrCodeBlock block) { CachedValue> data = block.getUserData(ASSIGNED_VARS); if (data == null) { @@ -181,27 +180,24 @@ public class GrReassignedLocalVarsChecker { final Set 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); diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties index 5ccb19b400aa..348e53ffbbb6 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties @@ -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 diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/confusing/GrReassignedInClosureLocalVarInspection.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/confusing/GrReassignedInClosureLocalVarInspection.java index 6f16535b484a..82a688f9bd89 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/confusing/GrReassignedInClosureLocalVarInspection.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/confusing/GrReassignedInClosureLocalVarInspection.java @@ -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(); } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java index b1a497ddc00f..bff12db841ac 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java @@ -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 res = findAccess(local, place, ahead, true); - return res.toArray(new ReadWriteVariableInstruction[res.size()]); - } - public static List 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()); diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrInspectionTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrInspectionTest.groovy index efed43e59ec5..9568abf5b7a9 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrInspectionTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/highlighting/GrInspectionTest.groovy @@ -149,7 +149,7 @@ boolean bar(def list) { test() { def var = "abc" def cl = { - var = new Date() + var = new Date() } cl() var.toUpperCase() @@ -158,7 +158,7 @@ test() { test2() { def var = "abc" def cl = { - var = 'cde' + var = 'cde' } cl() var.toUpperCase()