From 3e84cc9063724228ed315383aafc8736e3dd1de5 Mon Sep 17 00:00:00 2001 From: Sophie Dankel <47993817+sdankel@users.noreply.github.com> Date: Wed, 10 May 2023 03:24:02 -0700 Subject: [PATCH] Add links to hover documentation (#4532) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Closes https://github.com/FuelLabs/sway/issues/2851 https://github.com/FuelLabs/sway/issues/2852 Related, this code is needed for the links to work: https://github.com/FuelLabs/sway-vscode-plugin/pull/152 There are two types of links: go to definition and go to implementations. It should be working for: - structs - enums - traits - variables - functions - function parameters - struct fields Note: it isn't working for ABI at the moment. image image image ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [ ] I have requested a review from the relevant team or maintainers. --------- Co-authored-by: IGI-111 Co-authored-by: Kaya Gökalp Co-authored-by: Joshua Batty --- forc-pkg/src/pkg.rs | 2 +- sway-core/src/language/call_path.rs | 2 +- sway-core/src/language/ty/module.rs | 2 +- sway-core/src/semantic_analysis/module.rs | 2 +- .../src/semantic_analysis/namespace/items.rs | 26 ++++- .../src/semantic_analysis/namespace/module.rs | 8 +- .../semantic_analysis/namespace/trait_map.rs | 104 ++++++++++++++++-- .../capabilities/hover/hover_link_contents.rs | 47 +++++++- sway-lsp/src/capabilities/hover/mod.rs | 18 ++- sway-lsp/src/core/session.rs | 11 +- sway-lsp/src/core/sync.rs | 27 ++++- sway-lsp/src/core/token_map.rs | 6 +- sway-lsp/src/utils/document.rs | 8 ++ sway-lsp/tests/lib.rs | 2 +- 14 files changed, 234 insertions(+), 31 deletions(-) diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index 2451a4ce192..94e7b1e903e 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -2579,7 +2579,7 @@ pub fn check( ) .unwrap(), ); - lib_namespace_map.insert(node, namespace); + lib_namespace_map.insert(node, namespace.module().clone()); } source_map.insert_dependency(manifest.dir()); diff --git a/sway-core/src/language/call_path.rs b/sway-core/src/language/call_path.rs index a829442d4ef..18dcdcb869b 100644 --- a/sway-core/src/language/call_path.rs +++ b/sway-core/src/language/call_path.rs @@ -100,7 +100,7 @@ impl CallPath { /// /// Paths to _external_ libraries such `std::lib1::lib2::my_obj` are considered full already /// and are left unchanged since `std` is a root of the package `std`. - pub fn to_fullpath(&self, namespace: &mut Namespace) -> CallPath { + pub fn to_fullpath(&self, namespace: &Namespace) -> CallPath { if self.is_absolute { return self.clone(); } diff --git a/sway-core/src/language/ty/module.rs b/sway-core/src/language/ty/module.rs index 11eab930e30..d7012ca48f2 100644 --- a/sway-core/src/language/ty/module.rs +++ b/sway-core/src/language/ty/module.rs @@ -12,7 +12,7 @@ use crate::{ pub struct TyModule { pub span: Span, pub submodules: Vec<(ModName, TySubmodule)>, - pub namespace: namespace::Module, + pub namespace: namespace::Namespace, pub all_nodes: Vec, pub attributes: transform::AttributesMap, } diff --git a/sway-core/src/semantic_analysis/module.rs b/sway-core/src/semantic_analysis/module.rs index 9c323a2e947..a34fe2972f2 100644 --- a/sway-core/src/semantic_analysis/module.rs +++ b/sway-core/src/semantic_analysis/module.rs @@ -41,7 +41,7 @@ impl ty::TyModule { typed_nodes_res.map(|all_nodes| Self { span: span.clone(), submodules, - namespace: ctx.namespace.module().clone(), + namespace: ctx.namespace.clone(), all_nodes, attributes: attributes.clone(), }) diff --git a/sway-core/src/semantic_analysis/namespace/items.rs b/sway-core/src/semantic_analysis/namespace/items.rs index 0c213c6c7ce..a7c4d430872 100644 --- a/sway-core/src/semantic_analysis/namespace/items.rs +++ b/sway-core/src/semantic_analysis/namespace/items.rs @@ -2,7 +2,10 @@ use crate::{ decl_engine::*, engine_threading::Engines, error::*, - language::ty::{self, TyStorageDecl}, + language::{ + ty::{self, TyDecl, TyStorageDecl}, + CallPath, + }, namespace::*, type_system::*, }; @@ -167,6 +170,27 @@ impl Items { self.implemented_traits.get_items_for_type(engines, type_id) } + pub fn get_impl_spans_for_decl(&self, engines: Engines<'_>, ty_decl: &TyDecl) -> Vec { + ty_decl + .return_type(engines) + .value + .map(|type_id| { + self.implemented_traits + .get_impl_spans_for_type(engines, &type_id) + }) + .unwrap_or_default() + } + + pub fn get_impl_spans_for_type(&self, engines: Engines<'_>, type_id: &TypeId) -> Vec { + self.implemented_traits + .get_impl_spans_for_type(engines, type_id) + } + + pub fn get_impl_spans_for_trait_name(&self, trait_name: &CallPath) -> Vec { + self.implemented_traits + .get_impl_spans_for_trait_name(trait_name) + } + pub fn get_methods_for_type( &self, engines: Engines<'_>, diff --git a/sway-core/src/semantic_analysis/namespace/module.rs b/sway-core/src/semantic_analysis/namespace/module.rs index b34ec0a1649..74b2ca8a9d8 100644 --- a/sway-core/src/semantic_analysis/namespace/module.rs +++ b/sway-core/src/semantic_analysis/namespace/module.rs @@ -16,7 +16,7 @@ use super::{ items::{GlobImport, Items, SymbolMap}, root::Root, trait_map::TraitMap, - ModuleName, Path, + ModuleName, Path, PathBuf, }; use sway_ast::ItemConst; @@ -56,6 +56,11 @@ pub struct Module { /// Indicates whether the module is external to the current package. External modules are /// imported in the `Forc.toml` file. pub is_external: bool, + /// An absolute path from the `root` that represents the module location. + /// + /// When this is the root module, this is equal to `[]`. When this is a + /// submodule of the root called "foo", this would be equal to `[foo]`. + pub mod_path: PathBuf, } impl Default for Module { @@ -67,6 +72,7 @@ impl Default for Module { name: Default::default(), span: Default::default(), is_external: Default::default(), + mod_path: Default::default(), } } } diff --git a/sway-core/src/semantic_analysis/namespace/trait_map.rs b/sway-core/src/semantic_analysis/namespace/trait_map.rs index 24190ee32c6..1dc28ab43a8 100644 --- a/sway-core/src/semantic_analysis/namespace/trait_map.rs +++ b/sway-core/src/semantic_analysis/namespace/trait_map.rs @@ -71,10 +71,17 @@ impl OrdWithEngines for TraitKey { /// Map of name to [TyImplItem](ty::TyImplItem) type TraitItems = im::HashMap; +#[derive(Clone, Debug)] +struct TraitValue { + trait_items: TraitItems, + /// The span of the entire impl block. + impl_span: Span, +} + #[derive(Clone, Debug)] struct TraitEntry { key: TraitKey, - value: TraitItems, + value: TraitValue, } /// Map of trait name and type to [TraitItems]. @@ -144,7 +151,11 @@ impl TraitMap { name: map_trait_name, type_id: map_type_id, }, - value: map_trait_items, + value: + TraitValue { + trait_items: map_trait_items, + .. + }, } in self.trait_impls.iter() { let CallPath { @@ -233,7 +244,7 @@ impl TraitMap { }; // even if there is a conflicting definition, add the trait anyway - self.insert_inner(trait_name, type_id, trait_items, engines); + self.insert_inner(trait_name, impl_span.clone(), type_id, trait_items, engines); if errors.is_empty() { ok((), warnings, errors) @@ -245,6 +256,7 @@ impl TraitMap { fn insert_inner( &mut self, trait_name: TraitName, + impl_span: Span, type_id: TypeId, trait_methods: TraitItems, engines: Engines<'_>, @@ -253,10 +265,11 @@ impl TraitMap { name: trait_name, type_id, }; - let entry = TraitEntry { - key, - value: trait_methods, + let value = TraitValue { + trait_items: trait_methods, + impl_span, }; + let entry = TraitEntry { key, value }; let trait_impls: TraitImpls = vec![entry]; let trait_map = TraitMap { trait_impls }; @@ -362,7 +375,10 @@ impl TraitMap { .binary_search_by(|se| se.key.cmp(&oe.key, engines)); match pos { - Ok(pos) => self.trait_impls[pos].value.extend(oe.value.into_iter()), + Ok(pos) => self.trait_impls[pos] + .value + .trait_items + .extend(oe.value.trait_items.into_iter()), Err(pos) => self.trait_impls.insert(pos, oe), } } @@ -588,7 +604,11 @@ impl TraitMap { name: map_trait_name, type_id: map_type_id, }, - value: map_trait_items, + value: + TraitValue { + trait_items: map_trait_items, + impl_span, + }, } in self.trait_impls.iter() { for type_id in all_types.iter_mut() { @@ -596,6 +616,7 @@ impl TraitMap { if !type_info.can_change(decl_engine) && *type_id == *map_type_id { trait_map.insert_inner( map_trait_name.clone(), + impl_span.clone(), *type_id, map_trait_items.clone(), engines, @@ -631,7 +652,13 @@ impl TraitMap { } }) .collect(); - trait_map.insert_inner(map_trait_name.clone(), *type_id, trait_items, engines); + trait_map.insert_inner( + map_trait_name.clone(), + impl_span.clone(), + *type_id, + trait_items, + engines, + ); } } } @@ -663,13 +690,68 @@ impl TraitMap { } for entry in self.trait_impls.iter() { if are_equal_minus_dynamic_types(engines, type_id, entry.key.type_id) { - let mut trait_items = entry.value.values().cloned().collect::>(); + let mut trait_items = entry + .value + .trait_items + .values() + .cloned() + .collect::>(); items.append(&mut trait_items); } } items } + /// Find the spans of all impls for the given type. + /// + /// Notes: + /// - equivalency is defined (1) based on whether the types contains types + /// that are dynamic and can change and (2) whether the types hold + /// equivalency after (1) is fulfilled + /// - this method does not translate types from the found entries to the + /// `type_id` (like in `filter_by_type()`). This is because the only + /// entries that qualify as hits are equivalents of `type_id` + pub(crate) fn get_impl_spans_for_type( + &self, + engines: Engines<'_>, + type_id: &TypeId, + ) -> Vec { + let type_engine = engines.te(); + let mut spans = vec![]; + // small performance gain in bad case + if type_engine + .get(*type_id) + .eq(&TypeInfo::ErrorRecovery, engines) + { + return spans; + } + for entry in self.trait_impls.iter() { + if are_equal_minus_dynamic_types(engines, *type_id, entry.key.type_id) { + spans.push(entry.value.impl_span.clone()); + } + } + spans + } + + /// Find the entries in `self` with trait name `trait_name` and return the + /// spans of the impls. + pub(crate) fn get_impl_spans_for_trait_name(&self, trait_name: &CallPath) -> Vec { + self.trait_impls + .iter() + .filter_map(|entry| { + let map_trait_name = CallPath { + prefixes: entry.key.name.prefixes.clone(), + suffix: entry.key.name.suffix.name.clone(), + is_absolute: entry.key.name.is_absolute, + }; + if &map_trait_name == trait_name { + return Some(entry.value.impl_span.clone()); + } + None + }) + .collect() + } + /// Find the entries in `self` that are equivalent to `type_id` with trait /// name `trait_name`. /// @@ -704,7 +786,7 @@ impl TraitMap { if &map_trait_name == trait_name && are_equal_minus_dynamic_types(engines, type_id, e.key.type_id) { - let mut trait_items = e.value.values().cloned().collect::>(); + let mut trait_items = e.value.trait_items.values().cloned().collect::>(); items.append(&mut trait_items); } } diff --git a/sway-lsp/src/capabilities/hover/hover_link_contents.rs b/sway-lsp/src/capabilities/hover/hover_link_contents.rs index c67e112075d..4c619fa0a3d 100644 --- a/sway-lsp/src/capabilities/hover/hover_link_contents.rs +++ b/sway-lsp/src/capabilities/hover/hover_link_contents.rs @@ -3,7 +3,13 @@ use crate::{ utils::document::get_url_from_span, }; use std::sync::Arc; -use sway_core::{language::CallPath, Engines, TypeId, TypeInfo}; +use sway_core::{ + language::{ + ty::{TyDecl, TyTraitDecl}, + CallPath, + }, + Engines, TypeId, TypeInfo, +}; use sway_types::{Span, Spanned}; use tower_lsp::lsp_types::{Range, Url}; @@ -72,4 +78,43 @@ impl<'a> HoverLinkContents<'a> { }); }; } + + /// Adds all implementations of the given [TyTraitDecl] to the list of implementations. + pub fn add_implementations_for_trait(&mut self, trait_decl: &TyTraitDecl) { + if let Some(namespace) = self.session.namespace() { + let call_path = CallPath::from(trait_decl.name.clone()).to_fullpath(&namespace); + let impl_spans = namespace.get_impl_spans_for_trait_name(&call_path); + self.add_implementations(&trait_decl.span(), impl_spans); + } + } + + /// Adds implementations of the given type to the list of implementations using the [TyDecl]. + pub fn add_implementations_for_decl(&mut self, ty_decl: &TyDecl) { + if let Some(namespace) = self.session.namespace() { + let impl_spans = namespace.get_impl_spans_for_decl(self.engines, ty_decl); + self.add_implementations(&ty_decl.span(), impl_spans); + } + } + + /// Adds implementations of the given type to the list of implementations using the [TypeId]. + pub fn add_implementations_for_type(&mut self, decl_span: &Span, type_id: &TypeId) { + if let Some(namespace) = self.session.namespace() { + let impl_spans = namespace.get_impl_spans_for_type(self.engines, type_id); + self.add_implementations(decl_span, impl_spans); + } + } + + /// Adds implementations to the list of implementation spans, with the declaration span first. + /// Ensure that all paths are converted to workspace paths before adding them. + fn add_implementations(&mut self, decl_span: &Span, mut impl_spans: Vec) { + let mut all_spans = vec![decl_span.clone()]; + all_spans.append(&mut impl_spans); + all_spans.dedup(); + all_spans.iter().for_each(|span| { + let span_result = self.session.sync.temp_to_workspace_span(span); + if let Ok(span) = span_result { + self.implementations.push(span); + } + }); + } } diff --git a/sway-lsp/src/capabilities/hover/mod.rs b/sway-lsp/src/capabilities/hover/mod.rs index c121f0f6fc0..53802100106 100644 --- a/sway-lsp/src/capabilities/hover/mod.rs +++ b/sway-lsp/src/capabilities/hover/mod.rs @@ -160,6 +160,7 @@ fn hover_format( } ty::TyDecl::StructDecl(ty::StructDecl { decl_id, .. }) => { let struct_decl = decl_engine.get_struct(decl_id); + hover_link_contents.add_implementations_for_decl(decl); Some(format_visibility_hover( struct_decl.visibility, decl.friendly_type_name(), @@ -168,6 +169,7 @@ fn hover_format( } ty::TyDecl::TraitDecl(ty::TraitDecl { decl_id, .. }) => { let trait_decl = decl_engine.get_trait(decl_id); + hover_link_contents.add_implementations_for_trait(&trait_decl); Some(format_visibility_hover( trait_decl.visibility, decl.friendly_type_name(), @@ -176,6 +178,7 @@ fn hover_format( } ty::TyDecl::EnumDecl(ty::EnumDecl { decl_id, .. }) => { let enum_decl = decl_engine.get_enum(decl_id); + hover_link_contents.add_implementations_for_decl(decl); Some(format_visibility_hover( enum_decl.visibility, decl.friendly_type_name(), @@ -183,6 +186,7 @@ fn hover_format( )) } ty::TyDecl::AbiDecl(ty::AbiDecl { .. }) => { + hover_link_contents.add_implementations_for_decl(decl); Some(format!("{} {}", decl.friendly_type_name(), &token_name)) } _ => None, @@ -198,10 +202,16 @@ fn hover_format( ¶m.type_argument.type_id, )) } - TypedAstToken::TypedStructField(field) => Some(format_name_with_type( - field.name.as_str(), - &field.type_argument.type_id, - )), + TypedAstToken::TypedStructField(field) => { + hover_link_contents.add_implementations_for_type( + &field.type_argument.span(), + &field.type_argument.type_id, + ); + Some(format_name_with_type( + field.name.as_str(), + &field.type_argument.type_id, + )) + } TypedAstToken::TypedExpression(expr) => match expr.expression { ty::TyExpressionVariant::Literal { .. } => { Some(format!("{}", engines.help_out(expr.return_type))) diff --git a/sway-lsp/src/core/session.rs b/sway-lsp/src/core/session.rs index f9c45387374..7497b4eee6b 100644 --- a/sway-lsp/src/core/session.rs +++ b/sway-lsp/src/core/session.rs @@ -26,9 +26,9 @@ use sway_core::{ language::{ lexed::LexedProgram, parsed::{AstNode, ParseProgram}, - ty::{self}, + ty, }, - BuildTarget, CompileResult, Engines, TypeEngine, + BuildTarget, CompileResult, Engines, Namespace, TypeEngine, }; use sway_types::{Span, Spanned}; use sway_utils::helpers::get_sway_files; @@ -330,6 +330,13 @@ impl Session { None } + /// Returns the [Namespace] from the compiled program if it exists. + pub fn namespace(&self) -> Option { + let compiled_program = &*self.compiled_program.read(); + let program = compiled_program.typed.clone()?; + Some(program.root.namespace) + } + pub fn symbol_information(&self, url: &Url) -> Option> { let tokens = self.token_map.tokens_for_file(url); self.sync diff --git a/sway-lsp/src/core/sync.rs b/sway-lsp/src/core/sync.rs index 19b23c0a254..5f59288486a 100644 --- a/sway-lsp/src/core/sync.rs +++ b/sway-lsp/src/core/sync.rs @@ -1,6 +1,6 @@ use crate::{ error::{DirectoryError, DocumentError, LanguageServerError}, - utils::document::get_url_from_path, + utils::document::{get_path_from_url, get_url_from_path, get_url_from_span}, }; use dashmap::DashMap; use forc_pkg::{manifest::Dependency, PackageManifestFile}; @@ -14,6 +14,7 @@ use std::{ path::{Path, PathBuf}, sync::Arc, }; +use sway_types::Span; use tempfile::Builder; use tower_lsp::lsp_types::Url; @@ -113,6 +114,30 @@ impl SyncWorkspace { self.convert_url(uri, self.manifest_dir()?, self.temp_dir()?) } + /// If it is a path to a temp directory, convert the path in the [Span] to the same file in the user's + /// workspace. Otherwise, return the span as-is. + pub(crate) fn temp_to_workspace_span(&self, span: &Span) -> Result { + let url = get_url_from_span(span)?; + if self.is_path_in_temp_workspace(&url) { + let converted_url = self.convert_url(&url, self.manifest_dir()?, self.temp_dir()?)?; + let converted_path = get_path_from_url(&converted_url)?; + let converted_span = Span::new( + span.src().clone(), + span.start(), + span.end(), + Some(converted_path.clone().into()), + ); + match converted_span { + Some(span) => Ok(span), + None => Err(DirectoryError::SpanFromPathFailed { + path: converted_path.to_string_lossy().to_string(), + }), + } + } else { + Ok(span.clone()) + } + } + /// If path is part of the users workspace, then convert URL from temp to workspace dir. /// Otherwise, pass through if it points to a dependency path pub(crate) fn to_workspace_url(&self, url: Url) -> Option { diff --git a/sway-lsp/src/core/token_map.rs b/sway-lsp/src/core/token_map.rs index 7fdbe993c25..874d096fc31 100644 --- a/sway-lsp/src/core/token_map.rs +++ b/sway-lsp/src/core/token_map.rs @@ -1,10 +1,6 @@ use crate::core::token::{self, Token, TypedAstToken}; use dashmap::DashMap; -use sway_core::{ - language::ty::{self}, - type_system::TypeId, - Engines, -}; +use sway_core::{language::ty, type_system::TypeId, Engines}; use sway_types::{Ident, Span, Spanned}; use tower_lsp::lsp_types::{Position, Url}; diff --git a/sway-lsp/src/utils/document.rs b/sway-lsp/src/utils/document.rs index 77f265c5042..ea33979c1a1 100644 --- a/sway-lsp/src/utils/document.rs +++ b/sway-lsp/src/utils/document.rs @@ -10,6 +10,14 @@ pub fn get_url_from_path(path: &PathBuf) -> Result { }) } +/// Create a [PathBuf] from a [Url]. +pub fn get_path_from_url(url: &Url) -> Result { + url.to_file_path() + .map_err(|_| DirectoryError::PathFromUrlFailed { + url: url.to_string(), + }) +} + /// Create a [Url] from a [Span]. pub fn get_url_from_span(span: &Span) -> Result { if let Some(path) = span.path() { diff --git a/sway-lsp/tests/lib.rs b/sway-lsp/tests/lib.rs index a2706251e9e..a9414ebbe78 100644 --- a/sway-lsp/tests/lib.rs +++ b/sway-lsp/tests/lib.rs @@ -1539,7 +1539,7 @@ async fn hover_docs_for_self_keywords() { let _ = lsp::hover_request(&mut service, &hover, &mut i).await; hover.req_char = 24; - hover.documentation = vec!["```sway\nstruct MyStruct\n```\n---"]; + hover.documentation = vec!["```sway\nstruct MyStruct\n```\n---\n\n---\n[2 implementations](command:sway.peekLocations?%5B%7B%22locations%22%3A%5B%7B%22range%22%3A%7B%22end%22%3A%7B%22character%22%3A1%2C%22line%22%3A4%7D%2C%22start%22%3A%7B%22character%22%3A0%2C%22line%22%3A2%7D%7D%2C%22uri%22%3A%22file","sway%2Fsway-lsp%2Ftests%2Ffixtures%2Fcompletion%2Fsrc%2Fmain.sw%22%7D%2C%7B%22range%22%3A%7B%22end%22%3A%7B%22character%22%3A1%2C%22line%22%3A14%7D%2C%22start%22%3A%7B%22character%22%3A0%2C%22line%22%3A6%7D%7D%2C%22uri%22%3A%22file","sway%2Fsway-lsp%2Ftests%2Ffixtures%2Fcompletion%2Fsrc%2Fmain.sw%22%7D%5D%7D%5D \"Go to implementations\")"]; let _ = lsp::hover_request(&mut service, &hover, &mut i).await; }