From 23bbe7336a9edca8e4adfcd079f01f34fe0a6039 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 11 Oct 2023 14:33:43 -0500 Subject: [PATCH] Fix false negative in `outdated-version-block` when using greater than comparisons (#7920) Closes #7902 --- .../test/fixtures/pyupgrade/UP036_0.py | 6 ++ .../pyupgrade/rules/outdated_version_block.rs | 59 +++++++------------ ...__rules__pyupgrade__tests__UP036_0.py.snap | 18 ++++++ 3 files changed, 44 insertions(+), 39 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP036_0.py b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP036_0.py index 2cbdaf83359b4..a99e91f4533f6 100644 --- a/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP036_0.py +++ b/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP036_0.py @@ -196,3 +196,9 @@ def g(): if sys.version_info <= (3,10000000): print("py3") + +if sys.version_info > (3,12): + print("py3") + +if sys.version_info >= (3,12): + print("py3") diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/outdated_version_block.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/outdated_version_block.rs index bfaee87a73a32..784984e053eab 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/outdated_version_block.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/outdated_version_block.rs @@ -104,12 +104,18 @@ pub(crate) fn outdated_version_block(checker: &mut Checker, stmt_if: &StmtIf) { match comparison { Expr::Tuple(ast::ExprTuple { elts, .. }) => match op { - CmpOp::Lt | CmpOp::LtE => { + CmpOp::Lt | CmpOp::LtE | CmpOp::Gt | CmpOp::GtE => { let Some(version) = extract_version(elts) else { return; }; let target = checker.settings.target_version; - match compare_version(&version, target, op == &CmpOp::LtE) { + match version_always_less_than( + &version, + target, + // `x <= y` and `x > y` are cases where `x == y` will not stop the comparison + // from always evaluating to true or false respectively + op.is_lt_e() || op.is_gt(), + ) { Ok(false) => {} Ok(true) => { let mut diagnostic = Diagnostic::new( @@ -118,36 +124,11 @@ pub(crate) fn outdated_version_block(checker: &mut Checker, stmt_if: &StmtIf) { }, branch.test.range(), ); - if let Some(fix) = fix_always_false_branch(checker, stmt_if, &branch) { - diagnostic.set_fix(fix); - } - checker.diagnostics.push(diagnostic); - } - Err(_) => { - checker.diagnostics.push(Diagnostic::new( - OutdatedVersionBlock { - reason: Reason::Invalid, - }, - comparison.range(), - )); - } - } - } - CmpOp::Gt | CmpOp::GtE => { - let Some(version) = extract_version(elts) else { - return; - }; - let target = checker.settings.target_version; - match compare_version(&version, target, op == &CmpOp::GtE) { - Ok(false) => {} - Ok(true) => { - let mut diagnostic = Diagnostic::new( - OutdatedVersionBlock { - reason: Reason::Outdated, - }, - branch.test.range(), - ); - if let Some(fix) = fix_always_true_branch(checker, stmt_if, &branch) { + if let Some(fix) = if op.is_lt() || op.is_lt_e() { + fix_always_false_branch(checker, stmt_if, &branch) + } else { + fix_always_true_branch(checker, stmt_if, &branch) + } { diagnostic.set_fix(fix); } checker.diagnostics.push(diagnostic); @@ -211,15 +192,15 @@ pub(crate) fn outdated_version_block(checker: &mut Checker, stmt_if: &StmtIf) { } } -/// Returns true if the `target_version` is always less than the [`PythonVersion`]. -fn compare_version( - target_version: &[Int], +/// Returns true if the `check_version` is always less than the [`PythonVersion`]. +fn version_always_less_than( + check_version: &[Int], py_version: PythonVersion, or_equal: bool, ) -> Result { - let mut target_version_iter = target_version.iter(); + let mut check_version_iter = check_version.iter(); - let Some(if_major) = target_version_iter.next() else { + let Some(if_major) = check_version_iter.next() else { return Ok(false); }; let Some(if_major) = if_major.as_u8() else { @@ -232,7 +213,7 @@ fn compare_version( Ordering::Less => Ok(true), Ordering::Greater => Ok(false), Ordering::Equal => { - let Some(if_minor) = target_version_iter.next() else { + let Some(if_minor) = check_version_iter.next() else { return Ok(true); }; let Some(if_minor) = if_minor.as_u8() else { @@ -462,7 +443,7 @@ mod tests { expected: bool, ) -> Result<()> { let target_versions: Vec<_> = target_versions.iter().map(|int| Int::from(*int)).collect(); - let actual = compare_version(&target_versions, version, or_equal)?; + let actual = version_always_less_than(&target_versions, version, or_equal)?; assert_eq!(actual, expected); Ok(()) } diff --git a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP036_0.py.snap b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP036_0.py.snap index 6d66d23ef17be..0f42d7f67136b 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP036_0.py.snap +++ b/crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP036_0.py.snap @@ -712,4 +712,22 @@ UP036_0.py:197:24: UP036 Version specifier is invalid 198 | print("py3") | +UP036_0.py:203:4: UP036 [*] Version block is outdated for minimum Python version + | +201 | print("py3") +202 | +203 | if sys.version_info >= (3,12): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ UP036 +204 | print("py3") + | + = help: Remove outdated version block + +ℹ Suggested fix +200 200 | if sys.version_info > (3,12): +201 201 | print("py3") +202 202 | +203 |-if sys.version_info >= (3,12): +204 |- print("py3") + 203 |+print("py3") +