diff --git a/python/src/com/jetbrains/python/inspections/PyRedeclarationInspection.java b/python/src/com/jetbrains/python/inspections/PyRedeclarationInspection.java index 8dd09eed41c8..97a6d16fe1d9 100644 --- a/python/src/com/jetbrains/python/inspections/PyRedeclarationInspection.java +++ b/python/src/com/jetbrains/python/inspections/PyRedeclarationInspection.java @@ -15,6 +15,7 @@ */ package com.jetbrains.python.inspections; +import com.intellij.codeInsight.controlflow.ConditionalInstruction; import com.intellij.codeInsight.controlflow.ControlFlowUtil; import com.intellij.codeInsight.controlflow.Instruction; import com.intellij.codeInspection.LocalInspectionToolSession; @@ -34,6 +35,7 @@ import com.jetbrains.python.codeInsight.controlflow.ScopeOwner; import com.jetbrains.python.codeInsight.dataflow.scope.ScopeUtil; import com.jetbrains.python.inspections.quickfix.PyRenameElementQuickFix; import com.jetbrains.python.psi.*; +import com.jetbrains.python.psi.impl.PyEvaluator; import com.jetbrains.python.pyi.PyiUtil; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; @@ -93,10 +95,6 @@ public class PyRedeclarationInspection extends PyInspection { } } - private static boolean isConditional(@NotNull PsiElement node) { - return PsiTreeUtil.getParentOfType(node, PyIfStatement.class, PyConditionalExpression.class, PyTryExceptStatement.class) != null; - } - private static boolean isDecorated(@NotNull PyDecoratable node) { boolean isDecorated = false; final PyDecoratorList decoratorList = node.getDecoratorList(); @@ -110,9 +108,6 @@ public class PyRedeclarationInspection extends PyInspection { } private void processElement(@NotNull final PsiNameIdentifierOwner element) { - if (isConditional(element)) { - return; - } final String name = element.getName(); final ScopeOwner owner = ScopeUtil.getScopeOwner(element); if (owner != null && name != null) { @@ -130,6 +125,7 @@ public class PyRedeclarationInspection extends PyInspection { } final Ref readElementRef = Ref.create(null); final Ref writeElementRef = Ref.create(null); + final Ref underPossiblyFalseCondition = Ref.create(false); ControlFlowUtil.iteratePrev(startInstruction, instructions, instruction -> { if (instruction instanceof ReadWriteInstruction && instruction.num() != startInstruction) { final ReadWriteInstruction rwInstruction = (ReadWriteInstruction)instruction; @@ -143,7 +139,7 @@ public class PyRedeclarationInspection extends PyInspection { if (PyiUtil.isOverload(originalElement, myTypeEvalContext)) { return ControlFlowUtil.Operation.NEXT; } - else { + else if (!underPossiblyFalseCondition.get()) { writeElementRef.set(originalElement); } } @@ -151,6 +147,9 @@ public class PyRedeclarationInspection extends PyInspection { return ControlFlowUtil.Operation.CONTINUE; } } + if (possiblyFalseCondition(instruction)) { + underPossiblyFalseCondition.set(true); + } return ControlFlowUtil.Operation.NEXT; }); final PsiElement writeElement = writeElementRef.get(); @@ -169,6 +168,30 @@ public class PyRedeclarationInspection extends PyInspection { } } + private static boolean possiblyFalseCondition(@NotNull Instruction instruction) { + final PsiElement element = instruction.getElement(); + if (element == null) return false; + + if (element instanceof PyTryExceptStatement) return true; + + if (element instanceof PyForStatement) { + final PyForPart forPart = ((PyForStatement)element).getForPart(); + return !PyEvaluator.evaluateAsBoolean(forPart.getSource(), false); + } + + if (instruction instanceof ConditionalInstruction) { + final ConditionalInstruction conditionalInstruction = (ConditionalInstruction)instruction; + final PsiElement condition = conditionalInstruction.getCondition(); + if (condition instanceof PyExpression) { + return conditionalInstruction.getResult() + ? !PyEvaluator.evaluateAsBoolean((PyExpression)condition, false) + : PyEvaluator.evaluateAsBoolean((PyExpression)condition, true); + } + } + + return false; + } + private static boolean suggestRename(@NotNull PsiNameIdentifierOwner element, @NotNull PsiElement originalElement) { // Target expressions in the same scope are treated as the same variable if ((element instanceof PyTargetExpression) && originalElement instanceof PyTargetExpression) { diff --git a/python/testData/inspections/PyRedeclarationInspection/afterIfTrueElse.py b/python/testData/inspections/PyRedeclarationInspection/afterIfTrueElse.py new file mode 100644 index 000000000000..94c4fe13918e --- /dev/null +++ b/python/testData/inspections/PyRedeclarationInspection/afterIfTrueElse.py @@ -0,0 +1,7 @@ +x = default_value +y = 0 +if True: + x = y +else: + pass +x = y * 2 \ No newline at end of file diff --git a/python/testData/inspections/PyRedeclarationInspection/ifFalseElse.py b/python/testData/inspections/PyRedeclarationInspection/ifFalseElse.py new file mode 100644 index 000000000000..7fee25d7d00b --- /dev/null +++ b/python/testData/inspections/PyRedeclarationInspection/ifFalseElse.py @@ -0,0 +1,6 @@ +x = default_value +y = 0 +if False: + pass +else: + x = process(y) \ No newline at end of file diff --git a/python/testData/inspections/PyRedeclarationInspection/ifTrue.py b/python/testData/inspections/PyRedeclarationInspection/ifTrue.py new file mode 100644 index 000000000000..2fd6e9db5879 --- /dev/null +++ b/python/testData/inspections/PyRedeclarationInspection/ifTrue.py @@ -0,0 +1,4 @@ +x = default_value +y = 0 +if True: + x = process(y) \ No newline at end of file diff --git a/python/testData/inspections/PyRedeclarationInspection/possiblyEmptyFor.py b/python/testData/inspections/PyRedeclarationInspection/possiblyEmptyFor.py new file mode 100644 index 000000000000..bc238f78b687 --- /dev/null +++ b/python/testData/inspections/PyRedeclarationInspection/possiblyEmptyFor.py @@ -0,0 +1,3 @@ +x = default_value +for y in possibly_empty_list: + x = process(y) \ No newline at end of file diff --git a/python/testData/inspections/PyRedeclarationInspection/possiblyEmptyWhile.py b/python/testData/inspections/PyRedeclarationInspection/possiblyEmptyWhile.py new file mode 100644 index 000000000000..7f929cd3b73a --- /dev/null +++ b/python/testData/inspections/PyRedeclarationInspection/possiblyEmptyWhile.py @@ -0,0 +1,4 @@ +x = default_value +y = 0 +while y < stop: + x = process(y) \ No newline at end of file diff --git a/python/testData/inspections/PyRedeclarationInspection/possiblyFalseIf.py b/python/testData/inspections/PyRedeclarationInspection/possiblyFalseIf.py new file mode 100644 index 000000000000..4689d960bfbc --- /dev/null +++ b/python/testData/inspections/PyRedeclarationInspection/possiblyFalseIf.py @@ -0,0 +1,4 @@ +x = default_value +y = 0 +if condition: + x = process(y) \ No newline at end of file diff --git a/python/testData/inspections/PyRedeclarationInspection/possiblyTrueIf.py b/python/testData/inspections/PyRedeclarationInspection/possiblyTrueIf.py new file mode 100644 index 000000000000..46fbb6aeb192 --- /dev/null +++ b/python/testData/inspections/PyRedeclarationInspection/possiblyTrueIf.py @@ -0,0 +1,6 @@ +x = default_value +y = 0 +if condition: + pass +else: + x = process(y) \ No newline at end of file diff --git a/python/testData/inspections/PyRedeclarationInspection/while.py b/python/testData/inspections/PyRedeclarationInspection/while.py index eca7d219d3ad..b2f8295d0570 100644 --- a/python/testData/inspections/PyRedeclarationInspection/while.py +++ b/python/testData/inspections/PyRedeclarationInspection/while.py @@ -2,6 +2,6 @@ def test_while_loop(c): def foo(): pass - while c: + while True: def foo(): pass \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/inspections/PyRedeclarationInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/PyRedeclarationInspectionTest.java index e08b2d7210a2..4d70177033d8 100644 --- a/python/testSrc/com/jetbrains/python/inspections/PyRedeclarationInspectionTest.java +++ b/python/testSrc/com/jetbrains/python/inspections/PyRedeclarationInspectionTest.java @@ -130,6 +130,36 @@ public class PyRedeclarationInspectionTest extends PyInspectionTestCase { doTest(); } + // PY-19856 + public void testPossiblyEmptyFor() { + doTest(); + } + + // PY-19856 + public void testPossiblyEmptyWhile() { + doTest(); + } + + public void testIfFalseElse() { + doTest(); + } + + public void testIfTrue() { + doTest(); + } + + public void testPossiblyFalseIf() { + doTest(); + } + + public void testPossiblyTrueIf() { + doTest(); + } + + public void testAfterIfTrueElse() { + doTest(); + } + @NotNull @Override protected Class getInspectionClass() {