From 2838f7af983432890c44548c327cdd672bc4baad Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 1 Oct 2023 10:57:40 -0400 Subject: [PATCH] Skip all bracketed expressions when locating comparison ops (#7740) Closes https://github.com/astral-sh/ruff/issues/7737. --- .../resources/test/fixtures/pyflakes/F632.py | 4 + ..._rules__pyflakes__tests__F632_F632.py.snap | 39 ++++++ crates/ruff_python_parser/src/lib.rs | 123 ++++++++++-------- 3 files changed, 114 insertions(+), 52 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F632.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F632.py index 9f40368ad8..6e18123898 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyflakes/F632.py +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F632.py @@ -25,3 +25,7 @@ not ''} {2 is not ''} + +# Regression test for +if values[1is not None ] is not '-': + pass diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F632_F632.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F632_F632.py.snap index b23df34224..c1d0d79dd1 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F632_F632.py.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F632_F632.py.snap @@ -171,6 +171,8 @@ F632.py:26:2: F632 [*] Use `!=` to compare constant literals | __^ 27 | | not ''} | |_______^ F632 +28 | +29 | # Regression test for | = help: Replace `is not` with `!=` @@ -181,5 +183,42 @@ F632.py:26:2: F632 [*] Use `!=` to compare constant literals 26 |-{2 is 27 |- not ''} 26 |+{2 != ''} +28 27 | +29 28 | # Regression test for +30 29 | if values[1is not None ] is not '-': + +F632.py:30:4: F632 [*] Use `!=` to compare constant literals + | +29 | # Regression test for +30 | if values[1is not None ] is not '-': + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ F632 +31 | pass + | + = help: Replace `is not` with `!=` + +ℹ Fix +27 27 | not ''} +28 28 | +29 29 | # Regression test for +30 |-if values[1is not None ] is not '-': + 30 |+if values[1is not None ] != '-': +31 31 | pass + +F632.py:30:11: F632 [*] Use `!=` to compare constant literals + | +29 | # Regression test for +30 | if values[1is not None ] is not '-': + | ^^^^^^^^^^^^ F632 +31 | pass + | + = help: Replace `is not` with `!=` + +ℹ Fix +27 27 | not ''} +28 28 | +29 29 | # Regression test for +30 |-if values[1is not None ] is not '-': + 30 |+if values[1!= None ] is not '-': +31 31 | pass diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs index e3bc01d98d..28ff5229c1 100644 --- a/crates/ruff_python_parser/src/lib.rs +++ b/crates/ruff_python_parser/src/lib.rs @@ -180,66 +180,85 @@ pub fn locate_cmp_ops(expr: &Expr, source: &str) -> Vec { .peekable(); let mut ops: Vec = vec![]; - let mut count = 0u32; + + // Track the bracket depth. + let mut par_count = 0u32; + let mut sqb_count = 0u32; + let mut brace_count = 0u32; + loop { let Some((tok, range)) = tok_iter.next() else { break; }; - if matches!(tok, Tok::Lpar) { - count = count.saturating_add(1); - continue; - } else if matches!(tok, Tok::Rpar) { - count = count.saturating_sub(1); + + match tok { + Tok::Lpar => { + par_count = par_count.saturating_add(1); + } + Tok::Rpar => { + par_count = par_count.saturating_sub(1); + } + Tok::Lsqb => { + sqb_count = sqb_count.saturating_add(1); + } + Tok::Rsqb => { + sqb_count = sqb_count.saturating_sub(1); + } + Tok::Lbrace => { + brace_count = brace_count.saturating_add(1); + } + Tok::Rbrace => { + brace_count = brace_count.saturating_sub(1); + } + _ => {} + } + + if par_count > 0 || sqb_count > 0 || brace_count > 0 { continue; } - if count == 0 { - match tok { - Tok::Not => { - if let Some((_, next_range)) = - tok_iter.next_if(|(tok, _)| matches!(tok, Tok::In)) - { - ops.push(LocatedCmpOp::new( - TextRange::new(range.start(), next_range.end()), - CmpOp::NotIn, - )); - } + + match tok { + Tok::Not => { + if let Some((_, next_range)) = tok_iter.next_if(|(tok, _)| tok.is_in()) { + ops.push(LocatedCmpOp::new( + TextRange::new(range.start(), next_range.end()), + CmpOp::NotIn, + )); } - Tok::In => { - ops.push(LocatedCmpOp::new(range, CmpOp::In)); - } - Tok::Is => { - let op = if let Some((_, next_range)) = - tok_iter.next_if(|(tok, _)| matches!(tok, Tok::Not)) - { - LocatedCmpOp::new( - TextRange::new(range.start(), next_range.end()), - CmpOp::IsNot, - ) - } else { - LocatedCmpOp::new(range, CmpOp::Is) - }; - ops.push(op); - } - Tok::NotEqual => { - ops.push(LocatedCmpOp::new(range, CmpOp::NotEq)); - } - Tok::EqEqual => { - ops.push(LocatedCmpOp::new(range, CmpOp::Eq)); - } - Tok::GreaterEqual => { - ops.push(LocatedCmpOp::new(range, CmpOp::GtE)); - } - Tok::Greater => { - ops.push(LocatedCmpOp::new(range, CmpOp::Gt)); - } - Tok::LessEqual => { - ops.push(LocatedCmpOp::new(range, CmpOp::LtE)); - } - Tok::Less => { - ops.push(LocatedCmpOp::new(range, CmpOp::Lt)); - } - _ => {} } + Tok::In => { + ops.push(LocatedCmpOp::new(range, CmpOp::In)); + } + Tok::Is => { + let op = if let Some((_, next_range)) = tok_iter.next_if(|(tok, _)| tok.is_not()) { + LocatedCmpOp::new( + TextRange::new(range.start(), next_range.end()), + CmpOp::IsNot, + ) + } else { + LocatedCmpOp::new(range, CmpOp::Is) + }; + ops.push(op); + } + Tok::NotEqual => { + ops.push(LocatedCmpOp::new(range, CmpOp::NotEq)); + } + Tok::EqEqual => { + ops.push(LocatedCmpOp::new(range, CmpOp::Eq)); + } + Tok::GreaterEqual => { + ops.push(LocatedCmpOp::new(range, CmpOp::GtE)); + } + Tok::Greater => { + ops.push(LocatedCmpOp::new(range, CmpOp::Gt)); + } + Tok::LessEqual => { + ops.push(LocatedCmpOp::new(range, CmpOp::LtE)); + } + Tok::Less => { + ops.push(LocatedCmpOp::new(range, CmpOp::Lt)); + } + _ => {} } } ops