Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(lsp): refactor completions and add tests #9789

Merged
merged 3 commits into from
Mar 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions cli/lsp/capabilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,12 @@ pub fn server_capabilities(
)),
hover_provider: Some(HoverProviderCapability::Simple(true)),
completion_provider: Some(CompletionOptions {
all_commit_characters: None,
all_commit_characters: Some(vec![
".".to_string(),
",".to_string(),
";".to_string(),
"(".to_string(),
]),
trigger_characters: Some(vec![
".".to_string(),
"\"".to_string(),
Expand All @@ -66,7 +71,7 @@ pub fn server_capabilities(
"<".to_string(),
"#".to_string(),
]),
resolve_provider: None,
resolve_provider: Some(true),
work_done_progress_options: WorkDoneProgressOptions {
work_done_progress: None,
},
Expand All @@ -77,7 +82,7 @@ pub fn server_capabilities(
"(".to_string(),
"<".to_string(),
]),
retrigger_characters: None,
retrigger_characters: Some(vec![")".to_string()]),
work_done_progress_options: WorkDoneProgressOptions {
work_done_progress: None,
},
Expand Down
72 changes: 40 additions & 32 deletions cli/lsp/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub struct ClientCapabilities {
pub workspace_did_change_watched_files: bool,
}

#[derive(Debug, Default, Clone, Deserialize)]
#[derive(Debug, Clone, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct CodeLensSettings {
/// Flag for providing implementation code lenses.
Expand All @@ -30,13 +30,50 @@ pub struct CodeLensSettings {
pub references_all_functions: bool,
}

impl Default for CodeLensSettings {
fn default() -> Self {
Self {
implementations: false,
references: false,
references_all_functions: false,
}
}
}

#[derive(Debug, Clone, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct CompletionSettings {
#[serde(default)]
pub complete_function_calls: bool,
#[serde(default)]
pub names: bool,
#[serde(default)]
pub paths: bool,
#[serde(default)]
pub auto_imports: bool,
}

impl Default for CompletionSettings {
fn default() -> Self {
Self {
complete_function_calls: false,
names: true,
paths: true,
auto_imports: true,
}
}
}

#[derive(Debug, Default, Clone, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct WorkspaceSettings {
pub enable: bool,
pub config: Option<String>,
pub import_map: Option<String>,
pub code_lens: Option<CodeLensSettings>,
#[serde(default)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These give us default values if they aren't sent by the client, this makes it a lot easier dealing with the values.

pub code_lens: CodeLensSettings,
#[serde(default)]
pub suggest: CompletionSettings,

#[serde(default)]
pub lint: bool,
Expand All @@ -48,36 +85,7 @@ impl WorkspaceSettings {
/// Determine if any code lenses are enabled at all. This allows short
/// circuiting when there are no code lenses enabled.
pub fn enabled_code_lens(&self) -> bool {
if let Some(code_lens) = &self.code_lens {
// This should contain all the "top level" code lens references
code_lens.implementations || code_lens.references
} else {
false
}
}

pub fn enabled_code_lens_implementations(&self) -> bool {
if let Some(code_lens) = &self.code_lens {
code_lens.implementations
} else {
false
}
}

pub fn enabled_code_lens_references(&self) -> bool {
if let Some(code_lens) = &self.code_lens {
code_lens.references
} else {
false
}
}

pub fn enabled_code_lens_references_all_functions(&self) -> bool {
if let Some(code_lens) = &self.code_lens {
code_lens.references_all_functions
} else {
false
}
self.code_lens.implementations || self.code_lens.references
}
}

Expand Down
139 changes: 124 additions & 15 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,7 @@ impl Inner {
let mut code_lenses = cl.borrow_mut();

// TSC Implementations Code Lens
if self.config.settings.enabled_code_lens_implementations() {
if self.config.settings.code_lens.implementations {
let source = CodeLensSource::Implementations;
match i.kind {
tsc::ScriptElementKind::InterfaceElement => {
Expand All @@ -942,7 +942,7 @@ impl Inner {
}

// TSC References Code Lens
if self.config.settings.enabled_code_lens_references() {
if self.config.settings.code_lens.references {
let source = CodeLensSource::References;
if let Some(parent) = &mp {
if parent.kind == tsc::ScriptElementKind::EnumElement {
Expand All @@ -951,11 +951,7 @@ impl Inner {
}
match i.kind {
tsc::ScriptElementKind::FunctionElement => {
if self
.config
.settings
.enabled_code_lens_references_all_functions()
{
if self.config.settings.code_lens.references_all_functions {
code_lenses.push(i.to_code_lens(
&line_index,
&specifier,
Expand Down Expand Up @@ -1359,7 +1355,6 @@ impl Inner {
let specifier = self
.url_map
.normalize_url(&params.text_document_position.text_document.uri);
// TODO(lucacasonato): handle error correctly
let line_index =
if let Some(line_index) = self.get_line_index_sync(&specifier) {
line_index
Expand All @@ -1369,13 +1364,22 @@ impl Inner {
specifier
)));
};
let trigger_character = if let Some(context) = &params.context {
context.trigger_character.clone()
} else {
None
};
let position =
line_index.offset_tsc(params.text_document_position.position)?;
let req = tsc::RequestMethod::GetCompletions((
specifier,
line_index.offset_tsc(params.text_document_position.position)?,
tsc::UserPreferences {
// TODO(lucacasonato): enable this. see https://github.com/denoland/deno/pull/8651
include_completions_with_insert_text: Some(false),
..Default::default()
specifier.clone(),
position,
tsc::GetCompletionsAtPositionOptions {
user_preferences: tsc::UserPreferences {
include_completions_with_insert_text: Some(true),
..Default::default()
},
trigger_character,
},
));
let maybe_completion_info: Option<tsc::CompletionInfo> = self
Expand All @@ -1388,7 +1392,12 @@ impl Inner {
})?;

if let Some(completions) = maybe_completion_info {
let results = completions.into_completion_response(&line_index);
let results = completions.as_completion_response(
&line_index,
&self.config.settings.suggest,
&specifier,
position,
);
self.performance.measure(mark);
Ok(Some(results))
} else {
Expand All @@ -1397,6 +1406,47 @@ impl Inner {
}
}

async fn completion_resolve(
&mut self,
params: CompletionItem,
) -> LspResult<CompletionItem> {
let mark = self.performance.mark("completion_resolve");
if let Some(data) = &params.data {
let data: tsc::CompletionItemData = serde_json::from_value(data.clone())
.map_err(|err| {
error!("{}", err);
LspError::invalid_params(
"Could not decode data field of completion item.",
)
})?;
let req = tsc::RequestMethod::GetCompletionDetails(data.into());
let maybe_completion_info: Option<tsc::CompletionEntryDetails> = self
.ts_server
.request(self.snapshot(), req)
.await
.map_err(|err| {
error!("Unable to get completion info from TypeScript: {}", err);
LspError::internal_error()
})?;
if let Some(completion_info) = maybe_completion_info {
let completion_item = completion_info.as_completion_item(&params);
self.performance.measure(mark);
Ok(completion_item)
} else {
error!(
"Received an undefined response from tsc for completion details."
);
self.performance.measure(mark);
Ok(params)
}
} else {
self.performance.measure(mark);
Err(LspError::invalid_params(
"The completion item is missing the data field.",
))
}
}

async fn goto_implementation(
&mut self,
params: GotoImplementationParams,
Expand Down Expand Up @@ -1716,6 +1766,13 @@ impl lspower::LanguageServer for LanguageServer {
self.0.lock().await.completion(params).await
}

async fn completion_resolve(
&self,
params: CompletionItem,
) -> LspResult<CompletionItem> {
self.0.lock().await.completion_resolve(params).await
}

async fn goto_implementation(
&self,
params: GotoImplementationParams,
Expand Down Expand Up @@ -2741,6 +2798,58 @@ mod tests {
harness.run().await;
}

#[derive(Deserialize)]
struct CompletionResult {
pub result: Option<CompletionResponse>,
}

#[tokio::test]
async fn test_completions() {
let mut harness = LspTestHarness::new(vec![
("initialize_request.json", LspResponse::RequestAny),
("initialized_notification.json", LspResponse::None),
("did_open_notification_completions.json", LspResponse::None),
(
"completion_request.json",
LspResponse::RequestAssert(|value| {
let response: CompletionResult =
serde_json::from_value(value).unwrap();
let result = response.result.unwrap();
match result {
CompletionResponse::List(list) => {
// there should be at least 90 completions for `Deno.`
assert!(list.items.len() > 90);
}
_ => panic!("unexpected result"),
}
}),
),
(
"completion_resolve_request.json",
LspResponse::Request(
4,
json!({
"label": "build",
"kind": 6,
"detail": "const Deno.build: {\n target: string;\n arch: \"x86_64\";\n os: \"darwin\" | \"linux\" | \"windows\";\n vendor: string;\n env?: string | undefined;\n}",
"documentation": {
"kind": "markdown",
"value": "Build related information."
},
"sortText": "1",
"insertTextFormat": 1,
}),
),
),
(
"shutdown_request.json",
LspResponse::Request(3, json!(null)),
),
("exit_notification.json", LspResponse::None),
]);
harness.run().await;
}

#[derive(Deserialize)]
struct PerformanceAverages {
averages: Vec<PerformanceAverage>,
Expand Down
Loading