language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-17 17:10:53 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Remove contains_effect's dependency on Context (#3855)

Browse source
This commit is contained in:
Charlie Marsh 2023-04-03 12:08:13 -04:00 • committed by GitHub
parent b52cb93e58
commit 3744e9ab3f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 38 additions and 33 deletions
Download patch file Download diff file Expand all files Collapse all files

2
crates/ruff/src/rules/flake8_bugbear/rules/useless_expression.rs
View file

@ -54,7 +54,7 @@ pub fn useless_expression(checker: &mut Checker, value: &Expr) {
}
// Ignore statements that have side effects.
if contains_effect(&checker.ctx, value) {
if contains_effect(value, |id| checker.ctx.is_builtin(id)) {
// Flag attributes as useless expressions, even if they're attached to calls or other
// expressions.
if matches!(value.node, ExprKind::Attribute { .. }) {

10
crates/ruff/src/rules/flake8_simplify/rules/ast_bool_op.rs
View file

@ -213,7 +213,7 @@ pub fn duplicate_isinstance_call(checker: &mut Checker, expr: &Expr) {
} else {
unreachable!("Indices should only contain `isinstance` calls")
};
let fixable = !contains_effect(&checker.ctx, target);
let fixable = !contains_effect(target, |id| checker.ctx.is_builtin(id));
let mut diagnostic = Diagnostic::new(
DuplicateIsinstanceCall {
name: if let ExprKind::Name { id, .. } = &target.node {
@ -341,7 +341,7 @@ pub fn compare_with_tuple(checker: &mut Checker, expr: &Expr) {
// Avoid rewriting (e.g.) `a == "foo" or a == f()`.
if comparators
.iter()
.any(|expr| contains_effect(&checker.ctx, expr))
.any(|expr| contains_effect(expr, |id| checker.ctx.is_builtin(id)))
{
continue;
}
@ -423,7 +423,7 @@ pub fn expr_and_not_expr(checker: &mut Checker, expr: &Expr) {
return;
}
if contains_effect(&checker.ctx, expr) {
if contains_effect(expr, |id| checker.ctx.is_builtin(id)) {
return;
}
@ -477,7 +477,7 @@ pub fn expr_or_not_expr(checker: &mut Checker, expr: &Expr) {
return;
}
if contains_effect(&checker.ctx, expr) {
if contains_effect(expr, |id| checker.ctx.is_builtin(id)) {
return;
}
@ -522,7 +522,7 @@ pub fn is_short_circuit(
let mut location = expr.location;
for (value, next_value) in values.iter().tuple_windows() {
// Keep track of the location of the furthest-right, non-effectful expression.
if contains_effect(ctx, value) {
if contains_effect(value, |id| ctx.is_builtin(id)) {
location = next_value.location;
continue;
}

30
crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs
View file

@ -639,10 +639,9 @@ pub fn manual_dict_lookup(
let StmtKind::Return { value, .. } = &body[0].node else {
return;
};
if value
.as_ref()
.map_or(false, |value| contains_effect(&checker.ctx, value))
{
if value.as_ref().map_or(false, |value| {
contains_effect(value, |id| checker.ctx.is_builtin(id))
}) {
return;
}
@ -712,10 +711,9 @@ pub fn manual_dict_lookup(
let StmtKind::Return { value, .. } = &body[0].node else {
return;
};
if value
.as_ref()
.map_or(false, |value| contains_effect(&checker.ctx, value))
{
if value.as_ref().map_or(false, |value| {
contains_effect(value, |id| checker.ctx.is_builtin(id))
}) {
return;
};
@ -757,13 +755,13 @@ pub fn use_dict_get_with_default(
if body.len() != 1 || orelse.len() != 1 {
return;
}
let StmtKind::Assign { targets: body_var, value: body_val, ..} = &body[0].node else {
let StmtKind::Assign { targets: body_var, value: body_value, ..} = &body[0].node else {
return;
};
if body_var.len() != 1 {
return;
};
let StmtKind::Assign { targets: orelse_var, value: orelse_val, .. } = &orelse[0].node else {
let StmtKind::Assign { targets: orelse_var, value: orelse_value, .. } = &orelse[0].node else {
return;
};
if orelse_var.len() != 1 {
@ -775,15 +773,15 @@ pub fn use_dict_get_with_default(
if test_dict.len() != 1 {
return;
}
let (expected_var, expected_val, default_var, default_val) = match ops[..] {
[Cmpop::In] => (&body_var[0], body_val, &orelse_var[0], orelse_val),
[Cmpop::NotIn] => (&orelse_var[0], orelse_val, &body_var[0], body_val),
let (expected_var, expected_value, default_var, default_value) = match ops[..] {
[Cmpop::In] => (&body_var[0], body_value, &orelse_var[0], orelse_value),
[Cmpop::NotIn] => (&orelse_var[0], orelse_value, &body_var[0], body_value),
_ => {
return;
}
};
let test_dict = &test_dict[0];
let ExprKind::Subscript { value: expected_subscript, slice: expected_slice, .. } = &expected_val.node else {
let ExprKind::Subscript { value: expected_subscript, slice: expected_slice, .. } = &expected_value.node else {
return;
};
@ -797,7 +795,7 @@ pub fn use_dict_get_with_default(
}
// Check that the default value is not "complex".
if contains_effect(&checker.ctx, default_val) {
if contains_effect(default_value, |id| checker.ctx.is_builtin(id)) {
return;
}
@ -842,7 +840,7 @@ pub fn use_dict_get_with_default(
})),
args: vec![
create_expr(test_key.node.clone()),
create_expr(default_val.node.clone()),
create_expr(default_value.node.clone()),
],
keywords: vec![],
})),

6
crates/ruff/src/rules/pyflakes/rules/unused_variable.rs
View file

@ -202,7 +202,9 @@ fn remove_unused_variable(
range.location == target.location && range.end_location == target.end_location.unwrap()
}) {
if matches!(target.node, ExprKind::Name { .. }) {
return if targets.len() > 1 || contains_effect(&checker.ctx, value) {
return if targets.len() > 1
|| contains_effect(value, |id| checker.ctx.is_builtin(id))
{
// If the expression is complex (`x = foo()`), remove the assignment,
// but preserve the right-hand side.
Some((
@ -248,7 +250,7 @@ fn remove_unused_variable(
} = &stmt.node
{
if matches!(target.node, ExprKind::Name { .. }) {
return if contains_effect(&checker.ctx, value) {
return if contains_effect(value, |id| checker.ctx.is_builtin(id)) {
// If the expression is complex (`x = foo()`), remove the assignment,
// but preserve the right-hand side.
Some((

2
crates/ruff/src/rules/tryceratops/rules/try_consider_else.rs
View file

@ -28,7 +28,7 @@ pub fn try_consider_else(
if let Some(stmt) = body.last() {
if let StmtKind::Return { value } = &stmt.node {
if let Some(value) = value {
if contains_effect(&checker.ctx, value) {
if contains_effect(value, |id| checker.ctx.is_builtin(id)) {
return;
}
}

21
crates/ruff_python_ast/src/helpers.rs
View file

@ -60,7 +60,12 @@ pub fn contains_call_path(ctx: &Context, expr: &Expr, target: &[&str]) -> bool {
/// Return `true` if the `Expr` contains an expression that appears to include a
/// side-effect (like a function call).
pub fn contains_effect(ctx: &Context, expr: &Expr) -> bool {
///
/// Accepts a closure that determines whether a given name (e.g., `"list"`) is a Python builtin.
pub fn contains_effect<F>(expr: &Expr, is_builtin: F) -> bool
where
F: Fn(&str) -> bool,
{
any_over_expr(expr, &|expr| {
// Accept empty initializers.
if let ExprKind::Call {
@ -71,13 +76,13 @@ pub fn contains_effect(ctx: &Context, expr: &Expr) -> bool {
{
if args.is_empty() && keywords.is_empty() {
if let ExprKind::Name { id, .. } = &func.node {
let is_empty_initializer = (id == "set"
|| id == "list"
|| id == "tuple"
|| id == "dict"
|| id == "frozenset")
&& ctx.is_builtin(id);
return !is_empty_initializer;
if !matches!(id.as_str(), "set" | "list" | "tuple" | "dict" | "frozenset") {
return true;
}
if !is_builtin(id) {
return true;
}
return false;
}
}
}