From ac03de773f750f7b327720778e14d141f5e78d63 Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Fri, 7 Apr 2023 22:45:04 +0900 Subject: [PATCH 1/3] Add flag to disallow opaque types for `DisplayTarget::SourceCode` --- crates/hir-ty/src/display.rs | 40 +++++++++++++++---- crates/hir-ty/src/tests.rs | 4 +- .../src/handlers/add_explicit_type.rs | 2 +- .../src/handlers/add_return_type.rs | 2 +- .../src/handlers/extract_function.rs | 2 +- .../src/handlers/generate_constant.rs | 3 +- .../src/handlers/generate_enum_variant.rs | 2 +- .../src/handlers/generate_function.rs | 6 +-- .../src/handlers/promote_local_to_const.rs | 14 ++++--- .../replace_turbofish_with_explicit_type.rs | 2 +- .../src/completions/fn_param.rs | 2 +- crates/ide-completion/src/completions/type.rs | 2 +- crates/ide-db/src/path_transform.rs | 4 +- .../src/handlers/missing_fields.rs | 4 +- .../src/handlers/no_such_field.rs | 2 +- 15 files changed, 62 insertions(+), 29 deletions(-) diff --git a/crates/hir-ty/src/display.rs b/crates/hir-ty/src/display.rs index f892a815197..0eef25102e8 100644 --- a/crates/hir-ty/src/display.rs +++ b/crates/hir-ty/src/display.rs @@ -150,6 +150,7 @@ fn display_source_code<'a>( &'a self, db: &'a dyn HirDatabase, module_id: ModuleId, + allow_opaque: bool, ) -> Result { let mut result = String::new(); match self.hir_fmt(&mut HirFormatter { @@ -160,7 +161,7 @@ fn display_source_code<'a>( max_size: None, omit_verbose_types: false, closure_style: ClosureStyle::ImplFn, - display_target: DisplayTarget::SourceCode { module_id }, + display_target: DisplayTarget::SourceCode { module_id, allow_opaque }, }) { Ok(()) => {} Err(HirDisplayError::FmtError) => panic!("Writing to String can't fail!"), @@ -249,18 +250,26 @@ pub enum DisplayTarget { Diagnostics, /// Display types for inserting them in source files. /// The generated code should compile, so paths need to be qualified. - SourceCode { module_id: ModuleId }, + SourceCode { module_id: ModuleId, allow_opaque: bool }, /// Only for test purpose to keep real types Test, } impl DisplayTarget { - fn is_source_code(&self) -> bool { + fn is_source_code(self) -> bool { matches!(self, Self::SourceCode { .. }) } - fn is_test(&self) -> bool { + + fn is_test(self) -> bool { matches!(self, Self::Test) } + + fn allows_opaque(self) -> bool { + match self { + Self::SourceCode { allow_opaque, .. } => allow_opaque, + _ => true, + } + } } #[derive(Debug)] @@ -268,6 +277,7 @@ pub enum DisplaySourceCodeError { PathNotFound, UnknownType, Generator, + OpaqueType, } pub enum HirDisplayError { @@ -768,7 +778,7 @@ fn hir_fmt( }; write!(f, "{name}")?; } - DisplayTarget::SourceCode { module_id } => { + DisplayTarget::SourceCode { module_id, allow_opaque: _ } => { if let Some(path) = find_path::find_path( db.upcast(), ItemInNs::Types((*def_id).into()), @@ -906,6 +916,11 @@ fn should_show( f.end_location_link(); } TyKind::OpaqueType(opaque_ty_id, parameters) => { + if !f.display_target.allows_opaque() { + return Err(HirDisplayError::DisplaySourceCodeError( + DisplaySourceCodeError::OpaqueType, + )); + } let impl_trait_id = db.lookup_intern_impl_trait_id((*opaque_ty_id).into()); match impl_trait_id { ImplTraitId::ReturnTypeImplTrait(func, idx) => { @@ -953,8 +968,14 @@ fn should_show( } } TyKind::Closure(id, substs) => { - if f.display_target.is_source_code() && f.closure_style != ClosureStyle::ImplFn { - never!("Only `impl Fn` is valid for displaying closures in source code"); + if f.display_target.is_source_code() { + if !f.display_target.allows_opaque() { + return Err(HirDisplayError::DisplaySourceCodeError( + DisplaySourceCodeError::OpaqueType, + )); + } else if f.closure_style != ClosureStyle::ImplFn { + never!("Only `impl Fn` is valid for displaying closures in source code"); + } } match f.closure_style { ClosureStyle::Hide => return write!(f, "{TYPE_HINT_TRUNCATION}"), @@ -1053,6 +1074,11 @@ fn should_show( } TyKind::Alias(AliasTy::Projection(p_ty)) => p_ty.hir_fmt(f)?, TyKind::Alias(AliasTy::Opaque(opaque_ty)) => { + if !f.display_target.allows_opaque() { + return Err(HirDisplayError::DisplaySourceCodeError( + DisplaySourceCodeError::OpaqueType, + )); + } let impl_trait_id = db.lookup_intern_impl_trait_id(opaque_ty.opaque_ty_id.into()); match impl_trait_id { ImplTraitId::ReturnTypeImplTrait(func, idx) => { diff --git a/crates/hir-ty/src/tests.rs b/crates/hir-ty/src/tests.rs index 1e46bb1d040..245617ab82e 100644 --- a/crates/hir-ty/src/tests.rs +++ b/crates/hir-ty/src/tests.rs @@ -159,7 +159,7 @@ fn check_impl(ra_fixture: &str, allow_none: bool, only_types: bool, display_sour let range = node.as_ref().original_file_range(&db); if let Some(expected) = types.remove(&range) { let actual = if display_source { - ty.display_source_code(&db, def.module(&db)).unwrap() + ty.display_source_code(&db, def.module(&db), true).unwrap() } else { ty.display_test(&db).to_string() }; @@ -175,7 +175,7 @@ fn check_impl(ra_fixture: &str, allow_none: bool, only_types: bool, display_sour let range = node.as_ref().original_file_range(&db); if let Some(expected) = types.remove(&range) { let actual = if display_source { - ty.display_source_code(&db, def.module(&db)).unwrap() + ty.display_source_code(&db, def.module(&db), true).unwrap() } else { ty.display_test(&db).to_string() }; diff --git a/crates/ide-assists/src/handlers/add_explicit_type.rs b/crates/ide-assists/src/handlers/add_explicit_type.rs index 785ae3d09c6..8bc285614e0 100644 --- a/crates/ide-assists/src/handlers/add_explicit_type.rs +++ b/crates/ide-assists/src/handlers/add_explicit_type.rs @@ -69,7 +69,7 @@ pub(crate) fn add_explicit_type(acc: &mut Assists, ctx: &AssistContext<'_>) -> O return None; } - let inferred_type = ty.display_source_code(ctx.db(), module.into()).ok()?; + let inferred_type = ty.display_source_code(ctx.db(), module.into(), false).ok()?; acc.add( AssistId("add_explicit_type", AssistKind::RefactorRewrite), format!("Insert explicit type `{inferred_type}`"), diff --git a/crates/ide-assists/src/handlers/add_return_type.rs b/crates/ide-assists/src/handlers/add_return_type.rs index 879c478acf8..9e1022d809d 100644 --- a/crates/ide-assists/src/handlers/add_return_type.rs +++ b/crates/ide-assists/src/handlers/add_return_type.rs @@ -22,7 +22,7 @@ pub(crate) fn add_return_type(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opt if ty.is_unit() { return None; } - let ty = ty.display_source_code(ctx.db(), module.into()).ok()?; + let ty = ty.display_source_code(ctx.db(), module.into(), true).ok()?; acc.add( AssistId("add_return_type", AssistKind::RefactorRewrite), diff --git a/crates/ide-assists/src/handlers/extract_function.rs b/crates/ide-assists/src/handlers/extract_function.rs index bfa2890676b..728018506d1 100644 --- a/crates/ide-assists/src/handlers/extract_function.rs +++ b/crates/ide-assists/src/handlers/extract_function.rs @@ -1884,7 +1884,7 @@ fn with_tail_expr(block: ast::BlockExpr, tail_expr: ast::Expr) -> ast::BlockExpr } fn format_type(ty: &hir::Type, ctx: &AssistContext<'_>, module: hir::Module) -> String { - ty.display_source_code(ctx.db(), module.into()).ok().unwrap_or_else(|| "_".to_string()) + ty.display_source_code(ctx.db(), module.into(), true).ok().unwrap_or_else(|| "_".to_string()) } fn make_ty(ty: &hir::Type, ctx: &AssistContext<'_>, module: hir::Module) -> ast::Type { diff --git a/crates/ide-assists/src/handlers/generate_constant.rs b/crates/ide-assists/src/handlers/generate_constant.rs index 57bb679729f..eccd7675fba 100644 --- a/crates/ide-assists/src/handlers/generate_constant.rs +++ b/crates/ide-assists/src/handlers/generate_constant.rs @@ -46,7 +46,8 @@ pub(crate) fn generate_constant(acc: &mut Assists, ctx: &AssistContext<'_>) -> O let ty = ctx.sema.type_of_expr(&expr)?; let scope = ctx.sema.scope(statement.syntax())?; let constant_module = scope.module(); - let type_name = ty.original().display_source_code(ctx.db(), constant_module.into()).ok()?; + let type_name = + ty.original().display_source_code(ctx.db(), constant_module.into(), false).ok()?; let target = statement.syntax().parent()?.text_range(); let path = constant_token.syntax().ancestors().find_map(ast::Path::cast)?; diff --git a/crates/ide-assists/src/handlers/generate_enum_variant.rs b/crates/ide-assists/src/handlers/generate_enum_variant.rs index cd037f7492c..184f523e01b 100644 --- a/crates/ide-assists/src/handlers/generate_enum_variant.rs +++ b/crates/ide-assists/src/handlers/generate_enum_variant.rs @@ -192,7 +192,7 @@ fn expr_ty( scope: &hir::SemanticsScope<'_>, ) -> Option { let ty = ctx.sema.type_of_expr(&arg).map(|it| it.adjusted())?; - let text = ty.display_source_code(ctx.db(), scope.module().into()).ok()?; + let text = ty.display_source_code(ctx.db(), scope.module().into(), false).ok()?; Some(make::ty(&text)) } diff --git a/crates/ide-assists/src/handlers/generate_function.rs b/crates/ide-assists/src/handlers/generate_function.rs index 2372fe28e19..a5556878be6 100644 --- a/crates/ide-assists/src/handlers/generate_function.rs +++ b/crates/ide-assists/src/handlers/generate_function.rs @@ -438,7 +438,7 @@ fn make_return_type( Some(ty) if ty.is_unit() => (None, false), Some(ty) => { necessary_generic_params.extend(ty.generic_params(ctx.db())); - let rendered = ty.display_source_code(ctx.db(), target_module.into()); + let rendered = ty.display_source_code(ctx.db(), target_module.into(), true); match rendered { Ok(rendered) => (Some(make::ty(&rendered)), false), Err(_) => (Some(make::ty_placeholder()), true), @@ -992,9 +992,9 @@ fn maybe_displayed_type( let famous_defs = &FamousDefs(&ctx.sema, ctx.sema.scope(fn_arg.syntax())?.krate()); convert_reference_type(ty.strip_references(), ctx.db(), famous_defs) .map(|conversion| conversion.convert_type(ctx.db())) - .or_else(|| ty.display_source_code(ctx.db(), target_module.into()).ok()) + .or_else(|| ty.display_source_code(ctx.db(), target_module.into(), true).ok()) } else { - ty.display_source_code(ctx.db(), target_module.into()).ok() + ty.display_source_code(ctx.db(), target_module.into(), true).ok() } } diff --git a/crates/ide-assists/src/handlers/promote_local_to_const.rs b/crates/ide-assists/src/handlers/promote_local_to_const.rs index cbbea6c1e63..23153b4c566 100644 --- a/crates/ide-assists/src/handlers/promote_local_to_const.rs +++ b/crates/ide-assists/src/handlers/promote_local_to_const.rs @@ -57,11 +57,13 @@ pub(crate) fn promote_local_to_const(acc: &mut Assists, ctx: &AssistContext<'_>) let local = ctx.sema.to_def(&pat)?; let ty = ctx.sema.type_of_pat(&pat.into())?.original; - if ty.contains_unknown() || ty.is_closure() { - cov_mark::hit!(promote_lcoal_not_applicable_if_ty_not_inferred); - return None; - } - let ty = ty.display_source_code(ctx.db(), module.into()).ok()?; + let ty = match ty.display_source_code(ctx.db(), module.into(), false) { + Ok(ty) => ty, + Err(_) => { + cov_mark::hit!(promote_local_not_applicable_if_ty_not_inferred); + return None; + } + }; let initializer = let_stmt.initializer()?; if !is_body_const(&ctx.sema, &initializer) { @@ -187,7 +189,7 @@ fn foo() { #[test] fn not_applicable_unknown_ty() { - cov_mark::check!(promote_lcoal_not_applicable_if_ty_not_inferred); + cov_mark::check!(promote_local_not_applicable_if_ty_not_inferred); check_assist_not_applicable( promote_local_to_const, r" diff --git a/crates/ide-assists/src/handlers/replace_turbofish_with_explicit_type.rs b/crates/ide-assists/src/handlers/replace_turbofish_with_explicit_type.rs index 6626ce07959..43a97d7d3a5 100644 --- a/crates/ide-assists/src/handlers/replace_turbofish_with_explicit_type.rs +++ b/crates/ide-assists/src/handlers/replace_turbofish_with_explicit_type.rs @@ -55,7 +55,7 @@ pub(crate) fn replace_turbofish_with_explicit_type( let returned_type = match ctx.sema.type_of_expr(&initializer) { Some(returned_type) if !returned_type.original.contains_unknown() => { let module = ctx.sema.scope(let_stmt.syntax())?.module(); - returned_type.original.display_source_code(ctx.db(), module.into()).ok()? + returned_type.original.display_source_code(ctx.db(), module.into(), false).ok()? } _ => { cov_mark::hit!(fallback_to_turbofish_type_if_type_info_not_available); diff --git a/crates/ide-completion/src/completions/fn_param.rs b/crates/ide-completion/src/completions/fn_param.rs index d8b8a190eb8..734e1bed8df 100644 --- a/crates/ide-completion/src/completions/fn_param.rs +++ b/crates/ide-completion/src/completions/fn_param.rs @@ -127,7 +127,7 @@ fn params_from_stmt_list_scope( let module = scope.module().into(); scope.process_all_names(&mut |name, def| { if let hir::ScopeDef::Local(local) = def { - if let Ok(ty) = local.ty(ctx.db).display_source_code(ctx.db, module) { + if let Ok(ty) = local.ty(ctx.db).display_source_code(ctx.db, module, true) { cb(name, ty); } } diff --git a/crates/ide-completion/src/completions/type.rs b/crates/ide-completion/src/completions/type.rs index 2ad9520cd6e..e4705475638 100644 --- a/crates/ide-completion/src/completions/type.rs +++ b/crates/ide-completion/src/completions/type.rs @@ -242,7 +242,7 @@ pub(crate) fn complete_ascribed_type( } }? .adjusted(); - let ty_string = x.display_source_code(ctx.db, ctx.module.into()).ok()?; + let ty_string = x.display_source_code(ctx.db, ctx.module.into(), true).ok()?; acc.add(render_type_inference(ty_string, ctx)); None } diff --git a/crates/ide-db/src/path_transform.rs b/crates/ide-db/src/path_transform.rs index 6402a84a68b..0ee627a44c6 100644 --- a/crates/ide-db/src/path_transform.rs +++ b/crates/ide-db/src/path_transform.rs @@ -116,7 +116,9 @@ fn build_ctx(&self) -> Ctx<'a> { Some(( k, ast::make::ty( - &default.display_source_code(db, source_module.into()).ok()?, + &default + .display_source_code(db, source_module.into(), false) + .ok()?, ), )) } diff --git a/crates/ide-diagnostics/src/handlers/missing_fields.rs b/crates/ide-diagnostics/src/handlers/missing_fields.rs index 5c4327ff934..a33a2cd85e2 100644 --- a/crates/ide-diagnostics/src/handlers/missing_fields.rs +++ b/crates/ide-diagnostics/src/handlers/missing_fields.rs @@ -176,7 +176,9 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingFields) -> Option ast::Type { let ty_str = match ty.as_adt() { Some(adt) => adt.name(db).to_string(), - None => ty.display_source_code(db, module.into()).ok().unwrap_or_else(|| "_".to_string()), + None => { + ty.display_source_code(db, module.into(), false).ok().unwrap_or_else(|| "_".to_string()) + } }; make::ty(&ty_str) diff --git a/crates/ide-diagnostics/src/handlers/no_such_field.rs b/crates/ide-diagnostics/src/handlers/no_such_field.rs index 24c521ed1a8..625c95ce201 100644 --- a/crates/ide-diagnostics/src/handlers/no_such_field.rs +++ b/crates/ide-diagnostics/src/handlers/no_such_field.rs @@ -69,7 +69,7 @@ fn missing_record_expr_field_fixes( let new_field = make::record_field( None, make::name(record_expr_field.field_name()?.ident_token()?.text()), - make::ty(&new_field_type.display_source_code(sema.db, module.into()).ok()?), + make::ty(&new_field_type.display_source_code(sema.db, module.into(), true).ok()?), ); let last_field = record_fields.fields().last()?; From fcbc250723cdaf329ffd941dccb9e37568b88736 Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Thu, 6 Apr 2023 18:53:34 +0900 Subject: [PATCH 2/3] Add field for text edits to `InlayHint` --- crates/ide/src/inlay_hints.rs | 17 +++++++++++++++-- crates/ide/src/inlay_hints/adjustment.rs | 1 + crates/ide/src/inlay_hints/bind_pat.rs | 8 +++++--- crates/ide/src/inlay_hints/binding_mode.rs | 8 +++++++- crates/ide/src/inlay_hints/chaining.rs | 17 +++++++++++++++++ crates/ide/src/inlay_hints/closing_brace.rs | 1 + crates/ide/src/inlay_hints/closure_ret.rs | 1 + crates/ide/src/inlay_hints/discriminant.rs | 1 + crates/ide/src/inlay_hints/fn_lifetime_fn.rs | 3 +++ crates/ide/src/inlay_hints/implicit_static.rs | 1 + crates/ide/src/inlay_hints/param_name.rs | 1 + crates/rust-analyzer/src/to_proto.rs | 2 +- 12 files changed, 54 insertions(+), 7 deletions(-) diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs index e6360bc6ec1..42009ea6491 100644 --- a/crates/ide/src/inlay_hints.rs +++ b/crates/ide/src/inlay_hints.rs @@ -16,6 +16,7 @@ ast::{self, AstNode}, match_ast, NodeOrToken, SyntaxNode, TextRange, }; +use text_edit::TextEdit; use crate::{navigation_target::TryToNav, FileId}; @@ -113,14 +114,26 @@ pub struct InlayHint { pub kind: InlayKind, /// The actual label to show in the inlay hint. pub label: InlayHintLabel, + /// Text edit to apply when "accepting" this inlay hint. + pub text_edit: Option, } impl InlayHint { fn closing_paren(range: TextRange) -> InlayHint { - InlayHint { range, kind: InlayKind::ClosingParenthesis, label: InlayHintLabel::from(")") } + InlayHint { + range, + kind: InlayKind::ClosingParenthesis, + label: InlayHintLabel::from(")"), + text_edit: None, + } } fn opening_paren(range: TextRange) -> InlayHint { - InlayHint { range, kind: InlayKind::OpeningParenthesis, label: InlayHintLabel::from("(") } + InlayHint { + range, + kind: InlayKind::OpeningParenthesis, + label: InlayHintLabel::from("("), + text_edit: None, + } } } diff --git a/crates/ide/src/inlay_hints/adjustment.rs b/crates/ide/src/inlay_hints/adjustment.rs index 0b146091956..fea4b89b89f 100644 --- a/crates/ide/src/inlay_hints/adjustment.rs +++ b/crates/ide/src/inlay_hints/adjustment.rs @@ -135,6 +135,7 @@ pub(super) fn hints( ))), None, ), + text_edit: None, }); } if !postfix && needs_inner_parens { diff --git a/crates/ide/src/inlay_hints/bind_pat.rs b/crates/ide/src/inlay_hints/bind_pat.rs index 5f571d04482..4cd1fce8c92 100644 --- a/crates/ide/src/inlay_hints/bind_pat.rs +++ b/crates/ide/src/inlay_hints/bind_pat.rs @@ -12,9 +12,10 @@ match_ast, }; -use crate::{inlay_hints::closure_has_block_body, InlayHint, InlayHintsConfig, InlayKind}; - -use super::label_of_ty; +use crate::{ + inlay_hints::{closure_has_block_body, label_of_ty}, + InlayHint, InlayHintsConfig, InlayKind, +}; pub(super) fn hints( acc: &mut Vec, @@ -50,6 +51,7 @@ pub(super) fn hints( }, kind: InlayKind::Type, label, + text_edit: None, }); Some(()) diff --git a/crates/ide/src/inlay_hints/binding_mode.rs b/crates/ide/src/inlay_hints/binding_mode.rs index 5d9729263c2..3d7f969aaab 100644 --- a/crates/ide/src/inlay_hints/binding_mode.rs +++ b/crates/ide/src/inlay_hints/binding_mode.rs @@ -49,7 +49,12 @@ pub(super) fn hints( (true, false) => "&", _ => return, }; - acc.push(InlayHint { range, kind: InlayKind::BindingMode, label: r.to_string().into() }); + acc.push(InlayHint { + range, + kind: InlayKind::BindingMode, + label: r.to_string().into(), + text_edit: None, + }); }); match pat { ast::Pat::IdentPat(pat) if pat.ref_token().is_none() && pat.mut_token().is_none() => { @@ -63,6 +68,7 @@ pub(super) fn hints( range: pat.syntax().text_range(), kind: InlayKind::BindingMode, label: bm.to_string().into(), + text_edit: None, }); } ast::Pat::OrPat(pat) if !pattern_adjustments.is_empty() && outer_paren_pat.is_none() => { diff --git a/crates/ide/src/inlay_hints/chaining.rs b/crates/ide/src/inlay_hints/chaining.rs index 11e6dc05fa1..eed5a2554cc 100644 --- a/crates/ide/src/inlay_hints/chaining.rs +++ b/crates/ide/src/inlay_hints/chaining.rs @@ -61,6 +61,7 @@ pub(super) fn hints( range: expr.syntax().text_range(), kind: InlayKind::Chaining, label: label_of_ty(famous_defs, config, ty)?, + text_edit: None, }); } } @@ -120,6 +121,7 @@ fn main() { }, "", ], + text_edit: None, }, InlayHint { range: 147..154, @@ -140,6 +142,7 @@ fn main() { }, "", ], + text_edit: None, }, ] "#]], @@ -205,6 +208,7 @@ fn main() { }, "", ], + text_edit: None, }, InlayHint { range: 143..179, @@ -225,6 +229,7 @@ fn main() { }, "", ], + text_edit: None, }, ] "#]], @@ -274,6 +279,7 @@ fn main() { }, "", ], + text_edit: None, }, InlayHint { range: 143..179, @@ -294,6 +300,7 @@ fn main() { }, "", ], + text_edit: None, }, ] "#]], @@ -357,6 +364,7 @@ fn main() { }, ">", ], + text_edit: None, }, InlayHint { range: 246..265, @@ -390,6 +398,7 @@ fn main() { }, ">", ], + text_edit: None, }, ] "#]], @@ -455,6 +464,7 @@ fn main() { }, " = ()>", ], + text_edit: None, }, InlayHint { range: 174..224, @@ -488,6 +498,7 @@ fn main() { }, " = ()>", ], + text_edit: None, }, InlayHint { range: 174..206, @@ -521,6 +532,7 @@ fn main() { }, " = ()>", ], + text_edit: None, }, InlayHint { range: 174..189, @@ -541,6 +553,7 @@ fn main() { }, "", ], + text_edit: None, }, ] "#]], @@ -590,6 +603,7 @@ fn main() { }, "", ], + text_edit: None, }, InlayHint { range: 145..185, @@ -610,6 +624,7 @@ fn main() { }, "", ], + text_edit: None, }, InlayHint { range: 145..168, @@ -630,6 +645,7 @@ fn main() { }, "", ], + text_edit: None, }, InlayHint { range: 222..228, @@ -648,6 +664,7 @@ fn main() { tooltip: "", }, ], + text_edit: None, }, ] "#]], diff --git a/crates/ide/src/inlay_hints/closing_brace.rs b/crates/ide/src/inlay_hints/closing_brace.rs index 14c11be54ef..10b5acd064e 100644 --- a/crates/ide/src/inlay_hints/closing_brace.rs +++ b/crates/ide/src/inlay_hints/closing_brace.rs @@ -112,6 +112,7 @@ pub(super) fn hints( range: closing_token.text_range(), kind: InlayKind::ClosingBrace, label: InlayHintLabel::simple(label, None, linked_location), + text_edit: None, }); None diff --git a/crates/ide/src/inlay_hints/closure_ret.rs b/crates/ide/src/inlay_hints/closure_ret.rs index f03a18b8e96..eb5a464e7c8 100644 --- a/crates/ide/src/inlay_hints/closure_ret.rs +++ b/crates/ide/src/inlay_hints/closure_ret.rs @@ -43,6 +43,7 @@ pub(super) fn hints( range: param_list.syntax().text_range(), kind: InlayKind::ClosureReturnType, label: label_of_ty(famous_defs, config, ty)?, + text_edit: None, }); Some(()) } diff --git a/crates/ide/src/inlay_hints/discriminant.rs b/crates/ide/src/inlay_hints/discriminant.rs index 67eaa553ada..f9047efaf1a 100644 --- a/crates/ide/src/inlay_hints/discriminant.rs +++ b/crates/ide/src/inlay_hints/discriminant.rs @@ -75,6 +75,7 @@ fn variant_hints( })), None, ), + text_edit: None, }); Some(()) diff --git a/crates/ide/src/inlay_hints/fn_lifetime_fn.rs b/crates/ide/src/inlay_hints/fn_lifetime_fn.rs index b7182085b31..34eb5eb94c4 100644 --- a/crates/ide/src/inlay_hints/fn_lifetime_fn.rs +++ b/crates/ide/src/inlay_hints/fn_lifetime_fn.rs @@ -25,6 +25,7 @@ pub(super) fn hints( range: t.text_range(), kind: InlayKind::Lifetime, label: label.into(), + text_edit: None, }; let param_list = func.param_list()?; @@ -189,12 +190,14 @@ pub(super) fn hints( if is_empty { "" } else { ", " } ) .into(), + text_edit: None, }); } (None, allocated_lifetimes) => acc.push(InlayHint { range: func.name()?.syntax().text_range(), kind: InlayKind::GenericParamList, label: format!("<{}>", allocated_lifetimes.iter().format(", "),).into(), + text_edit: None, }), } Some(()) diff --git a/crates/ide/src/inlay_hints/implicit_static.rs b/crates/ide/src/inlay_hints/implicit_static.rs index 1122ee2e392..ba875649f79 100644 --- a/crates/ide/src/inlay_hints/implicit_static.rs +++ b/crates/ide/src/inlay_hints/implicit_static.rs @@ -34,6 +34,7 @@ pub(super) fn hints( range: t.text_range(), kind: InlayKind::Lifetime, label: "'static".to_owned().into(), + text_edit: None, }); } } diff --git a/crates/ide/src/inlay_hints/param_name.rs b/crates/ide/src/inlay_hints/param_name.rs index 9cdae632410..9729a43c220 100644 --- a/crates/ide/src/inlay_hints/param_name.rs +++ b/crates/ide/src/inlay_hints/param_name.rs @@ -57,6 +57,7 @@ pub(super) fn hints( range, kind: InlayKind::Parameter, label: InlayHintLabel::simple(param_name, None, linked_location), + text_edit: None, } }); diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 2b9dfecceff..cc72c2e10b5 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -510,7 +510,7 @@ pub(crate) fn inlay_hint( | InlayKind::AdjustmentPostfix | InlayKind::ClosingBrace => None, }, - text_edits: None, + text_edits: inlay_hint.text_edit.map(|it| text_edit_vec(line_index, it)), data: None, tooltip, label, From c978d4bf0c92e77ef94f354af8402af71bd7b5ab Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Sat, 8 Apr 2023 04:06:32 +0900 Subject: [PATCH 3/3] Implement text edits for inlay hints --- crates/ide/src/inlay_hints.rs | 50 +++++- crates/ide/src/inlay_hints/bind_pat.rs | 187 +++++++++++++++++++++- crates/ide/src/inlay_hints/chaining.rs | 11 +- crates/ide/src/inlay_hints/closure_ret.rs | 31 ++-- 4 files changed, 262 insertions(+), 17 deletions(-) diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs index 42009ea6491..7a8edfea835 100644 --- a/crates/ide/src/inlay_hints.rs +++ b/crates/ide/src/inlay_hints.rs @@ -14,7 +14,7 @@ use stdx::never; use syntax::{ ast::{self, AstNode}, - match_ast, NodeOrToken, SyntaxNode, TextRange, + match_ast, NodeOrToken, SyntaxNode, TextRange, TextSize, }; use text_edit::TextEdit; @@ -359,6 +359,23 @@ fn rec( Some(r) } +fn ty_to_text_edit( + sema: &Semantics<'_, RootDatabase>, + node_for_hint: &SyntaxNode, + ty: &hir::Type, + offset_to_insert: TextSize, + prefix: String, +) -> Option { + let scope = sema.scope(node_for_hint)?; + // FIXME: Limit the length and bail out on excess somehow? + let rendered = ty.display_source_code(scope.db, scope.module().into(), false).ok()?; + + let mut builder = TextEdit::builder(); + builder.insert(offset_to_insert, prefix); + builder.insert(offset_to_insert, rendered); + Some(builder.finish()) +} + // Feature: Inlay Hints // // rust-analyzer shows additional information inline with the source code. @@ -566,6 +583,37 @@ pub(super) fn check_expect(config: InlayHintsConfig, ra_fixture: &str, expect: E expect.assert_debug_eq(&inlay_hints) } + /// Computes inlay hints for the fixture, applies all the provided text edits and then runs + /// expect test. + #[track_caller] + pub(super) fn check_edit(config: InlayHintsConfig, ra_fixture: &str, expect: Expect) { + let (analysis, file_id) = fixture::file(ra_fixture); + let inlay_hints = analysis.inlay_hints(&config, file_id, None).unwrap(); + + let edits = inlay_hints + .into_iter() + .filter_map(|hint| hint.text_edit) + .reduce(|mut acc, next| { + acc.union(next).expect("merging text edits failed"); + acc + }) + .expect("no edit returned"); + + let mut actual = analysis.file_text(file_id).unwrap().to_string(); + edits.apply(&mut actual); + expect.assert_eq(&actual); + } + + #[track_caller] + pub(super) fn check_no_edit(config: InlayHintsConfig, ra_fixture: &str) { + let (analysis, file_id) = fixture::file(ra_fixture); + let inlay_hints = analysis.inlay_hints(&config, file_id, None).unwrap(); + + let edits: Vec<_> = inlay_hints.into_iter().filter_map(|hint| hint.text_edit).collect(); + + assert!(edits.is_empty(), "unexpected edits: {edits:?}"); + } + #[test] fn hints_disabled() { check_with_config( diff --git a/crates/ide/src/inlay_hints/bind_pat.rs b/crates/ide/src/inlay_hints/bind_pat.rs index 4cd1fce8c92..a131427f5fd 100644 --- a/crates/ide/src/inlay_hints/bind_pat.rs +++ b/crates/ide/src/inlay_hints/bind_pat.rs @@ -13,7 +13,7 @@ }; use crate::{ - inlay_hints::{closure_has_block_body, label_of_ty}, + inlay_hints::{closure_has_block_body, label_of_ty, ty_to_text_edit}, InlayHint, InlayHintsConfig, InlayKind, }; @@ -36,7 +36,7 @@ pub(super) fn hints( return None; } - let label = label_of_ty(famous_defs, config, ty)?; + let label = label_of_ty(famous_defs, config, ty.clone())?; if config.hide_named_constructor_hints && is_named_constructor(sema, pat, &label.to_string()).is_some() @@ -44,6 +44,23 @@ pub(super) fn hints( return None; } + let type_annotation_is_valid = desc_pat + .syntax() + .parent() + .map(|it| ast::LetStmt::can_cast(it.kind()) || ast::Param::can_cast(it.kind())) + .unwrap_or(false); + let text_edit = if type_annotation_is_valid { + ty_to_text_edit( + sema, + desc_pat.syntax(), + &ty, + pat.syntax().text_range().end(), + String::from(": "), + ) + } else { + None + }; + acc.push(InlayHint { range: match pat.name() { Some(name) => name.syntax().text_range(), @@ -51,7 +68,7 @@ pub(super) fn hints( }, kind: InlayKind::Type, label, - text_edit: None, + text_edit, }); Some(()) @@ -178,14 +195,16 @@ fn pat_is_enum_variant(db: &RootDatabase, bind_pat: &ast::IdentPat, pat_ty: &hir mod tests { // This module also contains tests for super::closure_ret + use expect_test::expect; use hir::ClosureStyle; use syntax::{TextRange, TextSize}; use test_utils::extract_annotations; - use crate::{fixture, inlay_hints::InlayHintsConfig}; + use crate::{fixture, inlay_hints::InlayHintsConfig, ClosureReturnTypeHints}; - use crate::inlay_hints::tests::{check, check_with_config, DISABLED_CONFIG, TEST_CONFIG}; - use crate::ClosureReturnTypeHints; + use crate::inlay_hints::tests::{ + check, check_edit, check_no_edit, check_with_config, DISABLED_CONFIG, TEST_CONFIG, + }; #[track_caller] fn check_types(ra_fixture: &str) { @@ -1014,4 +1033,160 @@ fn main() { }"#, ); } + + #[test] + fn edit_for_let_stmt() { + check_edit( + TEST_CONFIG, + r#" +struct S(T); +fn test(v: S<(S, S<()>)>, f: F) { + let a = v; + let S((b, c)) = v; + let a @ S((b, c)) = v; + let a = f; +} +"#, + expect![[r#" + struct S(T); + fn test(v: S<(S, S<()>)>, f: F) { + let a: S<(S, S<()>)> = v; + let S((b, c)) = v; + let a @ S((b, c)): S<(S, S<()>)> = v; + let a: F = f; + } + "#]], + ); + } + + #[test] + fn edit_for_closure_param() { + check_edit( + TEST_CONFIG, + r#" +fn test(t: T) { + let f = |a, b, c| {}; + let result = f(42, "", t); +} +"#, + expect![[r#" + fn test(t: T) { + let f = |a: i32, b: &str, c: T| {}; + let result: () = f(42, "", t); + } + "#]], + ); + } + + #[test] + fn edit_for_closure_ret() { + check_edit( + TEST_CONFIG, + r#" +struct S(T); +fn test() { + let f = || { 3 }; + let f = |a: S| { S(a) }; +} +"#, + expect![[r#" + struct S(T); + fn test() { + let f = || -> i32 { 3 }; + let f = |a: S| -> S> { S(a) }; + } + "#]], + ); + } + + #[test] + fn edit_prefixes_paths() { + check_edit( + TEST_CONFIG, + r#" +pub struct S(T); +mod middle { + pub struct S(T, U); + pub fn make() -> S, super::S> { loop {} } + + mod inner { + pub struct S(T); + } + + fn test() { + let a = make(); + } +} +"#, + expect![[r#" + pub struct S(T); + mod middle { + pub struct S(T, U); + pub fn make() -> S, super::S> { loop {} } + + mod inner { + pub struct S(T); + } + + fn test() { + let a: S, crate::S> = make(); + } + } + "#]], + ); + } + + #[test] + fn no_edit_for_top_pat_where_type_annotation_is_invalid() { + check_no_edit( + TEST_CONFIG, + r#" +fn test() { + if let a = 42 {} + while let a = 42 {} + match 42 { + a => (), + } +} +"#, + ) + } + + #[test] + fn no_edit_for_opaque_type() { + check_no_edit( + TEST_CONFIG, + r#" +trait Trait {} +struct S(T); +fn foo() -> impl Trait {} +fn bar() -> S {} +fn test() { + let a = foo(); + let a = bar(); + let f = || { foo() }; + let f = || { bar() }; +} +"#, + ); + } + + #[test] + fn no_edit_for_closure_return_without_body_block() { + // We can lift this limitation; see FIXME in closure_ret module. + let config = InlayHintsConfig { + closure_return_type_hints: ClosureReturnTypeHints::Always, + ..TEST_CONFIG + }; + check_no_edit( + config, + r#" +struct S(T); +fn test() { + let f = || 3; + let f = |a: S| S(a); +} +"#, + ); + } } diff --git a/crates/ide/src/inlay_hints/chaining.rs b/crates/ide/src/inlay_hints/chaining.rs index eed5a2554cc..6db9b8b5443 100644 --- a/crates/ide/src/inlay_hints/chaining.rs +++ b/crates/ide/src/inlay_hints/chaining.rs @@ -603,7 +603,16 @@ fn main() { }, "", ], - text_edit: None, + text_edit: Some( + TextEdit { + indels: [ + Indel { + insert: ": Struct", + delete: 130..130, + }, + ], + }, + ), }, InlayHint { range: 145..185, diff --git a/crates/ide/src/inlay_hints/closure_ret.rs b/crates/ide/src/inlay_hints/closure_ret.rs index eb5a464e7c8..6214e9c8e7f 100644 --- a/crates/ide/src/inlay_hints/closure_ret.rs +++ b/crates/ide/src/inlay_hints/closure_ret.rs @@ -1,14 +1,14 @@ //! Implementation of "closure return type" inlay hints. +//! +//! Tests live in [`bind_pat`][super::bind_pat] module. use ide_db::{base_db::FileId, famous_defs::FamousDefs}; use syntax::ast::{self, AstNode}; use crate::{ - inlay_hints::closure_has_block_body, ClosureReturnTypeHints, InlayHint, InlayHintsConfig, - InlayKind, + inlay_hints::{closure_has_block_body, label_of_ty, ty_to_text_edit}, + ClosureReturnTypeHints, InlayHint, InlayHintsConfig, InlayKind, }; -use super::label_of_ty; - pub(super) fn hints( acc: &mut Vec, famous_defs @ FamousDefs(sema, _): &FamousDefs<'_, '_>, @@ -24,26 +24,39 @@ pub(super) fn hints( return None; } - if !closure_has_block_body(&closure) - && config.closure_return_type_hints == ClosureReturnTypeHints::WithBlock - { + let has_block_body = closure_has_block_body(&closure); + if !has_block_body && config.closure_return_type_hints == ClosureReturnTypeHints::WithBlock { return None; } let param_list = closure.param_list()?; let closure = sema.descend_node_into_attributes(closure).pop()?; - let ty = sema.type_of_expr(&ast::Expr::ClosureExpr(closure))?.adjusted(); + let ty = sema.type_of_expr(&ast::Expr::ClosureExpr(closure.clone()))?.adjusted(); let callable = ty.as_callable(sema.db)?; let ty = callable.return_type(); if ty.is_unit() { return None; } + + // FIXME?: We could provide text edit to insert braces for closures with non-block body. + let text_edit = if has_block_body { + ty_to_text_edit( + sema, + closure.syntax(), + &ty, + param_list.syntax().text_range().end(), + String::from(" -> "), + ) + } else { + None + }; + acc.push(InlayHint { range: param_list.syntax().text_range(), kind: InlayKind::ClosureReturnType, label: label_of_ty(famous_defs, config, ty)?, - text_edit: None, + text_edit, }); Some(()) }