From b6d643866f5abab3b5f7b4084010de76c107f9cb Mon Sep 17 00:00:00 2001 From: Sophie Date: Mon, 24 Jun 2024 22:26:50 -0700 Subject: [PATCH 1/3] fix: only support hover links for vscode --- sway-lsp/src/capabilities/hover/mod.rs | 19 +++++++++++---- sway-lsp/src/config.rs | 11 +++++++++ sway-lsp/src/handlers/request.rs | 1 + sway-lsp/src/utils/markup.rs | 11 ++++++--- sway-lsp/tests/lib.rs | 32 +++++++++++++++++++++----- 5 files changed, 60 insertions(+), 14 deletions(-) diff --git a/sway-lsp/src/capabilities/hover/mod.rs b/sway-lsp/src/capabilities/hover/mod.rs index 26c7a5e0172..24491106e98 100644 --- a/sway-lsp/src/capabilities/hover/mod.rs +++ b/sway-lsp/src/capabilities/hover/mod.rs @@ -1,5 +1,7 @@ pub(crate) mod hover_link_contents; +use self::hover_link_contents::HoverLinkContents; +use crate::config::LspClientConfig; use crate::{ core::{ session::Session, @@ -9,23 +11,21 @@ use crate::{ attributes::doc_comment_attributes, keyword_docs::KeywordDocs, markdown, markup::Markup, }, }; +use lsp_types::{self, Position, Url}; use std::sync::Arc; use sway_core::{ language::{ty, Visibility}, Engines, TypeId, }; - -use lsp_types::{self, Position, Url}; use sway_types::{Span, Spanned}; -use self::hover_link_contents::HoverLinkContents; - /// Extracts the hover information for a token at the current position. pub fn hover_data( session: Arc, keyword_docs: &KeywordDocs, url: &Url, position: Position, + client_config: LspClientConfig, ) -> Option { let t = session.token_map().token_at_position(url, position)?; let (ident, token) = t.pair(); @@ -60,11 +60,18 @@ pub fn hover_data( &session.engines.read(), decl_token, &decl_ident.name, + client_config.clone(), ) } // The `TypeInfo` of the token does not contain an `Ident`. In this case, // we use the `Ident` of the token itself. - None => hover_format(session.clone(), &session.engines.read(), token, &ident.name), + None => hover_format( + session.clone(), + &session.engines.read(), + token, + &ident.name, + client_config.clone(), + ), }; Some(lsp_types::Hover { @@ -122,6 +129,7 @@ fn hover_format( engines: &Engines, token: &Token, ident_name: &str, + client_config: LspClientConfig, ) -> lsp_types::HoverContents { let decl_engine = engines.de(); let doc_comment = format_doc_attributes(engines, token); @@ -219,6 +227,7 @@ fn hover_format( engines.se(), &hover_link_contents.related_types, &hover_link_contents.implementations, + client_config, ); lsp_types::HoverContents::Markup(markup_content(&content)) diff --git a/sway-lsp/src/config.rs b/sway-lsp/src/config.rs index 0a3a5597462..0f5bb5039b2 100644 --- a/sway-lsp/src/config.rs +++ b/sway-lsp/src/config.rs @@ -4,6 +4,8 @@ use tracing::metadata::LevelFilter; #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Default)] #[serde(rename_all = "camelCase")] pub struct Config { + #[serde(default)] + pub client: LspClientConfig, #[serde(default)] pub debug: DebugConfig, #[serde(default)] @@ -20,6 +22,15 @@ pub struct Config { pub garbage_collection: GarbageCollectionConfig, } +#[derive(Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum LspClientConfig { + VsCode, + #[serde(other)] + #[default] + Other, +} + #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Default)] struct TraceConfig {} diff --git a/sway-lsp/src/handlers/request.rs b/sway-lsp/src/handlers/request.rs index d6d53a54a26..dd94d823959 100644 --- a/sway-lsp/src/handlers/request.rs +++ b/sway-lsp/src/handlers/request.rs @@ -132,6 +132,7 @@ pub async fn handle_hover( &state.keyword_docs, &uri, position, + state.config.read().client.clone(), )) } Err(err) => { diff --git a/sway-lsp/src/utils/markup.rs b/sway-lsp/src/utils/markup.rs index 9510674f435..b434f4f1f48 100644 --- a/sway-lsp/src/utils/markup.rs +++ b/sway-lsp/src/utils/markup.rs @@ -4,8 +4,8 @@ //! markdown for this purpose. //! Modified from rust-analyzer. use crate::{ - capabilities::hover::hover_link_contents::RelatedType, core::token::get_range_from_span, - utils::document::get_url_from_span, + capabilities::hover::hover_link_contents::RelatedType, config::LspClientConfig, + core::token::get_range_from_span, utils::document::get_url_from_span, }; use serde_json::{json, Value}; use std::fmt::{self}; @@ -56,13 +56,18 @@ impl Markup { } /// Adds go-to links if there are any related types, a link to view implementations if there are any, - /// or nothing if there are no related types or implementations. + /// or nothing if there are no related types or implementations. Only adds links for VSCode clients. pub fn maybe_add_links( self, source_engine: &SourceEngine, related_types: &[RelatedType], implementations: &[Span], + client_config: LspClientConfig, ) -> Self { + if client_config != LspClientConfig::VsCode { + return self; + } + if related_types.is_empty() { let locations = implementations .iter() diff --git a/sway-lsp/tests/lib.rs b/sway-lsp/tests/lib.rs index 77727dbeb1a..f26341b654f 100644 --- a/sway-lsp/tests/lib.rs +++ b/sway-lsp/tests/lib.rs @@ -4,6 +4,7 @@ use crate::integration::{code_actions, lsp}; use lsp_types::*; use std::{fs, path::PathBuf}; use sway_lsp::{ + config::LspClientConfig, handlers::{notification, request}, server_state::ServerState, }; @@ -1779,17 +1780,18 @@ fn hover_docs_with_code_examples() { } #[test] -fn hover_docs_for_self_keywords() { +fn hover_docs_for_self_keywords_vscode() { run_async!({ let server = ServerState::default(); + server.config.write().client = LspClientConfig::VsCode; let uri = open(&server, test_fixtures_dir().join("completion/src/main.sw")).await; let mut hover = HoverDocumentation { - req_uri: &uri, - req_line: 11, - req_char: 13, - documentation: vec!["\n```sway\nself\n```\n\n---\n\n The receiver of a method, or the current module.\n\n `self` is used in two situations: referencing the current module and marking\n the receiver of a method.\n\n In paths, `self` can be used to refer to the current module, either in a\n [`use`] statement or in a path to access an element:\n\n ```sway\n use std::contract_id::{self, ContractId};\n ```\n\n Is functionally the same as:\n\n ```sway\n use std::contract_id;\n use std::contract_id::ContractId;\n ```\n\n `self` as the current receiver for a method allows to omit the parameter\n type most of the time. With the exception of this particularity, `self` is\n used much like any other parameter:\n\n ```sway\n struct Foo(u32);\n\n impl Foo {\n // No `self`.\n fn new() -> Self {\n Self(0)\n }\n\n // Borrowing `self`.\n fn value(&self) -> u32 {\n self.0\n }\n\n // Updating `self` mutably.\n fn clear(ref mut self) {\n self.0 = 0\n }\n }\n ```"], - }; + req_uri: &uri, + req_line: 11, + req_char: 13, + documentation: vec!["\n```sway\nself\n```\n\n---\n\n The receiver of a method, or the current module.\n\n `self` is used in two situations: referencing the current module and marking\n the receiver of a method.\n\n In paths, `self` can be used to refer to the current module, either in a\n [`use`] statement or in a path to access an element:\n\n ```sway\n use std::contract_id::{self, ContractId};\n ```\n\n Is functionally the same as:\n\n ```sway\n use std::contract_id;\n use std::contract_id::ContractId;\n ```\n\n `self` as the current receiver for a method allows to omit the parameter\n type most of the time. With the exception of this particularity, `self` is\n used much like any other parameter:\n\n ```sway\n struct Foo(u32);\n\n impl Foo {\n // No `self`.\n fn new() -> Self {\n Self(0)\n }\n\n // Borrowing `self`.\n fn value(&self) -> u32 {\n self.0\n }\n\n // Updating `self` mutably.\n fn clear(ref mut self) {\n self.0 = 0\n }\n }\n ```"], + }; lsp::hover_request(&server, &hover).await; hover.req_char = 24; @@ -1799,6 +1801,24 @@ fn hover_docs_for_self_keywords() { }); } +#[test] +fn hover_docs_for_self_keywords() { + run_async!({ + let server = ServerState::default(); + let uri = open(&server, test_fixtures_dir().join("completion/src/main.sw")).await; + + let hover = HoverDocumentation { + req_uri: &uri, + req_line: 11, + req_char: 13, + documentation: vec!["\n```sway\nself\n```\n\n---\n\n The receiver of a method, or the current module.\n\n `self` is used in two situations: referencing the current module and marking\n the receiver of a method.\n\n In paths, `self` can be used to refer to the current module, either in a\n [`use`] statement or in a path to access an element:\n\n ```sway\n use std::contract_id::{self, ContractId};\n ```\n\n Is functionally the same as:\n\n ```sway\n use std::contract_id;\n use std::contract_id::ContractId;\n ```\n\n `self` as the current receiver for a method allows to omit the parameter\n type most of the time. With the exception of this particularity, `self` is\n used much like any other parameter:\n\n ```sway\n struct Foo(u32);\n\n impl Foo {\n // No `self`.\n fn new() -> Self {\n Self(0)\n }\n\n // Borrowing `self`.\n fn value(&self) -> u32 {\n self.0\n }\n\n // Updating `self` mutably.\n fn clear(ref mut self) {\n self.0 = 0\n }\n }\n ```"], + }; + + lsp::hover_request(&server, &hover).await; + let _ = server.shutdown_server(); + }); +} + #[test] fn hover_docs_for_boolean_keywords() { run_async!({ From c485b5be52b9ba05797c390277c4481e5470410b Mon Sep 17 00:00:00 2001 From: Sophie Date: Mon, 24 Jun 2024 22:32:25 -0700 Subject: [PATCH 2/3] update benchmark --- sway-lsp/benches/lsp_benchmarks/requests.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/sway-lsp/benches/lsp_benchmarks/requests.rs b/sway-lsp/benches/lsp_benchmarks/requests.rs index cc6fae96f58..d46a92b38b4 100644 --- a/sway-lsp/benches/lsp_benchmarks/requests.rs +++ b/sway-lsp/benches/lsp_benchmarks/requests.rs @@ -3,7 +3,9 @@ use lsp_types::{ CompletionResponse, DocumentSymbolResponse, Position, Range, TextDocumentContentChangeEvent, TextDocumentIdentifier, }; -use sway_lsp::{capabilities, lsp_ext::OnEnterParams, utils::keyword_docs::KeywordDocs}; +use sway_lsp::{ + capabilities, config::LspClientConfig, lsp_ext::OnEnterParams, utils::keyword_docs::KeywordDocs, +}; use tokio::runtime::Runtime; fn benchmarks(c: &mut Criterion) { @@ -37,7 +39,15 @@ fn benchmarks(c: &mut Criterion) { }); c.bench_function("hover", |b| { - b.iter(|| capabilities::hover::hover_data(session.clone(), &keyword_docs, &uri, position)) + b.iter(|| { + capabilities::hover::hover_data( + session.clone(), + &keyword_docs, + &uri, + position, + LspClientConfig::default(), + ) + }) }); c.bench_function("highlight", |b| { From a69dd9027ed491accb043feee6de0581f426cc2d Mon Sep 17 00:00:00 2001 From: Sophie Date: Tue, 25 Jun 2024 11:16:52 -0700 Subject: [PATCH 3/3] feedback --- sway-lsp/benches/lsp_benchmarks/requests.rs | 4 ++-- sway-lsp/src/capabilities/hover/mod.rs | 6 +++--- sway-lsp/src/config.rs | 4 ++-- sway-lsp/src/utils/markup.rs | 6 +++--- sway-lsp/tests/lib.rs | 7 ++++--- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/sway-lsp/benches/lsp_benchmarks/requests.rs b/sway-lsp/benches/lsp_benchmarks/requests.rs index d46a92b38b4..82a80a2a366 100644 --- a/sway-lsp/benches/lsp_benchmarks/requests.rs +++ b/sway-lsp/benches/lsp_benchmarks/requests.rs @@ -4,7 +4,7 @@ use lsp_types::{ TextDocumentIdentifier, }; use sway_lsp::{ - capabilities, config::LspClientConfig, lsp_ext::OnEnterParams, utils::keyword_docs::KeywordDocs, + capabilities, config::LspClient, lsp_ext::OnEnterParams, utils::keyword_docs::KeywordDocs, }; use tokio::runtime::Runtime; @@ -45,7 +45,7 @@ fn benchmarks(c: &mut Criterion) { &keyword_docs, &uri, position, - LspClientConfig::default(), + LspClient::default(), ) }) }); diff --git a/sway-lsp/src/capabilities/hover/mod.rs b/sway-lsp/src/capabilities/hover/mod.rs index 24491106e98..97ef886daf7 100644 --- a/sway-lsp/src/capabilities/hover/mod.rs +++ b/sway-lsp/src/capabilities/hover/mod.rs @@ -1,7 +1,7 @@ pub(crate) mod hover_link_contents; use self::hover_link_contents::HoverLinkContents; -use crate::config::LspClientConfig; +use crate::config::LspClient; use crate::{ core::{ session::Session, @@ -25,7 +25,7 @@ pub fn hover_data( keyword_docs: &KeywordDocs, url: &Url, position: Position, - client_config: LspClientConfig, + client_config: LspClient, ) -> Option { let t = session.token_map().token_at_position(url, position)?; let (ident, token) = t.pair(); @@ -129,7 +129,7 @@ fn hover_format( engines: &Engines, token: &Token, ident_name: &str, - client_config: LspClientConfig, + client_config: LspClient, ) -> lsp_types::HoverContents { let decl_engine = engines.de(); let doc_comment = format_doc_attributes(engines, token); diff --git a/sway-lsp/src/config.rs b/sway-lsp/src/config.rs index 0f5bb5039b2..7a440afeb57 100644 --- a/sway-lsp/src/config.rs +++ b/sway-lsp/src/config.rs @@ -5,7 +5,7 @@ use tracing::metadata::LevelFilter; #[serde(rename_all = "camelCase")] pub struct Config { #[serde(default)] - pub client: LspClientConfig, + pub client: LspClient, #[serde(default)] pub debug: DebugConfig, #[serde(default)] @@ -24,7 +24,7 @@ pub struct Config { #[derive(Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] -pub enum LspClientConfig { +pub enum LspClient { VsCode, #[serde(other)] #[default] diff --git a/sway-lsp/src/utils/markup.rs b/sway-lsp/src/utils/markup.rs index b434f4f1f48..0d2d641e4ea 100644 --- a/sway-lsp/src/utils/markup.rs +++ b/sway-lsp/src/utils/markup.rs @@ -4,7 +4,7 @@ //! markdown for this purpose. //! Modified from rust-analyzer. use crate::{ - capabilities::hover::hover_link_contents::RelatedType, config::LspClientConfig, + capabilities::hover::hover_link_contents::RelatedType, config::LspClient, core::token::get_range_from_span, utils::document::get_url_from_span, }; use serde_json::{json, Value}; @@ -62,9 +62,9 @@ impl Markup { source_engine: &SourceEngine, related_types: &[RelatedType], implementations: &[Span], - client_config: LspClientConfig, + client_config: LspClient, ) -> Self { - if client_config != LspClientConfig::VsCode { + if client_config != LspClient::VsCode { return self; } diff --git a/sway-lsp/tests/lib.rs b/sway-lsp/tests/lib.rs index f26341b654f..d4187a182f5 100644 --- a/sway-lsp/tests/lib.rs +++ b/sway-lsp/tests/lib.rs @@ -4,7 +4,7 @@ use crate::integration::{code_actions, lsp}; use lsp_types::*; use std::{fs, path::PathBuf}; use sway_lsp::{ - config::LspClientConfig, + config::LspClient, handlers::{notification, request}, server_state::ServerState, }; @@ -1636,9 +1636,10 @@ fn hover_docs_for_consts() { } #[test] -fn hover_docs_for_functions() { +fn hover_docs_for_functions_vscode() { run_async!({ let server = ServerState::default(); + server.config.write().client = LspClient::VsCode; let uri = open( &server, test_fixtures_dir().join("tokens/functions/src/main.sw"), @@ -1783,7 +1784,7 @@ fn hover_docs_with_code_examples() { fn hover_docs_for_self_keywords_vscode() { run_async!({ let server = ServerState::default(); - server.config.write().client = LspClientConfig::VsCode; + server.config.write().client = LspClient::VsCode; let uri = open(&server, test_fixtures_dir().join("completion/src/main.sw")).await; let mut hover = HoverDocumentation {