From dadad36bb9770f9b13ed84bc219ea0168a7a5bf1 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Mon, 18 Nov 2019 18:02:28 +0100 Subject: [PATCH 1/2] Move type inlay hint truncation to language server This commit implements a general truncation framework for HirFormatter that keeps track of how much has been output so far. This information can then be used to perform truncation inside the language server, instead of relying on the client. Initial support is implemented for truncating types hints using the maxInlayHintLength server config option. The existing solution in the VSCode extension has been removed in favor of letting the server truncate type hints. --- crates/ra_hir/src/ty.rs | 20 +++++++++ crates/ra_hir/src/ty/display.rs | 43 +++++++++++++++++-- crates/ra_ide_api/src/inlay_hints.rs | 37 +++++++++------- crates/ra_ide_api/src/lib.rs | 10 ++++- crates/ra_lsp_server/src/config.rs | 3 ++ crates/ra_lsp_server/src/main_loop.rs | 1 + .../ra_lsp_server/src/main_loop/handlers.rs | 2 +- crates/ra_lsp_server/src/world.rs | 1 + editors/code/src/commands/inlay_hints.ts | 14 +----- editors/code/src/server.ts | 1 + 10 files changed, 97 insertions(+), 35 deletions(-) diff --git a/crates/ra_hir/src/ty.rs b/crates/ra_hir/src/ty.rs index b7f50b714f..776613c7ce 100644 --- a/crates/ra_hir/src/ty.rs +++ b/crates/ra_hir/src/ty.rs @@ -800,6 +800,10 @@ impl HirDisplay for &Ty { impl HirDisplay for ApplicationTy { fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result { + if f.should_truncate() { + return write!(f, "…"); + } + match self.ctor { TypeCtor::Bool => write!(f, "bool")?, TypeCtor::Char => write!(f, "char")?, @@ -901,6 +905,10 @@ impl HirDisplay for ApplicationTy { impl HirDisplay for ProjectionTy { fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result { + if f.should_truncate() { + return write!(f, "…"); + } + let trait_name = self .associated_ty .parent_trait(f.db) @@ -919,6 +927,10 @@ impl HirDisplay for ProjectionTy { impl HirDisplay for Ty { fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result { + if f.should_truncate() { + return write!(f, "…"); + } + match self { Ty::Apply(a_ty) => a_ty.hir_fmt(f)?, Ty::Projection(p_ty) => p_ty.hir_fmt(f)?, @@ -1001,6 +1013,10 @@ impl HirDisplay for Ty { impl TraitRef { fn hir_fmt_ext(&self, f: &mut HirFormatter, use_as: bool) -> fmt::Result { + if f.should_truncate() { + return write!(f, "…"); + } + self.substs[0].hir_fmt(f)?; if use_as { write!(f, " as ")?; @@ -1031,6 +1047,10 @@ impl HirDisplay for &GenericPredicate { impl HirDisplay for GenericPredicate { fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result { + if f.should_truncate() { + return write!(f, "…"); + } + match self { GenericPredicate::Implemented(trait_ref) => trait_ref.hir_fmt(f)?, GenericPredicate::Projection(projection_pred) => { diff --git a/crates/ra_hir/src/ty/display.rs b/crates/ra_hir/src/ty/display.rs index 7910429d72..9bb3ece6c8 100644 --- a/crates/ra_hir/src/ty/display.rs +++ b/crates/ra_hir/src/ty/display.rs @@ -7,15 +7,30 @@ use crate::db::HirDatabase; pub struct HirFormatter<'a, 'b, DB> { pub db: &'a DB, fmt: &'a mut fmt::Formatter<'b>, + buf: String, + curr_size: usize, + max_size: Option, } pub trait HirDisplay { fn hir_fmt(&self, f: &mut HirFormatter) -> fmt::Result; + fn display<'a, DB>(&'a self, db: &'a DB) -> HirDisplayWrapper<'a, DB, Self> where Self: Sized, { - HirDisplayWrapper(db, self) + HirDisplayWrapper(db, self, None) + } + + fn display_truncated<'a, DB>( + &'a self, + db: &'a DB, + max_size: Option, + ) -> HirDisplayWrapper<'a, DB, Self> + where + Self: Sized, + { + HirDisplayWrapper(db, self, max_size) } } @@ -41,11 +56,25 @@ where /// This allows using the `write!` macro directly with a `HirFormatter`. pub fn write_fmt(&mut self, args: fmt::Arguments) -> fmt::Result { - fmt::write(self.fmt, args) + // We write to a buffer first to track output size + self.buf.clear(); + fmt::write(&mut self.buf, args)?; + self.curr_size += self.buf.len(); + + // Then we write to the internal formatter from the buffer + self.fmt.write_str(&self.buf) + } + + pub fn should_truncate(&self) -> bool { + if let Some(max_size) = self.max_size { + self.curr_size >= max_size + } else { + false + } } } -pub struct HirDisplayWrapper<'a, DB, T>(&'a DB, &'a T); +pub struct HirDisplayWrapper<'a, DB, T>(&'a DB, &'a T, Option); impl<'a, DB, T> fmt::Display for HirDisplayWrapper<'a, DB, T> where @@ -53,6 +82,12 @@ where T: HirDisplay, { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.1.hir_fmt(&mut HirFormatter { db: self.0, fmt: f }) + self.1.hir_fmt(&mut HirFormatter { + db: self.0, + fmt: f, + buf: String::with_capacity(20), + curr_size: 0, + max_size: self.2, + }) } } diff --git a/crates/ra_ide_api/src/inlay_hints.rs b/crates/ra_ide_api/src/inlay_hints.rs index 0cd9598483..6cd492b2a0 100644 --- a/crates/ra_ide_api/src/inlay_hints.rs +++ b/crates/ra_ide_api/src/inlay_hints.rs @@ -19,10 +19,15 @@ pub struct InlayHint { pub label: SmolStr, } -pub(crate) fn inlay_hints(db: &RootDatabase, file_id: FileId, file: &SourceFile) -> Vec { +pub(crate) fn inlay_hints( + db: &RootDatabase, + file_id: FileId, + file: &SourceFile, + max_inlay_hint_length: Option, +) -> Vec { file.syntax() .descendants() - .map(|node| get_inlay_hints(db, file_id, &node).unwrap_or_default()) + .map(|node| get_inlay_hints(db, file_id, &node, max_inlay_hint_length).unwrap_or_default()) .flatten() .collect() } @@ -31,6 +36,7 @@ fn get_inlay_hints( db: &RootDatabase, file_id: FileId, node: &SyntaxNode, + max_inlay_hint_length: Option, ) -> Option> { let analyzer = SourceAnalyzer::new(db, hir::Source::new(file_id.into(), node), None); match_ast! { @@ -40,7 +46,7 @@ fn get_inlay_hints( return None; } let pat = it.pat()?; - Some(get_pat_type_hints(db, &analyzer, pat, false)) + Some(get_pat_type_hints(db, &analyzer, pat, false, max_inlay_hint_length)) }, ast::LambdaExpr(it) => { it.param_list().map(|param_list| { @@ -48,22 +54,22 @@ fn get_inlay_hints( .params() .filter(|closure_param| closure_param.ascribed_type().is_none()) .filter_map(|closure_param| closure_param.pat()) - .map(|root_pat| get_pat_type_hints(db, &analyzer, root_pat, false)) + .map(|root_pat| get_pat_type_hints(db, &analyzer, root_pat, false, max_inlay_hint_length)) .flatten() .collect() }) }, ast::ForExpr(it) => { let pat = it.pat()?; - Some(get_pat_type_hints(db, &analyzer, pat, false)) + Some(get_pat_type_hints(db, &analyzer, pat, false, max_inlay_hint_length)) }, ast::IfExpr(it) => { let pat = it.condition()?.pat()?; - Some(get_pat_type_hints(db, &analyzer, pat, true)) + Some(get_pat_type_hints(db, &analyzer, pat, true, max_inlay_hint_length)) }, ast::WhileExpr(it) => { let pat = it.condition()?.pat()?; - Some(get_pat_type_hints(db, &analyzer, pat, true)) + Some(get_pat_type_hints(db, &analyzer, pat, true, max_inlay_hint_length)) }, ast::MatchArmList(it) => { Some( @@ -71,7 +77,7 @@ fn get_inlay_hints( .arms() .map(|match_arm| match_arm.pats()) .flatten() - .map(|root_pat| get_pat_type_hints(db, &analyzer, root_pat, true)) + .map(|root_pat| get_pat_type_hints(db, &analyzer, root_pat, true, max_inlay_hint_length)) .flatten() .collect(), ) @@ -86,6 +92,7 @@ fn get_pat_type_hints( analyzer: &SourceAnalyzer, root_pat: ast::Pat, skip_root_pat_hint: bool, + max_inlay_hint_length: Option, ) -> Vec { let original_pat = &root_pat.clone(); @@ -99,7 +106,7 @@ fn get_pat_type_hints( .map(|(range, pat_type)| InlayHint { range, kind: InlayKind::TypeHint, - label: pat_type.display(db).to_string().into(), + label: pat_type.display_truncated(db, max_inlay_hint_length).to_string().into(), }) .collect() } @@ -209,7 +216,7 @@ fn main() { }"#, ); - assert_debug_snapshot!(analysis.inlay_hints(file_id).unwrap(), @r###" + assert_debug_snapshot!(analysis.inlay_hints(file_id, None).unwrap(), @r###" [ InlayHint { range: [193; 197), @@ -278,7 +285,7 @@ fn main() { }"#, ); - assert_debug_snapshot!(analysis.inlay_hints(file_id).unwrap(), @r###" + assert_debug_snapshot!(analysis.inlay_hints(file_id, None).unwrap(), @r###" [ InlayHint { range: [21; 30), @@ -307,7 +314,7 @@ fn main() { }"#, ); - assert_debug_snapshot!(analysis.inlay_hints(file_id).unwrap(), @r###" + assert_debug_snapshot!(analysis.inlay_hints(file_id, None).unwrap(), @r###" [ InlayHint { range: [21; 30), @@ -355,7 +362,7 @@ fn main() { }"#, ); - assert_debug_snapshot!(analysis.inlay_hints(file_id).unwrap(), @r###" + assert_debug_snapshot!(analysis.inlay_hints(file_id, None).unwrap(), @r###" [ InlayHint { range: [166; 170), @@ -418,7 +425,7 @@ fn main() { }"#, ); - assert_debug_snapshot!(analysis.inlay_hints(file_id).unwrap(), @r###" + assert_debug_snapshot!(analysis.inlay_hints(file_id, None).unwrap(), @r###" [ InlayHint { range: [166; 170), @@ -481,7 +488,7 @@ fn main() { }"#, ); - assert_debug_snapshot!(analysis.inlay_hints(file_id).unwrap(), @r###" + assert_debug_snapshot!(analysis.inlay_hints(file_id, None).unwrap(), @r###" [ InlayHint { range: [311; 315), diff --git a/crates/ra_ide_api/src/lib.rs b/crates/ra_ide_api/src/lib.rs index 110ddcd626..fcb3da90e4 100644 --- a/crates/ra_ide_api/src/lib.rs +++ b/crates/ra_ide_api/src/lib.rs @@ -338,8 +338,14 @@ impl Analysis { } /// Returns a list of the places in the file where type hints can be displayed. - pub fn inlay_hints(&self, file_id: FileId) -> Cancelable> { - self.with_db(|db| inlay_hints::inlay_hints(db, file_id, &db.parse(file_id).tree())) + pub fn inlay_hints( + &self, + file_id: FileId, + max_inlay_hint_length: Option, + ) -> Cancelable> { + self.with_db(|db| { + inlay_hints::inlay_hints(db, file_id, &db.parse(file_id).tree(), max_inlay_hint_length) + }) } /// Returns the set of folding ranges. diff --git a/crates/ra_lsp_server/src/config.rs b/crates/ra_lsp_server/src/config.rs index 9871a3b37f..8045f3d60d 100644 --- a/crates/ra_lsp_server/src/config.rs +++ b/crates/ra_lsp_server/src/config.rs @@ -29,6 +29,8 @@ pub struct ServerConfig { pub lru_capacity: Option, + pub max_inlay_hint_length: Option, + /// For internal usage to make integrated tests faster. #[serde(deserialize_with = "nullable_bool_true")] pub with_sysroot: bool, @@ -44,6 +46,7 @@ impl Default for ServerConfig { exclude_globs: Vec::new(), use_client_watching: false, lru_capacity: None, + max_inlay_hint_length: None, with_sysroot: true, feature_flags: FxHashMap::default(), } diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs index 379dab4384..3e2ac36830 100644 --- a/crates/ra_lsp_server/src/main_loop.rs +++ b/crates/ra_lsp_server/src/main_loop.rs @@ -123,6 +123,7 @@ pub fn main_loop( .and_then(|it| it.folding_range.as_ref()) .and_then(|it| it.line_folding_only) .unwrap_or(false), + max_inlay_hint_length: config.max_inlay_hint_length, } }; diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs index 20f9aee138..7347a78c7a 100644 --- a/crates/ra_lsp_server/src/main_loop/handlers.rs +++ b/crates/ra_lsp_server/src/main_loop/handlers.rs @@ -870,7 +870,7 @@ pub fn handle_inlay_hints( let analysis = world.analysis(); let line_index = analysis.file_line_index(file_id)?; Ok(analysis - .inlay_hints(file_id)? + .inlay_hints(file_id, world.options.max_inlay_hint_length)? .into_iter() .map(|api_type| InlayHint { label: api_type.label.to_string(), diff --git a/crates/ra_lsp_server/src/world.rs b/crates/ra_lsp_server/src/world.rs index 51824e7a35..9bdea70c74 100644 --- a/crates/ra_lsp_server/src/world.rs +++ b/crates/ra_lsp_server/src/world.rs @@ -28,6 +28,7 @@ pub struct Options { pub publish_decorations: bool, pub supports_location_link: bool, pub line_folding_only: bool, + pub max_inlay_hint_length: Option, } /// `WorldState` is the primary mutable state of the language server diff --git a/editors/code/src/commands/inlay_hints.ts b/editors/code/src/commands/inlay_hints.ts index ffaaaebcb0..0dbdd94fba 100644 --- a/editors/code/src/commands/inlay_hints.ts +++ b/editors/code/src/commands/inlay_hints.ts @@ -87,7 +87,7 @@ export class HintsUpdater { range: hint.range, renderOptions: { after: { - contentText: `: ${this.truncateHint(hint.label)}` + contentText: `: ${hint.label}` } } })); @@ -98,18 +98,6 @@ export class HintsUpdater { } } - private truncateHint(label: string): string { - if (!Server.config.maxInlayHintLength) { - return label; - } - - let newLabel = label.substring(0, Server.config.maxInlayHintLength); - if (label.length > Server.config.maxInlayHintLength) { - newLabel += '…'; - } - return newLabel; - } - private async queryHints(documentUri: string): Promise { const request: InlayHintsParams = { textDocument: { uri: documentUri } diff --git a/editors/code/src/server.ts b/editors/code/src/server.ts index a3ef21a167..7907b70bc5 100644 --- a/editors/code/src/server.ts +++ b/editors/code/src/server.ts @@ -43,6 +43,7 @@ export class Server { initializationOptions: { publishDecorations: true, lruCapacity: Server.config.lruCapacity, + maxInlayHintLength: Server.config.maxInlayHintLength, excludeGlobs: Server.config.excludeGlobs, useClientWatching: Server.config.useClientWatching, featureFlags: Server.config.featureFlags From bf5ac4fe3952ee0db9fe18a185e39a209c58e79b Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Tue, 19 Nov 2019 17:40:38 +0100 Subject: [PATCH 2/2] Add test for inlay hint truncation --- crates/ra_ide_api/src/inlay_hints.rs | 37 ++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/crates/ra_ide_api/src/inlay_hints.rs b/crates/ra_ide_api/src/inlay_hints.rs index 6cd492b2a0..24a7ca5e71 100644 --- a/crates/ra_ide_api/src/inlay_hints.rs +++ b/crates/ra_ide_api/src/inlay_hints.rs @@ -514,4 +514,41 @@ fn main() { "### ); } + + #[test] + fn hint_truncation() { + let (analysis, file_id) = single_file( + r#" +struct Smol(T); + +struct VeryLongOuterName(T); + +fn main() { + let a = Smol(0u32); + let b = VeryLongOuterName(0usize); + let c = Smol(Smol(0u32)) +}"#, + ); + + assert_debug_snapshot!(analysis.inlay_hints(file_id, Some(8)).unwrap(), @r###" + [ + InlayHint { + range: [74; 75), + kind: TypeHint, + label: "Smol", + }, + InlayHint { + range: [98; 99), + kind: TypeHint, + label: "VeryLongOuterName<…>", + }, + InlayHint { + range: [137; 138), + kind: TypeHint, + label: "Smol>", + }, + ] + "### + ); + } }