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,
 };
 
 usecrate::{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),
  _ => returnNone,
 };
 
@@ -65,33 +72,33 @@ pub(crate) fn apply_demorgan(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opti
  editor.replace(demorganed.op_token()?, make.token(inv_token));
 
 letmut 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),
 ]);
 
-whileletSome((expr,dm)) = exprs.pop_front(){
+whileletSome((expr,demorganed, prec)) = exprs.pop_front(){
 ifletBinExpr(bin_expr) = &expr {
-ifletBinExpr(cbin_expr) = &dm{
+ifletBinExpr(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{
 letmut inv = invert_boolean_expression(&make, expr);
-ifneeds_parens_in_place_of(&inv,&dm.syntax().parent()?,&dm){
+ifprecedence(&inv).needs_parentheses_in(prec){
  inv = make.expr_paren(inv).into();
 }
- editor.replace(dm.syntax(), inv.syntax());
+ editor.replace(demorganed.syntax(), inv.syntax());
 }
 }else{
 returnNone;
 }
 }else{
-letmut inv = invert_boolean_expression(&make,dm.clone());
-ifneeds_parens_in_place_of(&inv,&dm.syntax().parent()?,&dm){
+letmut inv = invert_boolean_expression(&make,demorganed.clone());
+ifprecedence(&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()
 }
 
-fnneeds_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();
-
-letmut 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 {
 usesuper::*;

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 => returnSome((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){
 returnNone;
 }
 

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(
 }
 }
 
-fnneeds_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!()};
-letSome(receiver) = call.receiver()else{unreachable!()};
-let ast::Expr::ParenExpr(parens) = receiver else{unreachable!()};
-letSome(expr) = parens.expr()else{unreachable!()};
-
- expr.needs_parens_in(dummy_call.syntax().clone())
-}
-
 #[cfg(test)]
 mod tests {
 usecrate::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(
 fnneeds_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,
 }
 
+implExprPrecedence{
+pubfnneeds_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)]
 pubenumFixity{
 /// 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`.
-pubfnneeds_parens_in(&self,parent:SyntaxNode) -> bool{
+pubfnneeds_parens_in(&self,parent:&SyntaxNode) -> bool{
 match_ast!{
 match parent {
  ast::Expr(e) => self.needs_parens_in_expr(&e),