From d9d10df7a41bf9948442083a79377bbe3baeda68 Mon Sep 17 00:00:00 2001 From: Myriad-Dreamin <35292584+Myriad-Dreamin@users.noreply.github.com> Date: Thu, 17 Oct 2024 19:21:33 +0800 Subject: [PATCH] feat: provide parameter docs in hover tips (#702) * feat: supports parameter docs * dev: update snapshot --- crates/tinymist-query/src/analysis/global.rs | 6 +- crates/tinymist-query/src/analysis/tyck.rs | 20 ++-- .../tinymist-query/src/analysis/tyck/docs.rs | 22 +++- .../src/analysis/tyck/syntax.rs | 108 ++++++++---------- crates/tinymist-query/src/docs/symbol.rs | 41 ++++++- crates/tinymist-query/src/docs/tidy.rs | 27 ++++- .../src/fixtures/hover/param.typ | 5 + .../fixtures/hover/snaps/test@param.typ.snap | 9 ++ crates/tinymist-query/src/hover.rs | 34 +++--- crates/tinymist-query/src/tests.rs | 9 +- tests/e2e/main.rs | 4 +- 11 files changed, 190 insertions(+), 95 deletions(-) create mode 100644 crates/tinymist-query/src/fixtures/hover/param.typ create mode 100644 crates/tinymist-query/src/fixtures/hover/snaps/test@param.typ.snap diff --git a/crates/tinymist-query/src/analysis/global.rs b/crates/tinymist-query/src/analysis/global.rs index d185ea63..088f3c55 100644 --- a/crates/tinymist-query/src/analysis/global.rs +++ b/crates/tinymist-query/src/analysis/global.rs @@ -21,7 +21,7 @@ use crate::analysis::{ analyze_bib, analyze_expr_, analyze_import_, analyze_signature, post_type_check, BibInfo, DefUseInfo, DocString, ImportInfo, PathPreference, Signature, SignatureTarget, Ty, TypeScheme, }; -use crate::docs::{DocStringKind, SignatureDocs}; +use crate::docs::{DocStringKind, SignatureDocs, VarDocs}; use crate::syntax::{ construct_module_dependencies, find_expr_in_import, get_deref_target, resolve_id_by_path, scan_workspace_files, DerefTarget, LexicalHierarchy, ModuleDependency, @@ -573,6 +573,10 @@ impl<'w> AnalysisContext<'w> { res } + pub(crate) fn variable_docs(&mut self, pos: &LinkedNode) -> Option { + crate::docs::variable_docs(self, pos) + } + pub(crate) fn signature_docs(&mut self, runtime_fn: &Value) -> Option { crate::docs::signature_docs(self, runtime_fn, None) } diff --git a/crates/tinymist-query/src/analysis/tyck.rs b/crates/tinymist-query/src/analysis/tyck.rs index a97a5e30..ccbf404a 100644 --- a/crates/tinymist-query/src/analysis/tyck.rs +++ b/crates/tinymist-query/src/analysis/tyck.rs @@ -120,9 +120,19 @@ impl<'a, 'w> TypeChecker<'a, 'w> { var } - fn get_var(&mut self, s: Span, r: IdentRef) -> Option> { + fn get_var(&mut self, root: &LinkedNode<'_>, ident: ast::Ident) -> Option> { + let s = ident.span(); + let r = to_ident_ref(root, ident)?; let def_id = self.get_def_id(s, &r)?; + self.get_var_by_id(s, r.name.as_ref().into(), def_id) + } + fn get_var_by_id( + &mut self, + s: Span, + name: Interned, + def_id: DefId, + ) -> Option> { // todo: false positive of clippy #[allow(clippy::map_entry)] if !self.info.vars.contains_key(&def_id) { @@ -130,13 +140,7 @@ impl<'a, 'w> TypeChecker<'a, 'w> { let init_expr = self.init_var(def); self.info.vars.insert( def_id, - TypeVarBounds::new( - TypeVar { - name: r.name.as_str().into(), - def: def_id, - }, - init_expr, - ), + TypeVarBounds::new(TypeVar { name, def: def_id }, init_expr), ); } diff --git a/crates/tinymist-query/src/analysis/tyck/docs.rs b/crates/tinymist-query/src/analysis/tyck/docs.rs index 07af6605..96fcaa72 100644 --- a/crates/tinymist-query/src/analysis/tyck/docs.rs +++ b/crates/tinymist-query/src/analysis/tyck/docs.rs @@ -1,10 +1,15 @@ +use std::sync::OnceLock; + use reflexo::TakeAs; use typst::foundations::{IntoValue, Module, Str, Type}; use super::*; use crate::{ adt::snapshot_map::SnapshotMap, - docs::{convert_docs, identify_func_docs, identify_var_docs, DocStringKind}, + docs::{ + convert_docs, identify_func_docs, identify_var_docs, DocStringKind, UntypedSymbolDocs, + VarDocsT, + }, syntax::{find_docs_of, get_non_strict_def_target}, }; @@ -100,11 +105,22 @@ impl DocString { #[derive(Debug, Clone, Default)] pub struct VarDoc { /// The documentation of the variable - pub docs: Option, + pub docs: EcoString, /// The type of the variable pub ty: Option, } +impl VarDoc { + /// Convert the variable doc to an untyped version + pub fn to_untyped(&self) -> Arc { + Arc::new(UntypedSymbolDocs::Variable(VarDocsT { + docs: self.docs.clone(), + return_ty: (), + def_docs: OnceLock::new(), + })) + } +} + pub(crate) fn compute_docstring( ctx: &AnalysisContext, fid: TypstFileId, @@ -150,7 +166,7 @@ impl<'a, 'w> DocsChecker<'a, 'w> { params.insert( param.name.into(), VarDoc { - docs: Some(param.docs), + docs: param.docs, ty: self.check_type_strings(&module, ¶m.types), }, ); diff --git a/crates/tinymist-query/src/analysis/tyck/syntax.rs b/crates/tinymist-query/src/analysis/tyck/syntax.rs index c9ea3612..39af4da8 100644 --- a/crates/tinymist-query/src/analysis/tyck/syntax.rs +++ b/crates/tinymist-query/src/analysis/tyck/syntax.rs @@ -2,9 +2,7 @@ use super::*; use crate::analysis::ParamAttrs; -use crate::docs::{ - DocStringKind, SignatureDocsT, TidyVarDocsT, TypelessParamDocs, UntypedSymbolDocs, -}; +use crate::docs::{DocStringKind, SignatureDocsT, TypelessParamDocs, UntypedSymbolDocs}; use crate::ty::*; static EMPTY_DOCSTRING: LazyLock = LazyLock::new(DocString::default); @@ -177,19 +175,11 @@ impl<'a, 'w> TypeChecker<'a, 'w> { } fn check_ident(&mut self, root: LinkedNode<'_>, mode: InterpretMode) -> Option { - let ident: ast::Ident = root.cast()?; - let ident_ref = IdentRef { - name: ident.get().clone(), - range: root.range(), - }; - - self.get_var(root.span(), ident_ref) - .map(Ty::Var) - .or_else(|| { - let s = root.span(); - let v = resolve_global_value(self.ctx, root, mode == InterpretMode::Math)?; - Some(Ty::Value(InsTy::new_at(v, s))) - }) + self.get_var(&root, root.cast()?).map(Ty::Var).or_else(|| { + let s = root.span(); + let v = resolve_global_value(self.ctx, root, mode == InterpretMode::Math)?; + Some(Ty::Value(InsTy::new_at(v, s))) + }) } fn check_array(&mut self, root: LinkedNode<'_>) -> Option { @@ -372,9 +362,10 @@ impl<'a, 'w> TypeChecker<'a, 'w> { let closure: ast::Closure = root.cast()?; let def_id = closure .name() - .and_then(|n| self.get_def_id(n.span(), &to_ident_ref(&root, n)?))?; + .and_then(|n| self.get_def_id(n.span(), &to_ident_ref(&root, n)?)); - let docstring = self.check_docstring(&root, DocStringKind::Function, def_id); + let docstring = + def_id.and_then(|def_id| self.check_docstring(&root, DocStringKind::Function, def_id)); let docstring = docstring.as_deref().unwrap_or(&EMPTY_DOCSTRING); log::debug!("check closure: {:?} -> {docstring:#?}", closure.name()); @@ -398,7 +389,7 @@ impl<'a, 'w> TypeChecker<'a, 'w> { let param_doc = docstring.get_var(&name).unwrap_or(&EMPTY_VAR_DOC); pos_docs.push(TypelessParamDocs { name, - docs: param_doc.docs.clone().unwrap_or_default(), + docs: param_doc.docs.clone(), cano_type: (), default: None, attrs: ParamAttrs::positional(), @@ -414,9 +405,10 @@ impl<'a, 'w> TypeChecker<'a, 'w> { } } ast::Param::Named(e) => { - let name = e.name().get().into(); let exp = self.check_expr_in(e.expr().span(), root.clone()); - let v = Ty::Var(self.get_var(e.name().span(), to_ident_ref(&root, e.name())?)?); + let var = self.get_var(&root, e.name())?; + let name = var.name.clone(); + let v = Ty::Var(var.clone()); if let Some(annotated) = docstring.var_ty(&name) { self.constrain(&v, annotated); } @@ -431,30 +423,35 @@ impl<'a, 'w> TypeChecker<'a, 'w> { name.clone(), TypelessParamDocs { name: name.clone(), - docs: param_doc.docs.clone().unwrap_or_default(), + docs: param_doc.docs.clone(), cano_type: (), default: Some(e.expr().to_untyped().clone().into_text()), attrs: ParamAttrs::named(), }, ); + self.info.var_docs.insert(var.def, param_doc.to_untyped()); } // todo: spread left/right ast::Param::Spread(a) => { if let Some(e) = a.sink_ident() { + let var = self.get_var(&root, e)?; + let name = var.name.clone(); + let param_doc = docstring + .get_var(&var.name.clone()) + .unwrap_or(&EMPTY_VAR_DOC); + self.info.var_docs.insert(var.def, param_doc.to_untyped()); + let exp = Ty::Builtin(BuiltinTy::Args); - let v = Ty::Var(self.get_var(e.span(), to_ident_ref(&root, e)?)?); - if let Some(annotated) = docstring.var_ty(&e.get().as_str().into()) { + let v = Ty::Var(var); + if let Some(annotated) = docstring.var_ty(&name) { self.constrain(&v, annotated); } self.constrain(&exp, &v); rest = Some(v); - let param_doc = docstring - .get_var(&e.get().as_str().into()) - .unwrap_or(&EMPTY_VAR_DOC); rest_docs = Some(TypelessParamDocs { - name: e.get().as_str().into(), - docs: param_doc.docs.clone().unwrap_or_default(), + name, + docs: param_doc.docs.clone(), cano_type: (), default: None, attrs: ParamAttrs::variadic(), @@ -474,17 +471,20 @@ impl<'a, 'w> TypeChecker<'a, 'w> { } } - self.info.var_docs.insert( - def_id, - Arc::new(UntypedSymbolDocs::Function(Box::new(SignatureDocsT { - docs: docstring.docs.clone().unwrap_or_default(), - pos: pos_docs, - named: named_docs, - rest: rest_docs, - ret_ty: (), - def_docs: Default::default(), - }))), - ); + // todo: arrow function docs + if let Some(def_id) = def_id { + self.info.var_docs.insert( + def_id, + Arc::new(UntypedSymbolDocs::Function(Box::new(SignatureDocsT { + docs: docstring.docs.clone().unwrap_or_default(), + pos: pos_docs, + named: named_docs, + rest: rest_docs, + ret_ty: (), + def_docs: Default::default(), + }))), + ); + } let body = self.check_expr_in(closure.body().span(), root); let res_ty = if let Some(annotated) = &docstring.res_ty { @@ -535,7 +535,7 @@ impl<'a, 'w> TypeChecker<'a, 'w> { .map(|init| self.check_expr_in(init.span(), root.clone())) .unwrap_or_else(|| Ty::Builtin(BuiltinTy::Infer)); - let v = Ty::Var(self.get_var(c.span(), to_ident_ref(&root, c)?)?); + let v = Ty::Var(self.get_var(&root, c)?); self.constrain(&value, &v); // todo lbs is the lexical signature. } @@ -553,21 +553,6 @@ impl<'a, 'w> TypeChecker<'a, 'w> { let value = docstring.res_ty.clone().unwrap_or(value); self.check_pattern(pattern, value, docstring, root.clone()); - - if let ast::Pattern::Normal(ast::Expr::Ident(ident)) = pattern { - let def_id = Some(ident) - .and_then(|n| self.get_def_id(n.span(), &to_ident_ref(&root, n)?)); - - if let Some(def_id) = def_id { - self.info.var_docs.insert( - def_id, - Arc::new(UntypedSymbolDocs::Variable(TidyVarDocsT { - docs: docstring.docs.clone().unwrap_or_default(), - return_ty: (), - })), - ); - } - } } } @@ -701,14 +686,17 @@ impl<'a, 'w> TypeChecker<'a, 'w> { ) -> Option { Some(match pattern { ast::Pattern::Normal(ast::Expr::Ident(ident)) => { - let var = self.get_var(ident.span(), to_ident_ref(&root, ident)?)?; - let annotated = docs.var_ty(&var.name); + let var = self.get_var(&root, ident)?; + let def_id = var.def; + let docstring = docs.get_var(&var.name).unwrap_or(&EMPTY_VAR_DOC); let var = Ty::Var(var); - log::debug!("check pattern: {ident:?} with {value:?} and annotation {annotated:?}"); - if let Some(annotated) = annotated { + log::debug!("check pattern: {ident:?} with {value:?} and docs {docstring:?}"); + if let Some(annotated) = docstring.ty.as_ref() { self.constrain(&var, annotated); } self.constrain(&value, &var); + + self.info.var_docs.insert(def_id, docstring.to_untyped()); var } ast::Pattern::Normal(_) => Ty::Any, diff --git a/crates/tinymist-query/src/docs/symbol.rs b/crates/tinymist-query/src/docs/symbol.rs index 0d9eae3f..d9f36c97 100644 --- a/crates/tinymist-query/src/docs/symbol.rs +++ b/crates/tinymist-query/src/docs/symbol.rs @@ -8,6 +8,7 @@ use serde::{Deserialize, Serialize}; use tinymist_world::base::{EntryState, ShadowApi, TaskInputs}; use tinymist_world::LspWorld; use typst::foundations::{Bytes, Func, Value}; +use typst::syntax::LinkedNode; use typst::{ diag::StrResult, syntax::{FileId, VirtualPath}, @@ -16,7 +17,7 @@ use typst::{ use super::tidy::*; use crate::analysis::{ParamAttrs, ParamSpec}; use crate::docs::library; -use crate::ty::Interned; +use crate::ty::{DocSource, Interned}; use crate::upstream::plain_docs_sentence; use crate::{ty::Ty, AnalysisContext}; @@ -69,7 +70,7 @@ pub enum SymbolDocsT { Function(Box>), /// Documentation about a variable. #[serde(rename = "var")] - Variable(TidyVarDocsT), + Variable(VarDocsT), /// Documentation about a module. #[serde(rename = "module")] Module(TidyModuleDocs), @@ -306,6 +307,42 @@ fn format_ty(ty: Option<&Ty>, doc_ty: Option<&mut ShowTypeRepr>) -> TypeRepr { } } +pub(crate) fn variable_docs(ctx: &mut AnalysisContext, pos: &LinkedNode) -> Option { + let source = ctx.source_by_id(pos.span().id()?).ok()?; + let type_info = ctx.type_check(&source)?; + let ty = type_info.type_of_span(pos.span())?; + + // todo multiple sources + let mut srcs = ty.sources(); + srcs.sort(); + log::info!("check variable docs of ty: {ty:?} => {srcs:?}"); + let doc_source = srcs.into_iter().next()?; + + let return_ty = ty.describe().map(|short| (short, format!("{ty:?}"))); + match doc_source { + DocSource::Var(var) => { + let docs = type_info + .var_docs + .get(&var.def) + .map(|docs| docs.docs().clone()); + Some(VarDocs { + docs: docs.unwrap_or_default(), + return_ty, + def_docs: OnceLock::new(), + }) + } + DocSource::Ins(ins) => ins.syntax.as_ref().map(|src| { + let docs = src.doc.as_ref().into(); + VarDocs { + docs, + return_ty, + def_docs: OnceLock::new(), + } + }), + _ => None, + } +} + pub(crate) fn signature_docs( ctx: &mut AnalysisContext, runtime_fn: &Value, diff --git a/crates/tinymist-query/src/docs/tidy.rs b/crates/tinymist-query/src/docs/tidy.rs index 2897aac8..c92ab1ec 100644 --- a/crates/tinymist-query/src/docs/tidy.rs +++ b/crates/tinymist-query/src/docs/tidy.rs @@ -1,8 +1,12 @@ +use std::sync::OnceLock; + use ecow::EcoString; use itertools::Itertools; use serde::{Deserialize, Serialize}; use typst::diag::StrResult; +use crate::upstream::plain_docs_sentence; + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct TidyParamDocs { pub name: EcoString, @@ -19,14 +23,23 @@ pub struct TidyFuncDocs { } /// Documentation about a variable (without type information). -pub type UntypedVarDocs = TidyVarDocsT<()>; +pub type UntypedVarDocs = VarDocsT<()>; /// Documentation about a variable. -pub type TidyVarDocs = TidyVarDocsT>; +pub type VarDocs = VarDocsT>; #[derive(Debug, Clone, Serialize, Deserialize)] -pub struct TidyVarDocsT { +pub struct VarDocsT { pub docs: EcoString, pub return_ty: T, + #[serde(skip)] + pub def_docs: OnceLock, +} + +impl VarDocs { + pub fn def_docs(&self) -> &String { + self.def_docs + .get_or_init(|| plain_docs_sentence(&self.docs).into()) + } } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -146,7 +159,7 @@ pub fn identify_func_docs(converted: &str) -> StrResult { }) } -pub fn identify_var_docs(converted: EcoString) -> StrResult { +pub fn identify_var_docs(converted: EcoString) -> StrResult { let lines = converted.lines().collect::>(); let mut return_ty = None; @@ -180,7 +193,11 @@ pub fn identify_var_docs(converted: EcoString) -> StrResult { None => converted, }; - Ok(TidyVarDocs { docs, return_ty }) + Ok(VarDocs { + docs, + return_ty, + def_docs: OnceLock::new(), + }) } pub fn identify_tidy_module_docs(docs: EcoString) -> StrResult { diff --git a/crates/tinymist-query/src/fixtures/hover/param.typ b/crates/tinymist-query/src/fixtures/hover/param.typ new file mode 100644 index 00000000..dd7c8707 --- /dev/null +++ b/crates/tinymist-query/src/fixtures/hover/param.typ @@ -0,0 +1,5 @@ + +/// Test +/// +/// - param (int): The `parameter`. +#let f(/* ident after */ param: 1) = 1; diff --git a/crates/tinymist-query/src/fixtures/hover/snaps/test@param.typ.snap b/crates/tinymist-query/src/fixtures/hover/snaps/test@param.typ.snap new file mode 100644 index 00000000..e8239dd7 --- /dev/null +++ b/crates/tinymist-query/src/fixtures/hover/snaps/test@param.typ.snap @@ -0,0 +1,9 @@ +--- +source: crates/tinymist-query/src/hover.rs +expression: "JsonRepr::new_redacted(result, &REDACT_LOC)" +input_file: crates/tinymist-query/src/fixtures/hover/param.typ +--- +{ + "contents": "```typc\nlet param;\n```\n\n---\nThe `parameter`.", + "range": "3:25:3:30" +} diff --git a/crates/tinymist-query/src/hover.rs b/crates/tinymist-query/src/hover.rs index 5ea040e2..51454aba 100644 --- a/crates/tinymist-query/src/hover.rs +++ b/crates/tinymist-query/src/hover.rs @@ -259,7 +259,8 @@ fn def_tooltip( "let {name}({params}){result};", name = lnk.name, params = ParamTooltip(sig.as_ref()), - result = ResultTooltip(&lnk.name, sig.as_ref()) + result = + ResultTooltip(&lnk.name, sig.as_ref().and_then(|sig| sig.ret_ty.as_ref())) ), })); @@ -276,6 +277,8 @@ fn def_tooltip( } LexicalKind::Var(LexicalVarKind::Variable) => { let deref_node = deref_target.node(); + let sig = ctx.variable_docs(deref_target.node()); + // todo: check sensible length, value highlighting if let Some(values) = expr_tooltip(ctx.world(), deref_node) { match values { @@ -293,10 +296,19 @@ fn def_tooltip( results.push(MarkedString::LanguageString(LanguageString { language: "typc".to_owned(), - value: format!("let {name};", name = lnk.name), + value: format!( + "let {name}{result};", + name = lnk.name, + result = ResultTooltip( + &lnk.name, + sig.as_ref().and_then(|sig| sig.return_ty.as_ref()) + ) + ), })); - if let Some(doc) = DocTooltip::get(ctx, &lnk) { + if let Some(doc) = sig { + results.push(MarkedString::String(doc.def_docs().clone())); + } else if let Some(doc) = DocTooltip::get(ctx, &lnk) { results.push(MarkedString::String(doc)); } @@ -361,22 +373,18 @@ impl fmt::Display for ParamTooltip<'_> { } } -struct ResultTooltip<'a>(&'a str, Option<&'a SignatureDocs>); +struct ResultTooltip<'a>(&'a str, Option<&'a (String, String)>); impl fmt::Display for ResultTooltip<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let Some(sig) = self.1 else { + let Some((short, _)) = self.1 else { return Ok(()); }; - if let Some((short, _)) = &sig.ret_ty { - if short == self.0 { - return Ok(()); - } - - write!(f, " = {short}") - } else { - Ok(()) + if short == self.0 { + return Ok(()); } + + write!(f, " = {short}") } } diff --git a/crates/tinymist-query/src/tests.rs b/crates/tinymist-query/src/tests.rs index 9d10bf7b..78e26673 100644 --- a/crates/tinymist-query/src/tests.rs +++ b/crates/tinymist-query/src/tests.rs @@ -273,7 +273,14 @@ pub fn find_test_position_(s: &Source, offset: usize) -> LspPosition { continue; } if matches!(n.kind(), SyntaxKind::Named) { - n = n.children().last().unwrap(); + if match_ident { + n = n + .children() + .find(|n| matches!(n.kind(), SyntaxKind::Ident)) + .unwrap(); + } else { + n = n.children().last().unwrap(); + } continue; } if match_ident { diff --git a/tests/e2e/main.rs b/tests/e2e/main.rs index 5030f62c..b7b870c5 100644 --- a/tests/e2e/main.rs +++ b/tests/e2e/main.rs @@ -374,7 +374,7 @@ fn e2e() { }); let hash = replay_log(&tinymist_binary, &root.join("neovim")); - insta::assert_snapshot!(hash, @"siphash128_13:31b50bcd716fccacfd71057fd764a7bc"); + insta::assert_snapshot!(hash, @"siphash128_13:7ae5ec9c50b480649a00306d1359d9f0"); } { @@ -385,7 +385,7 @@ fn e2e() { }); let hash = replay_log(&tinymist_binary, &root.join("vscode")); - insta::assert_snapshot!(hash, @"siphash128_13:801a7b1171fad3200bbe8082f50a055f"); + insta::assert_snapshot!(hash, @"siphash128_13:4880731c9a64e74b858da04feede2cb3"); } }