mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-04 02:38:25 +00:00
Implement our own small-integer optimization (#7584)
## Summary This is a follow-up to #7469 that attempts to achieve similar gains, but without introducing malachite. Instead, this PR removes the `BigInt` type altogether, instead opting for a simple enum that allows us to store small integers directly and only allocate for values greater than `i64`: ```rust /// A Python integer literal. Represents both small (fits in an `i64`) and large integers. #[derive(Clone, PartialEq, Eq, Hash)] pub struct Int(Number); #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum Number { /// A "small" number that can be represented as an `i64`. Small(i64), /// A "large" number that cannot be represented as an `i64`. Big(Box<str>), } impl std::fmt::Display for Number { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Number::Small(value) => write!(f, "{value}"), Number::Big(value) => write!(f, "{value}"), } } } ``` We typically don't care about numbers greater than `isize` -- our only uses are comparisons against small constants (like `1`, `2`, `3`, etc.), so there's no real loss of information, except in one or two rules where we're now a little more conservative (with the worst-case being that we don't flag, e.g., an `itertools.pairwise` that uses an extremely large value for the slice start constant). For simplicity, a few diagnostics now show a dedicated message when they see integers that are out of the supported range (e.g., `outdated-version-block`). An additional benefit here is that we get to remove a few dependencies, especially `num-bigint`. ## Test Plan `cargo test`
This commit is contained in:
Charlie Marsh
GitHub
parent
65aebf127a
commit
93b5d8a0fb
40 changed files with 707 additions and 385 deletions
|
|
@ -45,8 +45,6 @@ libcst = { workspace = true }
|
|||
log = { workspace = true }
|
||||
memchr = { workspace = true }
|
||||
natord = { version = "1.0.9" }
|
||||
num-bigint = { workspace = true }
|
||||
num-traits = { workspace = true }
|
||||
once_cell = { workspace = true }
|
||||
path-absolutize = { workspace = true, features = [
|
||||
"once_cell_cache",
|
||||
|
|
|
|||
|
|
@ -20,3 +20,4 @@ os.chmod(keyfile, stat.S_IRWXO | stat.S_IRWXG | stat.S_IRWXU) # Error
|
|||
os.chmod("~/hidden_exec", stat.S_IXGRP) # Error
|
||||
os.chmod("~/hidden_exec", stat.S_IXOTH) # OK
|
||||
os.chmod("/etc/passwd", stat.S_IWOTH) # Error
|
||||
os.chmod("/etc/passwd", 0o100000000) # Error
|
||||
|
|
|
|||
|
|
@ -184,3 +184,15 @@ if sys.version_info < (3,12):
|
|||
|
||||
if sys.version_info <= (3,12):
|
||||
print("py3")
|
||||
|
||||
if sys.version_info <= (3,12):
|
||||
print("py3")
|
||||
|
||||
if sys.version_info == 10000000:
|
||||
print("py3")
|
||||
|
||||
if sys.version_info < (3,10000000):
|
||||
print("py3")
|
||||
|
||||
if sys.version_info <= (3,10000000):
|
||||
print("py3")
|
||||
|
|
|
|||
|
|
@ -1,8 +1,6 @@
|
|||
use num_bigint::BigInt;
|
||||
use ruff_python_ast::{self as ast, CmpOp, Constant, Expr};
|
||||
|
||||
use ruff_diagnostics::{Diagnostic, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::{self as ast, CmpOp, Constant, Expr};
|
||||
use ruff_text_size::Ranged;
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
|
|
@ -237,7 +235,7 @@ pub(crate) fn compare(checker: &mut Checker, left: &Expr, ops: &[CmpOp], compara
|
|||
..
|
||||
}) = slice.as_ref()
|
||||
{
|
||||
if *i == BigInt::from(0) {
|
||||
if *i == 0 {
|
||||
if let (
|
||||
[CmpOp::Eq | CmpOp::NotEq],
|
||||
[Expr::Constant(ast::ExprConstant {
|
||||
|
|
@ -246,13 +244,13 @@ pub(crate) fn compare(checker: &mut Checker, left: &Expr, ops: &[CmpOp], compara
|
|||
})],
|
||||
) = (ops, comparators)
|
||||
{
|
||||
if *n == BigInt::from(3) && checker.enabled(Rule::SysVersionInfo0Eq3) {
|
||||
if *n == 3 && checker.enabled(Rule::SysVersionInfo0Eq3) {
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(SysVersionInfo0Eq3, left.range()));
|
||||
}
|
||||
}
|
||||
} else if *i == BigInt::from(1) {
|
||||
} else if *i == 1 {
|
||||
if let (
|
||||
[CmpOp::Lt | CmpOp::LtE | CmpOp::Gt | CmpOp::GtE],
|
||||
[Expr::Constant(ast::ExprConstant {
|
||||
|
|
|
|||
|
|
@ -1,8 +1,6 @@
|
|||
use num_bigint::BigInt;
|
||||
use ruff_python_ast::{self as ast, Constant, Expr};
|
||||
|
||||
use ruff_diagnostics::{Diagnostic, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::{self as ast, Constant, Expr};
|
||||
use ruff_text_size::Ranged;
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
|
|
@ -184,11 +182,11 @@ pub(crate) fn subscript(checker: &mut Checker, value: &Expr, slice: &Expr) {
|
|||
..
|
||||
}) = upper.as_ref()
|
||||
{
|
||||
if *i == BigInt::from(1) && checker.enabled(Rule::SysVersionSlice1) {
|
||||
if *i == 1 && checker.enabled(Rule::SysVersionSlice1) {
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(SysVersionSlice1, value.range()));
|
||||
} else if *i == BigInt::from(3) && checker.enabled(Rule::SysVersionSlice3) {
|
||||
} else if *i == 3 && checker.enabled(Rule::SysVersionSlice3) {
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(SysVersionSlice3, value.range()));
|
||||
|
|
@ -200,11 +198,11 @@ pub(crate) fn subscript(checker: &mut Checker, value: &Expr, slice: &Expr) {
|
|||
value: Constant::Int(i),
|
||||
..
|
||||
}) => {
|
||||
if *i == BigInt::from(2) && checker.enabled(Rule::SysVersion2) {
|
||||
if *i == 2 && checker.enabled(Rule::SysVersion2) {
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(SysVersion2, value.range()));
|
||||
} else if *i == BigInt::from(0) && checker.enabled(Rule::SysVersion0) {
|
||||
} else if *i == 0 && checker.enabled(Rule::SysVersion0) {
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(SysVersion0, value.range()));
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
use num_traits::ToPrimitive;
|
||||
use anyhow::Result;
|
||||
|
||||
use ruff_diagnostics::{Diagnostic, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
|
|
@ -36,17 +36,28 @@ use crate::checkers::ast::Checker;
|
|||
/// - [Common Weakness Enumeration: CWE-732](https://cwe.mitre.org/data/definitions/732.html)
|
||||
#[violation]
|
||||
pub struct BadFilePermissions {
|
||||
mask: u16,
|
||||
reason: Reason,
|
||||
}
|
||||
|
||||
impl Violation for BadFilePermissions {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
let BadFilePermissions { mask } = self;
|
||||
format!("`os.chmod` setting a permissive mask `{mask:#o}` on file or directory")
|
||||
let BadFilePermissions { reason } = self;
|
||||
match reason {
|
||||
Reason::Permissive(mask) => {
|
||||
format!("`os.chmod` setting a permissive mask `{mask:#o}` on file or directory")
|
||||
}
|
||||
Reason::Invalid => format!("`os.chmod` setting an invalid mask on file or directory"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
enum Reason {
|
||||
Permissive(u16),
|
||||
Invalid,
|
||||
}
|
||||
|
||||
/// S103
|
||||
pub(crate) fn bad_file_permissions(checker: &mut Checker, call: &ast::ExprCall) {
|
||||
if checker
|
||||
|
|
@ -55,10 +66,26 @@ pub(crate) fn bad_file_permissions(checker: &mut Checker, call: &ast::ExprCall)
|
|||
.is_some_and(|call_path| matches!(call_path.as_slice(), ["os", "chmod"]))
|
||||
{
|
||||
if let Some(mode_arg) = call.arguments.find_argument("mode", 1) {
|
||||
if let Some(int_value) = int_value(mode_arg, checker.semantic()) {
|
||||
if (int_value & WRITE_WORLD > 0) || (int_value & EXECUTE_GROUP > 0) {
|
||||
match parse_mask(mode_arg, checker.semantic()) {
|
||||
// The mask couldn't be determined (e.g., it's dynamic).
|
||||
Ok(None) => {}
|
||||
// The mask is a valid integer value -- check for overly permissive permissions.
|
||||
Ok(Some(mask)) => {
|
||||
if (mask & WRITE_WORLD > 0) || (mask & EXECUTE_GROUP > 0) {
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
BadFilePermissions {
|
||||
reason: Reason::Permissive(mask),
|
||||
},
|
||||
mode_arg.range(),
|
||||
));
|
||||
}
|
||||
}
|
||||
// The mask is an invalid integer value (i.e., it's out of range).
|
||||
Err(_) => {
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
BadFilePermissions { mask: int_value },
|
||||
BadFilePermissions {
|
||||
reason: Reason::Invalid,
|
||||
},
|
||||
mode_arg.range(),
|
||||
));
|
||||
}
|
||||
|
|
@ -113,28 +140,37 @@ fn py_stat(call_path: &CallPath) -> Option<u16> {
|
|||
}
|
||||
}
|
||||
|
||||
fn int_value(expr: &Expr, semantic: &SemanticModel) -> Option<u16> {
|
||||
/// Return the mask value as a `u16`, if it can be determined. Returns an error if the mask is
|
||||
/// an integer value, but that value is out of range.
|
||||
fn parse_mask(expr: &Expr, semantic: &SemanticModel) -> Result<Option<u16>> {
|
||||
match expr {
|
||||
Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(value),
|
||||
value: Constant::Int(int),
|
||||
..
|
||||
}) => value.to_u16(),
|
||||
Expr::Attribute(_) => semantic.resolve_call_path(expr).as_ref().and_then(py_stat),
|
||||
}) => match int.as_u16() {
|
||||
Some(value) => Ok(Some(value)),
|
||||
None => anyhow::bail!("int value out of range"),
|
||||
},
|
||||
Expr::Attribute(_) => Ok(semantic.resolve_call_path(expr).as_ref().and_then(py_stat)),
|
||||
Expr::BinOp(ast::ExprBinOp {
|
||||
left,
|
||||
op,
|
||||
right,
|
||||
range: _,
|
||||
}) => {
|
||||
let left_value = int_value(left, semantic)?;
|
||||
let right_value = int_value(right, semantic)?;
|
||||
match op {
|
||||
let Some(left_value) = parse_mask(left, semantic)? else {
|
||||
return Ok(None);
|
||||
};
|
||||
let Some(right_value) = parse_mask(right, semantic)? else {
|
||||
return Ok(None);
|
||||
};
|
||||
Ok(match op {
|
||||
Operator::BitAnd => Some(left_value & right_value),
|
||||
Operator::BitOr => Some(left_value | right_value),
|
||||
Operator::BitXor => Some(left_value ^ right_value),
|
||||
_ => None,
|
||||
}
|
||||
})
|
||||
}
|
||||
_ => None,
|
||||
_ => Ok(None),
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,8 +1,6 @@
|
|||
use num_traits::{One, Zero};
|
||||
|
||||
use ruff_diagnostics::{Diagnostic, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::{self as ast, Constant, Expr};
|
||||
use ruff_python_ast::{self as ast, Constant, Expr, Int};
|
||||
use ruff_text_size::Ranged;
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
|
|
@ -52,16 +50,16 @@ pub(crate) fn snmp_insecure_version(checker: &mut Checker, call: &ast::ExprCall)
|
|||
})
|
||||
{
|
||||
if let Some(keyword) = call.arguments.find_keyword("mpModel") {
|
||||
if let Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(value),
|
||||
..
|
||||
}) = &keyword.value
|
||||
{
|
||||
if value.is_zero() || value.is_one() {
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(SnmpInsecureVersion, keyword.range()));
|
||||
}
|
||||
if matches!(
|
||||
keyword.value,
|
||||
Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(Int::ZERO | Int::ONE),
|
||||
..
|
||||
})
|
||||
) {
|
||||
checker
|
||||
.diagnostics
|
||||
.push(Diagnostic::new(SnmpInsecureVersion, keyword.range()));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -126,6 +126,15 @@ S103.py:22:25: S103 `os.chmod` setting a permissive mask `0o2` on file or direct
|
|||
21 | os.chmod("~/hidden_exec", stat.S_IXOTH) # OK
|
||||
22 | os.chmod("/etc/passwd", stat.S_IWOTH) # Error
|
||||
| ^^^^^^^^^^^^ S103
|
||||
23 | os.chmod("/etc/passwd", 0o100000000) # Error
|
||||
|
|
||||
|
||||
S103.py:23:25: S103 `os.chmod` setting an invalid mask on file or directory
|
||||
|
|
||||
21 | os.chmod("~/hidden_exec", stat.S_IXOTH) # OK
|
||||
22 | os.chmod("/etc/passwd", stat.S_IWOTH) # Error
|
||||
23 | os.chmod("/etc/passwd", 0o100000000) # Error
|
||||
| ^^^^^^^^^^^ S103
|
||||
|
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,3 @@
|
|||
use num_bigint::BigInt;
|
||||
|
||||
use ruff_diagnostics::{Diagnostic, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::{self as ast, Constant, Expr, UnaryOp};
|
||||
|
|
@ -93,7 +91,7 @@ pub(crate) fn unnecessary_subscript_reversal(
|
|||
else {
|
||||
return;
|
||||
};
|
||||
if *val != BigInt::from(1) {
|
||||
if *val != 1 {
|
||||
return;
|
||||
};
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
|
|
|
|||
|
|
@ -1,5 +1,3 @@
|
|||
use num_bigint::BigInt;
|
||||
|
||||
use ruff_diagnostics::Diagnostic;
|
||||
use ruff_diagnostics::{AlwaysAutofixableViolation, Fix};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
|
|
@ -75,7 +73,7 @@ pub(crate) fn unnecessary_range_start(checker: &mut Checker, call: &ast::ExprCal
|
|||
else {
|
||||
return;
|
||||
};
|
||||
if *value != BigInt::from(0) {
|
||||
if *value != 0 {
|
||||
return;
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -1,10 +1,7 @@
|
|||
use num_bigint::BigInt;
|
||||
use num_traits::{One, Zero};
|
||||
use ruff_python_ast::{self as ast, CmpOp, Constant, Expr};
|
||||
|
||||
use ruff_diagnostics::{Diagnostic, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::helpers::map_subscript;
|
||||
use ruff_python_ast::{self as ast, CmpOp, Constant, Expr, Int};
|
||||
use ruff_text_size::Ranged;
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
|
|
@ -249,18 +246,18 @@ impl ExpectedComparator {
|
|||
..
|
||||
}) = upper.as_ref()
|
||||
{
|
||||
if *upper == BigInt::one() {
|
||||
if *upper == 1 {
|
||||
return Some(ExpectedComparator::MajorTuple);
|
||||
}
|
||||
if *upper == BigInt::from(2) {
|
||||
if *upper == 2 {
|
||||
return Some(ExpectedComparator::MajorMinorTuple);
|
||||
}
|
||||
}
|
||||
}
|
||||
Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(n),
|
||||
value: Constant::Int(Int::ZERO),
|
||||
..
|
||||
}) if n.is_zero() => {
|
||||
}) => {
|
||||
return Some(ExpectedComparator::MajorDigit);
|
||||
}
|
||||
_ => (),
|
||||
|
|
|
|||
|
|
@ -1,9 +1,7 @@
|
|||
use num_traits::One;
|
||||
use ruff_python_ast::{self as ast, CmpOp, Constant, Expr};
|
||||
|
||||
use ruff_diagnostics::Diagnostic;
|
||||
use ruff_diagnostics::Violation;
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::{self as ast, CmpOp, Constant, Expr, Int};
|
||||
use ruff_text_size::Ranged;
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
|
|
@ -80,7 +78,13 @@ pub(crate) fn nunique_constant_series_check(
|
|||
}
|
||||
|
||||
// Right should be the integer 1.
|
||||
if !is_constant_one(right) {
|
||||
if !matches!(
|
||||
right,
|
||||
Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(Int::ONE),
|
||||
range: _,
|
||||
})
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -110,14 +114,3 @@ pub(crate) fn nunique_constant_series_check(
|
|||
expr.range(),
|
||||
));
|
||||
}
|
||||
|
||||
/// Return `true` if an [`Expr`] is a constant `1`.
|
||||
fn is_constant_one(expr: &Expr) -> bool {
|
||||
match expr {
|
||||
Expr::Constant(constant) => match &constant.value {
|
||||
Constant::Int(int) => int.is_one(),
|
||||
_ => false,
|
||||
},
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
use itertools::Itertools;
|
||||
use ruff_python_ast::{self as ast, Constant, Expr, UnaryOp};
|
||||
use ruff_python_ast::{self as ast, Constant, Expr, Int, UnaryOp};
|
||||
|
||||
use ruff_diagnostics::{Diagnostic, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
|
|
@ -83,7 +83,7 @@ fn is_magic_value(constant: &Constant, allowed_types: &[ConstantType]) -> bool {
|
|||
Constant::Str(ast::StringConstant { value, .. }) => {
|
||||
!matches!(value.as_str(), "" | "__main__")
|
||||
}
|
||||
Constant::Int(value) => !matches!(value.try_into(), Ok(0 | 1)),
|
||||
Constant::Int(value) => !matches!(*value, Int::ZERO | Int::ONE),
|
||||
Constant::Bytes(_) => true,
|
||||
Constant::Float(_) => true,
|
||||
Constant::Complex { .. } => true,
|
||||
|
|
|
|||
|
|
@ -1,8 +1,6 @@
|
|||
use std::fmt;
|
||||
use std::str::FromStr;
|
||||
|
||||
use num_bigint::BigInt;
|
||||
|
||||
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::{self as ast, Constant, Expr};
|
||||
|
|
@ -47,7 +45,7 @@ impl From<LiteralType> for Constant {
|
|||
value: Vec::new(),
|
||||
implicit_concatenated: false,
|
||||
}),
|
||||
LiteralType::Int => Constant::Int(BigInt::from(0)),
|
||||
LiteralType::Int => Constant::Int(0.into()),
|
||||
LiteralType::Float => Constant::Float(0.0),
|
||||
LiteralType::Bool => Constant::Bool(false),
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,12 +1,11 @@
|
|||
use std::cmp::Ordering;
|
||||
|
||||
use num_bigint::{BigInt, Sign};
|
||||
|
||||
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
|
||||
use anyhow::Result;
|
||||
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::stmt_if::{if_elif_branches, BranchKind, IfElifBranch};
|
||||
use ruff_python_ast::whitespace::indentation;
|
||||
use ruff_python_ast::{self as ast, CmpOp, Constant, ElifElseClause, Expr, StmtIf};
|
||||
use ruff_python_ast::{self as ast, CmpOp, Constant, ElifElseClause, Expr, Int, StmtIf};
|
||||
use ruff_text_size::{Ranged, TextLen, TextRange};
|
||||
|
||||
use crate::autofix::edits::delete_stmt;
|
||||
|
|
@ -47,19 +46,37 @@ use crate::settings::types::PythonVersion;
|
|||
/// ## References
|
||||
/// - [Python documentation: `sys.version_info`](https://docs.python.org/3/library/sys.html#sys.version_info)
|
||||
#[violation]
|
||||
pub struct OutdatedVersionBlock;
|
||||
pub struct OutdatedVersionBlock {
|
||||
reason: Reason,
|
||||
}
|
||||
|
||||
impl Violation for OutdatedVersionBlock {
|
||||
const AUTOFIX: AutofixKind = AutofixKind::Sometimes;
|
||||
|
||||
impl AlwaysAutofixableViolation for OutdatedVersionBlock {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
format!("Version block is outdated for minimum Python version")
|
||||
let OutdatedVersionBlock { reason } = self;
|
||||
match reason {
|
||||
Reason::Outdated => format!("Version block is outdated for minimum Python version"),
|
||||
Reason::Invalid => format!("Version specifier is invalid"),
|
||||
}
|
||||
}
|
||||
|
||||
fn autofix_title(&self) -> String {
|
||||
"Remove outdated version block".to_string()
|
||||
fn autofix_title(&self) -> Option<String> {
|
||||
let OutdatedVersionBlock { reason } = self;
|
||||
match reason {
|
||||
Reason::Outdated => Some("Remove outdated version block".to_string()),
|
||||
Reason::Invalid => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
enum Reason {
|
||||
Outdated,
|
||||
Invalid,
|
||||
}
|
||||
|
||||
/// UP036
|
||||
pub(crate) fn outdated_version_block(checker: &mut Checker, stmt_if: &StmtIf) {
|
||||
for branch in if_elif_branches(stmt_if) {
|
||||
|
|
@ -88,44 +105,19 @@ 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 => {
|
||||
let version = extract_version(elts);
|
||||
let Some(version) = extract_version(elts) else {
|
||||
return;
|
||||
};
|
||||
let target = checker.settings.target_version;
|
||||
if compare_version(&version, target, op == &CmpOp::LtE) {
|
||||
let mut diagnostic =
|
||||
Diagnostic::new(OutdatedVersionBlock, branch.test.range());
|
||||
if checker.patch(diagnostic.kind.rule()) {
|
||||
if let Some(fix) = fix_always_false_branch(checker, stmt_if, &branch) {
|
||||
diagnostic.set_fix(fix);
|
||||
}
|
||||
}
|
||||
checker.diagnostics.push(diagnostic);
|
||||
}
|
||||
}
|
||||
CmpOp::Gt | CmpOp::GtE => {
|
||||
let version = extract_version(elts);
|
||||
let target = checker.settings.target_version;
|
||||
if compare_version(&version, target, op == &CmpOp::GtE) {
|
||||
let mut diagnostic =
|
||||
Diagnostic::new(OutdatedVersionBlock, branch.test.range());
|
||||
if checker.patch(diagnostic.kind.rule()) {
|
||||
if let Some(fix) = fix_always_true_branch(checker, stmt_if, &branch) {
|
||||
diagnostic.set_fix(fix);
|
||||
}
|
||||
}
|
||||
checker.diagnostics.push(diagnostic);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
},
|
||||
Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(number),
|
||||
..
|
||||
}) => {
|
||||
if op == &CmpOp::Eq {
|
||||
match bigint_to_u32(number) {
|
||||
2 => {
|
||||
let mut diagnostic =
|
||||
Diagnostic::new(OutdatedVersionBlock, branch.test.range());
|
||||
match compare_version(&version, target, op == &CmpOp::LtE) {
|
||||
Ok(false) => {}
|
||||
Ok(true) => {
|
||||
let mut diagnostic = Diagnostic::new(
|
||||
OutdatedVersionBlock {
|
||||
reason: Reason::Outdated,
|
||||
},
|
||||
branch.test.range(),
|
||||
);
|
||||
if checker.patch(diagnostic.kind.rule()) {
|
||||
if let Some(fix) =
|
||||
fix_always_false_branch(checker, stmt_if, &branch)
|
||||
|
|
@ -135,9 +127,30 @@ pub(crate) fn outdated_version_block(checker: &mut Checker, stmt_if: &StmtIf) {
|
|||
}
|
||||
checker.diagnostics.push(diagnostic);
|
||||
}
|
||||
3 => {
|
||||
let mut diagnostic =
|
||||
Diagnostic::new(OutdatedVersionBlock, branch.test.range());
|
||||
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 checker.patch(diagnostic.kind.rule()) {
|
||||
if let Some(fix) = fix_always_true_branch(checker, stmt_if, &branch)
|
||||
{
|
||||
|
|
@ -146,6 +159,63 @@ pub(crate) fn outdated_version_block(checker: &mut Checker, stmt_if: &StmtIf) {
|
|||
}
|
||||
checker.diagnostics.push(diagnostic);
|
||||
}
|
||||
Err(_) => {
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
OutdatedVersionBlock {
|
||||
reason: Reason::Invalid,
|
||||
},
|
||||
comparison.range(),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
},
|
||||
Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(int),
|
||||
..
|
||||
}) => {
|
||||
if op == &CmpOp::Eq {
|
||||
match int.as_u8() {
|
||||
Some(2) => {
|
||||
let mut diagnostic = Diagnostic::new(
|
||||
OutdatedVersionBlock {
|
||||
reason: Reason::Outdated,
|
||||
},
|
||||
branch.test.range(),
|
||||
);
|
||||
if checker.patch(diagnostic.kind.rule()) {
|
||||
if let Some(fix) =
|
||||
fix_always_false_branch(checker, stmt_if, &branch)
|
||||
{
|
||||
diagnostic.set_fix(fix);
|
||||
}
|
||||
}
|
||||
checker.diagnostics.push(diagnostic);
|
||||
}
|
||||
Some(3) => {
|
||||
let mut diagnostic = Diagnostic::new(
|
||||
OutdatedVersionBlock {
|
||||
reason: Reason::Outdated,
|
||||
},
|
||||
branch.test.range(),
|
||||
);
|
||||
if checker.patch(diagnostic.kind.rule()) {
|
||||
if let Some(fix) = fix_always_true_branch(checker, stmt_if, &branch)
|
||||
{
|
||||
diagnostic.set_fix(fix);
|
||||
}
|
||||
}
|
||||
checker.diagnostics.push(diagnostic);
|
||||
}
|
||||
None => {
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
OutdatedVersionBlock {
|
||||
reason: Reason::Invalid,
|
||||
},
|
||||
comparison.range(),
|
||||
));
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
|
@ -156,31 +226,42 @@ 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: &[u32], py_version: PythonVersion, or_equal: bool) -> bool {
|
||||
fn compare_version(
|
||||
target_version: &[Int],
|
||||
py_version: PythonVersion,
|
||||
or_equal: bool,
|
||||
) -> Result<bool> {
|
||||
let mut target_version_iter = target_version.iter();
|
||||
|
||||
let Some(if_major) = target_version_iter.next() else {
|
||||
return false;
|
||||
return Ok(false);
|
||||
};
|
||||
let Some(if_major) = if_major.as_u8() else {
|
||||
return Err(anyhow::anyhow!("invalid major version: {if_major}"));
|
||||
};
|
||||
|
||||
let (py_major, py_minor) = py_version.as_tuple();
|
||||
|
||||
match if_major.cmp(&py_major.into()) {
|
||||
Ordering::Less => true,
|
||||
Ordering::Greater => false,
|
||||
match if_major.cmp(&py_major) {
|
||||
Ordering::Less => Ok(true),
|
||||
Ordering::Greater => Ok(false),
|
||||
Ordering::Equal => {
|
||||
let Some(if_minor) = target_version_iter.next() else {
|
||||
return true;
|
||||
return Ok(true);
|
||||
};
|
||||
if or_equal {
|
||||
let Some(if_minor) = if_minor.as_u8() else {
|
||||
return Err(anyhow::anyhow!("invalid minor version: {if_minor}"));
|
||||
};
|
||||
|
||||
Ok(if or_equal {
|
||||
// Ex) `sys.version_info <= 3.8`. If Python 3.8 is the minimum supported version,
|
||||
// the condition won't always evaluate to `false`, so we want to return `false`.
|
||||
*if_minor < py_minor.into()
|
||||
if_minor < py_minor
|
||||
} else {
|
||||
// Ex) `sys.version_info < 3.8`. If Python 3.8 is the minimum supported version,
|
||||
// the condition _will_ always evaluate to `false`, so we want to return `true`.
|
||||
*if_minor <= py_minor.into()
|
||||
}
|
||||
if_minor <= py_minor
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -353,31 +434,20 @@ fn fix_always_true_branch(
|
|||
}
|
||||
}
|
||||
|
||||
/// Converts a `BigInt` to a `u32`. If the number is negative, it will return 0.
|
||||
fn bigint_to_u32(number: &BigInt) -> u32 {
|
||||
let the_number = number.to_u32_digits();
|
||||
match the_number.0 {
|
||||
Sign::Minus | Sign::NoSign => 0,
|
||||
Sign::Plus => *the_number.1.first().unwrap(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Gets the version from the tuple
|
||||
fn extract_version(elts: &[Expr]) -> Vec<u32> {
|
||||
let mut version: Vec<u32> = vec![];
|
||||
/// Return the version tuple as a sequence of [`Int`] values.
|
||||
fn extract_version(elts: &[Expr]) -> Option<Vec<Int>> {
|
||||
let mut version: Vec<Int> = vec![];
|
||||
for elt in elts {
|
||||
if let Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(item),
|
||||
let Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(int),
|
||||
..
|
||||
}) = &elt
|
||||
{
|
||||
let number = bigint_to_u32(item);
|
||||
version.push(number);
|
||||
} else {
|
||||
return version;
|
||||
}
|
||||
else {
|
||||
return None;
|
||||
};
|
||||
version.push(int.clone());
|
||||
}
|
||||
version
|
||||
Some(version)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
@ -399,10 +469,13 @@ mod tests {
|
|||
#[test_case(PythonVersion::Py310, &[3, 11], true, false; "compare-3.11")]
|
||||
fn test_compare_version(
|
||||
version: PythonVersion,
|
||||
version_vec: &[u32],
|
||||
target_versions: &[u8],
|
||||
or_equal: bool,
|
||||
expected: bool,
|
||||
) {
|
||||
assert_eq!(compare_version(version_vec, version, or_equal), expected);
|
||||
) -> Result<()> {
|
||||
let target_versions: Vec<_> = target_versions.iter().map(|int| Int::from(*int)).collect();
|
||||
let actual = compare_version(&target_versions, version, or_equal)?;
|
||||
assert_eq!(actual, expected);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,12 +9,6 @@ use crate::autofix::edits::{pad, remove_argument, Parentheses};
|
|||
use crate::checkers::ast::Checker;
|
||||
use crate::registry::Rule;
|
||||
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
pub(crate) enum Reason {
|
||||
BytesLiteral,
|
||||
DefaultArgument,
|
||||
}
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for unnecessary calls to `encode` as UTF-8.
|
||||
///
|
||||
|
|
@ -56,6 +50,12 @@ impl AlwaysAutofixableViolation for UnnecessaryEncodeUTF8 {
|
|||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
enum Reason {
|
||||
BytesLiteral,
|
||||
DefaultArgument,
|
||||
}
|
||||
|
||||
const UTF8_LITERALS: &[&str] = &["utf-8", "utf8", "utf_8", "u8", "utf", "cp65001"];
|
||||
|
||||
fn match_encoded_variable(func: &Expr) -> Option<&Expr> {
|
||||
|
|
|
|||
|
|
@ -685,4 +685,31 @@ UP036_0.py:182:4: UP036 [*] Version block is outdated for minimum Python version
|
|||
185 183 | if sys.version_info <= (3,12):
|
||||
186 184 | print("py3")
|
||||
|
||||
UP036_0.py:191:24: UP036 Version specifier is invalid
|
||||
|
|
||||
189 | print("py3")
|
||||
190 |
|
||||
191 | if sys.version_info == 10000000:
|
||||
| ^^^^^^^^ UP036
|
||||
192 | print("py3")
|
||||
|
|
||||
|
||||
UP036_0.py:194:23: UP036 Version specifier is invalid
|
||||
|
|
||||
192 | print("py3")
|
||||
193 |
|
||||
194 | if sys.version_info < (3,10000000):
|
||||
| ^^^^^^^^^^^^ UP036
|
||||
195 | print("py3")
|
||||
|
|
||||
|
||||
UP036_0.py:197:24: UP036 Version specifier is invalid
|
||||
|
|
||||
195 | print("py3")
|
||||
196 |
|
||||
197 | if sys.version_info <= (3,10000000):
|
||||
| ^^^^^^^^^^^^ UP036
|
||||
198 | print("py3")
|
||||
|
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1,11 +1,9 @@
|
|||
use std::fmt;
|
||||
|
||||
use num_traits::Zero;
|
||||
|
||||
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast as ast;
|
||||
use ruff_python_ast::{Arguments, Constant, Expr};
|
||||
use ruff_python_ast::{Arguments, Constant, Expr, Int};
|
||||
use ruff_python_codegen::Generator;
|
||||
use ruff_text_size::{Ranged, TextRange};
|
||||
|
||||
|
|
@ -160,15 +158,13 @@ pub(crate) fn unnecessary_enumerate(checker: &mut Checker, stmt_for: &ast::StmtF
|
|||
// there's no clear fix.
|
||||
let start = arguments.find_argument("start", 1);
|
||||
if start.map_or(true, |start| {
|
||||
if let Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(value),
|
||||
..
|
||||
}) = start
|
||||
{
|
||||
value.is_zero()
|
||||
} else {
|
||||
false
|
||||
}
|
||||
matches!(
|
||||
start,
|
||||
Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(Int::ZERO),
|
||||
..
|
||||
})
|
||||
)
|
||||
}) {
|
||||
let replace_iter = Edit::range_replacement(
|
||||
generate_range_len_call(sequence, checker.generator()),
|
||||
|
|
|
|||
|
|
@ -1,8 +1,6 @@
|
|||
use num_traits::ToPrimitive;
|
||||
use ruff_python_ast::{self as ast, Constant, Expr, UnaryOp};
|
||||
|
||||
use ruff_diagnostics::{Diagnostic, Violation};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::{self as ast, Constant, Expr, Int};
|
||||
use ruff_text_size::Ranged;
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
|
|
@ -45,26 +43,17 @@ impl Violation for PairwiseOverZipped {
|
|||
|
||||
#[derive(Debug)]
|
||||
struct SliceInfo {
|
||||
arg_name: String,
|
||||
slice_start: Option<i64>,
|
||||
id: String,
|
||||
slice_start: Option<i32>,
|
||||
}
|
||||
|
||||
impl SliceInfo {
|
||||
pub(crate) fn new(arg_name: String, slice_start: Option<i64>) -> Self {
|
||||
Self {
|
||||
arg_name,
|
||||
slice_start,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Return the argument name, lower bound, and upper bound for an expression, if it's a slice.
|
||||
/// Return the argument name, lower bound, and upper bound for an expression, if it's a slice.
|
||||
fn match_slice_info(expr: &Expr) -> Option<SliceInfo> {
|
||||
let Expr::Subscript(ast::ExprSubscript { value, slice, .. }) = expr else {
|
||||
return None;
|
||||
};
|
||||
|
||||
let Expr::Name(ast::ExprName { id: arg_id, .. }) = value.as_ref() else {
|
||||
let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else {
|
||||
return None;
|
||||
};
|
||||
|
||||
|
|
@ -74,44 +63,40 @@ fn match_slice_info(expr: &Expr) -> Option<SliceInfo> {
|
|||
|
||||
// Avoid false positives for slices with a step.
|
||||
if let Some(step) = step {
|
||||
if let Some(step) = to_bound(step) {
|
||||
if step != 1 {
|
||||
return None;
|
||||
}
|
||||
} else {
|
||||
if !matches!(
|
||||
step.as_ref(),
|
||||
Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(Int::ONE),
|
||||
..
|
||||
})
|
||||
) {
|
||||
return None;
|
||||
}
|
||||
}
|
||||
|
||||
Some(SliceInfo::new(
|
||||
arg_id.to_string(),
|
||||
lower.as_ref().and_then(|expr| to_bound(expr)),
|
||||
))
|
||||
}
|
||||
|
||||
fn to_bound(expr: &Expr) -> Option<i64> {
|
||||
match expr {
|
||||
Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(value),
|
||||
..
|
||||
}) => value.to_i64(),
|
||||
Expr::UnaryOp(ast::ExprUnaryOp {
|
||||
op: UnaryOp::USub | UnaryOp::Invert,
|
||||
operand,
|
||||
// If the slice start is a non-constant, we can't be sure that it's successive.
|
||||
let slice_start = if let Some(lower) = lower.as_ref() {
|
||||
let Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(int),
|
||||
range: _,
|
||||
}) => {
|
||||
if let Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(value),
|
||||
..
|
||||
}) = operand.as_ref()
|
||||
{
|
||||
value.to_i64().map(|v| -v)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
}) = lower.as_ref()
|
||||
else {
|
||||
return None;
|
||||
};
|
||||
|
||||
let Some(slice_start) = int.as_i32() else {
|
||||
return None;
|
||||
};
|
||||
|
||||
Some(slice_start)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
Some(SliceInfo {
|
||||
id: id.to_string(),
|
||||
slice_start,
|
||||
})
|
||||
}
|
||||
|
||||
/// RUF007
|
||||
|
|
@ -121,9 +106,9 @@ pub(crate) fn pairwise_over_zipped(checker: &mut Checker, func: &Expr, args: &[E
|
|||
};
|
||||
|
||||
// Require exactly two positional arguments.
|
||||
if args.len() != 2 {
|
||||
let [first, second] = args else {
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
// Require the function to be the builtin `zip`.
|
||||
if !(id == "zip" && checker.semantic().is_builtin(id)) {
|
||||
|
|
@ -132,25 +117,28 @@ pub(crate) fn pairwise_over_zipped(checker: &mut Checker, func: &Expr, args: &[E
|
|||
|
||||
// Allow the first argument to be a `Name` or `Subscript`.
|
||||
let Some(first_arg_info) = ({
|
||||
if let Expr::Name(ast::ExprName { id, .. }) = &args[0] {
|
||||
Some(SliceInfo::new(id.to_string(), None))
|
||||
if let Expr::Name(ast::ExprName { id, .. }) = first {
|
||||
Some(SliceInfo {
|
||||
id: id.to_string(),
|
||||
slice_start: None,
|
||||
})
|
||||
} else {
|
||||
match_slice_info(&args[0])
|
||||
match_slice_info(first)
|
||||
}
|
||||
}) else {
|
||||
return;
|
||||
};
|
||||
|
||||
// Require second argument to be a `Subscript`.
|
||||
if !args[1].is_subscript_expr() {
|
||||
if !second.is_subscript_expr() {
|
||||
return;
|
||||
}
|
||||
let Some(second_arg_info) = match_slice_info(&args[1]) else {
|
||||
let Some(second_arg_info) = match_slice_info(second) else {
|
||||
return;
|
||||
};
|
||||
|
||||
// Verify that the arguments match the same name.
|
||||
if first_arg_info.arg_name != second_arg_info.arg_name {
|
||||
if first_arg_info.id != second_arg_info.id {
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,10 +1,8 @@
|
|||
use std::borrow::Cow;
|
||||
|
||||
use num_traits::Zero;
|
||||
|
||||
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::{self as ast, Arguments, Comprehension, Constant, Expr};
|
||||
use ruff_python_ast::{self as ast, Arguments, Comprehension, Constant, Expr, Int};
|
||||
use ruff_python_semantic::SemanticModel;
|
||||
use ruff_text_size::{Ranged, TextRange, TextSize};
|
||||
|
||||
|
|
@ -110,15 +108,13 @@ pub(crate) fn unnecessary_iterable_allocation_for_first_element(
|
|||
|
||||
/// Check that the slice [`Expr`] is a slice of the first element (e.g., `x[0]`).
|
||||
fn is_head_slice(expr: &Expr) -> bool {
|
||||
if let Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(value),
|
||||
..
|
||||
}) = expr
|
||||
{
|
||||
value.is_zero()
|
||||
} else {
|
||||
false
|
||||
}
|
||||
matches!(
|
||||
expr,
|
||||
Expr::Constant(ast::ExprConstant {
|
||||
value: Constant::Int(Int::ZERO),
|
||||
..
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue