From 992b4648d9f6a186c7c444a4db36b2bba00b09a5 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 3 Oct 2021 14:45:21 +0200 Subject: [PATCH 1/2] feat: Hide type inlay hints for constructors --- crates/ide/src/inlay_hints.rs | 96 +++++++++++++++++++++++++++++-- crates/test_utils/src/minicore.rs | 11 ++++ 2 files changed, 102 insertions(+), 5 deletions(-) diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs index 401feffff425..8dfceb35affd 100644 --- a/crates/ide/src/inlay_hints.rs +++ b/crates/ide/src/inlay_hints.rs @@ -198,28 +198,69 @@ fn get_bind_pat_hints( let descended = sema.descend_node_into_attributes(pat.clone()).pop(); let desc_pat = descended.as_ref().unwrap_or(pat); - let krate = sema.scope(desc_pat.syntax()).module().map(|it| it.krate()); - let famous_defs = FamousDefs(sema, krate); - let ty = sema.type_of_pat(&desc_pat.clone().into())?.original; if should_not_display_type_hint(sema, &pat, &ty) { return None; } + let krate = sema.scope(desc_pat.syntax()).module().map(|it| it.krate()); + let famous_defs = FamousDefs(sema, krate); + let label = hint_iterator(sema, &famous_defs, config, &ty); + + let label = match label { + Some(label) => label, + None => { + let ty = ty.display_truncated(sema.db, config.max_length).to_string(); + if Some(&*ty) == get_constructor_name(sema, pat).as_deref() { + return None; + } + ty.into() + } + }; + acc.push(InlayHint { range: match pat.name() { Some(name) => name.syntax().text_range(), None => pat.syntax().text_range(), }, kind: InlayKind::TypeHint, - label: hint_iterator(sema, &famous_defs, config, &ty) - .unwrap_or_else(|| ty.display_truncated(sema.db, config.max_length).to_string().into()), + label, }); Some(()) } +fn get_constructor_name(sema: &Semantics, pat: &ast::IdentPat) -> Option { + let it = pat.syntax().parent()?; + let expr = match_ast! { + match it { + ast::LetStmt(it) => it.initializer(), + ast::Condition(it) => it.expr(), + _ => None, + } + }; + + if let Some(expr) = expr { + let expr = sema.descend_node_into_attributes(expr.clone()).pop().unwrap_or(expr); + let expr = match expr { + ast::Expr::TryExpr(it) => it.expr(), + ast::Expr::AwaitExpr(it) => it.expr(), + expr => Some(expr), + }?; + let path = match expr { + ast::Expr::CallExpr(call) => match call.expr()? { + ast::Expr::PathExpr(p) => p.path(), + _ => None, + }, + _ => None, + }?; + let seg = path.qualifier()?.segment()?; + return Some(seg.to_string()); + } + None +} + /// Checks if the type is an Iterator from std::iter and replaces its hint with an `impl Iterator`. fn hint_iterator( sema: &Semantics, @@ -1234,6 +1275,51 @@ fn main() { ); } + #[test] + fn skip_constructor_type_hints() { + check_types( + r#" +//- minicore: try +use core::ops::ControlFlow; + +struct Struct; +struct TupleStruct(); + +impl Struct { + fn new() -> Self { + Struct + } + fn try_new() -> ControlFlow<(), Self> { + ControlFlow::Continue(Struct) + } +} + +struct Generic(T); +impl Generic { + fn new() -> Self { + Generic(0) + } +} + +fn main() { + let strukt = Struct::new(); + let tuple_struct = TupleStruct(); + // ^^^^^^^^^^^^ TupleStruct + let generic0 = Generic::new(); + // ^^^^^^^^ Generic + let generic1 = Generic::::new(); + // ^^^^^^^^ Generic + let generic2 = >::new(); + // ^^^^^^^^ Generic +} + +fn fallible() -> ControlFlow<()> { + let strukt = Struct::try_new()?; +} +"#, + ); + } + #[test] fn closures() { check( diff --git a/crates/test_utils/src/minicore.rs b/crates/test_utils/src/minicore.rs index 5e17047a407e..045b4898e508 100644 --- a/crates/test_utils/src/minicore.rs +++ b/crates/test_utils/src/minicore.rs @@ -300,6 +300,17 @@ pub mod ops { #[lang = "branch"] fn branch(self) -> ControlFlow; } + + impl Try for ControlFlow { + type Output = C; + type Residual = ControlFlow; + fn from_output(output: Self::Output) -> Self {} + fn branch(self) -> ControlFlow {} + } + + impl FromResidual for ControlFlow { + fn from_residual(residual: ControlFlow) -> Self {} + } } pub use self::try_::{ControlFlow, FromResidual, Try}; // endregion:try From a31bc668f50b7ad6f67028c80a8805457cc7ebdf Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 3 Oct 2021 15:31:35 +0200 Subject: [PATCH 2/2] Hide generic constructors if appropriate --- crates/ide/src/inlay_hints.rs | 89 +++++++++++++++++++++++------------ 1 file changed, 60 insertions(+), 29 deletions(-) diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs index 8dfceb35affd..3dcb510ab051 100644 --- a/crates/ide/src/inlay_hints.rs +++ b/crates/ide/src/inlay_hints.rs @@ -2,6 +2,7 @@ use either::Either; use hir::{known, Callable, HasVisibility, HirDisplay, Semantics, TypeInfo}; use ide_db::RootDatabase; use ide_db::{base_db::FileRange, helpers::FamousDefs}; +use itertools::Itertools; use stdx::to_lower_snake_case; use syntax::{ ast::{self, AstNode, HasArgList, HasName}, @@ -211,11 +212,11 @@ fn get_bind_pat_hints( let label = match label { Some(label) => label, None => { - let ty = ty.display_truncated(sema.db, config.max_length).to_string(); - if Some(&*ty) == get_constructor_name(sema, pat).as_deref() { + let ty_name = ty.display_truncated(sema.db, config.max_length).to_string(); + if is_named_constructor(sema, pat, &ty_name).is_some() { return None; } - ty.into() + ty_name.into() } }; @@ -231,34 +232,60 @@ fn get_bind_pat_hints( Some(()) } -fn get_constructor_name(sema: &Semantics, pat: &ast::IdentPat) -> Option { - let it = pat.syntax().parent()?; +fn is_named_constructor( + sema: &Semantics, + pat: &ast::IdentPat, + ty_name: &str, +) -> Option<()> { + let let_node = pat.syntax().parent()?; let expr = match_ast! { - match it { + match let_node { ast::LetStmt(it) => it.initializer(), ast::Condition(it) => it.expr(), _ => None, } + }?; + + let expr = sema.descend_node_into_attributes(expr.clone()).pop().unwrap_or(expr); + // unwrap postfix expressions + let expr = match expr { + ast::Expr::TryExpr(it) => it.expr(), + ast::Expr::AwaitExpr(it) => it.expr(), + expr => Some(expr), + }?; + let expr = match expr { + ast::Expr::CallExpr(call) => match call.expr()? { + ast::Expr::PathExpr(p) => p, + _ => return None, + }, + _ => return None, }; + let path = expr.path()?; - if let Some(expr) = expr { - let expr = sema.descend_node_into_attributes(expr.clone()).pop().unwrap_or(expr); - let expr = match expr { - ast::Expr::TryExpr(it) => it.expr(), - ast::Expr::AwaitExpr(it) => it.expr(), - expr => Some(expr), - }?; - let path = match expr { - ast::Expr::CallExpr(call) => match call.expr()? { - ast::Expr::PathExpr(p) => p.path(), - _ => None, - }, - _ => None, - }?; - let seg = path.qualifier()?.segment()?; - return Some(seg.to_string()); + // Check for tuple-struct or tuple-variant in which case we can check the last segment + let callable = sema.type_of_expr(&ast::Expr::PathExpr(expr))?.original.as_callable(sema.db); + let callable_kind = callable.map(|it| it.kind()); + if let Some(hir::CallableKind::TupleStruct(_) | hir::CallableKind::TupleEnumVariant(_)) = + callable_kind + { + if let Some(ctor) = path.segment() { + return (&ctor.to_string() == ty_name).then(|| ()); + } } - None + + // otherwise use the qualifying segment as the constructor name + let qual_seg = path.qualifier()?.segment()?; + let ctor_name = match qual_seg.kind()? { + ast::PathSegmentKind::Name(name_ref) => { + match qual_seg.generic_arg_list().map(|it| it.generic_args()) { + Some(generics) => format!("{}<{}>", name_ref, generics.format(", ")), + None => name_ref.to_string(), + } + } + ast::PathSegmentKind::Type { type_ref: Some(ty), trait_ref: None } => ty.to_string(), + _ => return None, + }; + (&ctor_name == ty_name).then(|| ()) } /// Checks if the type is an Iterator from std::iter and replaces its hint with an `impl Iterator`. @@ -511,10 +538,12 @@ mod tests { max_length: None, }; + #[track_caller] fn check(ra_fixture: &str) { check_with_config(TEST_CONFIG, ra_fixture); } + #[track_caller] fn check_params(ra_fixture: &str) { check_with_config( InlayHintsConfig { @@ -527,6 +556,7 @@ mod tests { ); } + #[track_caller] fn check_types(ra_fixture: &str) { check_with_config( InlayHintsConfig { @@ -539,6 +569,7 @@ mod tests { ); } + #[track_caller] fn check_chains(ra_fixture: &str) { check_with_config( InlayHintsConfig { @@ -551,6 +582,7 @@ mod tests { ); } + #[track_caller] fn check_with_config(config: InlayHintsConfig, ra_fixture: &str) { let (analysis, file_id) = fixture::file(&ra_fixture); let expected = extract_annotations(&*analysis.file_text(file_id).unwrap()); @@ -560,6 +592,7 @@ mod tests { assert_eq!(expected, actual, "\nExpected:\n{:#?}\n\nActual:\n{:#?}", expected, actual); } + #[track_caller] fn check_expect(config: InlayHintsConfig, ra_fixture: &str, expect: Expect) { let (analysis, file_id) = fixture::file(&ra_fixture); let inlay_hints = analysis.inlay_hints(&config, file_id).unwrap(); @@ -1232,11 +1265,12 @@ trait Display {} trait Sync {} fn main() { - let _v = Vec::>::new(); + // The block expression wrapping disables the constructor hint hiding logic + let _v = { Vec::>::new() }; //^^ Vec> - let _v = Vec::>::new(); + let _v = { Vec::>::new() }; //^^ Vec> - let _v = Vec::>::new(); + let _v = { Vec::>::new() }; //^^ Vec> } "#, @@ -1304,13 +1338,10 @@ impl Generic { fn main() { let strukt = Struct::new(); let tuple_struct = TupleStruct(); - // ^^^^^^^^^^^^ TupleStruct let generic0 = Generic::new(); // ^^^^^^^^ Generic let generic1 = Generic::::new(); - // ^^^^^^^^ Generic let generic2 = >::new(); - // ^^^^^^^^ Generic } fn fallible() -> ControlFlow<()> {