Merge pull request rust-lang#19251 from Veykril/push-tkmpqtzxynxk

Remove syntax editing from parenthesis computation

Author: Veykril

src/tools/rust-analyzer/crates/ide-assists/src/handlers/apply_demorgan.rs

@@ -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::*;

src/tools/rust-analyzer/crates/ide-assists/src/handlers/inline_local_variable.rs

@@ -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<_>>>()?;
 

src/tools/rust-analyzer/crates/ide-assists/src/handlers/remove_parentheses.rs

@@ -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;
     }
 

src/tools/rust-analyzer/crates/ide-assists/src/handlers/unqualify_method_call.rs

@@ -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};

src/tools/rust-analyzer/crates/ide/src/inlay_hints/adjustment.rs

@@ -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)
     }
 }

src/tools/rust-analyzer/crates/syntax/src/ast/prec.rs

@@ -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),