Skip to content

Commit

Permalink
Fix false negative in outdated-version-block when using greater tha…
Browse files Browse the repository at this point in the history
…n comparisons (#7920)

Closes #7902
  • Loading branch information
zanieb authored Oct 11, 2023
1 parent a71c4df commit 23bbe73
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);
Expand Down Expand Up @@ -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<bool> {
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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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(())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")


0 comments on commit 23bbe73

Please sign in to comment.