diff --git a/plugins/groovy/src/META-INF/plugin.xml b/plugins/groovy/src/META-INF/plugin.xml index c214cd7266aa..5ffff58acb87 100644 --- a/plugins/groovy/src/META-INF/plugin.xml +++ b/plugins/groovy/src/META-INF/plugin.xml @@ -661,6 +661,10 @@ groupName="Control Flow" enabledByDefault="true" level="WARNING" implementationClass="org.jetbrains.plugins.groovy.codeInspection.control.GroovyUnnecessaryReturnInspection"/> + scopes = collectVariables(scope); + + for (Map.Entry> entry : scopes.entrySet()) { + final PsiElement scopeToProcess = entry.getKey(); + + final Map variables = ContainerUtil.newHashMap(); + for (GrVariable var : entry.getValue()) { + variables.put(var.getName(), var); + } + + + final List result = checkFlow(getFlow(scopeToProcess), variables); + if (result != null) { + for (ReadWriteVariableInstruction instruction : result) { + if (variables.containsKey(instruction.getVariableName())) { + registerError(instruction.getElement(), + GroovyBundle.message("cannot.assign.a.value.to.final.field.0", instruction.getVariableName()), + LocalQuickFix.EMPTY_ARRAY, ProblemHighlightType.GENERIC_ERROR_OR_WARNING); + } + } + } + } + } + }; + } + + /** + * @return map: scope -> variables defined in the scope + */ + @NotNull + private static MultiMap collectVariables(@NotNull GroovyPsiElement scope) { + final MultiMap scopes = MultiMap.create(); + scope.accept(new GroovyRecursiveElementVisitor() { + @Override + public void visitVariable(GrVariable variable) { + super.visitVariable(variable); + if (!(variable instanceof PsiField) && variable.hasModifierProperty(PsiModifier.FINAL)) { + final PsiElement varScope = findScope(variable); + if (varScope != null) { + scopes.putValue(varScope, variable); + } + } + } + }); + return scopes; + } + + @NotNull + private static Instruction[] getFlow(@NotNull PsiElement element) { + return element instanceof GrControlFlowOwner + ? ((GrControlFlowOwner)element).getControlFlow() + : new ControlFlowBuilder(element.getProject()).buildControlFlow((GroovyPsiElement)element); + } + + @Nullable + private static List checkFlow(@NotNull Instruction[] flow, @NotNull Map variables) { + DFAEngine engine = new DFAEngine(flow, new MyDFAInstance(), new MySemilattice()); + final ArrayList dfaResult = engine.performDFAWithTimeout(); + if (dfaResult == null) return null; + + + List result = ContainerUtil.newArrayList(); + for (int i = 0; i < flow.length; i++) { + Instruction instruction = flow[i]; + if (instruction instanceof ReadWriteVariableInstruction && ((ReadWriteVariableInstruction)instruction).isWrite()) { + final MyData initialized = dfaResult.get(i); + final GrVariable var = variables.get(((ReadWriteVariableInstruction)instruction).getVariableName()); + if (var instanceof GrParameter && ((GrParameter)var).getDeclarationScope() instanceof GrForStatement) { + if (initialized.isInitialized(((ReadWriteVariableInstruction)instruction).getVariableName())) { + result.add((ReadWriteVariableInstruction)instruction); + } + } + else { + if (initialized.isOverInitialized(((ReadWriteVariableInstruction)instruction).getVariableName())) { + result.add((ReadWriteVariableInstruction)instruction); + } + } + } + } + + + return result; + } + + @Nullable + private static PsiElement findScope(@NotNull GrVariable variable) { + GroovyPsiElement result = PsiTreeUtil.getParentOfType(variable, GrControlStatement.class, GrControlFlowOwner.class); + if (result instanceof GrForStatement) { + final GrStatement body = ((GrForStatement)result).getBody(); + if (body != null) { + result = body; + } + } + return result; + } + + private static class MyDFAInstance implements DfaInstance { + @Override + public void fun(MyData e, Instruction instruction) { + if (instruction instanceof ReadWriteVariableInstruction && ((ReadWriteVariableInstruction)instruction).isWrite()) { + e.add(((ReadWriteVariableInstruction)instruction).getVariableName()); + } + } + + @NotNull + @Override + public MyData initial() { + return new MyData(); + } + + @Override + public boolean isForward() { + return true; + } + } + + private static class MySemilattice implements Semilattice { + @Override + public MyData join(ArrayList ins) { + return new MyData(ins); + } + + @Override + public boolean eq(MyData e1, MyData e2) { + return e1.equals(e2); + } + } + + private static class MyData { + private final Set myInitialized = ContainerUtil.newHashSet(); + private final Set myOverInitialized = ContainerUtil.newHashSet(); + + public MyData(List ins) { + for (MyData data : ins) { + myInitialized.addAll(data.myInitialized); + myOverInitialized.addAll(data.myOverInitialized); + } + } + + public MyData() {} + + public void add(String var) { + if (!myInitialized.add(var)) { + myOverInitialized.add(var); + } + } + + @Override + public boolean equals(Object obj) { + return obj instanceof MyData && + myInitialized.equals(((MyData)obj).myInitialized) && + myOverInitialized.equals(((MyData)obj).myOverInitialized); + } + + public boolean isOverInitialized(String var) { + return myOverInitialized.contains(var); + } + + public boolean isInitialized(String var) { + return myInitialized.contains(var); + } + } +} diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/inspections/GrFinalVariableAccessTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/inspections/GrFinalVariableAccessTest.groovy new file mode 100644 index 000000000000..96d8cd9e275f --- /dev/null +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/inspections/GrFinalVariableAccessTest.groovy @@ -0,0 +1,178 @@ +/* + * Copyright 2000-2013 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jetbrains.plugins.groovy.inspections + +import com.intellij.codeInspection.InspectionProfileEntry +import org.jetbrains.plugins.groovy.codeInspection.control.GrFinalVariableAccessInspection +import org.jetbrains.plugins.groovy.lang.highlighting.GrHighlightingTestBase + +/** + * @author Max Medvedev + */ +class GrFinalVariableAccessTest extends GrHighlightingTestBase { + @Override + InspectionProfileEntry[] getCustomInspections() { + return [new GrFinalVariableAccessInspection()] + } + + + void testSimpleVar() { + testHighlighting(''' + final foo = 5 + foo = 7 + print foo + ''') + } + + void testSplitInit() { + testHighlighting(''' + final foo + foo = 7 + foo = 8 + print foo + ''') + } + + void testIf1() { + testHighlighting(''' + final foo = 5 + if (cond) { + foo = 7 + } + print foo + ''') + } + + void testIf2() { + testHighlighting(''' + final foo + if (cond) { + foo = 7 + } + else { + foo = 2 + } + foo = 1 + print foo + ''') + } + + void testIf3() { + testHighlighting(''' + final foo + if (cond) { + foo = 7 + } + foo = 1 + print foo + ''') + } + + + void testFor() { + testHighlighting(''' + for (a in b) { + final x = 5 //all correct + print x + } + ''') + } + + void testFor2() { + testHighlighting(''' + final foo = 5 + for (a in b) { + foo = 5 + print foo + } + ''') + } + + void testFor3() { + testHighlighting(''' + for (a in b) + final foo = 5 //correct code + ''') + } + + + void testForParam() { + testHighlighting(''' + for (final i : [1, 2]) { + i = 5 + print i + } + ''') + } + + void testDuplicatedVar() { + testHighlighting(''' + if (cond) { + final foo = 5 + print foo + } + + if (otherCond) { + final foo = 2 //correct + foo = 4 + print foo + } + + if (anotherCond) + final foo = 3 //correct +''') + } + + void testDuplicatedVar2() { + testHighlighting(''' + if (cond) { + final foo = 5 + foo = 4 + print foo + } + + if (otherCond) { + foo = 4 + print foo + } + + if (anotherCond) + final foo = 3 //correct +''') + } + + void testDuplicatedVar3() { + testHighlighting(''' + class X { + def bar() { + if (cond) { + final foo = 5 + foo = 4 + print foo + } + + if (otherCond) { + foo = 4 + print foo + } + + if (anotherCond) + final foo = 3 //correct + } + } +''') + } +}