diff --git a/crates/ide/src/inlay_hints/adjustment.rs b/crates/ide/src/inlay_hints/adjustment.rs index 2acd4021cc15..d3b95750f7e1 100644 --- a/crates/ide/src/inlay_hints/adjustment.rs +++ b/crates/ide/src/inlay_hints/adjustment.rs @@ -13,11 +13,7 @@ use ide_db::famous_defs::FamousDefs; use ide_db::text_edit::TextEditBuilder; use span::EditionedFileId; -use stdx::never; -use syntax::{ - ast::{self, make, AstNode}, - ted, -}; +use syntax::ast::{self, prec::ExprPrecedence, AstNode}; use crate::{ AdjustmentHints, AdjustmentHintsMode, InlayHint, InlayHintLabel, InlayHintLabelPart, @@ -104,12 +100,14 @@ pub(super) fn hints( }; let iter: &mut dyn Iterator = iter.as_mut().either(|it| it as _, |it| it as _); + let mut has_adjustments = false; let mut allow_edit = !postfix; for Adjustment { source, target, kind } in iter { if source == target { cov_mark::hit!(same_type_adjustment); continue; } + has_adjustments = true; // FIXME: Add some nicer tooltips to each of these let (text, coercion) = match kind { @@ -172,6 +170,10 @@ pub(super) fn hints( }; if postfix { &mut post } else { &mut pre }.label.append_part(label); } + if !has_adjustments { + return None; + } + if !postfix && needs_inner_parens { pre.label.append_str("("); } @@ -254,71 +256,31 @@ fn mode_and_needs_parens_for_adjustment_hints( /// Returns whatever we need to add parentheses on the inside and/or outside of `expr`, /// if we are going to add (`postfix`) adjustments hints to it. fn needs_parens_for_adjustment_hints(expr: &ast::Expr, postfix: bool) -> (bool, bool) { - // This is a very miserable pile of hacks... - // - // `Expr::needs_parens_in` requires that the expression is the child of the other expression, - // that is supposed to be its parent. - // - // But we want to check what would happen if we add `*`/`.*` to the inner expression. - // To check for inner we need `` expr.needs_parens_in(`*expr`) ``, - // to check for outer we need `` `*expr`.needs_parens_in(parent) ``, - // where "expr" is the `expr` parameter, `*expr` is the edited `expr`, - // and "parent" is the parent of the original expression... - // - // For this we utilize mutable trees, which is a HACK, but it works. - // - // FIXME: comeup with a better API for `needs_parens_in`, so that we don't have to do *this* - - // Make `&expr`/`expr?` - let dummy_expr = { - // `make::*` function go through a string, so they parse wrongly. - // for example `` make::expr_try(`|| a`) `` would result in a - // `|| (a?)` and not `(|| a)?`. - // - // Thus we need dummy parens to preserve the relationship we want. - // The parens are then simply ignored by the following code. - let dummy_paren = make::expr_paren(expr.clone()); - if postfix { - make::expr_try(dummy_paren) - } else { - make::expr_ref(dummy_paren, false) - } - }; - - // Do the dark mutable tree magic. - // This essentially makes `dummy_expr` and `expr` switch places (families), - // so that `expr`'s parent is not `dummy_expr`'s parent. - let dummy_expr = dummy_expr.clone_for_update(); - let expr = expr.clone_for_update(); - ted::replace(expr.syntax(), dummy_expr.syntax()); - - let parent = dummy_expr.syntax().parent(); - let Some(expr) = (|| { - if postfix { - let ast::Expr::TryExpr(e) = &dummy_expr else { return None }; - let Some(ast::Expr::ParenExpr(e)) = e.expr() else { return None }; - - e.expr() - } else { - let ast::Expr::RefExpr(e) = &dummy_expr else { return None }; - let Some(ast::Expr::ParenExpr(e)) = e.expr() else { return None }; - - e.expr() - } - })() else { - never!("broken syntax tree?\n{:?}\n{:?}", expr, dummy_expr); - return (true, true); - }; - - // At this point - // - `parent` is the parent of the original expression - // - `dummy_expr` is the original expression wrapped in the operator we want (`*`/`.*`) - // - `expr` is the clone of the original expression (with `dummy_expr` as the parent) - - let needs_outer_parens = parent.is_some_and(|p| dummy_expr.needs_parens_in(p)); - let needs_inner_parens = expr.needs_parens_in(dummy_expr.syntax().clone()); - - (needs_outer_parens, needs_inner_parens) + 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 (except for jumps if they don't have a value) + let needs_inner_parens = prec < ExprPrecedence::Unambiguous && { + prec != ExprPrecedence::Jump || !expr.is_ret_like_with_no_value() + }; + // 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 parent = expr + .syntax() + .parent() + .and_then(ast::Expr::cast) + // if we are already wrapped, great, no need to wrap again + .filter(|it| !matches!(it, ast::Expr::ParenExpr(_))) + .map(|it| it.precedence()); + // 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(|prec| prec > ExprPrecedence::Prefix); + (needs_outer_parens, needs_inner_parens) + } } #[cfg(test)] diff --git a/crates/syntax/src/ast/prec.rs b/crates/syntax/src/ast/prec.rs index 28089ffb3771..5d33f132ac15 100644 --- a/crates/syntax/src/ast/prec.rs +++ b/crates/syntax/src/ast/prec.rs @@ -5,7 +5,122 @@ use crate::{ match_ast, AstNode, SyntaxNode, }; +#[derive(Debug, Clone, Copy, PartialEq, PartialOrd)] +pub enum ExprPrecedence { + // return, break, yield, closures + Jump, + // = += -= *= /= %= &= |= ^= <<= >>= + Assign, + // .. ..= + Range, + // || + LOr, + // && + LAnd, + // == != < > <= >= + Compare, + // | + BitOr, + // ^ + BitXor, + // & + BitAnd, + // << >> + Shift, + // + - + Sum, + // * / % + Product, + // as + Cast, + // unary - * ! & &mut + Prefix, + // paths, loops, function calls, array indexing, field expressions, method calls + Unambiguous, +} + +#[derive(PartialEq, Debug)] +pub enum Fixity { + /// The operator is left-associative + Left, + /// The operator is right-associative + Right, + /// The operator is not associative + None, +} + +pub fn precedence(expr: &ast::Expr) -> ExprPrecedence { + match expr { + Expr::ClosureExpr(closure) => match closure.ret_type() { + None => ExprPrecedence::Jump, + Some(_) => ExprPrecedence::Unambiguous, + }, + + Expr::BreakExpr(_) + | Expr::ContinueExpr(_) + | Expr::ReturnExpr(_) + | Expr::YeetExpr(_) + | Expr::YieldExpr(_) => ExprPrecedence::Jump, + + Expr::RangeExpr(..) => ExprPrecedence::Range, + + Expr::BinExpr(bin_expr) => match bin_expr.op_kind() { + Some(it) => match it { + BinaryOp::LogicOp(logic_op) => match logic_op { + ast::LogicOp::And => ExprPrecedence::LAnd, + ast::LogicOp::Or => ExprPrecedence::LOr, + }, + BinaryOp::ArithOp(arith_op) => match arith_op { + ast::ArithOp::Add | ast::ArithOp::Sub => ExprPrecedence::Sum, + ast::ArithOp::Div | ast::ArithOp::Rem | ast::ArithOp::Mul => { + ExprPrecedence::Product + } + ast::ArithOp::Shl | ast::ArithOp::Shr => ExprPrecedence::Shift, + ast::ArithOp::BitXor => ExprPrecedence::BitXor, + ast::ArithOp::BitOr => ExprPrecedence::BitOr, + ast::ArithOp::BitAnd => ExprPrecedence::BitAnd, + }, + BinaryOp::CmpOp(_) => ExprPrecedence::Compare, + BinaryOp::Assignment { .. } => ExprPrecedence::Assign, + }, + None => ExprPrecedence::Unambiguous, + }, + Expr::CastExpr(_) => ExprPrecedence::Cast, + + Expr::LetExpr(_) | Expr::PrefixExpr(_) | Expr::RefExpr(_) => ExprPrecedence::Prefix, + + Expr::ArrayExpr(_) + | Expr::AsmExpr(_) + | Expr::AwaitExpr(_) + | Expr::BecomeExpr(_) + | Expr::BlockExpr(_) + | Expr::CallExpr(_) + | Expr::FieldExpr(_) + | Expr::ForExpr(_) + | Expr::FormatArgsExpr(_) + | Expr::IfExpr(_) + | Expr::IndexExpr(_) + | Expr::Literal(_) + | Expr::LoopExpr(_) + | Expr::MacroExpr(_) + | Expr::MatchExpr(_) + | Expr::MethodCallExpr(_) + | Expr::OffsetOfExpr(_) + | Expr::ParenExpr(_) + | Expr::PathExpr(_) + | Expr::RecordExpr(_) + | Expr::TryExpr(_) + | Expr::TupleExpr(_) + | Expr::UnderscoreExpr(_) + | Expr::WhileExpr(_) => ExprPrecedence::Unambiguous, + } +} + impl Expr { + pub fn precedence(&self) -> ExprPrecedence { + precedence(self) + } + // Implementation is based on // - https://doc.rust-lang.org/reference/expressions.html#expression-precedence // - https://matklad.github.io/2020/04/13/simple-but-powerful-pratt-parsing.html @@ -261,7 +376,7 @@ impl Expr { } /// Returns true if self is one of `return`, `break`, `continue` or `yield` with **no associated value**. - fn is_ret_like_with_no_value(&self) -> bool { + pub fn is_ret_like_with_no_value(&self) -> bool { use Expr::*; match self {