From e92dc3fd80142d426cbf4bfb3b1915b9efe460d3 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 28 Feb 2025 09:45:24 +0100 Subject: [PATCH] Have inline_local_variable use precedence calculation for parentheses --- .../src/handlers/inline_const_as_literal.rs | 23 ++------ .../src/handlers/inline_local_variable.rs | 59 ++++++++----------- crates/ide/src/inlay_hints/adjustment.rs | 12 ++-- crates/syntax/src/ast/prec.rs | 13 +++- crates/syntax/src/ast/token_ext.rs | 4 +- 5 files changed, 44 insertions(+), 67 deletions(-) diff --git a/crates/ide-assists/src/handlers/inline_const_as_literal.rs b/crates/ide-assists/src/handlers/inline_const_as_literal.rs index c92c22378f..139078eee7 100644 --- a/crates/ide-assists/src/handlers/inline_const_as_literal.rs +++ b/crates/ide-assists/src/handlers/inline_const_as_literal.rs @@ -39,25 +39,10 @@ pub(crate) fn inline_const_as_literal(acc: &mut Assists, ctx: &AssistContext<'_> // FIXME: Add support to handle type aliases for builtin scalar types. validate_type_recursively(ctx, Some(&konst_ty), false, fuel)?; - let expr = konst.value(ctx.sema.db)?; - - let value = match expr { - ast::Expr::BlockExpr(_) - | ast::Expr::Literal(_) - | ast::Expr::RefExpr(_) - | ast::Expr::ArrayExpr(_) - | ast::Expr::TupleExpr(_) - | ast::Expr::IfExpr(_) - | ast::Expr::ParenExpr(_) - | ast::Expr::MatchExpr(_) - | ast::Expr::MacroExpr(_) - | ast::Expr::BinExpr(_) - | ast::Expr::CallExpr(_) => konst - .eval(ctx.sema.db) - .ok()? - .render(ctx.sema.db, konst.krate(ctx.sema.db).edition(ctx.sema.db)), - _ => return None, - }; + let value = konst + .eval(ctx.sema.db) + .ok()? + .render(ctx.sema.db, konst.krate(ctx.sema.db).edition(ctx.sema.db)); let id = AssistId("inline_const_as_literal", AssistKind::RefactorInline); diff --git a/crates/ide-assists/src/handlers/inline_local_variable.rs b/crates/ide-assists/src/handlers/inline_local_variable.rs index b9fc075ae8..5d4fbfc10a 100644 --- a/crates/ide-assists/src/handlers/inline_local_variable.rs +++ b/crates/ide-assists/src/handlers/inline_local_variable.rs @@ -5,7 +5,11 @@ use ide_db::{ EditionedFileId, RootDatabase, }; use syntax::{ - ast::{self, AstNode, AstToken, HasName}, + ast::{ + self, + prec::{precedence, ExprPrecedence}, + AstNode, AstToken, HasName, + }, SyntaxElement, TextRange, }; @@ -79,33 +83,16 @@ pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext<'_>) Some(u) => u, None => return Some((range, name_ref, false)), }; - let initializer = matches!( - initializer_expr, - ast::Expr::CallExpr(_) - | ast::Expr::IndexExpr(_) - | ast::Expr::MethodCallExpr(_) - | ast::Expr::FieldExpr(_) - | ast::Expr::TryExpr(_) - | ast::Expr::Literal(_) - | ast::Expr::TupleExpr(_) - | ast::Expr::ArrayExpr(_) - | ast::Expr::ParenExpr(_) - | ast::Expr::PathExpr(_) - | ast::Expr::BlockExpr(_), - ); - let parent = matches!( - usage_parent, - ast::Expr::TupleExpr(_) - | ast::Expr::ArrayExpr(_) - | ast::Expr::ParenExpr(_) - | ast::Expr::ForExpr(_) - | ast::Expr::WhileExpr(_) - | ast::Expr::BreakExpr(_) - | ast::Expr::ReturnExpr(_) - | ast::Expr::MatchExpr(_) - | ast::Expr::BlockExpr(_) - ); - Some((range, name_ref, !(initializer || parent))) + 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), + )) }) .collect::>>()?; @@ -281,11 +268,11 @@ fn foo() { r" fn bar(a: usize) {} fn foo() { - (1 + 1) + 1; - if (1 + 1) > 10 { + 1 + 1 + 1; + if 1 + 1 > 10 { } - while (1 + 1) > 10 { + while 1 + 1 > 10 { } let b = (1 + 1) * 10; @@ -350,14 +337,14 @@ fn foo() { r" fn bar(a: usize) -> usize { a } fn foo() { - (bar(1) as u64) + 1; - if (bar(1) as u64) > 10 { + bar(1) as u64 + 1; + if bar(1) as u64 > 10 { } - while (bar(1) as u64) > 10 { + while bar(1) as u64 > 10 { } - let b = (bar(1) as u64) * 10; + let b = bar(1) as u64 * 10; bar(bar(1) as u64); }", ); @@ -574,7 +561,7 @@ fn foo() { r" fn foo() { let bar = 10; - let b = (&bar) * 10; + let b = &bar * 10; }", ); } diff --git a/crates/ide/src/inlay_hints/adjustment.rs b/crates/ide/src/inlay_hints/adjustment.rs index 40156ace26..6b2e41f42b 100644 --- a/crates/ide/src/inlay_hints/adjustment.rs +++ b/crates/ide/src/inlay_hints/adjustment.rs @@ -259,10 +259,8 @@ fn needs_parens_for_adjustment_hints(expr: &ast::Expr, postfix: 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 (except for jumps if they don't have a value) - let needs_inner_parens = prec < ExprPrecedence::Postfix && { - prec != ExprPrecedence::Jump || !expr.is_ret_like_with_no_value() - }; + // any inner expression that is below + let needs_inner_parens = prec < 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) @@ -275,13 +273,13 @@ fn needs_parens_for_adjustment_hints(expr: &ast::Expr, postfix: bool) -> (bool, .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()); + .map(|it| it.precedence()) + .filter(|&prec| prec != ExprPrecedence::Unambiguous); // 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 - // that means only postfix ops let needs_outer_parens = - parent.is_some_and(|parent_prec| parent_prec == ExprPrecedence::Postfix); + parent.is_some_and(|parent_prec| parent_prec > ExprPrecedence::Prefix); (needs_outer_parens, needs_inner_parens) } } diff --git a/crates/syntax/src/ast/prec.rs b/crates/syntax/src/ast/prec.rs index a7f1a3788c..2a47b3bea5 100644 --- a/crates/syntax/src/ast/prec.rs +++ b/crates/syntax/src/ast/prec.rs @@ -7,7 +7,7 @@ use crate::{ #[derive(Debug, Clone, Copy, PartialEq, PartialOrd)] pub enum ExprPrecedence { - // return, break, yield, closures + // return val, break val, yield val, closures Jump, // = += -= *= /= %= &= |= ^= <<= >>= Assign, @@ -58,12 +58,18 @@ pub fn precedence(expr: &ast::Expr) -> ExprPrecedence { Some(_) => ExprPrecedence::Unambiguous, }, + Expr::BreakExpr(e) if e.expr().is_some() => ExprPrecedence::Jump, + Expr::BecomeExpr(e) if e.expr().is_some() => ExprPrecedence::Jump, + Expr::ReturnExpr(e) if e.expr().is_some() => ExprPrecedence::Jump, + Expr::YeetExpr(e) if e.expr().is_some() => ExprPrecedence::Jump, + Expr::YieldExpr(e) if e.expr().is_some() => ExprPrecedence::Jump, + Expr::BreakExpr(_) | Expr::BecomeExpr(_) - | Expr::ContinueExpr(_) | Expr::ReturnExpr(_) | Expr::YeetExpr(_) - | Expr::YieldExpr(_) => ExprPrecedence::Jump, + | Expr::YieldExpr(_) + | Expr::ContinueExpr(_) => ExprPrecedence::Unambiguous, Expr::RangeExpr(..) => ExprPrecedence::Range, @@ -387,6 +393,7 @@ impl Expr { BreakExpr(e) => e.expr().is_none(), ContinueExpr(_) => true, YieldExpr(e) => e.expr().is_none(), + BecomeExpr(e) => e.expr().is_none(), _ => false, } } diff --git a/crates/syntax/src/ast/token_ext.rs b/crates/syntax/src/ast/token_ext.rs index 7d5ca27043..df851ab5b2 100644 --- a/crates/syntax/src/ast/token_ext.rs +++ b/crates/syntax/src/ast/token_ext.rs @@ -269,7 +269,7 @@ impl ast::ByteString { } (Ok(c), true) => { buf.reserve_exact(text.len()); - buf.extend_from_slice(text[..prev_end].as_bytes()); + buf.extend_from_slice(&text.as_bytes()[..prev_end]); buf.push(c as u8); } (Err(e), _) => has_error = Some(e), @@ -333,7 +333,7 @@ impl ast::CString { } (Ok(u), true) => { buf.reserve_exact(text.len()); - buf.extend(text[..prev_end].as_bytes()); + buf.extend(&text.as_bytes()[..prev_end]); extend_unit(&mut buf, u); } (Err(e), _) => has_error = Some(e),