From 609eea57ac583b69e9e78a910fa7ea3c5b75f511 Mon Sep 17 00:00:00 2001 From: peter Date: Fri, 13 Oct 2017 21:08:46 +0200 Subject: [PATCH] IDEA-178434 "Unwrap 'if' statement" produces incompilable code if variable declared in the `if` body shadows the outer scope variable --- .../afterConflictingField.java | 13 +++++++ .../beforeConflictingField.java | 13 +++++++ .../ig/psiutils/DeclarationSearchUtils.java | 37 +++++++------------ 3 files changed, 39 insertions(+), 24 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterConflictingField.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeConflictingField.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterConflictingField.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterConflictingField.java new file mode 100644 index 000000000000..8272123082af --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterConflictingField.java @@ -0,0 +1,13 @@ +// "Unwrap 'if' statement" "true" +class X { + StringBuilder str = new StringBuilder(); + + void test(@org.jetbrains.annotations.NotNull String x) { + { + String str = x.trim(); + System.out.println(str); + } + + str.append("foo"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeConflictingField.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeConflictingField.java new file mode 100644 index 000000000000..fe73a8e86199 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeConflictingField.java @@ -0,0 +1,13 @@ +// "Unwrap 'if' statement" "true" +class X { + StringBuilder str = new StringBuilder(); + + void test(@org.jetbrains.annotations.NotNull String x) { + if(x != null) { + String str = x.trim(); + System.out.println(str); + } + + str.append("foo"); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/DeclarationSearchUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/DeclarationSearchUtils.java index 3235ffb7891a..5caa48373537 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/DeclarationSearchUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/DeclarationSearchUtils.java @@ -20,13 +20,15 @@ import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.controlFlow.DefUseUtil; import com.intellij.psi.search.GlobalSearchScope; +import com.intellij.psi.search.LocalSearchScope; import com.intellij.psi.search.PsiSearchHelper; import com.intellij.psi.search.SearchScope; +import com.intellij.psi.search.searches.ReferencesSearch; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.ArrayList; import java.util.List; public class DeclarationSearchUtils { @@ -51,8 +53,12 @@ public class DeclarationSearchUtils { if (statements.length == 0) { return false; } - final List followingBlocks = new ArrayList<>(); - collectFollowingBlocks(block.getParent().getNextSibling(), followingBlocks); + List affectedBlocks = ContainerUtil.newArrayList(parentBlock); + affectedBlocks.addAll(SyntaxTraverser.psiTraverser(block.getParent().getParent()) + .filter(PsiCodeBlock.class) + .filter(cb -> cb.getTextRange().getStartOffset() > block.getTextRange().getEndOffset()) + .toList()); + SearchScope affectedScope = new LocalSearchScope(affectedBlocks.toArray(PsiElement.EMPTY_ARRAY)); final Project project = block.getProject(); final JavaPsiFacade facade = JavaPsiFacade.getInstance(project); final PsiResolveHelper resolveHelper = facade.getResolveHelper(); @@ -71,13 +77,10 @@ public class DeclarationSearchUtils { if (variableName == null) { continue; } - final PsiVariable target = resolveHelper.resolveAccessibleReferencedVariable(variableName, parentBlock); - if (target instanceof PsiLocalVariable) { - return true; - } - for (PsiCodeBlock codeBlock : followingBlocks) { - final PsiVariable target1 = resolveHelper.resolveAccessibleReferencedVariable(variableName, codeBlock); - if (target1 instanceof PsiLocalVariable) { + for (PsiCodeBlock codeBlock : affectedBlocks) { + PsiVariable target = resolveHelper.resolveAccessibleReferencedVariable(variableName, codeBlock); + if (target instanceof PsiLocalVariable || + target instanceof PsiField && ReferencesSearch.search(target, affectedScope).findFirst() != null) { return true; } } @@ -86,20 +89,6 @@ public class DeclarationSearchUtils { return false; } - /** - * Depth first traversal to find all PsiCodeBlock children. - */ - private static void collectFollowingBlocks(PsiElement element, - List out) { - while (element != null) { - if (element instanceof PsiCodeBlock) { - out.add((PsiCodeBlock)element); - } - collectFollowingBlocks(element.getFirstChild(), out); - element = element.getNextSibling(); - } - } - public static PsiExpression findDefinition(@NotNull PsiReferenceExpression referenceExpression, @Nullable PsiVariable variable) { if (variable == null) {