From 08931c557595044d6278694a78ddefd1241696c8 Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Sat, 6 Jan 2024 20:40:45 -0500
Subject: [PATCH] Don't flag redefined-while-unsed in if branches

---
 .../resources/test/fixtures/pyflakes/F811_27.py      |  6 ++++++
 .../src/checkers/ast/analyze/deferred_scopes.rs      | 12 ++++++------
 crates/ruff_linter/src/rules/pyflakes/mod.rs         |  1 +
 ...ter__rules__pyflakes__tests__F811_F811_27.py.snap |  4 ++++
 crates/ruff_python_semantic/src/model.rs             |  9 +++------
 5 files changed, 20 insertions(+), 12 deletions(-)
 create mode 100644 crates/ruff_linter/resources/test/fixtures/pyflakes/F811_27.py
 create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_27.py.snap

diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_27.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_27.py
new file mode 100644
index 0000000000000..d2121a53a3ccc
--- /dev/null
+++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_27.py
@@ -0,0 +1,6 @@
+import os
+
+x = 1
+
+if x > 0:
+    import os
diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs
index 4c7a6bd999cd0..14bc02c0df76c 100644
--- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs
+++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs
@@ -144,9 +144,9 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
 
                     // If the bindings are in different forks, abort.
                     if shadowed.source.map_or(true, |left| {
-                        binding.source.map_or(true, |right| {
-                            checker.semantic.different_branches(left, right)
-                        })
+                        binding
+                            .source
+                            .map_or(true, |right| !checker.semantic.same_branch(left, right))
                     }) {
                         continue;
                     }
@@ -233,9 +233,9 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
 
                     // If the bindings are in different forks, abort.
                     if shadowed.source.map_or(true, |left| {
-                        binding.source.map_or(true, |right| {
-                            checker.semantic.different_branches(left, right)
-                        })
+                        binding
+                            .source
+                            .map_or(true, |right| !checker.semantic.same_branch(left, right))
                     }) {
                         continue;
                     }
diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs
index 9eb50868e6ad9..6f9ad095718bb 100644
--- a/crates/ruff_linter/src/rules/pyflakes/mod.rs
+++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs
@@ -120,6 +120,7 @@ mod tests {
     #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_24.py"))]
     #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_25.py"))]
     #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_26.py"))]
+    #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_27.py"))]
     #[test_case(Rule::UndefinedName, Path::new("F821_0.py"))]
     #[test_case(Rule::UndefinedName, Path::new("F821_1.py"))]
     #[test_case(Rule::UndefinedName, Path::new("F821_2.py"))]
diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_27.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_27.py.snap
new file mode 100644
index 0000000000000..d0b409f39ee0b
--- /dev/null
+++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_27.py.snap
@@ -0,0 +1,4 @@
+---
+source: crates/ruff_linter/src/rules/pyflakes/mod.rs
+---
+
diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs
index 578719a8e2268..27b6bed31e640 100644
--- a/crates/ruff_python_semantic/src/model.rs
+++ b/crates/ruff_python_semantic/src/model.rs
@@ -1163,11 +1163,11 @@ impl<'a> SemanticModel<'a> {
         false
     }
 
-    /// Returns `true` if `left` and `right` are on different branches of an `if`, `match`, or
+    /// Returns `true` if `left` and `right` are in the same branches of an `if`, `match`, or
     /// `try` statement.
     ///
     /// This implementation assumes that the statements are in the same scope.
-    pub fn different_branches(&self, left: NodeId, right: NodeId) -> bool {
+    pub fn same_branch(&self, left: NodeId, right: NodeId) -> bool {
         // Collect the branch path for the left statement.
         let left = self
             .nodes
@@ -1184,10 +1184,7 @@ impl<'a> SemanticModel<'a> {
             .flat_map(|branch_id| self.branches.ancestor_ids(*branch_id))
             .collect::<Vec<_>>();
 
-        !left
-            .iter()
-            .zip(right.iter())
-            .all(|(left, right)| left == right)
+        left == right
     }
 
     /// Returns `true` if the given expression is an unused variable, or consists solely of