mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-26 20:10:09 +00:00
Omit sys.version_info and sys.platform checks from ternary rule (#1756)
Resolves #1753.
This commit is contained in:
Charlie Marsh
GitHub
parent
9532f342a6
commit
f7ac28a935
3 changed files with 258 additions and 2 deletions
|
|
@ -29,3 +29,20 @@ else:
|
||||||
b = 1
|
b = 1
|
||||||
else:
|
else:
|
||||||
b = 2
|
b = 2
|
||||||
|
|
||||||
|
import sys
|
||||||
|
|
||||||
|
if sys.version_info >= (3, 9):
|
||||||
|
randbytes = random.randbytes
|
||||||
|
else:
|
||||||
|
randbytes = _get_random_bytes
|
||||||
|
|
||||||
|
if sys.platform == "darwin":
|
||||||
|
randbytes = random.randbytes
|
||||||
|
else:
|
||||||
|
randbytes = _get_random_bytes
|
||||||
|
|
||||||
|
if sys.platform.startswith("linux"):
|
||||||
|
randbytes = random.randbytes
|
||||||
|
else:
|
||||||
|
randbytes = _get_random_bytes
|
||||||
|
|
|
||||||
|
|
@ -179,6 +179,221 @@ pub fn match_call_path(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Return `true` if the `Expr` contains a reference to `${module}.${target}`.
|
||||||
|
pub fn contains_call_path(
|
||||||
|
expr: &Expr,
|
||||||
|
module: &str,
|
||||||
|
member: &str,
|
||||||
|
import_aliases: &FxHashMap<&str, &str>,
|
||||||
|
from_imports: &FxHashMap<&str, FxHashSet<&str>>,
|
||||||
|
) -> bool {
|
||||||
|
let call_path = collect_call_paths(expr);
|
||||||
|
if !call_path.is_empty() {
|
||||||
|
if match_call_path(
|
||||||
|
&dealias_call_path(call_path, import_aliases),
|
||||||
|
module,
|
||||||
|
member,
|
||||||
|
from_imports,
|
||||||
|
) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
match &expr.node {
|
||||||
|
ExprKind::BoolOp { values, .. } => values
|
||||||
|
.iter()
|
||||||
|
.any(|expr| contains_call_path(expr, module, member, import_aliases, from_imports)),
|
||||||
|
ExprKind::NamedExpr { target, value } => {
|
||||||
|
contains_call_path(target, module, member, import_aliases, from_imports)
|
||||||
|
|| contains_call_path(value, module, member, import_aliases, from_imports)
|
||||||
|
}
|
||||||
|
ExprKind::BinOp { left, right, .. } => {
|
||||||
|
contains_call_path(left, module, member, import_aliases, from_imports)
|
||||||
|
|| contains_call_path(right, module, member, import_aliases, from_imports)
|
||||||
|
}
|
||||||
|
ExprKind::UnaryOp { operand, .. } => {
|
||||||
|
contains_call_path(operand, module, member, import_aliases, from_imports)
|
||||||
|
}
|
||||||
|
ExprKind::Lambda { body, .. } => {
|
||||||
|
contains_call_path(body, module, member, import_aliases, from_imports)
|
||||||
|
}
|
||||||
|
ExprKind::IfExp { test, body, orelse } => {
|
||||||
|
contains_call_path(test, module, member, import_aliases, from_imports)
|
||||||
|
|| contains_call_path(body, module, member, import_aliases, from_imports)
|
||||||
|
|| contains_call_path(orelse, module, member, import_aliases, from_imports)
|
||||||
|
}
|
||||||
|
ExprKind::Dict { keys, values } => values
|
||||||
|
.iter()
|
||||||
|
.chain(keys.iter())
|
||||||
|
.any(|expr| contains_call_path(expr, module, member, import_aliases, from_imports)),
|
||||||
|
ExprKind::Set { elts } => elts
|
||||||
|
.iter()
|
||||||
|
.any(|expr| contains_call_path(expr, module, member, import_aliases, from_imports)),
|
||||||
|
ExprKind::ListComp { elt, generators } => {
|
||||||
|
contains_call_path(elt, module, member, import_aliases, from_imports)
|
||||||
|
|| generators.iter().any(|generator| {
|
||||||
|
contains_call_path(
|
||||||
|
&generator.target,
|
||||||
|
module,
|
||||||
|
member,
|
||||||
|
import_aliases,
|
||||||
|
from_imports,
|
||||||
|
) || contains_call_path(
|
||||||
|
&generator.iter,
|
||||||
|
module,
|
||||||
|
member,
|
||||||
|
import_aliases,
|
||||||
|
from_imports,
|
||||||
|
) || generator.ifs.iter().any(|expr| {
|
||||||
|
contains_call_path(expr, module, member, import_aliases, from_imports)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
}
|
||||||
|
ExprKind::SetComp { elt, generators } => {
|
||||||
|
contains_call_path(elt, module, member, import_aliases, from_imports)
|
||||||
|
|| generators.iter().any(|generator| {
|
||||||
|
contains_call_path(
|
||||||
|
&generator.target,
|
||||||
|
module,
|
||||||
|
member,
|
||||||
|
import_aliases,
|
||||||
|
from_imports,
|
||||||
|
) || contains_call_path(
|
||||||
|
&generator.iter,
|
||||||
|
module,
|
||||||
|
member,
|
||||||
|
import_aliases,
|
||||||
|
from_imports,
|
||||||
|
) || generator.ifs.iter().any(|expr| {
|
||||||
|
contains_call_path(expr, module, member, import_aliases, from_imports)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
}
|
||||||
|
ExprKind::DictComp {
|
||||||
|
key,
|
||||||
|
value,
|
||||||
|
generators,
|
||||||
|
} => {
|
||||||
|
contains_call_path(key, module, member, import_aliases, from_imports)
|
||||||
|
|| contains_call_path(value, module, member, import_aliases, from_imports)
|
||||||
|
|| generators.iter().any(|generator| {
|
||||||
|
contains_call_path(
|
||||||
|
&generator.target,
|
||||||
|
module,
|
||||||
|
member,
|
||||||
|
import_aliases,
|
||||||
|
from_imports,
|
||||||
|
) || contains_call_path(
|
||||||
|
&generator.iter,
|
||||||
|
module,
|
||||||
|
member,
|
||||||
|
import_aliases,
|
||||||
|
from_imports,
|
||||||
|
) || generator.ifs.iter().any(|expr| {
|
||||||
|
contains_call_path(expr, module, member, import_aliases, from_imports)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
}
|
||||||
|
ExprKind::GeneratorExp { elt, generators } => {
|
||||||
|
contains_call_path(elt, module, member, import_aliases, from_imports) || {
|
||||||
|
generators.iter().any(|generator| {
|
||||||
|
contains_call_path(
|
||||||
|
&generator.target,
|
||||||
|
module,
|
||||||
|
member,
|
||||||
|
import_aliases,
|
||||||
|
from_imports,
|
||||||
|
) || contains_call_path(
|
||||||
|
&generator.iter,
|
||||||
|
module,
|
||||||
|
member,
|
||||||
|
import_aliases,
|
||||||
|
from_imports,
|
||||||
|
) || generator.ifs.iter().any(|expr| {
|
||||||
|
contains_call_path(expr, module, member, import_aliases, from_imports)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
ExprKind::Await { value } => {
|
||||||
|
contains_call_path(value, module, member, import_aliases, from_imports)
|
||||||
|
}
|
||||||
|
ExprKind::Yield { value } => value.as_ref().map_or(false, |value| {
|
||||||
|
contains_call_path(value, module, member, import_aliases, from_imports)
|
||||||
|
}),
|
||||||
|
ExprKind::YieldFrom { value } => {
|
||||||
|
contains_call_path(value, module, member, import_aliases, from_imports)
|
||||||
|
}
|
||||||
|
ExprKind::Compare {
|
||||||
|
left,
|
||||||
|
ops: _,
|
||||||
|
comparators,
|
||||||
|
} => {
|
||||||
|
contains_call_path(left, module, member, import_aliases, from_imports)
|
||||||
|
|| comparators.iter().any(|expr| {
|
||||||
|
contains_call_path(expr, module, member, import_aliases, from_imports)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
ExprKind::Call {
|
||||||
|
func,
|
||||||
|
args,
|
||||||
|
keywords,
|
||||||
|
} => {
|
||||||
|
contains_call_path(func, module, member, import_aliases, from_imports)
|
||||||
|
|| args.iter().any(|expr| {
|
||||||
|
contains_call_path(expr, module, member, import_aliases, from_imports)
|
||||||
|
})
|
||||||
|
|| keywords.iter().any(|keyword| {
|
||||||
|
contains_call_path(
|
||||||
|
&keyword.node.value,
|
||||||
|
module,
|
||||||
|
member,
|
||||||
|
import_aliases,
|
||||||
|
from_imports,
|
||||||
|
)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
ExprKind::FormattedValue {
|
||||||
|
value, format_spec, ..
|
||||||
|
} => {
|
||||||
|
contains_call_path(value, module, member, import_aliases, from_imports)
|
||||||
|
|| format_spec.as_ref().map_or(false, |value| {
|
||||||
|
contains_call_path(value, module, member, import_aliases, from_imports)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
ExprKind::JoinedStr { values } => values
|
||||||
|
.iter()
|
||||||
|
.any(|expr| contains_call_path(expr, module, member, import_aliases, from_imports)),
|
||||||
|
ExprKind::Constant { .. } => false,
|
||||||
|
ExprKind::Attribute { value, .. } => {
|
||||||
|
contains_call_path(value, module, member, import_aliases, from_imports)
|
||||||
|
}
|
||||||
|
ExprKind::Subscript { value, slice, .. } => {
|
||||||
|
contains_call_path(value, module, member, import_aliases, from_imports)
|
||||||
|
|| contains_call_path(slice, module, member, import_aliases, from_imports)
|
||||||
|
}
|
||||||
|
ExprKind::Starred { value, ctx: _ } => {
|
||||||
|
contains_call_path(value, module, member, import_aliases, from_imports)
|
||||||
|
}
|
||||||
|
ExprKind::Name { .. } => false,
|
||||||
|
ExprKind::List { elts, .. } => elts
|
||||||
|
.iter()
|
||||||
|
.any(|expr| contains_call_path(expr, module, member, import_aliases, from_imports)),
|
||||||
|
ExprKind::Tuple { elts, .. } => elts
|
||||||
|
.iter()
|
||||||
|
.any(|expr| contains_call_path(expr, module, member, import_aliases, from_imports)),
|
||||||
|
ExprKind::Slice { lower, upper, step } => {
|
||||||
|
lower.as_ref().map_or(false, |value| {
|
||||||
|
contains_call_path(value, module, member, import_aliases, from_imports)
|
||||||
|
}) || upper.as_ref().map_or(false, |value| {
|
||||||
|
contains_call_path(value, module, member, import_aliases, from_imports)
|
||||||
|
}) || step.as_ref().map_or(false, |value| {
|
||||||
|
contains_call_path(value, module, member, import_aliases, from_imports)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
static DUNDER_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"__[^\s]+__").unwrap());
|
static DUNDER_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"__[^\s]+__").unwrap());
|
||||||
|
|
||||||
/// Return `true` if the `Stmt` is an assignment to a dunder (like `__all__`).
|
/// Return `true` if the `Stmt` is an assignment to a dunder (like `__all__`).
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,8 @@
|
||||||
use rustpython_ast::{Constant, Expr, ExprKind, Stmt, StmtKind};
|
use rustpython_ast::{Constant, Expr, ExprKind, Stmt, StmtKind};
|
||||||
|
|
||||||
use crate::ast::helpers::{create_expr, create_stmt, unparse_expr, unparse_stmt};
|
use crate::ast::helpers::{
|
||||||
|
contains_call_path, create_expr, create_stmt, unparse_expr, unparse_stmt,
|
||||||
|
};
|
||||||
use crate::ast::types::Range;
|
use crate::ast::types::Range;
|
||||||
use crate::autofix::Fix;
|
use crate::autofix::Fix;
|
||||||
use crate::checkers::ast::Checker;
|
use crate::checkers::ast::Checker;
|
||||||
|
|
@ -144,7 +146,28 @@ pub fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt, parent: Option<&
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
let target_var = &body_targets[0];
|
// Avoid suggesting ternary for `if sys.version_info >= ...`-style checks.
|
||||||
|
if contains_call_path(
|
||||||
|
test,
|
||||||
|
"sys",
|
||||||
|
"version_info",
|
||||||
|
&checker.import_aliases,
|
||||||
|
&checker.from_imports,
|
||||||
|
) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Avoid suggesting ternary for `if sys.platform.startswith("...")`-style
|
||||||
|
// checks.
|
||||||
|
if contains_call_path(
|
||||||
|
test,
|
||||||
|
"sys",
|
||||||
|
"platform",
|
||||||
|
&checker.import_aliases,
|
||||||
|
&checker.from_imports,
|
||||||
|
) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// It's part of a bigger if-elif block:
|
// It's part of a bigger if-elif block:
|
||||||
// https://github.com/MartinThoma/flake8-simplify/issues/115
|
// https://github.com/MartinThoma/flake8-simplify/issues/115
|
||||||
|
|
@ -176,6 +199,7 @@ pub fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt, parent: Option<&
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let target_var = &body_targets[0];
|
||||||
let ternary = ternary(target_var, body_value, test, orelse_value);
|
let ternary = ternary(target_var, body_value, test, orelse_value);
|
||||||
let content = unparse_stmt(&ternary, checker.style);
|
let content = unparse_stmt(&ternary, checker.style);
|
||||||
let mut diagnostic = Diagnostic::new(
|
let mut diagnostic = Diagnostic::new(
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue