Made PyRedeclarationInspection more precise by analyzing conditions (PY-19856)

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<PsiElement> readElementRef = Ref.create(null);
         final Ref<PsiElement> writeElementRef = Ref.create(null);
+        final Ref<Boolean> 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) {

python/testData/inspections/PyRedeclarationInspection/afterIfTrueElse.py

@@ -0,0 +1,7 @@
+x = default_value
+y = 0
+if True:
+    <warning descr="Redeclared 'x' defined above without usage">x</warning> = y
+else:
+    pass
+<warning descr="Redeclared 'x' defined above without usage">x</warning> = y * 2

python/testData/inspections/PyRedeclarationInspection/ifFalseElse.py

@@ -0,0 +1,6 @@
+x = default_value
+y = 0
+if False:
+    pass
+else:
+    <warning descr="Redeclared 'x' defined above without usage">x</warning> = process(y)

python/testData/inspections/PyRedeclarationInspection/ifTrue.py

@@ -0,0 +1,4 @@
+x = default_value
+y = 0
+if True:
+    <warning descr="Redeclared 'x' defined above without usage">x</warning> = process(y)

python/testData/inspections/PyRedeclarationInspection/possiblyEmptyFor.py

@@ -0,0 +1,3 @@
+x = default_value
+for y in possibly_empty_list:
+    x = process(y)

python/testData/inspections/PyRedeclarationInspection/possiblyEmptyWhile.py

@@ -0,0 +1,4 @@
+x = default_value
+y = 0
+while y < stop:
+    x = process(y)

python/testData/inspections/PyRedeclarationInspection/possiblyFalseIf.py

@@ -0,0 +1,4 @@
+x = default_value
+y = 0
+if condition:
+    x = process(y)

python/testData/inspections/PyRedeclarationInspection/possiblyTrueIf.py

@@ -0,0 +1,6 @@
+x = default_value
+y = 0
+if condition:
+    pass
+else:
+    x = process(y)

python/testData/inspections/PyRedeclarationInspection/while.py

@@ -2,6 +2,6 @@ def test_while_loop(c):
     def foo():
         pass
 
-    while c:
+    while True:
         def <warning descr="Redeclared 'foo' defined above without usage">foo</warning>():
             pass

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<? extends PyInspection> getInspectionClass() {