From 373a77e8c2ba61a0296e57b223bf59d3ac5097ce Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 16 Mar 2023 22:52:05 -0400 Subject: [PATCH] Avoid C1901 violations within subscripts (#3517) --- .../test/fixtures/pylint/compare_to_empty_string.py | 4 ++++ .../src/rules/pylint/rules/compare_to_empty_string.rs | 8 ++++++++ crates/ruff_python_ast/src/context.rs | 5 +++++ 3 files changed, 17 insertions(+) diff --git a/crates/ruff/resources/test/fixtures/pylint/compare_to_empty_string.py b/crates/ruff/resources/test/fixtures/pylint/compare_to_empty_string.py index 91b2926272..0fe9431420 100644 --- a/crates/ruff/resources/test/fixtures/pylint/compare_to_empty_string.py +++ b/crates/ruff/resources/test/fixtures/pylint/compare_to_empty_string.py @@ -17,3 +17,7 @@ def errors(): def ok(): if x and not y: print("x is not an empty string, but y is an empty string") + + +data.loc[data["a"] != ""] +data.loc[data["a"] != "", :] diff --git a/crates/ruff/src/rules/pylint/rules/compare_to_empty_string.rs b/crates/ruff/src/rules/pylint/rules/compare_to_empty_string.rs index 06e23b065f..82d68c9738 100644 --- a/crates/ruff/src/rules/pylint/rules/compare_to_empty_string.rs +++ b/crates/ruff/src/rules/pylint/rules/compare_to_empty_string.rs @@ -74,6 +74,14 @@ pub fn compare_to_empty_string( ops: &[Cmpop], comparators: &[Expr], ) { + // Omit string comparison rules within subscripts. This is mostly commonly used within + // DataFrame and np.ndarray indexing. + for parent in checker.ctx.expr_ancestors() { + if matches!(parent.node, ExprKind::Subscript { .. }) { + return; + } + } + let mut first = true; for ((lhs, rhs), op) in std::iter::once(left) .chain(comparators.iter()) diff --git a/crates/ruff_python_ast/src/context.rs b/crates/ruff_python_ast/src/context.rs index bf00878a2c..341ac047b8 100644 --- a/crates/ruff_python_ast/src/context.rs +++ b/crates/ruff_python_ast/src/context.rs @@ -251,6 +251,11 @@ impl<'a> Context<'a> { self.exprs.iter().rev().nth(2) } + /// Return an [`Iterator`] over the current `Expr` parents. + pub fn expr_ancestors(&self) -> impl Iterator> { + self.exprs.iter().rev().skip(1) + } + /// Return the `Stmt` that immediately follows the current `Stmt`, if any. pub fn current_sibling_stmt(&self) -> Option<&'a Stmt> { self.body.get(self.body_index + 1)