Skip to content

Commit

Permalink
Auto merge of #17187 - roife:fix-issue-17185, r=Veykril
Browse files Browse the repository at this point in the history
fix: keep parentheses when the precedence of inner expr is lower than the outer one

fix #17185

Additionally, this PR simplifies some code in `apply_demorgan`.
  • Loading branch information
bors committed May 13, 2024
2 parents 067d9d9 + 4ada826 commit 7aecf1a
Showing 1 changed file with 48 additions and 46 deletions.
94 changes: 48 additions & 46 deletions crates/ide-assists/src/handlers/apply_demorgan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ use ide_db::{
};
use syntax::{
ast::{self, make, AstNode, Expr::BinExpr, HasArgList},
ted::{self, Position},
SyntaxKind,
ted, SyntaxKind, T,
};

use crate::{utils::invert_boolean_expression, AssistContext, AssistId, AssistKind, Assists};
Expand Down Expand Up @@ -62,7 +61,7 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opti
let demorganed = bin_expr.clone_subtree().clone_for_update();

ted::replace(demorganed.op_token()?, ast::make::token(inv_token));
let mut exprs = VecDeque::from(vec![
let mut exprs = VecDeque::from([
(bin_expr.lhs()?, demorganed.lhs()?),
(bin_expr.rhs()?, demorganed.rhs()?),
]);
Expand Down Expand Up @@ -93,58 +92,38 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opti
}
}

let dm_lhs = demorganed.lhs()?;

acc.add_group(
&GroupLabel("Apply De Morgan's law".to_owned()),
AssistId("apply_demorgan", AssistKind::RefactorRewrite),
"Apply De Morgan's law",
op_range,
|edit| {
let demorganed = ast::Expr::BinExpr(demorganed);
let paren_expr = bin_expr.syntax().parent().and_then(ast::ParenExpr::cast);
let neg_expr = paren_expr
.clone()
.and_then(|paren_expr| paren_expr.syntax().parent())
.and_then(ast::PrefixExpr::cast)
.and_then(|prefix_expr| {
if prefix_expr.op_kind()? == ast::UnaryOp::Not {
Some(prefix_expr)
} else {
None
}
});
.filter(|prefix_expr| matches!(prefix_expr.op_kind(), Some(ast::UnaryOp::Not)))
.map(ast::Expr::PrefixExpr);

if let Some(paren_expr) = paren_expr {
if let Some(neg_expr) = neg_expr {
cov_mark::hit!(demorgan_double_negation);
edit.replace_ast(ast::Expr::PrefixExpr(neg_expr), demorganed.into());
let parent = neg_expr.syntax().parent();

if parent.is_some_and(|parent| demorganed.needs_parens_in(parent)) {
cov_mark::hit!(demorgan_keep_parens_for_op_precedence2);
edit.replace_ast(neg_expr, make::expr_paren(demorganed));
} else {
edit.replace_ast(neg_expr, demorganed);
};
} else {
cov_mark::hit!(demorgan_double_parens);
ted::insert_all_raw(
Position::before(dm_lhs.syntax()),
vec![
syntax::NodeOrToken::Token(ast::make::token(SyntaxKind::BANG)),
syntax::NodeOrToken::Token(ast::make::token(SyntaxKind::L_PAREN)),
],
);

ted::append_child_raw(
demorganed.syntax(),
syntax::NodeOrToken::Token(ast::make::token(SyntaxKind::R_PAREN)),
);

edit.replace_ast(ast::Expr::ParenExpr(paren_expr), demorganed.into());
edit.replace_ast(paren_expr.into(), add_bang_paren(demorganed));
}
} else {
ted::insert_all_raw(
Position::before(dm_lhs.syntax()),
vec![
syntax::NodeOrToken::Token(ast::make::token(SyntaxKind::BANG)),
syntax::NodeOrToken::Token(ast::make::token(SyntaxKind::L_PAREN)),
],
);
ted::append_child_raw(demorganed.syntax(), ast::make::token(SyntaxKind::R_PAREN));
edit.replace_ast(bin_expr, demorganed);
edit.replace_ast(bin_expr.into(), add_bang_paren(demorganed));
}
},
)
Expand Down Expand Up @@ -271,6 +250,11 @@ fn tail_cb_impl(edit: &mut SourceChangeBuilder, e: &ast::Expr) {
}
}

/// Add bang and parentheses to the expression.
fn add_bang_paren(expr: ast::Expr) -> ast::Expr {
make::expr_prefix(T![!], make::expr_paren(expr))
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -349,16 +333,14 @@ fn f() { !(S <= S || S < S) }
check_assist(apply_demorgan, "fn f() { (x ||$0 x) }", "fn f() { !(!x && !x) }")
}

// FIXME : This needs to go.
// // https://github.com/rust-lang/rust-analyzer/issues/10963
// #[test]
// fn demorgan_doesnt_hang() {
// check_assist(
// apply_demorgan,
// "fn f() { 1 || 3 &&$0 4 || 5 }",
// "fn f() { !(!1 || !3 || !4) || 5 }",
// )
// }
#[test]
fn demorgan_doesnt_hang() {
check_assist(
apply_demorgan,
"fn f() { 1 || 3 &&$0 4 || 5 }",
"fn f() { 1 || !(!3 || !4) || 5 }",
)
}

#[test]
fn demorgan_keep_pars_for_op_precedence() {
Expand All @@ -375,6 +357,21 @@ fn f() { !(S <= S || S < S) }
);
}

#[test]
fn demorgan_keep_pars_for_op_precedence2() {
cov_mark::check!(demorgan_keep_parens_for_op_precedence2);
check_assist(
apply_demorgan,
"fn f() { (a && !(b &&$0 c); }",
"fn f() { (a && (!b || !c); }",
);
}

#[test]
fn demorgan_keep_pars_for_op_precedence3() {
check_assist(apply_demorgan, "fn f() { (a || !(b &&$0 c); }", "fn f() { (a || !b || !c; }");
}

#[test]
fn demorgan_removes_pars_in_eq_precedence() {
check_assist(
Expand All @@ -384,6 +381,11 @@ fn f() { !(S <= S || S < S) }
)
}

#[test]
fn demorgan_removes_pars_for_op_precedence2() {
check_assist(apply_demorgan, "fn f() { (a || !(b ||$0 c); }", "fn f() { (a || !b && !c; }");
}

#[test]
fn demorgan_iterator_any_all_reverse() {
check_assist(
Expand Down

0 comments on commit 7aecf1a

Please sign in to comment.