Skip to content

Commit

Permalink
Goto links for LSP hover (FuelLabs#4539)
Browse files Browse the repository at this point in the history
## Description

This is a subset of https://github.com/FuelLabs/sway/pull/4532/files.
I'm trying to narrow down why some of the e2e tests are failing. This PR
contains the changes for the "Go to type" links only, excluding the
changes in sway-core that were needed for "Goto implementations" to
work.

Closes FuelLabs#2852

<img width="673" alt="image"
src="https://user-images.githubusercontent.com/47993817/236371214-0b82b509-9a59-4575-b1d1-5acd505da255.png">

## 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.
  • Loading branch information
sdankel authored May 6, 2023
1 parent 97f3c5f commit 85040f5
Show file tree
Hide file tree
Showing 15 changed files with 275 additions and 55 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions sway-lsp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ tokio = { version = "1.3", features = ["io-std", "io-util", "macros", "net", "rt
toml_edit = "0.19"
tower-lsp = { version = "0.18", features = ["proposed"] }
tracing = "0.1"
urlencoding = "2.1.2"

[dev-dependencies]
assert-json-diff = "2.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ impl<'a> CodeAction<'a, TyStructDecl> for StructNewCodeAction<'a> {
fn new(ctx: CodeActionContext<'a>, decl: &'a TyStructDecl) -> Self {
// Before the other functions are called, we need to determine if the new function
// should be generated in a new impl block, an existing impl block, or not at all.
// First, find the first impl block for this struct if it exists.
// Find the first impl block for this struct if it exists.
let existing_impl_decl = ctx
.tokens
.iter()
Expand All @@ -32,7 +32,6 @@ impl<'a> CodeAction<'a, TyStructDecl> for StructNewCodeAction<'a> {
None
}
});

Self {
decl,
uri: ctx.uri,
Expand Down
75 changes: 75 additions & 0 deletions sway-lsp/src/capabilities/hover/hover_link_contents.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
use crate::{
core::{session::Session, token::get_range_from_span},
utils::document::get_url_from_span,
};
use std::sync::Arc;
use sway_core::{language::CallPath, Engines, TypeId, TypeInfo};

use sway_types::{Span, Spanned};
use tower_lsp::lsp_types::{Range, Url};

#[derive(Debug, Clone)]
pub struct RelatedType {
pub name: String,
pub uri: Url,
pub range: Range,
pub callpath: CallPath,
}

#[derive(Debug, Clone)]
pub struct HoverLinkContents<'a> {
pub related_types: Vec<RelatedType>,
pub implementations: Vec<Span>,
session: Arc<Session>,
engines: Engines<'a>,
}

impl<'a> HoverLinkContents<'a> {
pub fn new(session: Arc<Session>, engines: Engines<'a>) -> Self {
Self {
related_types: Vec::new(),
implementations: Vec::new(),
session,
engines,
}
}

/// Adds the given type and any related type parameters to the list of related types.
pub fn add_related_types(&mut self, type_id: &TypeId) {
let type_info = self.engines.te().get(*type_id);
match type_info {
TypeInfo::Enum(decl_ref) => {
let decl = self.engines.de().get_enum(&decl_ref);
self.add_related_type(decl_ref.name().to_string(), &decl.span(), decl.call_path);
decl.type_parameters
.iter()
.for_each(|type_param| self.add_related_types(&type_param.type_id));
}
TypeInfo::Struct(decl_ref) => {
let decl = self.engines.de().get_struct(&decl_ref);
self.add_related_type(decl_ref.name().to_string(), &decl.span(), decl.call_path);
decl.type_parameters
.iter()
.for_each(|type_param| self.add_related_types(&type_param.type_id));
}
_ => {}
}
}

/// Adds a single type to the list of related types.
fn add_related_type(&mut self, name: String, span: &Span, callpath: CallPath) {
if let Ok(mut uri) = get_url_from_span(span) {
let converted_url = self.session.sync.temp_to_workspace_url(&uri);
if let Ok(url) = converted_url {
uri = url;
}
let range = get_range_from_span(span);
self.related_types.push(RelatedType {
name,
uri,
range,
callpath,
});
};
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
pub(crate) mod hover_link_contents;

use crate::{
core::{
session::Session,
Expand All @@ -9,12 +11,18 @@ use crate::{
};
use std::sync::Arc;
use sway_core::{
language::{ty, Visibility},
language::{
ty::{self},
Visibility,
},
Engines, TypeId,
};

use sway_types::{Ident, Span, Spanned};
use tower_lsp::lsp_types::{self, Position, Url};

use self::hover_link_contents::HoverLinkContents;

/// Extracts the hover information for a token at the current position.
pub fn hover_data(
session: Arc<Session>,
Expand Down Expand Up @@ -59,7 +67,7 @@ pub fn hover_data(
None => (ident, token),
};

let contents = hover_format(engines, &decl_token, &decl_ident);
let contents = hover_format(session.clone(), engines, &decl_token, &decl_ident);
Some(lsp_types::Hover {
contents,
range: Some(range),
Expand Down Expand Up @@ -116,7 +124,12 @@ fn markup_content(markup: Markup) -> lsp_types::MarkupContent {
lsp_types::MarkupContent { kind, value }
}

fn hover_format(engines: Engines<'_>, token: &Token, ident: &Ident) -> lsp_types::HoverContents {
fn hover_format(
session: Arc<Session>,
engines: Engines<'_>,
token: &Token,
ident: &Ident,
) -> lsp_types::HoverContents {
let decl_engine = engines.de();

let token_name: String = ident.as_str().into();
Expand All @@ -127,14 +140,18 @@ fn hover_format(engines: Engines<'_>, token: &Token, ident: &Ident) -> lsp_types
format!("{name}: {type_name}")
};

let value = token
// Used to collect all the information we need to generate links for the hover component.
let mut hover_link_contents = HoverLinkContents::new(session, engines);

let sway_block = token
.typed
.as_ref()
.and_then(|typed_token| match typed_token {
TypedAstToken::TypedDeclaration(decl) => match decl {
ty::TyDecl::VariableDecl(var_decl) => {
let type_name =
format!("{}", engines.help_out(var_decl.type_ascription.type_id));
hover_link_contents.add_related_types(&var_decl.type_ascription.type_id);
Some(format_variable_hover(
var_decl.mutability.is_mutable(),
&type_name,
Expand Down Expand Up @@ -165,18 +182,22 @@ fn hover_format(engines: Engines<'_>, token: &Token, ident: &Ident) -> lsp_types
&token_name,
))
}
ty::TyDecl::AbiDecl { .. } => {
ty::TyDecl::AbiDecl(ty::AbiDecl { .. }) => {
Some(format!("{} {}", decl.friendly_type_name(), &token_name))
}
_ => None,
},
TypedAstToken::TypedFunctionDeclaration(func) => {
hover_link_contents.add_related_types(&func.return_type.type_id);
Some(extract_fn_signature(&func.span()))
}
TypedAstToken::TypedFunctionParameter(param) => Some(format_name_with_type(
param.name.as_str(),
&param.type_argument.type_id,
)),
TypedAstToken::TypedFunctionParameter(param) => {
hover_link_contents.add_related_types(&param.type_argument.type_id);
Some(format_name_with_type(
param.name.as_str(),
&param.type_argument.type_id,
))
}
TypedAstToken::TypedStructField(field) => Some(format_name_with_type(
field.name.as_str(),
&field.type_argument.type_id,
Expand All @@ -190,7 +211,13 @@ fn hover_format(engines: Engines<'_>, token: &Token, ident: &Ident) -> lsp_types
_ => None,
});

let content = Markup::new().maybe_add_sway_block(value).text(&doc_comment);
let content = Markup::new()
.maybe_add_sway_block(sway_block)
.text(&doc_comment)
.maybe_add_links(
hover_link_contents.related_types,
hover_link_contents.implementations,
);

lsp_types::HoverContents::Markup(markup_content(content))
}
3 changes: 2 additions & 1 deletion sway-lsp/src/capabilities/rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
token_map::TokenMapExt,
},
error::{LanguageServerError, RenameError},
utils::document::get_url_from_path,
};
use std::{collections::HashMap, sync::Arc};
use sway_core::{language::ty, Engines};
Expand Down Expand Up @@ -77,7 +78,7 @@ pub fn rename(
range.start.character -= RAW_IDENTIFIER.len() as u32;
}
if let Some(path) = ident.span().path() {
let url = session.sync.url_from_path(path).ok()?;
let url = get_url_from_path(path).ok()?;
if let Some(url) = session.sync.to_workspace_url(url) {
let edit = TextEdit::new(range, new_name.clone());
return Some((url, vec![edit]));
Expand Down
2 changes: 1 addition & 1 deletion sway-lsp/src/core/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use sway_core::{
language::{
lexed::LexedProgram,
parsed::{AstNode, ParseProgram},
ty,
ty::{self},
},
BuildTarget, CompileResult, Engines, TypeEngine,
};
Expand Down
20 changes: 8 additions & 12 deletions sway-lsp/src/core/sync.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use crate::error::{DirectoryError, DocumentError, LanguageServerError};
use crate::{
error::{DirectoryError, DocumentError, LanguageServerError},
utils::document::get_url_from_path,
};
use dashmap::DashMap;
use forc_pkg::{manifest::Dependency, PackageManifestFile};
use notify::RecursiveMode;
Expand Down Expand Up @@ -96,7 +99,7 @@ impl SyncWorkspace {

/// Check if the current path is part of the users workspace.
/// Returns false if the path is from a dependancy
pub(crate) fn is_path_in_workspace(&self, uri: &Url) -> bool {
pub(crate) fn is_path_in_temp_workspace(&self, uri: &Url) -> bool {
uri.as_ref().contains(SyncWorkspace::LSP_TEMP_PREFIX)
}

Expand All @@ -105,15 +108,15 @@ impl SyncWorkspace {
self.convert_url(uri, self.temp_dir()?, self.manifest_dir()?)
}

/// Convert the Url path from the temp folder to point to the same file in the users workspace
/// Convert the [Url] path from the temp folder to point to the same file in the users workspace.
pub(crate) fn temp_to_workspace_url(&self, uri: &Url) -> Result<Url, DirectoryError> {
self.convert_url(uri, self.manifest_dir()?, self.temp_dir()?)
}

/// 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<Url> {
if self.is_path_in_workspace(&url) {
if self.is_path_in_temp_workspace(&url) {
Some(self.temp_to_workspace_url(&url).ok()?)
} else {
Some(url)
Expand Down Expand Up @@ -192,20 +195,13 @@ impl SyncWorkspace {
.ok_or(DirectoryError::TempDirNotFound)
}

/// Create a URL from a [PathBuf].
pub fn url_from_path(&self, path: &PathBuf) -> Result<Url, DirectoryError> {
Url::from_file_path(path).map_err(|_| DirectoryError::UrlFromPathFailed {
path: path.to_string_lossy().to_string(),
})
}

fn convert_url(&self, uri: &Url, from: PathBuf, to: PathBuf) -> Result<Url, DirectoryError> {
let path = from.join(
PathBuf::from(uri.path())
.strip_prefix(to)
.map_err(DirectoryError::StripPrefixError)?,
);
self.url_from_path(&path)
get_url_from_path(&path)
}
}

Expand Down
6 changes: 5 additions & 1 deletion sway-lsp/src/core/token_map.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use crate::core::token::{self, Token, TypedAstToken};
use dashmap::DashMap;
use sway_core::{language::ty, type_system::TypeId, Engines};
use sway_core::{
language::ty::{self},
type_system::TypeId,
Engines,
};
use sway_types::{Ident, Span, Spanned};
use tower_lsp::lsp_types::{Position, Url};

Expand Down
6 changes: 6 additions & 0 deletions sway-lsp/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ pub enum DirectoryError {
StripPrefixError(std::path::StripPrefixError),
#[error("Unable to create Url from path {:?}", path)]
UrlFromPathFailed { path: String },
#[error("Unable to create Url from span {:?}", span)]
UrlFromSpanFailed { span: String },
#[error("Unable to create path from Url {:?}", url)]
PathFromUrlFailed { url: String },
#[error("Unable to create span from path {:?}", path)]
SpanFromPathFailed { path: String },
}

#[derive(Debug, Error, PartialEq, Eq)]
Expand Down
22 changes: 22 additions & 0 deletions sway-lsp/src/utils/document.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use crate::error::DirectoryError;
use std::path::PathBuf;
use sway_types::Span;
use tower_lsp::lsp_types::Url;

/// Create a [Url] from a [PathBuf].
pub fn get_url_from_path(path: &PathBuf) -> Result<Url, DirectoryError> {
Url::from_file_path(path).map_err(|_| DirectoryError::UrlFromPathFailed {
path: path.to_string_lossy().to_string(),
})
}

/// Create a [Url] from a [Span].
pub fn get_url_from_span(span: &Span) -> Result<Url, DirectoryError> {
if let Some(path) = span.path() {
get_url_from_path(path)
} else {
Err(DirectoryError::UrlFromSpanFailed {
span: span.as_str().to_string(),
})
}
}
Loading

0 comments on commit 85040f5

Please sign in to comment.