Remove syntax editing from parenthesis computation
This commit is contained in:
parent
ff7de58156
commit
629baf217c
6 changed files with 50 additions and 87 deletions
|
@ -6,9 +6,16 @@ use ide_db::{
|
|||
syntax_helpers::node_ext::{for_each_tail_expr, walk_expr},
|
||||
};
|
||||
use syntax::{
|
||||
ast::{self, syntax_factory::SyntaxFactory, AstNode, Expr::BinExpr, HasArgList},
|
||||
ast::{
|
||||
self,
|
||||
prec::{precedence, ExprPrecedence},
|
||||
syntax_factory::SyntaxFactory,
|
||||
AstNode,
|
||||
Expr::BinExpr,
|
||||
HasArgList,
|
||||
},
|
||||
syntax_editor::{Position, SyntaxEditor},
|
||||
SyntaxKind, SyntaxNode, T,
|
||||
SyntaxKind, T,
|
||||
};
|
||||
|
||||
use crate::{utils::invert_boolean_expression, AssistContext, AssistId, AssistKind, Assists};
|
||||
|
@ -52,9 +59,9 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opti
|
|||
}
|
||||
|
||||
let op = bin_expr.op_kind()?;
|
||||
let inv_token = match op {
|
||||
ast::BinaryOp::LogicOp(ast::LogicOp::And) => SyntaxKind::PIPE2,
|
||||
ast::BinaryOp::LogicOp(ast::LogicOp::Or) => SyntaxKind::AMP2,
|
||||
let (inv_token, prec) = match op {
|
||||
ast::BinaryOp::LogicOp(ast::LogicOp::And) => (SyntaxKind::PIPE2, ExprPrecedence::LOr),
|
||||
ast::BinaryOp::LogicOp(ast::LogicOp::Or) => (SyntaxKind::AMP2, ExprPrecedence::LAnd),
|
||||
_ => return None,
|
||||
};
|
||||
|
||||
|
@ -65,33 +72,33 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opti
|
|||
editor.replace(demorganed.op_token()?, make.token(inv_token));
|
||||
|
||||
let mut exprs = VecDeque::from([
|
||||
(bin_expr.lhs()?, demorganed.lhs()?),
|
||||
(bin_expr.rhs()?, demorganed.rhs()?),
|
||||
(bin_expr.lhs()?, demorganed.lhs()?, prec),
|
||||
(bin_expr.rhs()?, demorganed.rhs()?, prec),
|
||||
]);
|
||||
|
||||
while let Some((expr, dm)) = exprs.pop_front() {
|
||||
while let Some((expr, demorganed, prec)) = exprs.pop_front() {
|
||||
if let BinExpr(bin_expr) = &expr {
|
||||
if let BinExpr(cbin_expr) = &dm {
|
||||
if let BinExpr(cbin_expr) = &demorganed {
|
||||
if op == bin_expr.op_kind()? {
|
||||
editor.replace(cbin_expr.op_token()?, make.token(inv_token));
|
||||
exprs.push_back((bin_expr.lhs()?, cbin_expr.lhs()?));
|
||||
exprs.push_back((bin_expr.rhs()?, cbin_expr.rhs()?));
|
||||
exprs.push_back((bin_expr.lhs()?, cbin_expr.lhs()?, prec));
|
||||
exprs.push_back((bin_expr.rhs()?, cbin_expr.rhs()?, prec));
|
||||
} else {
|
||||
let mut inv = invert_boolean_expression(&make, expr);
|
||||
if needs_parens_in_place_of(&inv, &dm.syntax().parent()?, &dm) {
|
||||
if precedence(&inv).needs_parentheses_in(prec) {
|
||||
inv = make.expr_paren(inv).into();
|
||||
}
|
||||
editor.replace(dm.syntax(), inv.syntax());
|
||||
editor.replace(demorganed.syntax(), inv.syntax());
|
||||
}
|
||||
} else {
|
||||
return None;
|
||||
}
|
||||
} else {
|
||||
let mut inv = invert_boolean_expression(&make, dm.clone());
|
||||
if needs_parens_in_place_of(&inv, &dm.syntax().parent()?, &dm) {
|
||||
let mut inv = invert_boolean_expression(&make, demorganed.clone());
|
||||
if precedence(&inv).needs_parentheses_in(prec) {
|
||||
inv = make.expr_paren(inv).into();
|
||||
}
|
||||
editor.replace(dm.syntax(), inv.syntax());
|
||||
editor.replace(demorganed.syntax(), inv.syntax());
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -121,7 +128,7 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opti
|
|||
let parent = neg_expr.syntax().parent();
|
||||
editor = builder.make_editor(neg_expr.syntax());
|
||||
|
||||
if parent.is_some_and(|parent| demorganed.needs_parens_in(parent)) {
|
||||
if parent.is_some_and(|parent| demorganed.needs_parens_in(&parent)) {
|
||||
cov_mark::hit!(demorgan_keep_parens_for_op_precedence2);
|
||||
editor.replace(neg_expr.syntax(), make.expr_paren(demorganed).syntax());
|
||||
} else {
|
||||
|
@ -271,30 +278,6 @@ fn add_bang_paren(make: &SyntaxFactory, expr: ast::Expr) -> ast::Expr {
|
|||
make.expr_prefix(T![!], make.expr_paren(expr).into()).into()
|
||||
}
|
||||
|
||||
fn needs_parens_in_place_of(
|
||||
this: &ast::Expr,
|
||||
parent: &SyntaxNode,
|
||||
in_place_of: &ast::Expr,
|
||||
) -> bool {
|
||||
assert_eq!(Some(parent), in_place_of.syntax().parent().as_ref());
|
||||
|
||||
let child_idx = parent
|
||||
.children()
|
||||
.enumerate()
|
||||
.find_map(|(i, it)| if &it == in_place_of.syntax() { Some(i) } else { None })
|
||||
.unwrap();
|
||||
let parent = parent.clone_subtree();
|
||||
let subtree_place = parent.children().nth(child_idx).unwrap();
|
||||
|
||||
let mut editor = SyntaxEditor::new(parent);
|
||||
editor.replace(subtree_place, this.syntax());
|
||||
let edit = editor.finish();
|
||||
|
||||
let replaced = edit.new_root().children().nth(child_idx).unwrap();
|
||||
let replaced = ast::Expr::cast(replaced).unwrap();
|
||||
replaced.needs_parens_in(edit.new_root().clone())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
|
|
@ -5,11 +5,7 @@ use ide_db::{
|
|||
EditionedFileId, RootDatabase,
|
||||
};
|
||||
use syntax::{
|
||||
ast::{
|
||||
self,
|
||||
prec::{precedence, ExprPrecedence},
|
||||
AstNode, AstToken, HasName,
|
||||
},
|
||||
ast::{self, AstNode, AstToken, HasName},
|
||||
SyntaxElement, TextRange,
|
||||
};
|
||||
|
||||
|
@ -77,22 +73,12 @@ pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext<'_>)
|
|||
}
|
||||
let usage_node =
|
||||
name_ref.syntax().ancestors().find(|it| ast::PathExpr::can_cast(it.kind()));
|
||||
let usage_parent_option =
|
||||
usage_node.and_then(|it| it.parent()).and_then(ast::Expr::cast);
|
||||
let usage_parent_option = usage_node.and_then(|it| it.parent());
|
||||
let usage_parent = match usage_parent_option {
|
||||
Some(u) => u,
|
||||
None => return Some((range, name_ref, false)),
|
||||
};
|
||||
let initializer = precedence(&initializer_expr);
|
||||
let parent = precedence(&usage_parent);
|
||||
Some((
|
||||
range,
|
||||
name_ref,
|
||||
parent != ExprPrecedence::Unambiguous
|
||||
&& initializer < parent
|
||||
// initializer == ExprPrecedence::Prefix -> parent != ExprPrecedence::Jump
|
||||
&& (initializer != ExprPrecedence::Prefix || parent != ExprPrecedence::Jump),
|
||||
))
|
||||
Some((range, name_ref, initializer_expr.needs_parens_in(&usage_parent)))
|
||||
})
|
||||
.collect::<Option<Vec<_>>>()?;
|
||||
|
||||
|
|
|
@ -34,7 +34,7 @@ pub(crate) fn remove_parentheses(acc: &mut Assists, ctx: &AssistContext<'_>) ->
|
|||
let expr = parens.expr()?;
|
||||
|
||||
let parent = parens.syntax().parent()?;
|
||||
if expr.needs_parens_in(parent) {
|
||||
if expr.needs_parens_in(&parent) {
|
||||
return None;
|
||||
}
|
||||
|
||||
|
|
|
@ -1,6 +1,6 @@
|
|||
use ide_db::imports::insert_use::ImportScope;
|
||||
use syntax::{
|
||||
ast::{self, make, AstNode, HasArgList},
|
||||
ast::{self, prec::ExprPrecedence, AstNode, HasArgList},
|
||||
TextRange,
|
||||
};
|
||||
|
||||
|
@ -55,7 +55,7 @@ pub(crate) fn unqualify_method_call(acc: &mut Assists, ctx: &AssistContext<'_>)
|
|||
TextRange::new(path.syntax().text_range().start(), l_paren.text_range().end());
|
||||
|
||||
// Parens around `expr` if needed
|
||||
let parens = needs_parens_as_receiver(&first_arg).then(|| {
|
||||
let parens = first_arg.precedence().needs_parentheses_in(ExprPrecedence::Postfix).then(|| {
|
||||
let range = first_arg.syntax().text_range();
|
||||
(range.start(), range.end())
|
||||
});
|
||||
|
@ -124,24 +124,6 @@ fn add_import(
|
|||
}
|
||||
}
|
||||
|
||||
fn needs_parens_as_receiver(expr: &ast::Expr) -> bool {
|
||||
// Make `(expr).dummy()`
|
||||
let dummy_call = make::expr_method_call(
|
||||
make::expr_paren(expr.clone()),
|
||||
make::name_ref("dummy"),
|
||||
make::arg_list([]),
|
||||
);
|
||||
|
||||
// Get the `expr` clone with the right parent back
|
||||
// (unreachable!s are fine since we've just constructed the expression)
|
||||
let ast::Expr::MethodCallExpr(call) = &dummy_call else { unreachable!() };
|
||||
let Some(receiver) = call.receiver() else { unreachable!() };
|
||||
let ast::Expr::ParenExpr(parens) = receiver else { unreachable!() };
|
||||
let Some(expr) = parens.expr() else { unreachable!() };
|
||||
|
||||
expr.needs_parens_in(dummy_call.syntax().clone())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::tests::{check_assist, check_assist_not_applicable};
|
||||
|
|
|
@ -258,15 +258,12 @@ fn mode_and_needs_parens_for_adjustment_hints(
|
|||
fn needs_parens_for_adjustment_hints(expr: &ast::Expr, postfix: bool) -> (bool, bool) {
|
||||
let prec = expr.precedence();
|
||||
if postfix {
|
||||
// postfix ops have higher precedence than any other operator, so we need to wrap
|
||||
// any inner expression that is below
|
||||
let needs_inner_parens = prec < ExprPrecedence::Postfix;
|
||||
let needs_inner_parens = prec.needs_parentheses_in(ExprPrecedence::Postfix);
|
||||
// given we are the higher precedence, no parent expression will have stronger requirements
|
||||
let needs_outer_parens = false;
|
||||
(needs_outer_parens, needs_inner_parens)
|
||||
} else {
|
||||
// We need to wrap all binary like things, thats everything below prefix except for jumps
|
||||
let needs_inner_parens = prec < ExprPrecedence::Prefix && prec != ExprPrecedence::Jump;
|
||||
let needs_inner_parens = prec.needs_parentheses_in(ExprPrecedence::Prefix);
|
||||
let parent = expr
|
||||
.syntax()
|
||||
.parent()
|
||||
|
@ -278,8 +275,8 @@ fn needs_parens_for_adjustment_hints(expr: &ast::Expr, postfix: bool) -> (bool,
|
|||
|
||||
// if we have no parent, we don't need outer parens to disambiguate
|
||||
// otherwise anything with higher precedence than what we insert needs to wrap us
|
||||
let needs_outer_parens =
|
||||
parent.is_some_and(|parent_prec| parent_prec > ExprPrecedence::Prefix);
|
||||
let needs_outer_parens = parent
|
||||
.is_some_and(|parent_prec| ExprPrecedence::Prefix.needs_parentheses_in(parent_prec));
|
||||
(needs_outer_parens, needs_inner_parens)
|
||||
}
|
||||
}
|
||||
|
|
|
@ -41,6 +41,21 @@ pub enum ExprPrecedence {
|
|||
Unambiguous,
|
||||
}
|
||||
|
||||
impl ExprPrecedence {
|
||||
pub fn needs_parentheses_in(self, other: ExprPrecedence) -> bool {
|
||||
match other {
|
||||
ExprPrecedence::Unambiguous => false,
|
||||
// postfix ops have higher precedence than any other operator, so we need to wrap
|
||||
// any inner expression that is below
|
||||
ExprPrecedence::Postfix => self < ExprPrecedence::Postfix,
|
||||
// We need to wrap all binary like things, thats everything below prefix except for
|
||||
// jumps (as those are prefix operations as well)
|
||||
ExprPrecedence::Prefix => ExprPrecedence::Jump < self && self < ExprPrecedence::Prefix,
|
||||
parent => self <= parent,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(PartialEq, Debug)]
|
||||
pub enum Fixity {
|
||||
/// The operator is left-associative
|
||||
|
@ -137,7 +152,7 @@ impl Expr {
|
|||
// - https://github.com/rust-lang/rust/blob/b6852428a8ea9728369b64b9964cad8e258403d3/compiler/rustc_ast/src/util/parser.rs#L296
|
||||
|
||||
/// Returns `true` if `self` would need to be wrapped in parentheses given that its parent is `parent`.
|
||||
pub fn needs_parens_in(&self, parent: SyntaxNode) -> bool {
|
||||
pub fn needs_parens_in(&self, parent: &SyntaxNode) -> bool {
|
||||
match_ast! {
|
||||
match parent {
|
||||
ast::Expr(e) => self.needs_parens_in_expr(&e),
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue