From 533a9b108677f1560fe55882771a0be2bb0b0fd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=82=B3=E6=9D=83?= <695601626@qq.com> Date: Wed, 16 Oct 2024 00:10:07 +0800 Subject: chore: upgrade to rust 1.81.0 (#26261) --- cli/lsp/tsc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cli/lsp') diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index c8b5c47f8..cfab39b20 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -3939,7 +3939,7 @@ pub struct OutliningSpan { kind: OutliningSpanKind, } -const FOLD_END_PAIR_CHARACTERS: &[u8] = &[b'}', b']', b')', b'`']; +const FOLD_END_PAIR_CHARACTERS: &[u8] = b"}])`"; impl OutliningSpan { pub fn to_folding_range( -- cgit v1.2.3 From 06778e4e9b0a856ac39cc01f0813e6076eb150d6 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Wed, 16 Oct 2024 17:05:13 +0100 Subject: feat(lsp): "deno/didRefreshDenoConfigurationTree" notifications (#26215) --- cli/lsp/capabilities.rs | 4 ++-- cli/lsp/client.rs | 35 +++++++++++++++++++++++++++++++++ cli/lsp/config.rs | 48 +++++++++++++++++++++++++++++++++++++++++++--- cli/lsp/language_server.rs | 5 +++++ cli/lsp/lsp_custom.rs | 30 ++++++++++++++++++++++++++--- 5 files changed, 114 insertions(+), 8 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/capabilities.rs b/cli/lsp/capabilities.rs index e93d3b7c2..5cdb1224d 100644 --- a/cli/lsp/capabilities.rs +++ b/cli/lsp/capabilities.rs @@ -147,11 +147,11 @@ pub fn server_capabilities( moniker_provider: None, experimental: Some(json!({ "denoConfigTasks": true, - "testingApi":true, + "testingApi": true, + "didRefreshDenoConfigurationTreeNotifications": true, })), inlay_hint_provider: Some(OneOf::Left(true)), position_encoding: None, - // TODO(nayeemrmn): Support pull-based diagnostics. diagnostic_provider: None, inline_value_provider: None, inline_completion_provider: None, diff --git a/cli/lsp/client.rs b/cli/lsp/client.rs index b3f0d64fa..65865d5b3 100644 --- a/cli/lsp/client.rs +++ b/cli/lsp/client.rs @@ -92,6 +92,19 @@ impl Client { }); } + pub fn send_did_refresh_deno_configuration_tree_notification( + &self, + params: lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams, + ) { + // do on a task in case the caller currently is in the lsp lock + let client = self.0.clone(); + spawn(async move { + client + .send_did_refresh_deno_configuration_tree_notification(params) + .await; + }); + } + pub fn send_did_change_deno_configuration_notification( &self, params: lsp_custom::DidChangeDenoConfigurationNotificationParams, @@ -169,6 +182,10 @@ trait ClientTrait: Send + Sync { params: lsp_custom::DiagnosticBatchNotificationParams, ); async fn send_test_notification(&self, params: TestingNotification); + async fn send_did_refresh_deno_configuration_tree_notification( + &self, + params: lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams, + ); async fn send_did_change_deno_configuration_notification( &self, params: lsp_custom::DidChangeDenoConfigurationNotificationParams, @@ -249,6 +266,18 @@ impl ClientTrait for TowerClient { } } + async fn send_did_refresh_deno_configuration_tree_notification( + &self, + params: lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams, + ) { + self + .0 + .send_notification::( + params, + ) + .await + } + async fn send_did_change_deno_configuration_notification( &self, params: lsp_custom::DidChangeDenoConfigurationNotificationParams, @@ -366,6 +395,12 @@ impl ClientTrait for ReplClient { async fn send_test_notification(&self, _params: TestingNotification) {} + async fn send_did_refresh_deno_configuration_tree_notification( + &self, + _params: lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams, + ) { + } + async fn send_did_change_deno_configuration_notification( &self, _params: lsp_custom::DidChangeDenoConfigurationNotificationParams, diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 07fdd3c65..74f3583d6 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -50,6 +50,8 @@ use std::sync::Arc; use tower_lsp::lsp_types as lsp; use super::logging::lsp_log; +use super::lsp_custom; +use super::urls::url_to_uri; use crate::args::discover_npmrc_from_workspace; use crate::args::has_flag_env_var; use crate::args::CliLockfile; @@ -1716,14 +1718,14 @@ impl ConfigTree { .unwrap_or_else(|| Arc::new(FmtConfig::new_with_base(PathBuf::from("/")))) } - /// Returns (scope_uri, type). + /// Returns (scope_url, type). pub fn watched_file_type( &self, specifier: &ModuleSpecifier, ) -> Option<(&ModuleSpecifier, ConfigWatchedFileType)> { - for (scope_uri, data) in self.scopes.iter() { + for (scope_url, data) in self.scopes.iter() { if let Some(typ) = data.watched_files.get(specifier) { - return Some((scope_uri, *typ)); + return Some((scope_url, *typ)); } } None @@ -1747,6 +1749,46 @@ impl ConfigTree { .any(|data| data.watched_files.contains_key(specifier)) } + pub fn to_did_refresh_params( + &self, + ) -> lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams { + let data = self + .scopes + .values() + .filter_map(|data| { + let workspace_root_scope_uri = + Some(data.member_dir.workspace.root_dir()) + .filter(|s| *s != data.member_dir.dir_url()) + .and_then(|s| url_to_uri(s).ok()); + Some(lsp_custom::DenoConfigurationData { + scope_uri: url_to_uri(&data.scope).ok()?, + deno_json: data.maybe_deno_json().and_then(|c| { + if workspace_root_scope_uri.is_some() + && Some(&c.specifier) + == data + .member_dir + .workspace + .root_deno_json() + .map(|c| &c.specifier) + { + return None; + } + Some(lsp::TextDocumentIdentifier { + uri: url_to_uri(&c.specifier).ok()?, + }) + }), + package_json: data.maybe_pkg_json().and_then(|p| { + Some(lsp::TextDocumentIdentifier { + uri: url_to_uri(&p.specifier()).ok()?, + }) + }), + workspace_root_scope_uri, + }) + }) + .collect(); + lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams { data } + } + pub async fn refresh( &mut self, settings: &Settings, diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 8269dc851..908afa165 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -963,6 +963,11 @@ impl Inner { .tree .refresh(&self.config.settings, &self.workspace_files, &file_fetcher) .await; + self + .client + .send_did_refresh_deno_configuration_tree_notification( + self.config.tree.to_did_refresh_params(), + ); for config_file in self.config.tree.config_files() { (|| { let compiler_options = config_file.to_compiler_options().ok()?.options; diff --git a/cli/lsp/lsp_custom.rs b/cli/lsp/lsp_custom.rs index 5f485db7a..b570b6d0e 100644 --- a/cli/lsp/lsp_custom.rs +++ b/cli/lsp/lsp_custom.rs @@ -46,6 +46,30 @@ pub struct DiagnosticBatchNotificationParams { pub messages_len: usize, } +#[derive(Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct DenoConfigurationData { + pub scope_uri: lsp::Uri, + pub workspace_root_scope_uri: Option, + pub deno_json: Option, + pub package_json: Option, +} + +#[derive(Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct DidRefreshDenoConfigurationTreeNotificationParams { + pub data: Vec, +} + +pub enum DidRefreshDenoConfigurationTreeNotification {} + +impl lsp::notification::Notification + for DidRefreshDenoConfigurationTreeNotification +{ + type Params = DidRefreshDenoConfigurationTreeNotificationParams; + const METHOD: &'static str = "deno/didRefreshDenoConfigurationTree"; +} + #[derive(Debug, Eq, Hash, PartialEq, Copy, Clone, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub enum DenoConfigurationChangeType { @@ -88,13 +112,15 @@ pub struct DidChangeDenoConfigurationNotificationParams { pub changes: Vec, } +// TODO(nayeemrmn): This is being replaced by +// `DidRefreshDenoConfigurationTreeNotification` for Deno > v2.0.0. Remove it +// soon. pub enum DidChangeDenoConfigurationNotification {} impl lsp::notification::Notification for DidChangeDenoConfigurationNotification { type Params = DidChangeDenoConfigurationNotificationParams; - const METHOD: &'static str = "deno/didChangeDenoConfiguration"; } @@ -102,7 +128,6 @@ pub enum DidUpgradeCheckNotification {} impl lsp::notification::Notification for DidUpgradeCheckNotification { type Params = DidUpgradeCheckNotificationParams; - const METHOD: &'static str = "deno/didUpgradeCheck"; } @@ -125,6 +150,5 @@ pub enum DiagnosticBatchNotification {} impl lsp::notification::Notification for DiagnosticBatchNotification { type Params = DiagnosticBatchNotificationParams; - const METHOD: &'static str = "deno/internalTestDiagnosticBatch"; } -- cgit v1.2.3 From 1fd8d092eafc168af779040e93129d14e5a7187d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 16 Oct 2024 20:05:47 +0100 Subject: =?UTF-8?q?Revert=20"feat(lsp):=20"deno/didRefreshDenoConfiguratio?= =?UTF-8?q?nTree"=20notificatio=E2=80=A6=20(#26320)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …ns (#26215)" This reverts commit 06778e4e9b0a856ac39cc01f0813e6076eb150d6 because benchmarks are failing on `main`. --- cli/lsp/capabilities.rs | 4 ++-- cli/lsp/client.rs | 35 --------------------------------- cli/lsp/config.rs | 48 +++------------------------------------------- cli/lsp/language_server.rs | 5 ----- cli/lsp/lsp_custom.rs | 30 +++-------------------------- 5 files changed, 8 insertions(+), 114 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/capabilities.rs b/cli/lsp/capabilities.rs index 5cdb1224d..e93d3b7c2 100644 --- a/cli/lsp/capabilities.rs +++ b/cli/lsp/capabilities.rs @@ -147,11 +147,11 @@ pub fn server_capabilities( moniker_provider: None, experimental: Some(json!({ "denoConfigTasks": true, - "testingApi": true, - "didRefreshDenoConfigurationTreeNotifications": true, + "testingApi":true, })), inlay_hint_provider: Some(OneOf::Left(true)), position_encoding: None, + // TODO(nayeemrmn): Support pull-based diagnostics. diagnostic_provider: None, inline_value_provider: None, inline_completion_provider: None, diff --git a/cli/lsp/client.rs b/cli/lsp/client.rs index 65865d5b3..b3f0d64fa 100644 --- a/cli/lsp/client.rs +++ b/cli/lsp/client.rs @@ -92,19 +92,6 @@ impl Client { }); } - pub fn send_did_refresh_deno_configuration_tree_notification( - &self, - params: lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams, - ) { - // do on a task in case the caller currently is in the lsp lock - let client = self.0.clone(); - spawn(async move { - client - .send_did_refresh_deno_configuration_tree_notification(params) - .await; - }); - } - pub fn send_did_change_deno_configuration_notification( &self, params: lsp_custom::DidChangeDenoConfigurationNotificationParams, @@ -182,10 +169,6 @@ trait ClientTrait: Send + Sync { params: lsp_custom::DiagnosticBatchNotificationParams, ); async fn send_test_notification(&self, params: TestingNotification); - async fn send_did_refresh_deno_configuration_tree_notification( - &self, - params: lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams, - ); async fn send_did_change_deno_configuration_notification( &self, params: lsp_custom::DidChangeDenoConfigurationNotificationParams, @@ -266,18 +249,6 @@ impl ClientTrait for TowerClient { } } - async fn send_did_refresh_deno_configuration_tree_notification( - &self, - params: lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams, - ) { - self - .0 - .send_notification::( - params, - ) - .await - } - async fn send_did_change_deno_configuration_notification( &self, params: lsp_custom::DidChangeDenoConfigurationNotificationParams, @@ -395,12 +366,6 @@ impl ClientTrait for ReplClient { async fn send_test_notification(&self, _params: TestingNotification) {} - async fn send_did_refresh_deno_configuration_tree_notification( - &self, - _params: lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams, - ) { - } - async fn send_did_change_deno_configuration_notification( &self, _params: lsp_custom::DidChangeDenoConfigurationNotificationParams, diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 74f3583d6..07fdd3c65 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -50,8 +50,6 @@ use std::sync::Arc; use tower_lsp::lsp_types as lsp; use super::logging::lsp_log; -use super::lsp_custom; -use super::urls::url_to_uri; use crate::args::discover_npmrc_from_workspace; use crate::args::has_flag_env_var; use crate::args::CliLockfile; @@ -1718,14 +1716,14 @@ impl ConfigTree { .unwrap_or_else(|| Arc::new(FmtConfig::new_with_base(PathBuf::from("/")))) } - /// Returns (scope_url, type). + /// Returns (scope_uri, type). pub fn watched_file_type( &self, specifier: &ModuleSpecifier, ) -> Option<(&ModuleSpecifier, ConfigWatchedFileType)> { - for (scope_url, data) in self.scopes.iter() { + for (scope_uri, data) in self.scopes.iter() { if let Some(typ) = data.watched_files.get(specifier) { - return Some((scope_url, *typ)); + return Some((scope_uri, *typ)); } } None @@ -1749,46 +1747,6 @@ impl ConfigTree { .any(|data| data.watched_files.contains_key(specifier)) } - pub fn to_did_refresh_params( - &self, - ) -> lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams { - let data = self - .scopes - .values() - .filter_map(|data| { - let workspace_root_scope_uri = - Some(data.member_dir.workspace.root_dir()) - .filter(|s| *s != data.member_dir.dir_url()) - .and_then(|s| url_to_uri(s).ok()); - Some(lsp_custom::DenoConfigurationData { - scope_uri: url_to_uri(&data.scope).ok()?, - deno_json: data.maybe_deno_json().and_then(|c| { - if workspace_root_scope_uri.is_some() - && Some(&c.specifier) - == data - .member_dir - .workspace - .root_deno_json() - .map(|c| &c.specifier) - { - return None; - } - Some(lsp::TextDocumentIdentifier { - uri: url_to_uri(&c.specifier).ok()?, - }) - }), - package_json: data.maybe_pkg_json().and_then(|p| { - Some(lsp::TextDocumentIdentifier { - uri: url_to_uri(&p.specifier()).ok()?, - }) - }), - workspace_root_scope_uri, - }) - }) - .collect(); - lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams { data } - } - pub async fn refresh( &mut self, settings: &Settings, diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 908afa165..8269dc851 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -963,11 +963,6 @@ impl Inner { .tree .refresh(&self.config.settings, &self.workspace_files, &file_fetcher) .await; - self - .client - .send_did_refresh_deno_configuration_tree_notification( - self.config.tree.to_did_refresh_params(), - ); for config_file in self.config.tree.config_files() { (|| { let compiler_options = config_file.to_compiler_options().ok()?.options; diff --git a/cli/lsp/lsp_custom.rs b/cli/lsp/lsp_custom.rs index b570b6d0e..5f485db7a 100644 --- a/cli/lsp/lsp_custom.rs +++ b/cli/lsp/lsp_custom.rs @@ -46,30 +46,6 @@ pub struct DiagnosticBatchNotificationParams { pub messages_len: usize, } -#[derive(Debug, Deserialize, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct DenoConfigurationData { - pub scope_uri: lsp::Uri, - pub workspace_root_scope_uri: Option, - pub deno_json: Option, - pub package_json: Option, -} - -#[derive(Debug, Deserialize, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct DidRefreshDenoConfigurationTreeNotificationParams { - pub data: Vec, -} - -pub enum DidRefreshDenoConfigurationTreeNotification {} - -impl lsp::notification::Notification - for DidRefreshDenoConfigurationTreeNotification -{ - type Params = DidRefreshDenoConfigurationTreeNotificationParams; - const METHOD: &'static str = "deno/didRefreshDenoConfigurationTree"; -} - #[derive(Debug, Eq, Hash, PartialEq, Copy, Clone, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub enum DenoConfigurationChangeType { @@ -112,15 +88,13 @@ pub struct DidChangeDenoConfigurationNotificationParams { pub changes: Vec, } -// TODO(nayeemrmn): This is being replaced by -// `DidRefreshDenoConfigurationTreeNotification` for Deno > v2.0.0. Remove it -// soon. pub enum DidChangeDenoConfigurationNotification {} impl lsp::notification::Notification for DidChangeDenoConfigurationNotification { type Params = DidChangeDenoConfigurationNotificationParams; + const METHOD: &'static str = "deno/didChangeDenoConfiguration"; } @@ -128,6 +102,7 @@ pub enum DidUpgradeCheckNotification {} impl lsp::notification::Notification for DidUpgradeCheckNotification { type Params = DidUpgradeCheckNotificationParams; + const METHOD: &'static str = "deno/didUpgradeCheck"; } @@ -150,5 +125,6 @@ pub enum DiagnosticBatchNotification {} impl lsp::notification::Notification for DiagnosticBatchNotification { type Params = DiagnosticBatchNotificationParams; + const METHOD: &'static str = "deno/internalTestDiagnosticBatch"; } -- cgit v1.2.3 From 72dd74d83a89d3db17e6ed5d0b8c7f4ba3f3d233 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Wed, 16 Oct 2024 22:43:26 +0100 Subject: Reland feat(lsp): deno/didRefreshDenoConfigurationTree notifications (#26325) --- cli/lsp/capabilities.rs | 4 ++-- cli/lsp/client.rs | 35 +++++++++++++++++++++++++++++++++ cli/lsp/config.rs | 48 +++++++++++++++++++++++++++++++++++++++++++--- cli/lsp/language_server.rs | 5 +++++ cli/lsp/lsp_custom.rs | 30 ++++++++++++++++++++++++++--- 5 files changed, 114 insertions(+), 8 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/capabilities.rs b/cli/lsp/capabilities.rs index e93d3b7c2..5cdb1224d 100644 --- a/cli/lsp/capabilities.rs +++ b/cli/lsp/capabilities.rs @@ -147,11 +147,11 @@ pub fn server_capabilities( moniker_provider: None, experimental: Some(json!({ "denoConfigTasks": true, - "testingApi":true, + "testingApi": true, + "didRefreshDenoConfigurationTreeNotifications": true, })), inlay_hint_provider: Some(OneOf::Left(true)), position_encoding: None, - // TODO(nayeemrmn): Support pull-based diagnostics. diagnostic_provider: None, inline_value_provider: None, inline_completion_provider: None, diff --git a/cli/lsp/client.rs b/cli/lsp/client.rs index b3f0d64fa..65865d5b3 100644 --- a/cli/lsp/client.rs +++ b/cli/lsp/client.rs @@ -92,6 +92,19 @@ impl Client { }); } + pub fn send_did_refresh_deno_configuration_tree_notification( + &self, + params: lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams, + ) { + // do on a task in case the caller currently is in the lsp lock + let client = self.0.clone(); + spawn(async move { + client + .send_did_refresh_deno_configuration_tree_notification(params) + .await; + }); + } + pub fn send_did_change_deno_configuration_notification( &self, params: lsp_custom::DidChangeDenoConfigurationNotificationParams, @@ -169,6 +182,10 @@ trait ClientTrait: Send + Sync { params: lsp_custom::DiagnosticBatchNotificationParams, ); async fn send_test_notification(&self, params: TestingNotification); + async fn send_did_refresh_deno_configuration_tree_notification( + &self, + params: lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams, + ); async fn send_did_change_deno_configuration_notification( &self, params: lsp_custom::DidChangeDenoConfigurationNotificationParams, @@ -249,6 +266,18 @@ impl ClientTrait for TowerClient { } } + async fn send_did_refresh_deno_configuration_tree_notification( + &self, + params: lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams, + ) { + self + .0 + .send_notification::( + params, + ) + .await + } + async fn send_did_change_deno_configuration_notification( &self, params: lsp_custom::DidChangeDenoConfigurationNotificationParams, @@ -366,6 +395,12 @@ impl ClientTrait for ReplClient { async fn send_test_notification(&self, _params: TestingNotification) {} + async fn send_did_refresh_deno_configuration_tree_notification( + &self, + _params: lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams, + ) { + } + async fn send_did_change_deno_configuration_notification( &self, _params: lsp_custom::DidChangeDenoConfigurationNotificationParams, diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 07fdd3c65..74f3583d6 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -50,6 +50,8 @@ use std::sync::Arc; use tower_lsp::lsp_types as lsp; use super::logging::lsp_log; +use super::lsp_custom; +use super::urls::url_to_uri; use crate::args::discover_npmrc_from_workspace; use crate::args::has_flag_env_var; use crate::args::CliLockfile; @@ -1716,14 +1718,14 @@ impl ConfigTree { .unwrap_or_else(|| Arc::new(FmtConfig::new_with_base(PathBuf::from("/")))) } - /// Returns (scope_uri, type). + /// Returns (scope_url, type). pub fn watched_file_type( &self, specifier: &ModuleSpecifier, ) -> Option<(&ModuleSpecifier, ConfigWatchedFileType)> { - for (scope_uri, data) in self.scopes.iter() { + for (scope_url, data) in self.scopes.iter() { if let Some(typ) = data.watched_files.get(specifier) { - return Some((scope_uri, *typ)); + return Some((scope_url, *typ)); } } None @@ -1747,6 +1749,46 @@ impl ConfigTree { .any(|data| data.watched_files.contains_key(specifier)) } + pub fn to_did_refresh_params( + &self, + ) -> lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams { + let data = self + .scopes + .values() + .filter_map(|data| { + let workspace_root_scope_uri = + Some(data.member_dir.workspace.root_dir()) + .filter(|s| *s != data.member_dir.dir_url()) + .and_then(|s| url_to_uri(s).ok()); + Some(lsp_custom::DenoConfigurationData { + scope_uri: url_to_uri(&data.scope).ok()?, + deno_json: data.maybe_deno_json().and_then(|c| { + if workspace_root_scope_uri.is_some() + && Some(&c.specifier) + == data + .member_dir + .workspace + .root_deno_json() + .map(|c| &c.specifier) + { + return None; + } + Some(lsp::TextDocumentIdentifier { + uri: url_to_uri(&c.specifier).ok()?, + }) + }), + package_json: data.maybe_pkg_json().and_then(|p| { + Some(lsp::TextDocumentIdentifier { + uri: url_to_uri(&p.specifier()).ok()?, + }) + }), + workspace_root_scope_uri, + }) + }) + .collect(); + lsp_custom::DidRefreshDenoConfigurationTreeNotificationParams { data } + } + pub async fn refresh( &mut self, settings: &Settings, diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 8269dc851..908afa165 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -963,6 +963,11 @@ impl Inner { .tree .refresh(&self.config.settings, &self.workspace_files, &file_fetcher) .await; + self + .client + .send_did_refresh_deno_configuration_tree_notification( + self.config.tree.to_did_refresh_params(), + ); for config_file in self.config.tree.config_files() { (|| { let compiler_options = config_file.to_compiler_options().ok()?.options; diff --git a/cli/lsp/lsp_custom.rs b/cli/lsp/lsp_custom.rs index 5f485db7a..b570b6d0e 100644 --- a/cli/lsp/lsp_custom.rs +++ b/cli/lsp/lsp_custom.rs @@ -46,6 +46,30 @@ pub struct DiagnosticBatchNotificationParams { pub messages_len: usize, } +#[derive(Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct DenoConfigurationData { + pub scope_uri: lsp::Uri, + pub workspace_root_scope_uri: Option, + pub deno_json: Option, + pub package_json: Option, +} + +#[derive(Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct DidRefreshDenoConfigurationTreeNotificationParams { + pub data: Vec, +} + +pub enum DidRefreshDenoConfigurationTreeNotification {} + +impl lsp::notification::Notification + for DidRefreshDenoConfigurationTreeNotification +{ + type Params = DidRefreshDenoConfigurationTreeNotificationParams; + const METHOD: &'static str = "deno/didRefreshDenoConfigurationTree"; +} + #[derive(Debug, Eq, Hash, PartialEq, Copy, Clone, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub enum DenoConfigurationChangeType { @@ -88,13 +112,15 @@ pub struct DidChangeDenoConfigurationNotificationParams { pub changes: Vec, } +// TODO(nayeemrmn): This is being replaced by +// `DidRefreshDenoConfigurationTreeNotification` for Deno > v2.0.0. Remove it +// soon. pub enum DidChangeDenoConfigurationNotification {} impl lsp::notification::Notification for DidChangeDenoConfigurationNotification { type Params = DidChangeDenoConfigurationNotificationParams; - const METHOD: &'static str = "deno/didChangeDenoConfiguration"; } @@ -102,7 +128,6 @@ pub enum DidUpgradeCheckNotification {} impl lsp::notification::Notification for DidUpgradeCheckNotification { type Params = DidUpgradeCheckNotificationParams; - const METHOD: &'static str = "deno/didUpgradeCheck"; } @@ -125,6 +150,5 @@ pub enum DiagnosticBatchNotification {} impl lsp::notification::Notification for DiagnosticBatchNotification { type Params = DiagnosticBatchNotificationParams; - const METHOD: &'static str = "deno/internalTestDiagnosticBatch"; } -- cgit v1.2.3 From 9fe2bf42dc584779cc43f0ec15a5a3d6dddca283 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Mon, 21 Oct 2024 17:15:52 +0100 Subject: feat(lsp): interactive inlay hints (#26382) --- cli/lsp/language_server.rs | 2 +- cli/lsp/tsc.rs | 65 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 3 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 908afa165..33ae539f8 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -3812,7 +3812,7 @@ impl Inner { let maybe_inlay_hints = maybe_inlay_hints.map(|hints| { hints .iter() - .map(|hint| hint.to_lsp(line_index.clone())) + .map(|hint| hint.to_lsp(line_index.clone(), self)) .collect() }); self.performance.measure(mark); diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index cfab39b20..0bc7d1600 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -2182,6 +2182,50 @@ impl NavigateToItem { } } +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct InlayHintDisplayPart { + pub text: String, + pub span: Option, + pub file: Option, +} + +impl InlayHintDisplayPart { + pub fn to_lsp( + &self, + language_server: &language_server::Inner, + ) -> lsp::InlayHintLabelPart { + let location = self.file.as_ref().map(|f| { + let specifier = + resolve_url(f).unwrap_or_else(|_| INVALID_SPECIFIER.clone()); + let file_referrer = + language_server.documents.get_file_referrer(&specifier); + let uri = language_server + .url_map + .specifier_to_uri(&specifier, file_referrer.as_deref()) + .unwrap_or_else(|_| INVALID_URI.clone()); + let range = self + .span + .as_ref() + .and_then(|s| { + let asset_or_doc = + language_server.get_asset_or_document(&specifier).ok()?; + Some(s.to_range(asset_or_doc.line_index())) + }) + .unwrap_or_else(|| { + lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 0)) + }); + lsp::Location { uri, range } + }); + lsp::InlayHintLabelPart { + value: self.text.clone(), + tooltip: None, + location, + command: None, + } + } +} + #[derive(Debug, Clone, Deserialize)] pub enum InlayHintKind { Type, @@ -2203,6 +2247,7 @@ impl InlayHintKind { #[serde(rename_all = "camelCase")] pub struct InlayHint { pub text: String, + pub display_parts: Option>, pub position: u32, pub kind: InlayHintKind, pub whitespace_before: Option, @@ -2210,10 +2255,23 @@ pub struct InlayHint { } impl InlayHint { - pub fn to_lsp(&self, line_index: Arc) -> lsp::InlayHint { + pub fn to_lsp( + &self, + line_index: Arc, + language_server: &language_server::Inner, + ) -> lsp::InlayHint { lsp::InlayHint { position: line_index.position_tsc(self.position.into()), - label: lsp::InlayHintLabel::String(self.text.clone()), + label: if let Some(display_parts) = &self.display_parts { + lsp::InlayHintLabel::LabelParts( + display_parts + .iter() + .map(|p| p.to_lsp(language_server)) + .collect(), + ) + } else { + lsp::InlayHintLabel::String(self.text.clone()) + }, kind: self.kind.to_lsp(), padding_left: self.whitespace_before, padding_right: self.whitespace_after, @@ -4892,6 +4950,8 @@ pub struct UserPreferences { pub allow_rename_of_import_path: Option, #[serde(skip_serializing_if = "Option::is_none")] pub auto_import_file_exclude_patterns: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub interactive_inlay_hints: Option, } impl UserPreferences { @@ -4909,6 +4969,7 @@ impl UserPreferences { include_completions_with_snippet_text: Some( config.snippet_support_capable(), ), + interactive_inlay_hints: Some(true), provide_refactor_not_applicable_reason: Some(true), quote_preference: Some(fmt_config.into()), use_label_details_in_completion_entries: Some(true), -- cgit v1.2.3 From 9e25a4ebbfa848e58415f267d5991bead7de4808 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Mon, 21 Oct 2024 22:16:39 +0100 Subject: fix(lsp): import-map-remap quickfix for type imports (#26454) --- cli/lsp/diagnostics.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'cli/lsp') diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index caabd3f04..83c00d27e 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -1499,7 +1499,11 @@ fn diagnose_dependency( .data_for_specifier(referrer_doc.file_referrer().unwrap_or(referrer)) .and_then(|d| d.resolver.maybe_import_map()); if let Some(import_map) = import_map { - if let Resolution::Ok(resolved) = &dependency.maybe_code { + let resolved = dependency + .maybe_code + .ok() + .or_else(|| dependency.maybe_type.ok()); + if let Some(resolved) = resolved { if let Some(to) = import_map.lookup(&resolved.specifier, referrer) { if dependency_key != to { diagnostics.push( -- cgit v1.2.3 From 5f0bb3c6f4328003012e98ba70ce18e4e2e842de Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Thu, 24 Oct 2024 20:03:56 +0200 Subject: fix: `.npmrc` settings not being passed to install/add command (#26473) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We weren't passing the resolved npmrc settings to the install commands. This lead us to always fall back to the default registry url instead of using the one from npmrc. Fixes https://github.com/denoland/deno/issues/26139 Fixes https://github.com/denoland/deno/issues/26033 Fixes https://github.com/denoland/deno/issues/25924 Fixes https://github.com/denoland/deno/issues/25822 Fixes https://github.com/denoland/deno/issues/26152 --------- Co-authored-by: Bartek Iwańczuk --- cli/lsp/npm.rs | 6 +++++- cli/lsp/registries.rs | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) (limited to 'cli/lsp') diff --git a/cli/lsp/npm.rs b/cli/lsp/npm.rs index 8bdeb7e7d..2decfc342 100644 --- a/cli/lsp/npm.rs +++ b/cli/lsp/npm.rs @@ -4,6 +4,7 @@ use dashmap::DashMap; use deno_core::anyhow::anyhow; use deno_core::error::AnyError; use deno_core::serde_json; +use deno_npm::npm_rc::NpmRc; use deno_semver::package::PackageNv; use deno_semver::Version; use serde::Deserialize; @@ -25,7 +26,10 @@ pub struct CliNpmSearchApi { impl CliNpmSearchApi { pub fn new(file_fetcher: Arc) -> Self { - let resolver = NpmFetchResolver::new(file_fetcher.clone()); + let resolver = NpmFetchResolver::new( + file_fetcher.clone(), + Arc::new(NpmRc::default().as_resolved(npm_registry_url()).unwrap()), + ); Self { file_fetcher, resolver, diff --git a/cli/lsp/registries.rs b/cli/lsp/registries.rs index 5f7ce0082..ade353e68 100644 --- a/cli/lsp/registries.rs +++ b/cli/lsp/registries.rs @@ -482,6 +482,7 @@ impl ModuleRegistry { .fetch_with_options(FetchOptions { specifier: &specifier, permissions: FetchPermissionsOptionRef::AllowAll, + maybe_auth: None, maybe_accept: Some("application/vnd.deno.reg.v2+json, application/vnd.deno.reg.v1+json;q=0.9, application/json;q=0.8"), maybe_cache_setting: None, }) -- cgit v1.2.3 From 38c7af456518ac4fdbd36e3bfa731c38195bb773 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Fri, 25 Oct 2024 18:35:09 +0100 Subject: feat(lsp): "typescript.preferences.preferTypeOnlyAutoImports" setting (#26546) --- cli/lsp/config.rs | 5 +++++ cli/lsp/tsc.rs | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) (limited to 'cli/lsp') diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 74f3583d6..3ffc9e657 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -439,6 +439,8 @@ pub struct LanguagePreferences { pub use_aliases_for_renames: bool, #[serde(default)] pub quote_style: QuoteStyle, + #[serde(default)] + pub prefer_type_only_auto_imports: bool, } impl Default for LanguagePreferences { @@ -449,6 +451,7 @@ impl Default for LanguagePreferences { auto_import_file_exclude_patterns: vec![], use_aliases_for_renames: true, quote_style: Default::default(), + prefer_type_only_auto_imports: false, } } } @@ -2251,6 +2254,7 @@ mod tests { auto_import_file_exclude_patterns: vec![], use_aliases_for_renames: true, quote_style: QuoteStyle::Auto, + prefer_type_only_auto_imports: false, }, suggest: CompletionSettings { complete_function_calls: false, @@ -2296,6 +2300,7 @@ mod tests { auto_import_file_exclude_patterns: vec![], use_aliases_for_renames: true, quote_style: QuoteStyle::Auto, + prefer_type_only_auto_imports: false, }, suggest: CompletionSettings { complete_function_calls: false, diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 0bc7d1600..0f31d7dd3 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -4952,6 +4952,8 @@ pub struct UserPreferences { pub auto_import_file_exclude_patterns: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub interactive_inlay_hints: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub prefer_type_only_auto_imports: Option, } impl UserPreferences { @@ -5074,6 +5076,9 @@ impl UserPreferences { } else { Some(language_settings.preferences.quote_style) }, + prefer_type_only_auto_imports: Some( + language_settings.preferences.prefer_type_only_auto_imports, + ), ..base_preferences } } @@ -6215,7 +6220,7 @@ mod tests { let change = changes.text_changes.first().unwrap(); assert_eq!( change.new_text, - "import type { someLongVariable } from './b.ts'\n" + "import { someLongVariable } from './b.ts'\n" ); } -- cgit v1.2.3 From d92d2fe9b0bd5e6e29cb3b6f924472aec972b636 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Fri, 25 Oct 2024 21:52:50 +0100 Subject: fix(lsp): make missing import action fix infallible (#26539) --- cli/lsp/analysis.rs | 105 ++++++++++++++++++++++++---------------------------- 1 file changed, 49 insertions(+), 56 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 8c2e8bb1d..39f1ae648 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -18,7 +18,6 @@ use deno_lint::diagnostic::LintDiagnosticRange; use deno_ast::SourceRange; use deno_ast::SourceRangedForSpanned; use deno_ast::SourceTextInfo; -use deno_core::anyhow::anyhow; use deno_core::error::custom_error; use deno_core::error::AnyError; use deno_core::serde::Deserialize; @@ -40,6 +39,7 @@ use import_map::ImportMap; use node_resolver::NpmResolver; use once_cell::sync::Lazy; use regex::Regex; +use std::borrow::Cow; use std::cmp::Ordering; use std::collections::HashMap; use std::collections::HashSet; @@ -598,68 +598,62 @@ pub fn fix_ts_import_changes( /// Fix tsc import code actions so that the module specifier is correct for /// resolution by Deno (includes the extension). -fn fix_ts_import_action( +fn fix_ts_import_action<'a>( referrer: &ModuleSpecifier, - action: &tsc::CodeFixAction, + action: &'a tsc::CodeFixAction, import_mapper: &TsResponseImportMapper, -) -> Result, AnyError> { - if matches!( +) -> Option> { + if !matches!( action.fix_name.as_str(), "import" | "fixMissingFunctionDeclaration" ) { - let change = action + return Some(Cow::Borrowed(action)); + } + let specifier = (|| { + let text_change = action.changes.first()?.text_changes.first()?; + let captures = IMPORT_SPECIFIER_RE.captures(&text_change.new_text)?; + Some(captures.get(1)?.as_str()) + })(); + let Some(specifier) = specifier else { + return Some(Cow::Borrowed(action)); + }; + if let Some(new_specifier) = + import_mapper.check_unresolved_specifier(specifier, referrer) + { + let description = action.description.replace(specifier, &new_specifier); + let changes = action .changes - .first() - .ok_or_else(|| anyhow!("Unexpected action changes."))?; - let text_change = change - .text_changes - .first() - .ok_or_else(|| anyhow!("Missing text change."))?; - if let Some(captures) = IMPORT_SPECIFIER_RE.captures(&text_change.new_text) - { - let specifier = captures - .get(1) - .ok_or_else(|| anyhow!("Missing capture."))? - .as_str(); - if let Some(new_specifier) = - import_mapper.check_unresolved_specifier(specifier, referrer) - { - let description = action.description.replace(specifier, &new_specifier); - let changes = action - .changes + .iter() + .map(|c| { + let text_changes = c + .text_changes .iter() - .map(|c| { - let text_changes = c - .text_changes - .iter() - .map(|tc| tsc::TextChange { - span: tc.span.clone(), - new_text: tc.new_text.replace(specifier, &new_specifier), - }) - .collect(); - tsc::FileTextChanges { - file_name: c.file_name.clone(), - text_changes, - is_new_file: c.is_new_file, - } + .map(|tc| tsc::TextChange { + span: tc.span.clone(), + new_text: tc.new_text.replace(specifier, &new_specifier), }) .collect(); - - return Ok(Some(tsc::CodeFixAction { - description, - changes, - commands: None, - fix_name: action.fix_name.clone(), - fix_id: None, - fix_all_description: None, - })); - } else if !import_mapper.is_valid_import(specifier, referrer) { - return Ok(None); - } - } + tsc::FileTextChanges { + file_name: c.file_name.clone(), + text_changes, + is_new_file: c.is_new_file, + } + }) + .collect(); + + Some(Cow::Owned(tsc::CodeFixAction { + description, + changes, + commands: None, + fix_name: action.fix_name.clone(), + fix_id: None, + fix_all_description: None, + })) + } else if !import_mapper.is_valid_import(specifier, referrer) { + None + } else { + Some(Cow::Borrowed(action)) } - - Ok(Some(action.clone())) } /// Determines if two TypeScript diagnostic codes are effectively equivalent. @@ -1004,8 +998,7 @@ impl CodeActionCollection { specifier, action, &language_server.get_ts_response_import_mapper(specifier), - )? - else { + ) else { return Ok(()); }; let edit = ts_changes_to_edit(&action.changes, language_server)?; @@ -1027,7 +1020,7 @@ impl CodeActionCollection { }); self .actions - .push(CodeActionKind::Tsc(code_action, action.clone())); + .push(CodeActionKind::Tsc(code_action, action.as_ref().clone())); if let Some(fix_id) = &action.fix_id { if let Some(CodeActionKind::Tsc(existing_fix_all, existing_action)) = -- cgit v1.2.3 From 2f0c25d33fc0c8bb8e6e92826b2549f429d68d42 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Thu, 31 Oct 2024 10:52:43 +0000 Subject: fix(lsp): include unstable features from editor settings (#26655) --- cli/lsp/config.rs | 12 +++++++++++- cli/lsp/language_server.rs | 10 +++------- cli/lsp/resolver.rs | 4 ++-- 3 files changed, 16 insertions(+), 10 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 3ffc9e657..cac0637a0 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -41,6 +41,7 @@ use deno_runtime::deno_node::PackageJson; use indexmap::IndexSet; use lsp_types::ClientCapabilities; use std::collections::BTreeMap; +use std::collections::BTreeSet; use std::collections::HashMap; use std::ops::Deref; use std::ops::DerefMut; @@ -1190,6 +1191,7 @@ pub struct ConfigData { pub resolver: Arc, pub sloppy_imports_resolver: Option>, pub import_map_from_settings: Option, + pub unstable: BTreeSet, watched_files: HashMap, } @@ -1587,9 +1589,16 @@ impl ConfigData { .join("\n") ); } + let unstable = member_dir + .workspace + .unstable_features() + .iter() + .chain(settings.unstable.as_deref()) + .cloned() + .collect::>(); let unstable_sloppy_imports = std::env::var("DENO_UNSTABLE_SLOPPY_IMPORTS") .is_ok() - || member_dir.workspace.has_unstable("sloppy-imports"); + || unstable.contains("sloppy-imports"); let sloppy_imports_resolver = unstable_sloppy_imports.then(|| { Arc::new(CliSloppyImportsResolver::new( SloppyImportsCachedFs::new_without_stat_cache(Arc::new( @@ -1630,6 +1639,7 @@ impl ConfigData { lockfile, npmrc, import_map_from_settings, + unstable, watched_files, } } diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 33ae539f8..a592245ce 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1384,14 +1384,10 @@ impl Inner { .clone(); fmt_options.use_tabs = Some(!params.options.insert_spaces); fmt_options.indent_width = Some(params.options.tab_size as u8); - let maybe_workspace = self - .config - .tree - .data_for_specifier(&specifier) - .map(|d| &d.member_dir.workspace); + let config_data = self.config.tree.data_for_specifier(&specifier); let unstable_options = UnstableFmtOptions { - component: maybe_workspace - .map(|w| w.has_unstable("fmt-component")) + component: config_data + .map(|d| d.unstable.contains("fmt-component")) .unwrap_or(false), }; let document = document.clone(); diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index c89273147..00587f8f5 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -555,8 +555,8 @@ fn create_graph_resolver( workspace.to_maybe_jsx_import_source_config().ok().flatten() }), maybe_vendor_dir: config_data.and_then(|d| d.vendor_dir.as_ref()), - bare_node_builtins_enabled: workspace - .is_some_and(|workspace| workspace.has_unstable("bare-node-builtins")), + bare_node_builtins_enabled: config_data + .is_some_and(|d| d.unstable.contains("bare-node-builtins")), sloppy_imports_resolver: config_data .and_then(|d| d.sloppy_imports_resolver.clone()), })) -- cgit v1.2.3 From 826e42a5b5880c974ae019a7a21aade6a718062c Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 1 Nov 2024 12:27:00 -0400 Subject: fix: improved support for cjs and cts modules (#26558) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * cts support * better cjs/cts type checking * deno compile cjs/cts support * More efficient detect cjs (going towards stabilization) * Determination of whether .js, .ts, .jsx, or .tsx is cjs or esm is only done after loading * Support `import x = require(...);` Co-authored-by: Bartek Iwańczuk --- cli/lsp/analysis.rs | 21 ++-- cli/lsp/code_lens.rs | 4 +- cli/lsp/config.rs | 2 +- cli/lsp/documents.rs | 20 +--- cli/lsp/language_server.rs | 2 +- cli/lsp/resolver.rs | 228 +++++++++++++++++++++++++++++++++--------- cli/lsp/testing/collectors.rs | 2 +- cli/lsp/tsc.rs | 27 +++-- 8 files changed, 220 insertions(+), 86 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 39f1ae648..0769c9047 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -36,7 +36,6 @@ use deno_semver::package::PackageReq; use deno_semver::package::PackageReqReference; use deno_semver::Version; use import_map::ImportMap; -use node_resolver::NpmResolver; use once_cell::sync::Lazy; use regex::Regex; use std::borrow::Cow; @@ -336,7 +335,12 @@ impl<'a> TsResponseImportMapper<'a> { .resolver .maybe_managed_npm_resolver(Some(&self.file_referrer)) { - if npm_resolver.in_npm_package(specifier) { + let in_npm_pkg = self + .resolver + .maybe_node_resolver(Some(&self.file_referrer)) + .map(|n| n.in_npm_package(specifier)) + .unwrap_or(false); + if in_npm_pkg { if let Ok(Some(pkg_id)) = npm_resolver.resolve_pkg_id_from_specifier(specifier) { @@ -1199,14 +1203,11 @@ impl CodeActionCollection { }), ); - match parsed_source.program_ref() { - deno_ast::swc::ast::Program::Module(module) => module - .body - .iter() - .find(|i| i.range().contains(&specifier_range)) - .map(|i| text_info.line_and_column_index(i.range().start)), - deno_ast::swc::ast::Program::Script(_) => None, - } + parsed_source + .program_ref() + .body() + .find(|i| i.range().contains(&specifier_range)) + .map(|i| text_info.line_and_column_index(i.range().start)) } async fn deno_types_for_npm_action( diff --git a/cli/lsp/code_lens.rs b/cli/lsp/code_lens.rs index e117888fb..a57ca3ac9 100644 --- a/cli/lsp/code_lens.rs +++ b/cli/lsp/code_lens.rs @@ -421,7 +421,7 @@ pub fn collect_test( ) -> Result, AnyError> { let mut collector = DenoTestCollector::new(specifier.clone(), parsed_source.clone()); - parsed_source.module().visit_with(&mut collector); + parsed_source.program().visit_with(&mut collector); Ok(collector.take()) } @@ -581,7 +581,7 @@ mod tests { .unwrap(); let mut collector = DenoTestCollector::new(specifier, parsed_module.clone()); - parsed_module.module().visit_with(&mut collector); + parsed_module.program().visit_with(&mut collector); assert_eq!( collector.take(), vec![ diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index cac0637a0..34bf64446 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -985,7 +985,7 @@ impl Config { | MediaType::Tsx => Some(&workspace_settings.typescript), MediaType::Json | MediaType::Wasm - | MediaType::TsBuildInfo + | MediaType::Css | MediaType::SourceMap | MediaType::Unknown => None, } diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 7d1ca6810..8609aed05 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -272,7 +272,7 @@ fn get_maybe_test_module_fut( parsed_source.specifier().clone(), parsed_source.text_info_lazy().clone(), ); - parsed_source.module().visit_with(&mut collector); + parsed_source.program().visit_with(&mut collector); Arc::new(collector.take()) }) .map(Result::ok) @@ -332,12 +332,8 @@ impl Document { .filter(|s| cache.is_valid_file_referrer(s)) .cloned() .or(file_referrer); - let media_type = resolve_media_type( - &specifier, - maybe_headers.as_ref(), - maybe_language_id, - &resolver, - ); + let media_type = + resolve_media_type(&specifier, maybe_headers.as_ref(), maybe_language_id); let (maybe_parsed_source, maybe_module) = if media_type_is_diagnosable(media_type) { parse_and_analyze_module( @@ -399,7 +395,6 @@ impl Document { &self.specifier, self.maybe_headers.as_ref(), self.maybe_language_id, - &resolver, ); let dependencies; let maybe_types_dependency; @@ -764,14 +759,7 @@ fn resolve_media_type( specifier: &ModuleSpecifier, maybe_headers: Option<&HashMap>, maybe_language_id: Option, - resolver: &LspResolver, ) -> MediaType { - if resolver.in_node_modules(specifier) { - if let Some(media_type) = resolver.node_media_type(specifier) { - return media_type; - } - } - if let Some(language_id) = maybe_language_id { return MediaType::from_specifier_and_content_type( specifier, @@ -1561,7 +1549,7 @@ fn parse_source( text: Arc, media_type: MediaType, ) -> ParsedSourceResult { - deno_ast::parse_module(deno_ast::ParseParams { + deno_ast::parse_program(deno_ast::ParseParams { specifier, text, media_type, diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index a592245ce..4fa0e3afb 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -904,7 +904,7 @@ impl Inner { | MediaType::Tsx => {} MediaType::Wasm | MediaType::SourceMap - | MediaType::TsBuildInfo + | MediaType::Css | MediaType::Unknown => { if path.extension().and_then(|s| s.to_str()) != Some("jsonc") { continue; diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 00587f8f5..9ce76005e 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -2,6 +2,8 @@ use dashmap::DashMap; use deno_ast::MediaType; +use deno_ast::ParsedSource; +use deno_cache_dir::npm::NpmCacheDir; use deno_cache_dir::HttpCache; use deno_config::workspace::PackageJsonDepResolution; use deno_config::workspace::WorkspaceResolver; @@ -14,15 +16,15 @@ use deno_path_util::url_to_file_path; use deno_runtime::deno_fs; use deno_runtime::deno_node::NodeResolver; use deno_runtime::deno_node::PackageJson; +use deno_runtime::deno_node::PackageJsonResolver; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageNv; use deno_semver::package::PackageReq; use indexmap::IndexMap; use node_resolver::errors::ClosestPkgJsonError; -use node_resolver::NodeResolution; +use node_resolver::InNpmPackageChecker; use node_resolver::NodeResolutionMode; -use node_resolver::NpmResolver; use std::borrow::Cow; use std::collections::BTreeMap; use std::collections::BTreeSet; @@ -36,6 +38,7 @@ use crate::args::create_default_npmrc; use crate::args::CacheSetting; use crate::args::CliLockfile; use crate::args::NpmInstallDepsProvider; +use crate::cache::DenoCacheEnvFsAdapter; use crate::graph_util::CliJsrUrlProvider; use crate::http_util::HttpClientProvider; use crate::lsp::config::Config; @@ -43,26 +46,32 @@ use crate::lsp::config::ConfigData; use crate::lsp::logging::lsp_warn; use crate::npm::create_cli_npm_resolver_for_lsp; use crate::npm::CliByonmNpmResolverCreateOptions; +use crate::npm::CliManagedInNpmPkgCheckerCreateOptions; +use crate::npm::CliManagedNpmResolverCreateOptions; use crate::npm::CliNpmResolver; use crate::npm::CliNpmResolverCreateOptions; -use crate::npm::CliNpmResolverManagedCreateOptions; use crate::npm::CliNpmResolverManagedSnapshotOption; +use crate::npm::CreateInNpmPkgCheckerOptions; use crate::npm::ManagedCliNpmResolver; -use crate::resolver::CjsResolutionStore; +use crate::resolver::CjsTracker; +use crate::resolver::CjsTrackerOptions; use crate::resolver::CliDenoResolverFs; use crate::resolver::CliGraphResolver; use crate::resolver::CliGraphResolverOptions; use crate::resolver::CliNodeResolver; use crate::resolver::WorkerCliNpmGraphResolver; +use crate::tsc::into_specifier_and_media_type; use crate::util::progress_bar::ProgressBar; use crate::util::progress_bar::ProgressBarStyle; #[derive(Debug, Clone)] struct LspScopeResolver { + cjs_tracker: Option>, graph_resolver: Arc, jsr_resolver: Option>, npm_resolver: Option>, node_resolver: Option>, + pkg_json_resolver: Option>, redirect_resolver: Option>, graph_imports: Arc>, config_data: Option>, @@ -71,10 +80,12 @@ struct LspScopeResolver { impl Default for LspScopeResolver { fn default() -> Self { Self { + cjs_tracker: None, graph_resolver: create_graph_resolver(None, None, None), jsr_resolver: None, npm_resolver: None, node_resolver: None, + pkg_json_resolver: None, redirect_resolver: None, graph_imports: Default::default(), config_data: None, @@ -90,14 +101,35 @@ impl LspScopeResolver { ) -> Self { let mut npm_resolver = None; let mut node_resolver = None; + let mut lsp_cjs_tracker = None; + let fs = Arc::new(deno_fs::RealFs); + let pkg_json_resolver = Arc::new(PackageJsonResolver::new( + deno_runtime::deno_node::DenoFsNodeResolverEnv::new(fs.clone()), + )); if let Some(http_client) = http_client_provider { npm_resolver = create_npm_resolver( config_data.map(|d| d.as_ref()), cache, http_client, + &pkg_json_resolver, ) .await; - node_resolver = create_node_resolver(npm_resolver.as_ref()); + if let Some(npm_resolver) = &npm_resolver { + let in_npm_pkg_checker = create_in_npm_pkg_checker(npm_resolver); + let cjs_tracker = create_cjs_tracker( + in_npm_pkg_checker.clone(), + pkg_json_resolver.clone(), + ); + lsp_cjs_tracker = + Some(Arc::new(LspCjsTracker::new(cjs_tracker.clone()))); + node_resolver = Some(create_node_resolver( + cjs_tracker, + fs.clone(), + in_npm_pkg_checker, + npm_resolver, + pkg_json_resolver.clone(), + )); + } } let graph_resolver = create_graph_resolver( config_data.map(|d| d.as_ref()), @@ -134,10 +166,12 @@ impl LspScopeResolver { }) .unwrap_or_default(); Self { + cjs_tracker: lsp_cjs_tracker, graph_resolver, jsr_resolver, npm_resolver, node_resolver, + pkg_json_resolver: Some(pkg_json_resolver), redirect_resolver, graph_imports, config_data: config_data.cloned(), @@ -147,18 +181,40 @@ impl LspScopeResolver { fn snapshot(&self) -> Arc { let npm_resolver = self.npm_resolver.as_ref().map(|r| r.clone_snapshotted()); - let node_resolver = create_node_resolver(npm_resolver.as_ref()); + let fs = Arc::new(deno_fs::RealFs); + let pkg_json_resolver = Arc::new(PackageJsonResolver::new( + deno_runtime::deno_node::DenoFsNodeResolverEnv::new(fs.clone()), + )); + let mut node_resolver = None; + let mut lsp_cjs_tracker = None; + if let Some(npm_resolver) = &npm_resolver { + let in_npm_pkg_checker = create_in_npm_pkg_checker(npm_resolver); + let cjs_tracker = create_cjs_tracker( + in_npm_pkg_checker.clone(), + pkg_json_resolver.clone(), + ); + lsp_cjs_tracker = Some(Arc::new(LspCjsTracker::new(cjs_tracker.clone()))); + node_resolver = Some(create_node_resolver( + cjs_tracker, + fs, + in_npm_pkg_checker, + npm_resolver, + pkg_json_resolver.clone(), + )); + } let graph_resolver = create_graph_resolver( self.config_data.as_deref(), npm_resolver.as_ref(), node_resolver.as_ref(), ); Arc::new(Self { + cjs_tracker: lsp_cjs_tracker, graph_resolver, jsr_resolver: self.jsr_resolver.clone(), npm_resolver, node_resolver, redirect_resolver: self.redirect_resolver.clone(), + pkg_json_resolver: Some(pkg_json_resolver), graph_imports: self.graph_imports.clone(), config_data: self.config_data.clone(), }) @@ -261,6 +317,22 @@ impl LspResolver { resolver.graph_resolver.create_graph_npm_resolver() } + pub fn maybe_cjs_tracker( + &self, + file_referrer: Option<&ModuleSpecifier>, + ) -> Option<&Arc> { + let resolver = self.get_scope_resolver(file_referrer); + resolver.cjs_tracker.as_ref() + } + + pub fn maybe_node_resolver( + &self, + file_referrer: Option<&ModuleSpecifier>, + ) -> Option<&Arc> { + let resolver = self.get_scope_resolver(file_referrer); + resolver.node_resolver.as_ref() + } + pub fn maybe_managed_npm_resolver( &self, file_referrer: Option<&ModuleSpecifier>, @@ -328,7 +400,7 @@ impl LspResolver { ) -> Option<(ModuleSpecifier, MediaType)> { let resolver = self.get_scope_resolver(file_referrer); let node_resolver = resolver.node_resolver.as_ref()?; - Some(NodeResolution::into_specifier_and_media_type(Some( + Some(into_specifier_and_media_type(Some( node_resolver .resolve_req_reference(req_ref, referrer, NodeResolutionMode::Types) .ok()?, @@ -346,14 +418,10 @@ impl LspResolver { .contains("/node_modules/") } - let global_npm_resolver = self - .get_scope_resolver(Some(specifier)) - .npm_resolver - .as_ref() - .and_then(|npm_resolver| npm_resolver.as_managed()) - .filter(|r| r.root_node_modules_path().is_none()); - if let Some(npm_resolver) = &global_npm_resolver { - if npm_resolver.in_npm_package(specifier) { + if let Some(node_resolver) = + &self.get_scope_resolver(Some(specifier)).node_resolver + { + if node_resolver.in_npm_package(specifier) { return true; } } @@ -361,18 +429,6 @@ impl LspResolver { has_node_modules_dir(specifier) } - pub fn node_media_type( - &self, - specifier: &ModuleSpecifier, - ) -> Option { - let resolver = self.get_scope_resolver(Some(specifier)); - let node_resolver = resolver.node_resolver.as_ref()?; - let resolution = node_resolver - .url_to_node_resolution(specifier.clone()) - .ok()?; - Some(NodeResolution::into_specifier_and_media_type(Some(resolution)).1) - } - pub fn is_bare_package_json_dep( &self, specifier_text: &str, @@ -398,10 +454,10 @@ impl LspResolver { referrer: &ModuleSpecifier, ) -> Result>, ClosestPkgJsonError> { let resolver = self.get_scope_resolver(Some(referrer)); - let Some(node_resolver) = resolver.node_resolver.as_ref() else { + let Some(pkg_json_resolver) = resolver.pkg_json_resolver.as_ref() else { return Ok(None); }; - node_resolver.get_closest_package_json(referrer) + pkg_json_resolver.get_closest_package_json(referrer) } pub fn resolve_redirects( @@ -457,11 +513,13 @@ async fn create_npm_resolver( config_data: Option<&ConfigData>, cache: &LspCache, http_client_provider: &Arc, + pkg_json_resolver: &Arc, ) -> Option> { let enable_byonm = config_data.map(|d| d.byonm).unwrap_or(false); let options = if enable_byonm { CliNpmResolverCreateOptions::Byonm(CliByonmNpmResolverCreateOptions { fs: CliDenoResolverFs(Arc::new(deno_fs::RealFs)), + pkg_json_resolver: pkg_json_resolver.clone(), root_node_modules_dir: config_data.and_then(|config_data| { config_data.node_modules_dir.clone().or_else(|| { url_to_file_path(&config_data.scope) @@ -471,7 +529,15 @@ async fn create_npm_resolver( }), }) } else { - CliNpmResolverCreateOptions::Managed(CliNpmResolverManagedCreateOptions { + let npmrc = config_data + .and_then(|d| d.npmrc.clone()) + .unwrap_or_else(create_default_npmrc); + let npm_cache_dir = Arc::new(NpmCacheDir::new( + &DenoCacheEnvFsAdapter(&deno_fs::RealFs), + cache.deno_dir().npm_folder_path(), + npmrc.get_all_known_registries_urls(), + )); + CliNpmResolverCreateOptions::Managed(CliManagedNpmResolverCreateOptions { http_client_provider: http_client_provider.clone(), snapshot: match config_data.and_then(|d| d.lockfile.as_ref()) { Some(lockfile) => { @@ -485,7 +551,7 @@ async fn create_npm_resolver( // updating it. Only the cache request should update the lockfile. maybe_lockfile: None, fs: Arc::new(deno_fs::RealFs), - npm_global_cache_dir: cache.deno_dir().npm_folder_path(), + npm_cache_dir, // Use an "only" cache setting in order to make the // user do an explicit "cache" command and prevent // the cache from being filled with lots of packages while @@ -496,9 +562,7 @@ async fn create_npm_resolver( .and_then(|d| d.node_modules_dir.clone()), // only used for top level install, so we can ignore this npm_install_deps_provider: Arc::new(NpmInstallDepsProvider::empty()), - npmrc: config_data - .and_then(|d| d.npmrc.clone()) - .unwrap_or_else(create_default_npmrc), + npmrc, npm_system_info: NpmSystemInfo::default(), lifecycle_scripts: Default::default(), }) @@ -506,28 +570,59 @@ async fn create_npm_resolver( Some(create_cli_npm_resolver_for_lsp(options).await) } -fn create_node_resolver( - npm_resolver: Option<&Arc>, -) -> Option> { - use once_cell::sync::Lazy; +fn create_cjs_tracker( + in_npm_pkg_checker: Arc, + pkg_json_resolver: Arc, +) -> Arc { + Arc::new(CjsTracker::new( + in_npm_pkg_checker, + pkg_json_resolver, + CjsTrackerOptions { + // todo(dsherret): support in the lsp by stabilizing the feature + // so that we don't have to pipe the config in here + unstable_detect_cjs: false, + }, + )) +} - // it's not ideal to share this across all scopes and to - // never clear it, but it's fine for the time being - static CJS_RESOLUTIONS: Lazy> = - Lazy::new(Default::default); +fn create_in_npm_pkg_checker( + npm_resolver: &Arc, +) -> Arc { + crate::npm::create_in_npm_pkg_checker(match npm_resolver.as_inner() { + crate::npm::InnerCliNpmResolverRef::Byonm(_) => { + CreateInNpmPkgCheckerOptions::Byonm + } + crate::npm::InnerCliNpmResolverRef::Managed(m) => { + CreateInNpmPkgCheckerOptions::Managed( + CliManagedInNpmPkgCheckerCreateOptions { + root_cache_dir_url: m.global_cache_root_url(), + maybe_node_modules_path: m.maybe_node_modules_path(), + }, + ) + } + }) +} - let npm_resolver = npm_resolver?; - let fs = Arc::new(deno_fs::RealFs); +fn create_node_resolver( + cjs_tracker: Arc, + fs: Arc, + in_npm_pkg_checker: Arc, + npm_resolver: &Arc, + pkg_json_resolver: Arc, +) -> Arc { let node_resolver_inner = Arc::new(NodeResolver::new( deno_runtime::deno_node::DenoFsNodeResolverEnv::new(fs.clone()), + in_npm_pkg_checker.clone(), npm_resolver.clone().into_npm_resolver(), + pkg_json_resolver.clone(), )); - Some(Arc::new(CliNodeResolver::new( - CJS_RESOLUTIONS.clone(), + Arc::new(CliNodeResolver::new( + cjs_tracker.clone(), fs, + in_npm_pkg_checker, node_resolver_inner, npm_resolver.clone(), - ))) + )) } fn create_graph_resolver( @@ -702,6 +797,45 @@ impl RedirectResolver { } } +#[derive(Debug)] +pub struct LspCjsTracker { + cjs_tracker: Arc, +} + +impl LspCjsTracker { + pub fn new(cjs_tracker: Arc) -> Self { + Self { cjs_tracker } + } + + pub fn is_cjs( + &self, + specifier: &ModuleSpecifier, + media_type: MediaType, + maybe_parsed_source: Option<&ParsedSource>, + ) -> bool { + if let Some(module_kind) = + self.cjs_tracker.get_known_kind(specifier, media_type) + { + module_kind.is_cjs() + } else { + let maybe_is_script = maybe_parsed_source.map(|p| p.compute_is_script()); + maybe_is_script + .and_then(|is_script| { + self + .cjs_tracker + .is_cjs_with_known_is_script(specifier, media_type, is_script) + .ok() + }) + .unwrap_or_else(|| { + self + .cjs_tracker + .is_maybe_cjs(specifier, media_type) + .unwrap_or(false) + }) + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/cli/lsp/testing/collectors.rs b/cli/lsp/testing/collectors.rs index 2f2ddb877..2dd7ec0d9 100644 --- a/cli/lsp/testing/collectors.rs +++ b/cli/lsp/testing/collectors.rs @@ -650,7 +650,7 @@ pub mod tests { .unwrap(); let text_info = parsed_module.text_info_lazy().clone(); let mut collector = TestCollector::new(specifier, text_info); - parsed_module.module().visit_with(&mut collector); + parsed_module.program().visit_with(&mut collector); collector.take() } diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 0f31d7dd3..5fcdb3575 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -4362,14 +4362,25 @@ fn op_load<'s>( None } else { let asset_or_document = state.get_asset_or_document(&specifier); - asset_or_document.map(|doc| LoadResponse { - data: doc.text(), - script_kind: crate::tsc::as_ts_script_kind(doc.media_type()), - version: state.script_version(&specifier), - is_cjs: matches!( - doc.media_type(), - MediaType::Cjs | MediaType::Cts | MediaType::Dcts - ), + asset_or_document.map(|doc| { + let maybe_cjs_tracker = state + .state_snapshot + .resolver + .maybe_cjs_tracker(Some(&specifier)); + LoadResponse { + data: doc.text(), + script_kind: crate::tsc::as_ts_script_kind(doc.media_type()), + version: state.script_version(&specifier), + is_cjs: maybe_cjs_tracker + .map(|t| { + t.is_cjs( + &specifier, + doc.media_type(), + doc.maybe_parsed_source().and_then(|p| p.as_ref().ok()), + ) + }) + .unwrap_or(false), + } }) }; -- cgit v1.2.3 From d95f06f20b97839d9a2aaeb590f4ed30b6383b0f Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Mon, 4 Nov 2024 16:36:21 +0000 Subject: perf(lsp): don't walk coverage directory (#26715) --- cli/lsp/language_server.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'cli/lsp') diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 4fa0e3afb..61a02f036 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -863,7 +863,10 @@ impl Inner { // We ignore these directories by default because there is a // high likelihood they aren't relevant. Someone can opt-into // them by specifying one of them as an enabled path. - if matches!(dir_name.as_str(), "vendor" | "node_modules" | ".git") { + if matches!( + dir_name.as_str(), + "vendor" | "coverage" | "node_modules" | ".git" + ) { continue; } // ignore cargo target directories for anyone using Deno with Rust @@ -3944,7 +3947,9 @@ mod tests { fn test_walk_workspace() { let temp_dir = TempDir::new(); temp_dir.create_dir_all("root1/vendor/"); + temp_dir.create_dir_all("root1/coverage/"); temp_dir.write("root1/vendor/mod.ts", ""); // no, vendor + temp_dir.write("root1/coverage/mod.ts", ""); // no, coverage temp_dir.create_dir_all("root1/node_modules/"); temp_dir.write("root1/node_modules/mod.ts", ""); // no, node_modules -- cgit v1.2.3 From 5088b25f2315fa45e912377356a89ba2a44dbcda Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Wed, 6 Nov 2024 06:26:46 +0000 Subject: feat(lsp): auto-import completions from byonm dependencies (#26680) --- cli/lsp/analysis.rs | 76 ++++++++++++++++++++++----------- cli/lsp/documents.rs | 28 ------------- cli/lsp/language_server.rs | 13 ++++-- cli/lsp/resolver.rs | 45 ++++++++++++++++++++ cli/lsp/tsc.rs | 102 ++++++++++++++++++++++++++++++++------------- 5 files changed, 179 insertions(+), 85 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 0769c9047..98215855c 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -12,7 +12,9 @@ use super::urls::url_to_uri; use crate::args::jsr_url; use crate::lsp::search::PackageSearchApi; use crate::tools::lint::CliLinter; +use crate::util::path::relative_specifier; use deno_config::workspace::MappedResolution; +use deno_graph::source::ResolutionMode; use deno_lint::diagnostic::LintDiagnosticRange; use deno_ast::SourceRange; @@ -228,6 +230,7 @@ pub struct TsResponseImportMapper<'a> { documents: &'a Documents, maybe_import_map: Option<&'a ImportMap>, resolver: &'a LspResolver, + tsc_specifier_map: &'a tsc::TscSpecifierMap, file_referrer: ModuleSpecifier, } @@ -236,12 +239,14 @@ impl<'a> TsResponseImportMapper<'a> { documents: &'a Documents, maybe_import_map: Option<&'a ImportMap>, resolver: &'a LspResolver, + tsc_specifier_map: &'a tsc::TscSpecifierMap, file_referrer: &ModuleSpecifier, ) -> Self { Self { documents, maybe_import_map, resolver, + tsc_specifier_map, file_referrer: file_referrer.clone(), } } @@ -387,6 +392,11 @@ impl<'a> TsResponseImportMapper<'a> { } } } + } else if let Some(dep_name) = self + .resolver + .file_url_to_package_json_dep(specifier, Some(&self.file_referrer)) + { + return Some(dep_name); } // check if the import map has this specifier @@ -457,19 +467,36 @@ impl<'a> TsResponseImportMapper<'a> { specifier: &str, referrer: &ModuleSpecifier, ) -> Option { - if let Ok(specifier) = referrer.join(specifier) { - if let Some(specifier) = self.check_specifier(&specifier, referrer) { - return Some(specifier); - } - } - let specifier = specifier.strip_suffix(".js").unwrap_or(specifier); - for ext in SUPPORTED_EXTENSIONS { - let specifier_with_ext = format!("{specifier}{ext}"); - if self - .documents - .contains_import(&specifier_with_ext, referrer) + let specifier_stem = specifier.strip_suffix(".js").unwrap_or(specifier); + let specifiers = std::iter::once(Cow::Borrowed(specifier)).chain( + SUPPORTED_EXTENSIONS + .iter() + .map(|ext| Cow::Owned(format!("{specifier_stem}{ext}"))), + ); + for specifier in specifiers { + if let Some(specifier) = self + .resolver + .as_graph_resolver(Some(&self.file_referrer)) + .resolve( + &specifier, + &deno_graph::Range { + specifier: referrer.clone(), + start: deno_graph::Position::zeroed(), + end: deno_graph::Position::zeroed(), + }, + ResolutionMode::Types, + ) + .ok() + .and_then(|s| self.tsc_specifier_map.normalize(s.as_str()).ok()) + .filter(|s| self.documents.exists(s, Some(&self.file_referrer))) { - return Some(specifier_with_ext); + if let Some(specifier) = self + .check_specifier(&specifier, referrer) + .or_else(|| relative_specifier(referrer, &specifier)) + .filter(|s| !s.contains("/node_modules/")) + { + return Some(specifier); + } } } None @@ -559,8 +586,9 @@ fn try_reverse_map_package_json_exports( pub fn fix_ts_import_changes( referrer: &ModuleSpecifier, changes: &[tsc::FileTextChanges], - import_mapper: &TsResponseImportMapper, + language_server: &language_server::Inner, ) -> Result, AnyError> { + let import_mapper = language_server.get_ts_response_import_mapper(referrer); let mut r = Vec::new(); for change in changes { let mut text_changes = Vec::new(); @@ -605,7 +633,7 @@ pub fn fix_ts_import_changes( fn fix_ts_import_action<'a>( referrer: &ModuleSpecifier, action: &'a tsc::CodeFixAction, - import_mapper: &TsResponseImportMapper, + language_server: &language_server::Inner, ) -> Option> { if !matches!( action.fix_name.as_str(), @@ -621,6 +649,7 @@ fn fix_ts_import_action<'a>( let Some(specifier) = specifier else { return Some(Cow::Borrowed(action)); }; + let import_mapper = language_server.get_ts_response_import_mapper(referrer); if let Some(new_specifier) = import_mapper.check_unresolved_specifier(specifier, referrer) { @@ -728,7 +757,7 @@ pub fn ts_changes_to_edit( })) } -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct CodeActionData { pub specifier: ModuleSpecifier, @@ -998,11 +1027,8 @@ impl CodeActionCollection { "The action returned from TypeScript is unsupported.", )); } - let Some(action) = fix_ts_import_action( - specifier, - action, - &language_server.get_ts_response_import_mapper(specifier), - ) else { + let Some(action) = fix_ts_import_action(specifier, action, language_server) + else { return Ok(()); }; let edit = ts_changes_to_edit(&action.changes, language_server)?; @@ -1051,10 +1077,12 @@ impl CodeActionCollection { specifier: &ModuleSpecifier, diagnostic: &lsp::Diagnostic, ) { - let data = Some(json!({ - "specifier": specifier, - "fixId": action.fix_id, - })); + let data = action.fix_id.as_ref().map(|fix_id| { + json!(CodeActionData { + specifier: specifier.clone(), + fix_id: fix_id.clone(), + }) + }); let title = if let Some(description) = &action.fix_all_description { description.clone() } else { diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 8609aed05..ce13c3215 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1059,34 +1059,6 @@ impl Documents { self.cache.is_valid_file_referrer(specifier) } - /// Return `true` if the provided specifier can be resolved to a document, - /// otherwise `false`. - pub fn contains_import( - &self, - specifier: &str, - referrer: &ModuleSpecifier, - ) -> bool { - let file_referrer = self.get_file_referrer(referrer); - let maybe_specifier = self - .resolver - .as_graph_resolver(file_referrer.as_deref()) - .resolve( - specifier, - &deno_graph::Range { - specifier: referrer.clone(), - start: deno_graph::Position::zeroed(), - end: deno_graph::Position::zeroed(), - }, - ResolutionMode::Types, - ) - .ok(); - if let Some(import_specifier) = maybe_specifier { - self.exists(&import_specifier, file_referrer.as_deref()) - } else { - false - } - } - pub fn resolve_document_specifier( &self, specifier: &ModuleSpecifier, diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 61a02f036..2554fa34b 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1837,7 +1837,7 @@ impl Inner { fix_ts_import_changes( &code_action_data.specifier, &combined_code_actions.changes, - &self.get_ts_response_import_mapper(&code_action_data.specifier), + self, ) .map_err(|err| { error!("Unable to remap changes: {:#}", err); @@ -1890,7 +1890,7 @@ impl Inner { refactor_edit_info.edits = fix_ts_import_changes( &action_data.specifier, &refactor_edit_info.edits, - &self.get_ts_response_import_mapper(&action_data.specifier), + self, ) .map_err(|err| { error!("Unable to remap changes: {:#}", err); @@ -1921,7 +1921,8 @@ impl Inner { // todo(dsherret): this should probably just take the resolver itself // as the import map is an implementation detail .and_then(|d| d.resolver.maybe_import_map()), - self.resolver.as_ref(), + &self.resolver, + &self.ts_server.specifier_map, file_referrer, ) } @@ -2284,7 +2285,11 @@ impl Inner { .into(), scope.cloned(), ) - .await; + .await + .unwrap_or_else(|err| { + error!("Unable to get completion info from TypeScript: {:#}", err); + None + }); if let Some(completions) = maybe_completion_info { response = Some( diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 9ce76005e..f5df24d57 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -74,6 +74,7 @@ struct LspScopeResolver { pkg_json_resolver: Option>, redirect_resolver: Option>, graph_imports: Arc>, + package_json_deps_by_resolution: Arc>, config_data: Option>, } @@ -88,6 +89,7 @@ impl Default for LspScopeResolver { pkg_json_resolver: None, redirect_resolver: None, graph_imports: Default::default(), + package_json_deps_by_resolution: Default::default(), config_data: None, } } @@ -165,6 +167,33 @@ impl LspScopeResolver { ) }) .unwrap_or_default(); + let package_json_deps_by_resolution = (|| { + let node_resolver = node_resolver.as_ref()?; + let package_json = config_data?.maybe_pkg_json()?; + let referrer = package_json.specifier(); + let dependencies = package_json.dependencies.as_ref()?; + let result = dependencies + .iter() + .flat_map(|(name, _)| { + let req_ref = + NpmPackageReqReference::from_str(&format!("npm:{name}")).ok()?; + let specifier = into_specifier_and_media_type(Some( + node_resolver + .resolve_req_reference( + &req_ref, + &referrer, + NodeResolutionMode::Types, + ) + .ok()?, + )) + .0; + Some((specifier, name.clone())) + }) + .collect(); + Some(result) + })(); + let package_json_deps_by_resolution = + Arc::new(package_json_deps_by_resolution.unwrap_or_default()); Self { cjs_tracker: lsp_cjs_tracker, graph_resolver, @@ -174,6 +203,7 @@ impl LspScopeResolver { pkg_json_resolver: Some(pkg_json_resolver), redirect_resolver, graph_imports, + package_json_deps_by_resolution, config_data: config_data.cloned(), } } @@ -216,6 +246,9 @@ impl LspScopeResolver { redirect_resolver: self.redirect_resolver.clone(), pkg_json_resolver: Some(pkg_json_resolver), graph_imports: self.graph_imports.clone(), + package_json_deps_by_resolution: self + .package_json_deps_by_resolution + .clone(), config_data: self.config_data.clone(), }) } @@ -407,6 +440,18 @@ impl LspResolver { ))) } + pub fn file_url_to_package_json_dep( + &self, + specifier: &ModuleSpecifier, + file_referrer: Option<&ModuleSpecifier>, + ) -> Option { + let resolver = self.get_scope_resolver(file_referrer); + resolver + .package_json_deps_by_resolution + .get(specifier) + .cloned() + } + pub fn in_node_modules(&self, specifier: &ModuleSpecifier) -> bool { fn has_node_modules_dir(specifier: &ModuleSpecifier) -> bool { // consider any /node_modules/ directory as being in the node_modules diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 5fcdb3575..6f63ced5b 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -236,7 +236,7 @@ pub struct TsServer { performance: Arc, sender: mpsc::UnboundedSender, receiver: Mutex>>, - specifier_map: Arc, + pub specifier_map: Arc, inspector_server: Mutex>>, pending_change: Mutex>, } @@ -882,20 +882,22 @@ impl TsServer { options: GetCompletionsAtPositionOptions, format_code_settings: FormatCodeSettings, scope: Option, - ) -> Option { + ) -> Result, AnyError> { let req = TscRequest::GetCompletionsAtPosition(Box::new(( self.specifier_map.denormalize(&specifier), position, options, format_code_settings, ))); - match self.request(snapshot, req, scope).await { - Ok(maybe_info) => maybe_info, - Err(err) => { - log::error!("Unable to get completion info from TypeScript: {:#}", err); - None - } - } + self + .request::>(snapshot, req, scope) + .await + .map(|mut info| { + if let Some(info) = &mut info { + info.normalize(&self.specifier_map); + } + info + }) } pub async fn get_completion_details( @@ -3642,6 +3644,12 @@ pub struct CompletionInfo { } impl CompletionInfo { + fn normalize(&mut self, specifier_map: &TscSpecifierMap) { + for entry in &mut self.entries { + entry.normalize(specifier_map); + } + } + pub fn as_completion_response( &self, line_index: Arc, @@ -3703,11 +3711,17 @@ pub struct CompletionItemData { #[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] -struct CompletionEntryDataImport { +struct CompletionEntryDataAutoImport { module_specifier: String, file_name: String, } +#[derive(Debug)] +pub struct CompletionNormalizedAutoImportData { + raw: CompletionEntryDataAutoImport, + normalized: ModuleSpecifier, +} + #[derive(Debug, Default, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct CompletionEntry { @@ -3740,9 +3754,28 @@ pub struct CompletionEntry { is_import_statement_completion: Option, #[serde(skip_serializing_if = "Option::is_none")] data: Option, + /// This is not from tsc, we add it for convenience during normalization. + /// Represents `self.data.file_name`, but normalized. + #[serde(skip)] + auto_import_data: Option, } impl CompletionEntry { + fn normalize(&mut self, specifier_map: &TscSpecifierMap) { + let Some(data) = &self.data else { + return; + }; + let Ok(raw) = + serde_json::from_value::(data.clone()) + else { + return; + }; + if let Ok(normalized) = specifier_map.normalize(&raw.file_name) { + self.auto_import_data = + Some(CompletionNormalizedAutoImportData { raw, normalized }); + } + } + fn get_commit_characters( &self, info: &CompletionInfo, @@ -3891,25 +3924,24 @@ impl CompletionEntry { if let Some(source) = &self.source { let mut display_source = source.clone(); - if let Some(data) = &self.data { - if let Ok(import_data) = - serde_json::from_value::(data.clone()) + if let Some(import_data) = &self.auto_import_data { + if let Some(new_module_specifier) = language_server + .get_ts_response_import_mapper(specifier) + .check_specifier(&import_data.normalized, specifier) + .or_else(|| relative_specifier(specifier, &import_data.normalized)) { - if let Ok(import_specifier) = resolve_url(&import_data.file_name) { - if let Some(new_module_specifier) = language_server - .get_ts_response_import_mapper(specifier) - .check_specifier(&import_specifier, specifier) - .or_else(|| relative_specifier(specifier, &import_specifier)) - { - display_source.clone_from(&new_module_specifier); - if new_module_specifier != import_data.module_specifier { - specifier_rewrite = - Some((import_data.module_specifier, new_module_specifier)); - } - } else if source.starts_with(jsr_url().as_str()) { - return None; - } + if new_module_specifier.contains("/node_modules/") { + return None; + } + display_source.clone_from(&new_module_specifier); + if new_module_specifier != import_data.raw.module_specifier { + specifier_rewrite = Some(( + import_data.raw.module_specifier.clone(), + new_module_specifier, + )); } + } else if source.starts_with(jsr_url().as_str()) { + return None; } } // We want relative or bare (import-mapped or otherwise) specifiers to @@ -4212,6 +4244,13 @@ impl TscSpecifierMap { return specifier.to_string(); } let mut specifier = original.to_string(); + if specifier.contains("/node_modules/.deno/") + && !specifier.contains("/node_modules/@types/node/") + { + // The ts server doesn't give completions from files in + // `node_modules/.deno/`. We work around it like this. + specifier = specifier.replace("/node_modules/", "/$node_modules/"); + } let media_type = MediaType::from_specifier(original); // If the URL-inferred media type doesn't correspond to tsc's path-inferred // media type, force it to be the same by appending an extension. @@ -4329,7 +4368,7 @@ fn op_is_cancelled(state: &mut OpState) -> bool { fn op_is_node_file(state: &mut OpState, #[string] path: String) -> bool { let state = state.borrow::(); let mark = state.performance.mark("tsc.op.op_is_node_file"); - let r = match ModuleSpecifier::parse(&path) { + let r = match state.specifier_map.normalize(path) { Ok(specifier) => state.state_snapshot.resolver.in_node_modules(&specifier), Err(_) => false, }; @@ -4609,7 +4648,10 @@ fn op_script_names(state: &mut OpState) -> ScriptNames { for doc in &docs { let specifier = doc.specifier(); let is_open = doc.is_open(); - if is_open || specifier.scheme() == "file" { + if is_open + || (specifier.scheme() == "file" + && !state.state_snapshot.resolver.in_node_modules(specifier)) + { let script_names = doc .scope() .and_then(|s| result.by_scope.get_mut(s)) @@ -6035,6 +6077,7 @@ mod tests { Some(temp_dir.url()), ) .await + .unwrap() .unwrap(); assert_eq!(info.entries.len(), 22); let details = ts_server @@ -6194,6 +6237,7 @@ mod tests { Some(temp_dir.url()), ) .await + .unwrap() .unwrap(); let entry = info .entries -- cgit v1.2.3 From 7d326c269cfb8463d530ee5dd4bcabb7499e1ba7 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Tue, 12 Nov 2024 13:15:32 +0000 Subject: fix(lsp): skip code action edits that can't be converted (#26831) --- cli/lsp/analysis.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 98215855c..683a59c21 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -10,6 +10,7 @@ use super::tsc; use super::urls::url_to_uri; use crate::args::jsr_url; +use crate::lsp::logging::lsp_warn; use crate::lsp::search::PackageSearchApi; use crate::tools::lint::CliLinter; use crate::util::path::relative_specifier; @@ -747,8 +748,14 @@ pub fn ts_changes_to_edit( ) -> Result, AnyError> { let mut text_document_edits = Vec::new(); for change in changes { - let text_document_edit = change.to_text_document_edit(language_server)?; - text_document_edits.push(text_document_edit); + let edit = match change.to_text_document_edit(language_server) { + Ok(e) => e, + Err(err) => { + lsp_warn!("Couldn't covert text document edit: {:#}", err); + continue; + } + }; + text_document_edits.push(edit); } Ok(Some(lsp::WorkspaceEdit { changes: None, -- cgit v1.2.3 From f091d1ad69b4e5217ae3272b641171781a372c4f Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 13 Nov 2024 10:10:09 -0500 Subject: feat(node): stabilize detecting if CJS via `"type": "commonjs"` in a package.json (#26439) This will respect `"type": "commonjs"` in a package.json to determine if `.js`/`.jsx`/`.ts`/.tsx` files are CJS or ESM. If the file is found to be ESM it will be loaded as ESM though. --- cli/lsp/analysis.rs | 37 ++++-- cli/lsp/completions.rs | 22 ++-- cli/lsp/config.rs | 12 ++ cli/lsp/diagnostics.rs | 1 + cli/lsp/documents.rs | 136 +++++++++++++++++---- cli/lsp/language_server.rs | 40 ++++++- cli/lsp/repl.rs | 2 +- cli/lsp/resolver.rs | 287 +++++++++++++++++++++++++++++---------------- cli/lsp/tsc.rs | 34 +++--- 9 files changed, 400 insertions(+), 171 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 683a59c21..9f26de70c 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -39,6 +39,7 @@ use deno_semver::package::PackageReq; use deno_semver::package::PackageReqReference; use deno_semver::Version; use import_map::ImportMap; +use node_resolver::NodeModuleKind; use once_cell::sync::Lazy; use regex::Regex; use std::borrow::Cow; @@ -467,6 +468,7 @@ impl<'a> TsResponseImportMapper<'a> { &self, specifier: &str, referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, ) -> Option { let specifier_stem = specifier.strip_suffix(".js").unwrap_or(specifier); let specifiers = std::iter::once(Cow::Borrowed(specifier)).chain( @@ -477,7 +479,7 @@ impl<'a> TsResponseImportMapper<'a> { for specifier in specifiers { if let Some(specifier) = self .resolver - .as_graph_resolver(Some(&self.file_referrer)) + .as_cli_resolver(Some(&self.file_referrer)) .resolve( &specifier, &deno_graph::Range { @@ -485,6 +487,7 @@ impl<'a> TsResponseImportMapper<'a> { start: deno_graph::Position::zeroed(), end: deno_graph::Position::zeroed(), }, + referrer_kind, ResolutionMode::Types, ) .ok() @@ -507,10 +510,11 @@ impl<'a> TsResponseImportMapper<'a> { &self, specifier_text: &str, referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, ) -> bool { self .resolver - .as_graph_resolver(Some(&self.file_referrer)) + .as_cli_resolver(Some(&self.file_referrer)) .resolve( specifier_text, &deno_graph::Range { @@ -518,6 +522,7 @@ impl<'a> TsResponseImportMapper<'a> { start: deno_graph::Position::zeroed(), end: deno_graph::Position::zeroed(), }, + referrer_kind, deno_graph::source::ResolutionMode::Types, ) .is_ok() @@ -586,6 +591,7 @@ fn try_reverse_map_package_json_exports( /// like an import and rewrite the import specifier to include the extension pub fn fix_ts_import_changes( referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, changes: &[tsc::FileTextChanges], language_server: &language_server::Inner, ) -> Result, AnyError> { @@ -602,8 +608,8 @@ pub fn fix_ts_import_changes( if let Some(captures) = IMPORT_SPECIFIER_RE.captures(line) { let specifier = captures.iter().skip(1).find_map(|s| s).unwrap().as_str(); - if let Some(new_specifier) = - import_mapper.check_unresolved_specifier(specifier, referrer) + if let Some(new_specifier) = import_mapper + .check_unresolved_specifier(specifier, referrer, referrer_kind) { line.replace(specifier, &new_specifier) } else { @@ -633,6 +639,7 @@ pub fn fix_ts_import_changes( /// resolution by Deno (includes the extension). fn fix_ts_import_action<'a>( referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, action: &'a tsc::CodeFixAction, language_server: &language_server::Inner, ) -> Option> { @@ -652,7 +659,7 @@ fn fix_ts_import_action<'a>( }; let import_mapper = language_server.get_ts_response_import_mapper(referrer); if let Some(new_specifier) = - import_mapper.check_unresolved_specifier(specifier, referrer) + import_mapper.check_unresolved_specifier(specifier, referrer, referrer_kind) { let description = action.description.replace(specifier, &new_specifier); let changes = action @@ -683,7 +690,7 @@ fn fix_ts_import_action<'a>( fix_id: None, fix_all_description: None, })) - } else if !import_mapper.is_valid_import(specifier, referrer) { + } else if !import_mapper.is_valid_import(specifier, referrer, referrer_kind) { None } else { Some(Cow::Borrowed(action)) @@ -1017,6 +1024,7 @@ impl CodeActionCollection { pub fn add_ts_fix_action( &mut self, specifier: &ModuleSpecifier, + specifier_kind: NodeModuleKind, action: &tsc::CodeFixAction, diagnostic: &lsp::Diagnostic, language_server: &language_server::Inner, @@ -1034,7 +1042,8 @@ impl CodeActionCollection { "The action returned from TypeScript is unsupported.", )); } - let Some(action) = fix_ts_import_action(specifier, action, language_server) + let Some(action) = + fix_ts_import_action(specifier, specifier_kind, action, language_server) else { return Ok(()); }; @@ -1276,6 +1285,9 @@ impl CodeActionCollection { import_start_from_specifier(document, i) })?; let referrer = document.specifier(); + let referrer_kind = language_server + .is_cjs_resolver + .get_doc_module_kind(document); let file_referrer = document.file_referrer(); let config_data = language_server .config @@ -1298,10 +1310,11 @@ impl CodeActionCollection { if !config_data.byonm { return None; } - if !language_server - .resolver - .is_bare_package_json_dep(&dep_key, referrer) - { + if !language_server.resolver.is_bare_package_json_dep( + &dep_key, + referrer, + referrer_kind, + ) { return None; } NpmPackageReqReference::from_str(&format!("npm:{}", &dep_key)).ok()? @@ -1320,7 +1333,7 @@ impl CodeActionCollection { } if language_server .resolver - .npm_to_file_url(&npm_ref, document.specifier(), file_referrer) + .npm_to_file_url(&npm_ref, referrer, referrer_kind, file_referrer) .is_some() { // The package import has types. diff --git a/cli/lsp/completions.rs b/cli/lsp/completions.rs index 1590743b2..3ee8ae93e 100644 --- a/cli/lsp/completions.rs +++ b/cli/lsp/completions.rs @@ -9,6 +9,7 @@ use super::jsr::CliJsrSearchApi; use super::lsp_custom; use super::npm::CliNpmSearchApi; use super::registries::ModuleRegistry; +use super::resolver::LspIsCjsResolver; use super::resolver::LspResolver; use super::search::PackageSearchApi; use super::tsc; @@ -35,6 +36,7 @@ use deno_semver::package::PackageNv; use import_map::ImportMap; use indexmap::IndexSet; use lsp_types::CompletionList; +use node_resolver::NodeModuleKind; use once_cell::sync::Lazy; use regex::Regex; use tower_lsp::lsp_types as lsp; @@ -159,15 +161,17 @@ pub async fn get_import_completions( jsr_search_api: &CliJsrSearchApi, npm_search_api: &CliNpmSearchApi, documents: &Documents, + is_cjs_resolver: &LspIsCjsResolver, resolver: &LspResolver, maybe_import_map: Option<&ImportMap>, ) -> Option { let document = documents.get(specifier)?; + let specifier_kind = is_cjs_resolver.get_doc_module_kind(&document); let file_referrer = document.file_referrer(); let (text, _, range) = document.get_maybe_dependency(position)?; let range = to_narrow_lsp_range(document.text_info(), &range); let resolved = resolver - .as_graph_resolver(file_referrer) + .as_cli_resolver(file_referrer) .resolve( &text, &Range { @@ -175,6 +179,7 @@ pub async fn get_import_completions( start: deno_graph::Position::zeroed(), end: deno_graph::Position::zeroed(), }, + specifier_kind, ResolutionMode::Execution, ) .ok(); @@ -201,7 +206,7 @@ pub async fn get_import_completions( // completions for import map specifiers Some(lsp::CompletionResponse::List(completion_list)) } else if let Some(completion_list) = - get_local_completions(specifier, &text, &range, resolver) + get_local_completions(specifier, specifier_kind, &text, &range, resolver) { // completions for local relative modules Some(lsp::CompletionResponse::List(completion_list)) @@ -355,24 +360,26 @@ fn get_import_map_completions( /// Return local completions that are relative to the base specifier. fn get_local_completions( - base: &ModuleSpecifier, + referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, text: &str, range: &lsp::Range, resolver: &LspResolver, ) -> Option { - if base.scheme() != "file" { + if referrer.scheme() != "file" { return None; } let parent = &text[..text.char_indices().rfind(|(_, c)| *c == '/')?.0 + 1]; let resolved_parent = resolver - .as_graph_resolver(Some(base)) + .as_cli_resolver(Some(referrer)) .resolve( parent, &Range { - specifier: base.clone(), + specifier: referrer.clone(), start: deno_graph::Position::zeroed(), end: deno_graph::Position::zeroed(), }, + referrer_kind, ResolutionMode::Execution, ) .ok()?; @@ -385,7 +392,7 @@ fn get_local_completions( let de = de.ok()?; let label = de.path().file_name()?.to_string_lossy().to_string(); let entry_specifier = resolve_path(de.path().to_str()?, &cwd).ok()?; - if entry_specifier == *base { + if entry_specifier == *referrer { return None; } let full_text = format!("{parent}{label}"); @@ -905,6 +912,7 @@ mod tests { ModuleSpecifier::from_file_path(file_c).expect("could not create"); let actual = get_local_completions( &specifier, + NodeModuleKind::Esm, "./", &lsp::Range { start: lsp::Position { diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 34bf64446..ea77e36bc 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -4,6 +4,7 @@ use deno_ast::MediaType; use deno_config::deno_json::DenoJsonCache; use deno_config::deno_json::FmtConfig; use deno_config::deno_json::FmtOptionsConfig; +use deno_config::deno_json::JsxImportSourceConfig; use deno_config::deno_json::LintConfig; use deno_config::deno_json::NodeModulesDirMode; use deno_config::deno_json::TestConfig; @@ -1654,6 +1655,17 @@ impl ConfigData { self.member_dir.maybe_pkg_json() } + pub fn maybe_jsx_import_source_config( + &self, + ) -> Option { + self + .member_dir + .workspace + .to_maybe_jsx_import_source_config() + .ok() + .flatten() + } + pub fn scope_contains_specifier(&self, specifier: &ModuleSpecifier) -> bool { specifier.as_str().starts_with(self.scope.as_str()) || self diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 83c00d27e..e4fb82e58 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -1707,6 +1707,7 @@ mod tests { documents: Arc::new(documents), assets: Default::default(), config: Arc::new(config), + is_cjs_resolver: Default::default(), resolver, }, ) diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index ce13c3215..b62fa8553 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -3,7 +3,9 @@ use super::cache::calculate_fs_version; use super::cache::LspCache; use super::config::Config; +use super::resolver::LspIsCjsResolver; use super::resolver::LspResolver; +use super::resolver::SingleReferrerGraphResolver; use super::testing::TestCollector; use super::testing::TestModule; use super::text::LineIndex; @@ -33,6 +35,7 @@ use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; use indexmap::IndexMap; use indexmap::IndexSet; +use node_resolver::NodeModuleKind; use std::borrow::Cow; use std::collections::BTreeMap; use std::collections::BTreeSet; @@ -293,6 +296,8 @@ pub struct Document { /// Contains the last-known-good set of dependencies from parsing the module. config: Arc, dependencies: Arc>, + /// If this is maybe a CJS script and maybe not an ES module. + is_script: Option, // TODO(nayeemrmn): This is unused, use it for scope attribution for remote // modules. file_referrer: Option, @@ -323,6 +328,7 @@ impl Document { maybe_lsp_version: Option, maybe_language_id: Option, maybe_headers: Option>, + is_cjs_resolver: &LspIsCjsResolver, resolver: Arc, config: Arc, cache: &Arc, @@ -342,6 +348,7 @@ impl Document { maybe_headers.as_ref(), media_type, file_referrer.as_ref(), + is_cjs_resolver, &resolver, ) } else { @@ -367,6 +374,7 @@ impl Document { file_referrer.as_ref(), ), file_referrer, + is_script: maybe_module.as_ref().map(|m| m.is_script), maybe_types_dependency, line_index, maybe_language_id, @@ -388,6 +396,7 @@ impl Document { fn with_new_config( &self, + is_cjs_resolver: &LspIsCjsResolver, resolver: Arc, config: Arc, ) -> Arc { @@ -399,6 +408,7 @@ impl Document { let dependencies; let maybe_types_dependency; let maybe_parsed_source; + let is_script; let maybe_test_module_fut; if media_type != self.media_type { let parsed_source_result = @@ -408,6 +418,7 @@ impl Document { &parsed_source_result, self.maybe_headers.as_ref(), self.file_referrer.as_ref(), + is_cjs_resolver, &resolver, ) .ok(); @@ -415,6 +426,7 @@ impl Document { .as_ref() .map(|m| Arc::new(m.dependencies.clone())) .unwrap_or_default(); + is_script = maybe_module.as_ref().map(|m| m.is_script); maybe_types_dependency = maybe_module .as_ref() .and_then(|m| Some(Arc::new(m.maybe_types_dependency.clone()?))); @@ -422,10 +434,19 @@ impl Document { maybe_test_module_fut = get_maybe_test_module_fut(maybe_parsed_source.as_ref(), &config); } else { - let graph_resolver = - resolver.as_graph_resolver(self.file_referrer.as_ref()); + let cli_resolver = resolver.as_cli_resolver(self.file_referrer.as_ref()); let npm_resolver = resolver.create_graph_npm_resolver(self.file_referrer.as_ref()); + let config_data = resolver.as_config_data(self.file_referrer.as_ref()); + let jsx_import_source_config = + config_data.and_then(|d| d.maybe_jsx_import_source_config()); + let resolver = SingleReferrerGraphResolver { + valid_referrer: &self.specifier, + referrer_kind: is_cjs_resolver + .get_lsp_referrer_kind(&self.specifier, self.is_script), + cli_resolver, + jsx_import_source_config: jsx_import_source_config.as_ref(), + }; dependencies = Arc::new( self .dependencies @@ -436,7 +457,7 @@ impl Document { d.with_new_resolver( s, &CliJsrUrlProvider, - Some(graph_resolver), + Some(&resolver), Some(&npm_resolver), ), ) @@ -446,10 +467,11 @@ impl Document { maybe_types_dependency = self.maybe_types_dependency.as_ref().map(|d| { Arc::new(d.with_new_resolver( &CliJsrUrlProvider, - Some(graph_resolver), + Some(&resolver), Some(&npm_resolver), )) }); + is_script = self.is_script; maybe_parsed_source = self.maybe_parsed_source().cloned(); maybe_test_module_fut = self .maybe_test_module_fut @@ -461,6 +483,7 @@ impl Document { // updated properties dependencies, file_referrer: self.file_referrer.clone(), + is_script, maybe_types_dependency, maybe_navigation_tree: Mutex::new(None), // maintain - this should all be copies/clones @@ -485,6 +508,7 @@ impl Document { fn with_change( &self, + is_cjs_resolver: &LspIsCjsResolver, version: i32, changes: Vec, ) -> Result, AnyError> { @@ -518,6 +542,7 @@ impl Document { self.maybe_headers.as_ref(), media_type, self.file_referrer.as_ref(), + is_cjs_resolver, self.resolver.as_ref(), ) } else { @@ -541,6 +566,7 @@ impl Document { get_maybe_test_module_fut(maybe_parsed_source.as_ref(), &self.config); Ok(Arc::new(Self { config: self.config.clone(), + is_script: maybe_module.as_ref().map(|m| m.is_script), specifier: self.specifier.clone(), file_referrer: self.file_referrer.clone(), maybe_fs_version: self.maybe_fs_version.clone(), @@ -575,6 +601,7 @@ impl Document { ), maybe_language_id: self.maybe_language_id, dependencies: self.dependencies.clone(), + is_script: self.is_script, maybe_types_dependency: self.maybe_types_dependency.clone(), text: self.text.clone(), text_info_cell: once_cell::sync::OnceCell::new(), @@ -602,6 +629,7 @@ impl Document { ), maybe_language_id: self.maybe_language_id, dependencies: self.dependencies.clone(), + is_script: self.is_script, maybe_types_dependency: self.maybe_types_dependency.clone(), text: self.text.clone(), text_info_cell: once_cell::sync::OnceCell::new(), @@ -650,6 +678,13 @@ impl Document { }) } + /// If this is maybe a CJS script and maybe not an ES module. + /// + /// Use `LspIsCjsResolver` to determine for sure. + pub fn is_script(&self) -> Option { + self.is_script + } + pub fn line_index(&self) -> Arc { self.line_index.clone() } @@ -797,6 +832,7 @@ impl FileSystemDocuments { pub fn get( &self, specifier: &ModuleSpecifier, + is_cjs_resolver: &LspIsCjsResolver, resolver: &Arc, config: &Arc, cache: &Arc, @@ -820,7 +856,14 @@ impl FileSystemDocuments { }; if dirty { // attempt to update the file on the file system - self.refresh_document(specifier, resolver, config, cache, file_referrer) + self.refresh_document( + specifier, + is_cjs_resolver, + resolver, + config, + cache, + file_referrer, + ) } else { old_doc } @@ -831,6 +874,7 @@ impl FileSystemDocuments { fn refresh_document( &self, specifier: &ModuleSpecifier, + is_cjs_resolver: &LspIsCjsResolver, resolver: &Arc, config: &Arc, cache: &Arc, @@ -847,6 +891,7 @@ impl FileSystemDocuments { None, None, None, + is_cjs_resolver, resolver.clone(), config.clone(), cache, @@ -863,6 +908,7 @@ impl FileSystemDocuments { None, None, None, + is_cjs_resolver, resolver.clone(), config.clone(), cache, @@ -890,6 +936,7 @@ impl FileSystemDocuments { None, None, maybe_headers, + is_cjs_resolver, resolver.clone(), config.clone(), cache, @@ -930,6 +977,11 @@ pub struct Documents { /// The DENO_DIR that the documents looks for non-file based modules. cache: Arc, config: Arc, + /// Resolver for detecting if a document is CJS or ESM. + is_cjs_resolver: Arc, + /// A resolver that takes into account currently loaded import map and JSX + /// settings. + resolver: Arc, /// A flag that indicates that stated data is potentially invalid and needs to /// be recalculated before being considered valid. dirty: bool, @@ -937,9 +989,6 @@ pub struct Documents { open_docs: HashMap>, /// Documents stored on the file system. file_system_docs: Arc, - /// A resolver that takes into account currently loaded import map and JSX - /// settings. - resolver: Arc, /// The npm package requirements found in npm specifiers. npm_reqs_by_scope: Arc, BTreeSet>>, @@ -970,6 +1019,7 @@ impl Documents { // the cache for remote modules here in order to get the // x-typescript-types? None, + &self.is_cjs_resolver, self.resolver.clone(), self.config.clone(), &self.cache, @@ -1004,7 +1054,7 @@ impl Documents { )) })?; self.dirty = true; - let doc = doc.with_change(version, changes)?; + let doc = doc.with_change(&self.is_cjs_resolver, version, changes)?; self.open_docs.insert(doc.specifier().clone(), doc.clone()); Ok(doc) } @@ -1133,6 +1183,7 @@ impl Documents { if let Some(old_doc) = old_doc { self.file_system_docs.get( specifier, + &self.is_cjs_resolver, &self.resolver, &self.config, &self.cache, @@ -1157,6 +1208,7 @@ impl Documents { } else { self.file_system_docs.get( &specifier, + &self.is_cjs_resolver, &self.resolver, &self.config, &self.cache, @@ -1215,12 +1267,15 @@ impl Documents { referrer: &ModuleSpecifier, file_referrer: Option<&ModuleSpecifier>, ) -> Vec> { - let document = self.get(referrer); - let file_referrer = document + let referrer_doc = self.get(referrer); + let file_referrer = referrer_doc .as_ref() .and_then(|d| d.file_referrer()) .or(file_referrer); - let dependencies = document.as_ref().map(|d| d.dependencies()); + let dependencies = referrer_doc.as_ref().map(|d| d.dependencies()); + let referrer_kind = self + .is_cjs_resolver + .get_maybe_doc_module_kind(referrer, referrer_doc.as_deref()); let mut results = Vec::new(); for raw_specifier in raw_specifiers { if raw_specifier.starts_with("asset:") { @@ -1237,31 +1292,35 @@ impl Documents { results.push(self.resolve_dependency( specifier, referrer, + referrer_kind, file_referrer, )); } else if let Some(specifier) = dep.maybe_code.maybe_specifier() { results.push(self.resolve_dependency( specifier, referrer, + referrer_kind, file_referrer, )); } else { results.push(None); } } else if let Ok(specifier) = - self.resolver.as_graph_resolver(file_referrer).resolve( + self.resolver.as_cli_resolver(file_referrer).resolve( raw_specifier, &deno_graph::Range { specifier: referrer.clone(), start: deno_graph::Position::zeroed(), end: deno_graph::Position::zeroed(), }, + referrer_kind, ResolutionMode::Types, ) { results.push(self.resolve_dependency( &specifier, referrer, + referrer_kind, file_referrer, )); } else { @@ -1280,7 +1339,11 @@ impl Documents { ) { self.config = Arc::new(config.clone()); self.cache = Arc::new(cache.clone()); + self.is_cjs_resolver = Arc::new(LspIsCjsResolver::new(cache)); self.resolver = resolver.clone(); + + node_resolver::PackageJsonThreadLocalCache::clear(); + { let fs_docs = &self.file_system_docs; // Clean up non-existent documents. @@ -1300,14 +1363,21 @@ impl Documents { if !config.specifier_enabled(doc.specifier()) { continue; } - *doc = doc.with_new_config(self.resolver.clone(), self.config.clone()); + *doc = doc.with_new_config( + &self.is_cjs_resolver, + self.resolver.clone(), + self.config.clone(), + ); } for mut doc in self.file_system_docs.docs.iter_mut() { if !config.specifier_enabled(doc.specifier()) { continue; } - *doc.value_mut() = - doc.with_new_config(self.resolver.clone(), self.config.clone()); + *doc.value_mut() = doc.with_new_config( + &self.is_cjs_resolver, + self.resolver.clone(), + self.config.clone(), + ); } self.open_docs = open_docs; let mut preload_count = 0; @@ -1324,6 +1394,7 @@ impl Documents { { fs_docs.refresh_document( specifier, + &self.is_cjs_resolver, &self.resolver, &self.config, &self.cache, @@ -1409,6 +1480,7 @@ impl Documents { &self, specifier: &ModuleSpecifier, referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, file_referrer: Option<&ModuleSpecifier>, ) -> Option<(ModuleSpecifier, MediaType)> { if let Some(module_name) = specifier.as_str().strip_prefix("node:") { @@ -1422,10 +1494,12 @@ impl Documents { let mut specifier = specifier.clone(); let mut media_type = None; if let Ok(npm_ref) = NpmPackageReqReference::from_specifier(&specifier) { - let (s, mt) = - self - .resolver - .npm_to_file_url(&npm_ref, referrer, file_referrer)?; + let (s, mt) = self.resolver.npm_to_file_url( + &npm_ref, + referrer, + referrer_kind, + file_referrer, + )?; specifier = s; media_type = Some(mt); } @@ -1435,7 +1509,8 @@ impl Documents { return Some((specifier, media_type)); }; if let Some(types) = doc.maybe_types_dependency().maybe_specifier() { - self.resolve_dependency(types, &specifier, file_referrer) + let specifier_kind = self.is_cjs_resolver.get_doc_module_kind(&doc); + self.resolve_dependency(types, &specifier, specifier_kind, file_referrer) } else { Some((doc.specifier().clone(), doc.media_type())) } @@ -1503,6 +1578,7 @@ fn parse_and_analyze_module( maybe_headers: Option<&HashMap>, media_type: MediaType, file_referrer: Option<&ModuleSpecifier>, + is_cjs_resolver: &LspIsCjsResolver, resolver: &LspResolver, ) -> (Option, Option) { let parsed_source_result = parse_source(specifier.clone(), text, media_type); @@ -1511,6 +1587,7 @@ fn parse_and_analyze_module( &parsed_source_result, maybe_headers, file_referrer, + is_cjs_resolver, resolver, ); (Some(parsed_source_result), Some(module_result)) @@ -1536,11 +1613,26 @@ fn analyze_module( parsed_source_result: &ParsedSourceResult, maybe_headers: Option<&HashMap>, file_referrer: Option<&ModuleSpecifier>, + is_cjs_resolver: &LspIsCjsResolver, resolver: &LspResolver, ) -> ModuleResult { match parsed_source_result { Ok(parsed_source) => { let npm_resolver = resolver.create_graph_npm_resolver(file_referrer); + let cli_resolver = resolver.as_cli_resolver(file_referrer); + let config_data = resolver.as_config_data(file_referrer); + let valid_referrer = specifier.clone(); + let jsx_import_source_config = + config_data.and_then(|d| d.maybe_jsx_import_source_config()); + let resolver = SingleReferrerGraphResolver { + valid_referrer: &valid_referrer, + referrer_kind: is_cjs_resolver.get_lsp_referrer_kind( + &specifier, + Some(parsed_source.compute_is_script()), + ), + cli_resolver, + jsx_import_source_config: jsx_import_source_config.as_ref(), + }; Ok(deno_graph::parse_module_from_ast( deno_graph::ParseModuleFromAstOptions { graph_kind: deno_graph::GraphKind::TypesOnly, @@ -1551,7 +1643,7 @@ fn analyze_module( // dynamic imports like import(`./dir/${something}`) in the LSP file_system: &deno_graph::source::NullFileSystem, jsr_url_provider: &CliJsrUrlProvider, - maybe_resolver: Some(resolver.as_graph_resolver(file_referrer)), + maybe_resolver: Some(&resolver), maybe_npm_resolver: Some(&npm_resolver), }, )) diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 2554fa34b..b2bd72416 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -22,6 +22,7 @@ use deno_semver::jsr::JsrPackageReqReference; use indexmap::Equivalent; use indexmap::IndexSet; use log::error; +use node_resolver::NodeModuleKind; use serde::Deserialize; use serde_json::from_value; use std::collections::BTreeMap; @@ -77,6 +78,7 @@ use super::parent_process_checker; use super::performance::Performance; use super::refactor; use super::registries::ModuleRegistry; +use super::resolver::LspIsCjsResolver; use super::resolver::LspResolver; use super::testing; use super::text; @@ -144,6 +146,7 @@ pub struct StateSnapshot { pub project_version: usize, pub assets: AssetsSnapshot, pub config: Arc, + pub is_cjs_resolver: Arc, pub documents: Arc, pub resolver: Arc, } @@ -203,6 +206,7 @@ pub struct Inner { pub documents: Documents, http_client_provider: Arc, initial_cwd: PathBuf, + pub is_cjs_resolver: Arc, jsr_search_api: CliJsrSearchApi, /// Handles module registries, which allow discovery of modules module_registry: ModuleRegistry, @@ -480,6 +484,7 @@ impl Inner { let initial_cwd = std::env::current_dir().unwrap_or_else(|_| { panic!("Could not resolve current working directory") }); + let is_cjs_resolver = Arc::new(LspIsCjsResolver::new(&cache)); Self { assets, @@ -491,6 +496,7 @@ impl Inner { documents, http_client_provider, initial_cwd: initial_cwd.clone(), + is_cjs_resolver, jsr_search_api, project_version: 0, task_queue: Default::default(), @@ -601,6 +607,7 @@ impl Inner { project_version: self.project_version, assets: self.assets.snapshot(), config: Arc::new(self.config.clone()), + is_cjs_resolver: self.is_cjs_resolver.clone(), documents: Arc::new(self.documents.clone()), resolver: self.resolver.snapshot(), }) @@ -622,6 +629,7 @@ impl Inner { } }); self.cache = LspCache::new(global_cache_url); + self.is_cjs_resolver = Arc::new(LspIsCjsResolver::new(&self.cache)); let deno_dir = self.cache.deno_dir(); let workspace_settings = self.config.workspace_settings(); let maybe_root_path = self @@ -982,7 +990,7 @@ impl Inner { spawn(async move { let specifier = { let inner = ls.inner.read().await; - let resolver = inner.resolver.as_graph_resolver(Some(&referrer)); + let resolver = inner.resolver.as_cli_resolver(Some(&referrer)); let Ok(specifier) = resolver.resolve( &specifier, &deno_graph::Range { @@ -990,6 +998,7 @@ impl Inner { start: deno_graph::Position::zeroed(), end: deno_graph::Position::zeroed(), }, + NodeModuleKind::Esm, deno_graph::source::ResolutionMode::Types, ) else { return; @@ -1622,6 +1631,10 @@ impl Inner { let file_diagnostics = self .diagnostics_server .get_ts_diagnostics(&specifier, asset_or_doc.document_lsp_version()); + let specifier_kind = asset_or_doc + .document() + .map(|d| self.is_cjs_resolver.get_doc_module_kind(d)) + .unwrap_or(NodeModuleKind::Esm); let mut includes_no_cache = false; for diagnostic in &fixable_diagnostics { match diagnostic.source.as_deref() { @@ -1660,7 +1673,13 @@ impl Inner { .await; for action in actions { code_actions - .add_ts_fix_action(&specifier, &action, diagnostic, self) + .add_ts_fix_action( + &specifier, + specifier_kind, + &action, + diagnostic, + self, + ) .map_err(|err| { error!("Unable to convert fix: {:#}", err); LspError::internal_error() @@ -1806,10 +1825,9 @@ impl Inner { error!("Unable to decode code action data: {:#}", err); LspError::invalid_params("The CodeAction's data is invalid.") })?; - let scope = self - .get_asset_or_document(&code_action_data.specifier) - .ok() - .and_then(|d| d.scope().cloned()); + let maybe_asset_or_doc = + self.get_asset_or_document(&code_action_data.specifier).ok(); + let scope = maybe_asset_or_doc.as_ref().and_then(|d| d.scope().cloned()); let combined_code_actions = self .ts_server .get_combined_code_fix( @@ -1836,6 +1854,11 @@ impl Inner { let changes = if code_action_data.fix_id == "fixMissingImport" { fix_ts_import_changes( &code_action_data.specifier, + maybe_asset_or_doc + .as_ref() + .and_then(|d| d.document()) + .map(|d| self.is_cjs_resolver.get_doc_module_kind(d)) + .unwrap_or(NodeModuleKind::Esm), &combined_code_actions.changes, self, ) @@ -1889,6 +1912,10 @@ impl Inner { if kind_suffix == ".rewrite.function.returnType" { refactor_edit_info.edits = fix_ts_import_changes( &action_data.specifier, + asset_or_doc + .document() + .map(|d| self.is_cjs_resolver.get_doc_module_kind(d)) + .unwrap_or(NodeModuleKind::Esm), &refactor_edit_info.edits, self, ) @@ -2238,6 +2265,7 @@ impl Inner { &self.jsr_search_api, &self.npm_search_api, &self.documents, + &self.is_cjs_resolver, self.resolver.as_ref(), self .config diff --git a/cli/lsp/repl.rs b/cli/lsp/repl.rs index fa5809045..b4aaa8cd0 100644 --- a/cli/lsp/repl.rs +++ b/cli/lsp/repl.rs @@ -263,7 +263,7 @@ impl ReplLanguageServer { } fn get_document_uri(&self) -> Uri { - uri_parse_unencoded(self.cwd_uri.join("$deno$repl.ts").unwrap().as_str()) + uri_parse_unencoded(self.cwd_uri.join("$deno$repl.mts").unwrap().as_str()) .unwrap() } } diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index f5df24d57..37f63b912 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -2,16 +2,18 @@ use dashmap::DashMap; use deno_ast::MediaType; -use deno_ast::ParsedSource; use deno_cache_dir::npm::NpmCacheDir; use deno_cache_dir::HttpCache; +use deno_config::deno_json::JsxImportSourceConfig; use deno_config::workspace::PackageJsonDepResolution; use deno_config::workspace::WorkspaceResolver; use deno_core::url::Url; -use deno_graph::source::Resolver; +use deno_graph::source::ResolutionMode; use deno_graph::GraphImport; use deno_graph::ModuleSpecifier; +use deno_graph::Range; use deno_npm::NpmSystemInfo; +use deno_path_util::url_from_directory_path; use deno_path_util::url_to_file_path; use deno_runtime::deno_fs; use deno_runtime::deno_node::NodeResolver; @@ -24,6 +26,7 @@ use deno_semver::package::PackageReq; use indexmap::IndexMap; use node_resolver::errors::ClosestPkgJsonError; use node_resolver::InNpmPackageChecker; +use node_resolver::NodeModuleKind; use node_resolver::NodeResolutionMode; use std::borrow::Cow; use std::collections::BTreeMap; @@ -33,6 +36,7 @@ use std::collections::HashSet; use std::sync::Arc; use super::cache::LspCache; +use super::documents::Document; use super::jsr::JsrCacheResolver; use crate::args::create_default_npmrc; use crate::args::CacheSetting; @@ -53,21 +57,20 @@ use crate::npm::CliNpmResolverCreateOptions; use crate::npm::CliNpmResolverManagedSnapshotOption; use crate::npm::CreateInNpmPkgCheckerOptions; use crate::npm::ManagedCliNpmResolver; -use crate::resolver::CjsTracker; -use crate::resolver::CjsTrackerOptions; use crate::resolver::CliDenoResolverFs; -use crate::resolver::CliGraphResolver; -use crate::resolver::CliGraphResolverOptions; use crate::resolver::CliNodeResolver; +use crate::resolver::CliResolver; +use crate::resolver::CliResolverOptions; +use crate::resolver::IsCjsResolver; use crate::resolver::WorkerCliNpmGraphResolver; use crate::tsc::into_specifier_and_media_type; +use crate::util::fs::canonicalize_path_maybe_not_exists; use crate::util::progress_bar::ProgressBar; use crate::util::progress_bar::ProgressBarStyle; #[derive(Debug, Clone)] struct LspScopeResolver { - cjs_tracker: Option>, - graph_resolver: Arc, + resolver: Arc, jsr_resolver: Option>, npm_resolver: Option>, node_resolver: Option>, @@ -81,8 +84,7 @@ struct LspScopeResolver { impl Default for LspScopeResolver { fn default() -> Self { Self { - cjs_tracker: None, - graph_resolver: create_graph_resolver(None, None, None), + resolver: create_cli_resolver(None, None, None), jsr_resolver: None, npm_resolver: None, node_resolver: None, @@ -103,7 +105,6 @@ impl LspScopeResolver { ) -> Self { let mut npm_resolver = None; let mut node_resolver = None; - let mut lsp_cjs_tracker = None; let fs = Arc::new(deno_fs::RealFs); let pkg_json_resolver = Arc::new(PackageJsonResolver::new( deno_runtime::deno_node::DenoFsNodeResolverEnv::new(fs.clone()), @@ -118,14 +119,7 @@ impl LspScopeResolver { .await; if let Some(npm_resolver) = &npm_resolver { let in_npm_pkg_checker = create_in_npm_pkg_checker(npm_resolver); - let cjs_tracker = create_cjs_tracker( - in_npm_pkg_checker.clone(), - pkg_json_resolver.clone(), - ); - lsp_cjs_tracker = - Some(Arc::new(LspCjsTracker::new(cjs_tracker.clone()))); node_resolver = Some(create_node_resolver( - cjs_tracker, fs.clone(), in_npm_pkg_checker, npm_resolver, @@ -133,7 +127,7 @@ impl LspScopeResolver { )); } } - let graph_resolver = create_graph_resolver( + let cli_resolver = create_cli_resolver( config_data.map(|d| d.as_ref()), npm_resolver.as_ref(), node_resolver.as_ref(), @@ -146,7 +140,9 @@ impl LspScopeResolver { cache.for_specifier(config_data.map(|d| d.scope.as_ref())), config_data.and_then(|d| d.lockfile.clone()), ))); - let npm_graph_resolver = graph_resolver.create_graph_npm_resolver(); + let npm_graph_resolver = cli_resolver.create_graph_npm_resolver(); + let maybe_jsx_import_source_config = + config_data.and_then(|d| d.maybe_jsx_import_source_config()); let graph_imports = config_data .and_then(|d| d.member_dir.workspace.to_compiler_option_types().ok()) .map(|imports| { @@ -154,11 +150,18 @@ impl LspScopeResolver { imports .into_iter() .map(|(referrer, imports)| { + let resolver = SingleReferrerGraphResolver { + valid_referrer: &referrer, + referrer_kind: NodeModuleKind::Esm, + cli_resolver: &cli_resolver, + jsx_import_source_config: maybe_jsx_import_source_config + .as_ref(), + }; let graph_import = GraphImport::new( &referrer, imports, &CliJsrUrlProvider, - Some(graph_resolver.as_ref()), + Some(&resolver), Some(&npm_graph_resolver), ); (referrer, graph_import) @@ -182,6 +185,8 @@ impl LspScopeResolver { .resolve_req_reference( &req_ref, &referrer, + // todo(dsherret): this is wrong because it doesn't consider CJS referrers + NodeModuleKind::Esm, NodeResolutionMode::Types, ) .ok()?, @@ -195,8 +200,7 @@ impl LspScopeResolver { let package_json_deps_by_resolution = Arc::new(package_json_deps_by_resolution.unwrap_or_default()); Self { - cjs_tracker: lsp_cjs_tracker, - graph_resolver, + resolver: cli_resolver, jsr_resolver, npm_resolver, node_resolver, @@ -216,30 +220,22 @@ impl LspScopeResolver { deno_runtime::deno_node::DenoFsNodeResolverEnv::new(fs.clone()), )); let mut node_resolver = None; - let mut lsp_cjs_tracker = None; if let Some(npm_resolver) = &npm_resolver { let in_npm_pkg_checker = create_in_npm_pkg_checker(npm_resolver); - let cjs_tracker = create_cjs_tracker( - in_npm_pkg_checker.clone(), - pkg_json_resolver.clone(), - ); - lsp_cjs_tracker = Some(Arc::new(LspCjsTracker::new(cjs_tracker.clone()))); node_resolver = Some(create_node_resolver( - cjs_tracker, fs, in_npm_pkg_checker, npm_resolver, pkg_json_resolver.clone(), )); } - let graph_resolver = create_graph_resolver( + let graph_resolver = create_cli_resolver( self.config_data.as_deref(), npm_resolver.as_ref(), node_resolver.as_ref(), ); Arc::new(Self { - cjs_tracker: lsp_cjs_tracker, - graph_resolver, + resolver: graph_resolver, jsr_resolver: self.jsr_resolver.clone(), npm_resolver, node_resolver, @@ -334,12 +330,12 @@ impl LspResolver { } } - pub fn as_graph_resolver( + pub fn as_cli_resolver( &self, file_referrer: Option<&ModuleSpecifier>, - ) -> &dyn Resolver { + ) -> &CliResolver { let resolver = self.get_scope_resolver(file_referrer); - resolver.graph_resolver.as_ref() + resolver.resolver.as_ref() } pub fn create_graph_npm_resolver( @@ -347,15 +343,15 @@ impl LspResolver { file_referrer: Option<&ModuleSpecifier>, ) -> WorkerCliNpmGraphResolver { let resolver = self.get_scope_resolver(file_referrer); - resolver.graph_resolver.create_graph_npm_resolver() + resolver.resolver.create_graph_npm_resolver() } - pub fn maybe_cjs_tracker( + pub fn as_config_data( &self, file_referrer: Option<&ModuleSpecifier>, - ) -> Option<&Arc> { + ) -> Option<&Arc> { let resolver = self.get_scope_resolver(file_referrer); - resolver.cjs_tracker.as_ref() + resolver.config_data.as_ref() } pub fn maybe_node_resolver( @@ -429,13 +425,19 @@ impl LspResolver { &self, req_ref: &NpmPackageReqReference, referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, file_referrer: Option<&ModuleSpecifier>, ) -> Option<(ModuleSpecifier, MediaType)> { let resolver = self.get_scope_resolver(file_referrer); let node_resolver = resolver.node_resolver.as_ref()?; Some(into_specifier_and_media_type(Some( node_resolver - .resolve_req_reference(req_ref, referrer, NodeResolutionMode::Types) + .resolve_req_reference( + req_ref, + referrer, + referrer_kind, + NodeResolutionMode::Types, + ) .ok()?, ))) } @@ -478,6 +480,7 @@ impl LspResolver { &self, specifier_text: &str, referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, ) -> bool { let resolver = self.get_scope_resolver(Some(referrer)); let Some(node_resolver) = resolver.node_resolver.as_ref() else { @@ -487,6 +490,7 @@ impl LspResolver { .resolve_if_for_npm_pkg( specifier_text, referrer, + referrer_kind, NodeResolutionMode::Types, ) .ok() @@ -615,21 +619,6 @@ async fn create_npm_resolver( Some(create_cli_npm_resolver_for_lsp(options).await) } -fn create_cjs_tracker( - in_npm_pkg_checker: Arc, - pkg_json_resolver: Arc, -) -> Arc { - Arc::new(CjsTracker::new( - in_npm_pkg_checker, - pkg_json_resolver, - CjsTrackerOptions { - // todo(dsherret): support in the lsp by stabilizing the feature - // so that we don't have to pipe the config in here - unstable_detect_cjs: false, - }, - )) -} - fn create_in_npm_pkg_checker( npm_resolver: &Arc, ) -> Arc { @@ -649,7 +638,6 @@ fn create_in_npm_pkg_checker( } fn create_node_resolver( - cjs_tracker: Arc, fs: Arc, in_npm_pkg_checker: Arc, npm_resolver: &Arc, @@ -662,7 +650,6 @@ fn create_node_resolver( pkg_json_resolver.clone(), )); Arc::new(CliNodeResolver::new( - cjs_tracker.clone(), fs, in_npm_pkg_checker, node_resolver_inner, @@ -670,13 +657,12 @@ fn create_node_resolver( )) } -fn create_graph_resolver( +fn create_cli_resolver( config_data: Option<&ConfigData>, npm_resolver: Option<&Arc>, node_resolver: Option<&Arc>, -) -> Arc { - let workspace = config_data.map(|d| &d.member_dir.workspace); - Arc::new(CliGraphResolver::new(CliGraphResolverOptions { +) -> Arc { + Arc::new(CliResolver::new(CliResolverOptions { node_resolver: node_resolver.cloned(), npm_resolver: npm_resolver.cloned(), workspace_resolver: config_data.map(|d| d.resolver.clone()).unwrap_or_else( @@ -691,9 +677,6 @@ fn create_graph_resolver( )) }, ), - maybe_jsx_import_source_config: workspace.and_then(|workspace| { - workspace.to_maybe_jsx_import_source_config().ok().flatten() - }), maybe_vendor_dir: config_data.and_then(|d| d.vendor_dir.as_ref()), bare_node_builtins_enabled: config_data .is_some_and(|d| d.unstable.contains("bare-node-builtins")), @@ -726,6 +709,141 @@ impl std::fmt::Debug for RedirectResolver { } } +#[derive(Debug)] +pub struct LspIsCjsResolver { + inner: IsCjsResolver, +} + +impl Default for LspIsCjsResolver { + fn default() -> Self { + LspIsCjsResolver::new(&Default::default()) + } +} + +impl LspIsCjsResolver { + pub fn new(cache: &LspCache) -> Self { + #[derive(Debug)] + struct LspInNpmPackageChecker { + global_cache_dir: ModuleSpecifier, + } + + impl LspInNpmPackageChecker { + pub fn new(cache: &LspCache) -> Self { + let npm_folder_path = cache.deno_dir().npm_folder_path(); + Self { + global_cache_dir: url_from_directory_path( + &canonicalize_path_maybe_not_exists(&npm_folder_path) + .unwrap_or(npm_folder_path), + ) + .unwrap_or_else(|_| { + ModuleSpecifier::parse("file:///invalid/").unwrap() + }), + } + } + } + + impl InNpmPackageChecker for LspInNpmPackageChecker { + fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool { + if specifier.scheme() != "file" { + return false; + } + if specifier + .as_str() + .starts_with(self.global_cache_dir.as_str()) + { + return true; + } + specifier.as_str().contains("/node_modules/") + } + } + + let fs = Arc::new(deno_fs::RealFs); + let pkg_json_resolver = Arc::new(PackageJsonResolver::new( + deno_runtime::deno_node::DenoFsNodeResolverEnv::new(fs.clone()), + )); + + LspIsCjsResolver { + inner: IsCjsResolver::new( + Arc::new(LspInNpmPackageChecker::new(cache)), + pkg_json_resolver, + crate::resolver::IsCjsResolverOptions { + detect_cjs: true, + is_node_main: false, + }, + ), + } + } + + pub fn get_maybe_doc_module_kind( + &self, + specifier: &ModuleSpecifier, + maybe_document: Option<&Document>, + ) -> NodeModuleKind { + self.get_lsp_referrer_kind( + specifier, + maybe_document.and_then(|d| d.is_script()), + ) + } + + pub fn get_doc_module_kind(&self, document: &Document) -> NodeModuleKind { + self.get_lsp_referrer_kind(document.specifier(), document.is_script()) + } + + pub fn get_lsp_referrer_kind( + &self, + specifier: &ModuleSpecifier, + is_script: Option, + ) -> NodeModuleKind { + self.inner.get_lsp_referrer_kind(specifier, is_script) + } +} + +#[derive(Debug)] +pub struct SingleReferrerGraphResolver<'a> { + pub valid_referrer: &'a ModuleSpecifier, + pub referrer_kind: NodeModuleKind, + pub cli_resolver: &'a CliResolver, + pub jsx_import_source_config: Option<&'a JsxImportSourceConfig>, +} + +impl<'a> deno_graph::source::Resolver for SingleReferrerGraphResolver<'a> { + fn default_jsx_import_source(&self) -> Option { + self + .jsx_import_source_config + .and_then(|c| c.default_specifier.clone()) + } + + fn default_jsx_import_source_types(&self) -> Option { + self + .jsx_import_source_config + .and_then(|c| c.default_types_specifier.clone()) + } + + fn jsx_import_source_module(&self) -> &str { + self + .jsx_import_source_config + .map(|c| c.module.as_str()) + .unwrap_or(deno_graph::source::DEFAULT_JSX_IMPORT_SOURCE_MODULE) + } + + fn resolve( + &self, + specifier_text: &str, + referrer_range: &Range, + mode: ResolutionMode, + ) -> Result { + // this resolver assumes it will only be used with a single referrer + // with the provided referrer kind + debug_assert_eq!(referrer_range.specifier, *self.valid_referrer); + self.cli_resolver.resolve( + specifier_text, + referrer_range, + self.referrer_kind, + mode, + ) + } +} + impl RedirectResolver { fn new( cache: Arc, @@ -842,45 +960,6 @@ impl RedirectResolver { } } -#[derive(Debug)] -pub struct LspCjsTracker { - cjs_tracker: Arc, -} - -impl LspCjsTracker { - pub fn new(cjs_tracker: Arc) -> Self { - Self { cjs_tracker } - } - - pub fn is_cjs( - &self, - specifier: &ModuleSpecifier, - media_type: MediaType, - maybe_parsed_source: Option<&ParsedSource>, - ) -> bool { - if let Some(module_kind) = - self.cjs_tracker.get_known_kind(specifier, media_type) - { - module_kind.is_cjs() - } else { - let maybe_is_script = maybe_parsed_source.map(|p| p.compute_is_script()); - maybe_is_script - .and_then(|is_script| { - self - .cjs_tracker - .is_cjs_with_known_is_script(specifier, media_type, is_script) - .ok() - }) - .unwrap_or_else(|| { - self - .cjs_tracker - .is_maybe_cjs(specifier, media_type) - .unwrap_or(false) - }) - } - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 6f63ced5b..c9b24176a 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -69,6 +69,7 @@ use indexmap::IndexMap; use indexmap::IndexSet; use lazy_regex::lazy_regex; use log::error; +use node_resolver::NodeModuleKind; use once_cell::sync::Lazy; use regex::Captures; use regex::Regex; @@ -4401,25 +4402,15 @@ fn op_load<'s>( None } else { let asset_or_document = state.get_asset_or_document(&specifier); - asset_or_document.map(|doc| { - let maybe_cjs_tracker = state - .state_snapshot - .resolver - .maybe_cjs_tracker(Some(&specifier)); - LoadResponse { - data: doc.text(), - script_kind: crate::tsc::as_ts_script_kind(doc.media_type()), - version: state.script_version(&specifier), - is_cjs: maybe_cjs_tracker - .map(|t| { - t.is_cjs( - &specifier, - doc.media_type(), - doc.maybe_parsed_source().and_then(|p| p.as_ref().ok()), - ) - }) - .unwrap_or(false), - } + asset_or_document.map(|doc| LoadResponse { + data: doc.text(), + script_kind: crate::tsc::as_ts_script_kind(doc.media_type()), + version: state.script_version(&specifier), + is_cjs: doc + .document() + .map(|d| state.state_snapshot.is_cjs_resolver.get_doc_module_kind(d)) + .unwrap_or(NodeModuleKind::Esm) + == NodeModuleKind::Cjs, }) }; @@ -4662,6 +4653,10 @@ fn op_script_names(state: &mut OpState) -> ScriptNames { let (types, _) = documents.resolve_dependency( types, specifier, + state + .state_snapshot + .is_cjs_resolver + .get_doc_module_kind(doc), doc.file_referrer(), )?; let types_doc = documents.get_or_load(&types, doc.file_referrer())?; @@ -5544,6 +5539,7 @@ mod tests { documents: Arc::new(documents), assets: Default::default(), config: Arc::new(config), + is_cjs_resolver: Default::default(), resolver, }); let performance = Arc::new(Performance::default()); -- cgit v1.2.3 From 4e899d48cffa95617266dd8f9aef54603a87ad82 Mon Sep 17 00:00:00 2001 From: snek Date: Thu, 14 Nov 2024 13:16:28 +0100 Subject: fix: otel resiliency (#26857) Improving the breadth of collected data, and ensuring that the collected data is more likely to be successfully reported. - Use `log` crate in more places - Hook up `log` crate to otel - Switch to process-wide otel processors - Handle places that use `process::exit` Also adds a more robust testing framework, with a deterministic tracing setting. Refs: https://github.com/denoland/deno/issues/26852 --- cli/lsp/parent_process_checker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cli/lsp') diff --git a/cli/lsp/parent_process_checker.rs b/cli/lsp/parent_process_checker.rs index e5b2b2f23..b8a42cd1a 100644 --- a/cli/lsp/parent_process_checker.rs +++ b/cli/lsp/parent_process_checker.rs @@ -11,7 +11,7 @@ pub fn start(parent_process_id: u32) { std::thread::sleep(Duration::from_secs(10)); if !is_process_active(parent_process_id) { - std::process::exit(1); + deno_runtime::exit(1); } }); } -- cgit v1.2.3 From 617350e79c58b6e01984e3d7c7436d243d0e5cff Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 14 Nov 2024 15:24:25 -0500 Subject: refactor(resolver): move more resolution code into deno_resolver (#26873) Follow-up to cjs refactor. This moves most of the resolution code into the deno_resolver crate. Still pending is the npm resolution code. --- cli/lsp/analysis.rs | 5 +- cli/lsp/resolver.rs | 427 +++++++++++++++++++++++++++++----------------------- 2 files changed, 243 insertions(+), 189 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 9f26de70c..044b1573b 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -344,9 +344,8 @@ impl<'a> TsResponseImportMapper<'a> { { let in_npm_pkg = self .resolver - .maybe_node_resolver(Some(&self.file_referrer)) - .map(|n| n.in_npm_package(specifier)) - .unwrap_or(false); + .in_npm_pkg_checker(Some(&self.file_referrer)) + .in_npm_package(specifier); if in_npm_pkg { if let Ok(Some(pkg_id)) = npm_resolver.resolve_pkg_id_from_specifier(specifier) diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 37f63b912..399b89638 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -15,6 +15,9 @@ use deno_graph::Range; use deno_npm::NpmSystemInfo; use deno_path_util::url_from_directory_path; use deno_path_util::url_to_file_path; +use deno_resolver::npm::NpmReqResolverOptions; +use deno_resolver::DenoResolverOptions; +use deno_resolver::NodeAndNpmReqResolver; use deno_runtime::deno_fs; use deno_runtime::deno_node::NodeResolver; use deno_runtime::deno_node::PackageJson; @@ -43,6 +46,7 @@ use crate::args::CacheSetting; use crate::args::CliLockfile; use crate::args::NpmInstallDepsProvider; use crate::cache::DenoCacheEnvFsAdapter; +use crate::factory::Deferred; use crate::graph_util::CliJsrUrlProvider; use crate::http_util::HttpClientProvider; use crate::lsp::config::Config; @@ -57,8 +61,9 @@ use crate::npm::CliNpmResolverCreateOptions; use crate::npm::CliNpmResolverManagedSnapshotOption; use crate::npm::CreateInNpmPkgCheckerOptions; use crate::npm::ManagedCliNpmResolver; +use crate::resolver::CliDenoResolver; use crate::resolver::CliDenoResolverFs; -use crate::resolver::CliNodeResolver; +use crate::resolver::CliNpmReqResolver; use crate::resolver::CliResolver; use crate::resolver::CliResolverOptions; use crate::resolver::IsCjsResolver; @@ -71,10 +76,12 @@ use crate::util::progress_bar::ProgressBarStyle; #[derive(Debug, Clone)] struct LspScopeResolver { resolver: Arc, + in_npm_pkg_checker: Arc, jsr_resolver: Option>, npm_resolver: Option>, - node_resolver: Option>, - pkg_json_resolver: Option>, + node_resolver: Option>, + npm_pkg_req_resolver: Option>, + pkg_json_resolver: Arc, redirect_resolver: Option>, graph_imports: Arc>, package_json_deps_by_resolution: Arc>, @@ -83,12 +90,15 @@ struct LspScopeResolver { impl Default for LspScopeResolver { fn default() -> Self { + let factory = ResolverFactory::new(None); Self { - resolver: create_cli_resolver(None, None, None), + resolver: factory.cli_resolver().clone(), + in_npm_pkg_checker: factory.in_npm_pkg_checker().clone(), jsr_resolver: None, npm_resolver: None, node_resolver: None, - pkg_json_resolver: None, + npm_pkg_req_resolver: None, + pkg_json_resolver: factory.pkg_json_resolver().clone(), redirect_resolver: None, graph_imports: Default::default(), package_json_deps_by_resolution: Default::default(), @@ -103,35 +113,16 @@ impl LspScopeResolver { cache: &LspCache, http_client_provider: Option<&Arc>, ) -> Self { - let mut npm_resolver = None; - let mut node_resolver = None; - let fs = Arc::new(deno_fs::RealFs); - let pkg_json_resolver = Arc::new(PackageJsonResolver::new( - deno_runtime::deno_node::DenoFsNodeResolverEnv::new(fs.clone()), - )); - if let Some(http_client) = http_client_provider { - npm_resolver = create_npm_resolver( - config_data.map(|d| d.as_ref()), - cache, - http_client, - &pkg_json_resolver, - ) - .await; - if let Some(npm_resolver) = &npm_resolver { - let in_npm_pkg_checker = create_in_npm_pkg_checker(npm_resolver); - node_resolver = Some(create_node_resolver( - fs.clone(), - in_npm_pkg_checker, - npm_resolver, - pkg_json_resolver.clone(), - )); - } + let mut factory = ResolverFactory::new(config_data); + if let Some(http_client_provider) = http_client_provider { + factory.init_npm_resolver(http_client_provider, cache).await; } - let cli_resolver = create_cli_resolver( - config_data.map(|d| d.as_ref()), - npm_resolver.as_ref(), - node_resolver.as_ref(), - ); + let in_npm_pkg_checker = factory.in_npm_pkg_checker().clone(); + let npm_resolver = factory.npm_resolver().cloned(); + let node_resolver = factory.node_resolver().cloned(); + let npm_pkg_req_resolver = factory.npm_pkg_req_resolver().cloned(); + let cli_resolver = factory.cli_resolver().clone(); + let pkg_json_resolver = factory.pkg_json_resolver().clone(); let jsr_resolver = Some(Arc::new(JsrCacheResolver::new( cache.for_specifier(config_data.map(|d| d.scope.as_ref())), config_data.map(|d| d.as_ref()), @@ -171,7 +162,7 @@ impl LspScopeResolver { }) .unwrap_or_default(); let package_json_deps_by_resolution = (|| { - let node_resolver = node_resolver.as_ref()?; + let npm_pkg_req_resolver = npm_pkg_req_resolver.as_ref()?; let package_json = config_data?.maybe_pkg_json()?; let referrer = package_json.specifier(); let dependencies = package_json.dependencies.as_ref()?; @@ -181,7 +172,7 @@ impl LspScopeResolver { let req_ref = NpmPackageReqReference::from_str(&format!("npm:{name}")).ok()?; let specifier = into_specifier_and_media_type(Some( - node_resolver + npm_pkg_req_resolver .resolve_req_reference( &req_ref, &referrer, @@ -201,10 +192,12 @@ impl LspScopeResolver { Arc::new(package_json_deps_by_resolution.unwrap_or_default()); Self { resolver: cli_resolver, + in_npm_pkg_checker, jsr_resolver, + npm_pkg_req_resolver, npm_resolver, node_resolver, - pkg_json_resolver: Some(pkg_json_resolver), + pkg_json_resolver, redirect_resolver, graph_imports, package_json_deps_by_resolution, @@ -213,34 +206,21 @@ impl LspScopeResolver { } fn snapshot(&self) -> Arc { + let mut factory = ResolverFactory::new(self.config_data.as_ref()); let npm_resolver = self.npm_resolver.as_ref().map(|r| r.clone_snapshotted()); - let fs = Arc::new(deno_fs::RealFs); - let pkg_json_resolver = Arc::new(PackageJsonResolver::new( - deno_runtime::deno_node::DenoFsNodeResolverEnv::new(fs.clone()), - )); - let mut node_resolver = None; if let Some(npm_resolver) = &npm_resolver { - let in_npm_pkg_checker = create_in_npm_pkg_checker(npm_resolver); - node_resolver = Some(create_node_resolver( - fs, - in_npm_pkg_checker, - npm_resolver, - pkg_json_resolver.clone(), - )); + factory.set_npm_resolver(npm_resolver.clone()); } - let graph_resolver = create_cli_resolver( - self.config_data.as_deref(), - npm_resolver.as_ref(), - node_resolver.as_ref(), - ); Arc::new(Self { - resolver: graph_resolver, + resolver: factory.cli_resolver().clone(), + in_npm_pkg_checker: factory.in_npm_pkg_checker().clone(), jsr_resolver: self.jsr_resolver.clone(), - npm_resolver, - node_resolver, + npm_pkg_req_resolver: factory.npm_pkg_req_resolver().cloned(), + npm_resolver: factory.npm_resolver().cloned(), + node_resolver: factory.node_resolver().cloned(), redirect_resolver: self.redirect_resolver.clone(), - pkg_json_resolver: Some(pkg_json_resolver), + pkg_json_resolver: factory.pkg_json_resolver().clone(), graph_imports: self.graph_imports.clone(), package_json_deps_by_resolution: self .package_json_deps_by_resolution @@ -354,12 +334,12 @@ impl LspResolver { resolver.config_data.as_ref() } - pub fn maybe_node_resolver( + pub fn in_npm_pkg_checker( &self, file_referrer: Option<&ModuleSpecifier>, - ) -> Option<&Arc> { + ) -> &Arc { let resolver = self.get_scope_resolver(file_referrer); - resolver.node_resolver.as_ref() + &resolver.in_npm_pkg_checker } pub fn maybe_managed_npm_resolver( @@ -429,9 +409,9 @@ impl LspResolver { file_referrer: Option<&ModuleSpecifier>, ) -> Option<(ModuleSpecifier, MediaType)> { let resolver = self.get_scope_resolver(file_referrer); - let node_resolver = resolver.node_resolver.as_ref()?; + let npm_pkg_req_resolver = resolver.npm_pkg_req_resolver.as_ref()?; Some(into_specifier_and_media_type(Some( - node_resolver + npm_pkg_req_resolver .resolve_req_reference( req_ref, referrer, @@ -483,10 +463,11 @@ impl LspResolver { referrer_kind: NodeModuleKind, ) -> bool { let resolver = self.get_scope_resolver(Some(referrer)); - let Some(node_resolver) = resolver.node_resolver.as_ref() else { + let Some(npm_pkg_req_resolver) = resolver.npm_pkg_req_resolver.as_ref() + else { return false; }; - node_resolver + npm_pkg_req_resolver .resolve_if_for_npm_pkg( specifier_text, referrer, @@ -503,10 +484,9 @@ impl LspResolver { referrer: &ModuleSpecifier, ) -> Result>, ClosestPkgJsonError> { let resolver = self.get_scope_resolver(Some(referrer)); - let Some(pkg_json_resolver) = resolver.pkg_json_resolver.as_ref() else { - return Ok(None); - }; - pkg_json_resolver.get_closest_package_json(referrer) + resolver + .pkg_json_resolver + .get_closest_package_json(referrer) } pub fn resolve_redirects( @@ -558,131 +538,206 @@ impl LspResolver { } } -async fn create_npm_resolver( - config_data: Option<&ConfigData>, - cache: &LspCache, - http_client_provider: &Arc, - pkg_json_resolver: &Arc, -) -> Option> { - let enable_byonm = config_data.map(|d| d.byonm).unwrap_or(false); - let options = if enable_byonm { - CliNpmResolverCreateOptions::Byonm(CliByonmNpmResolverCreateOptions { - fs: CliDenoResolverFs(Arc::new(deno_fs::RealFs)), - pkg_json_resolver: pkg_json_resolver.clone(), - root_node_modules_dir: config_data.and_then(|config_data| { - config_data.node_modules_dir.clone().or_else(|| { - url_to_file_path(&config_data.scope) - .ok() - .map(|p| p.join("node_modules/")) - }) - }), - }) - } else { - let npmrc = config_data - .and_then(|d| d.npmrc.clone()) - .unwrap_or_else(create_default_npmrc); - let npm_cache_dir = Arc::new(NpmCacheDir::new( - &DenoCacheEnvFsAdapter(&deno_fs::RealFs), - cache.deno_dir().npm_folder_path(), - npmrc.get_all_known_registries_urls(), - )); - CliNpmResolverCreateOptions::Managed(CliManagedNpmResolverCreateOptions { - http_client_provider: http_client_provider.clone(), - snapshot: match config_data.and_then(|d| d.lockfile.as_ref()) { - Some(lockfile) => { - CliNpmResolverManagedSnapshotOption::ResolveFromLockfile( - lockfile.clone(), - ) - } - None => CliNpmResolverManagedSnapshotOption::Specified(None), - }, - // Don't provide the lockfile. We don't want these resolvers - // updating it. Only the cache request should update the lockfile. - maybe_lockfile: None, - fs: Arc::new(deno_fs::RealFs), - npm_cache_dir, - // Use an "only" cache setting in order to make the - // user do an explicit "cache" command and prevent - // the cache from being filled with lots of packages while - // the user is typing. - cache_setting: CacheSetting::Only, - text_only_progress_bar: ProgressBar::new(ProgressBarStyle::TextOnly), - maybe_node_modules_path: config_data - .and_then(|d| d.node_modules_dir.clone()), - // only used for top level install, so we can ignore this - npm_install_deps_provider: Arc::new(NpmInstallDepsProvider::empty()), - npmrc, - npm_system_info: NpmSystemInfo::default(), - lifecycle_scripts: Default::default(), - }) - }; - Some(create_cli_npm_resolver_for_lsp(options).await) +#[derive(Default)] +struct ResolverFactoryServices { + cli_resolver: Deferred>, + in_npm_pkg_checker: Deferred>, + node_resolver: Deferred>>, + npm_pkg_req_resolver: Deferred>>, + npm_resolver: Option>, +} + +struct ResolverFactory<'a> { + config_data: Option<&'a Arc>, + fs: Arc, + pkg_json_resolver: Arc, + services: ResolverFactoryServices, } -fn create_in_npm_pkg_checker( - npm_resolver: &Arc, -) -> Arc { - crate::npm::create_in_npm_pkg_checker(match npm_resolver.as_inner() { - crate::npm::InnerCliNpmResolverRef::Byonm(_) => { - CreateInNpmPkgCheckerOptions::Byonm +impl<'a> ResolverFactory<'a> { + pub fn new(config_data: Option<&'a Arc>) -> Self { + let fs = Arc::new(deno_fs::RealFs); + let pkg_json_resolver = Arc::new(PackageJsonResolver::new( + deno_runtime::deno_node::DenoFsNodeResolverEnv::new(fs.clone()), + )); + Self { + config_data, + fs, + pkg_json_resolver, + services: Default::default(), } - crate::npm::InnerCliNpmResolverRef::Managed(m) => { - CreateInNpmPkgCheckerOptions::Managed( - CliManagedInNpmPkgCheckerCreateOptions { - root_cache_dir_url: m.global_cache_root_url(), - maybe_node_modules_path: m.maybe_node_modules_path(), + } + + async fn init_npm_resolver( + &mut self, + http_client_provider: &Arc, + cache: &LspCache, + ) { + let enable_byonm = self.config_data.map(|d| d.byonm).unwrap_or(false); + let options = if enable_byonm { + CliNpmResolverCreateOptions::Byonm(CliByonmNpmResolverCreateOptions { + fs: CliDenoResolverFs(Arc::new(deno_fs::RealFs)), + pkg_json_resolver: self.pkg_json_resolver.clone(), + root_node_modules_dir: self.config_data.and_then(|config_data| { + config_data.node_modules_dir.clone().or_else(|| { + url_to_file_path(&config_data.scope) + .ok() + .map(|p| p.join("node_modules/")) + }) + }), + }) + } else { + let npmrc = self + .config_data + .and_then(|d| d.npmrc.clone()) + .unwrap_or_else(create_default_npmrc); + let npm_cache_dir = Arc::new(NpmCacheDir::new( + &DenoCacheEnvFsAdapter(self.fs.as_ref()), + cache.deno_dir().npm_folder_path(), + npmrc.get_all_known_registries_urls(), + )); + CliNpmResolverCreateOptions::Managed(CliManagedNpmResolverCreateOptions { + http_client_provider: http_client_provider.clone(), + snapshot: match self.config_data.and_then(|d| d.lockfile.as_ref()) { + Some(lockfile) => { + CliNpmResolverManagedSnapshotOption::ResolveFromLockfile( + lockfile.clone(), + ) + } + None => CliNpmResolverManagedSnapshotOption::Specified(None), + }, + // Don't provide the lockfile. We don't want these resolvers + // updating it. Only the cache request should update the lockfile. + maybe_lockfile: None, + fs: Arc::new(deno_fs::RealFs), + npm_cache_dir, + // Use an "only" cache setting in order to make the + // user do an explicit "cache" command and prevent + // the cache from being filled with lots of packages while + // the user is typing. + cache_setting: CacheSetting::Only, + text_only_progress_bar: ProgressBar::new(ProgressBarStyle::TextOnly), + maybe_node_modules_path: self + .config_data + .and_then(|d| d.node_modules_dir.clone()), + // only used for top level install, so we can ignore this + npm_install_deps_provider: Arc::new(NpmInstallDepsProvider::empty()), + npmrc, + npm_system_info: NpmSystemInfo::default(), + lifecycle_scripts: Default::default(), + }) + }; + self.set_npm_resolver(create_cli_npm_resolver_for_lsp(options).await); + } + + pub fn set_npm_resolver(&mut self, npm_resolver: Arc) { + self.services.npm_resolver = Some(npm_resolver); + } + + pub fn npm_resolver(&self) -> Option<&Arc> { + self.services.npm_resolver.as_ref() + } + + pub fn cli_resolver(&self) -> &Arc { + self.services.cli_resolver.get_or_init(|| { + let npm_req_resolver = self.npm_pkg_req_resolver().cloned(); + let deno_resolver = Arc::new(CliDenoResolver::new(DenoResolverOptions { + in_npm_pkg_checker: self.in_npm_pkg_checker().clone(), + node_and_req_resolver: match (self.node_resolver(), npm_req_resolver) { + (Some(node_resolver), Some(npm_req_resolver)) => { + Some(NodeAndNpmReqResolver { + node_resolver: node_resolver.clone(), + npm_req_resolver, + }) + } + _ => None, + }, + sloppy_imports_resolver: self + .config_data + .and_then(|d| d.sloppy_imports_resolver.clone()), + workspace_resolver: self + .config_data + .map(|d| d.resolver.clone()) + .unwrap_or_else(|| { + Arc::new(WorkspaceResolver::new_raw( + // this is fine because this is only used before initialization + Arc::new(ModuleSpecifier::parse("file:///").unwrap()), + None, + Vec::new(), + Vec::new(), + PackageJsonDepResolution::Disabled, + )) + }), + is_byonm: self.config_data.map(|d| d.byonm).unwrap_or(false), + maybe_vendor_dir: self.config_data.and_then(|d| d.vendor_dir.as_ref()), + })); + Arc::new(CliResolver::new(CliResolverOptions { + deno_resolver, + npm_resolver: self.npm_resolver().cloned(), + bare_node_builtins_enabled: self + .config_data + .is_some_and(|d| d.unstable.contains("bare-node-builtins")), + })) + }) + } + + pub fn pkg_json_resolver(&self) -> &Arc { + &self.pkg_json_resolver + } + + pub fn in_npm_pkg_checker(&self) -> &Arc { + self.services.in_npm_pkg_checker.get_or_init(|| { + crate::npm::create_in_npm_pkg_checker( + match self.services.npm_resolver.as_ref().map(|r| r.as_inner()) { + Some(crate::npm::InnerCliNpmResolverRef::Byonm(_)) | None => { + CreateInNpmPkgCheckerOptions::Byonm + } + Some(crate::npm::InnerCliNpmResolverRef::Managed(m)) => { + CreateInNpmPkgCheckerOptions::Managed( + CliManagedInNpmPkgCheckerCreateOptions { + root_cache_dir_url: m.global_cache_root_url(), + maybe_node_modules_path: m.maybe_node_modules_path(), + }, + ) + } }, ) - } - }) -} + }) + } -fn create_node_resolver( - fs: Arc, - in_npm_pkg_checker: Arc, - npm_resolver: &Arc, - pkg_json_resolver: Arc, -) -> Arc { - let node_resolver_inner = Arc::new(NodeResolver::new( - deno_runtime::deno_node::DenoFsNodeResolverEnv::new(fs.clone()), - in_npm_pkg_checker.clone(), - npm_resolver.clone().into_npm_resolver(), - pkg_json_resolver.clone(), - )); - Arc::new(CliNodeResolver::new( - fs, - in_npm_pkg_checker, - node_resolver_inner, - npm_resolver.clone(), - )) -} + pub fn node_resolver(&self) -> Option<&Arc> { + self + .services + .node_resolver + .get_or_init(|| { + let npm_resolver = self.services.npm_resolver.as_ref()?; + Some(Arc::new(NodeResolver::new( + deno_runtime::deno_node::DenoFsNodeResolverEnv::new(self.fs.clone()), + self.in_npm_pkg_checker().clone(), + npm_resolver.clone().into_npm_pkg_folder_resolver(), + self.pkg_json_resolver.clone(), + ))) + }) + .as_ref() + } -fn create_cli_resolver( - config_data: Option<&ConfigData>, - npm_resolver: Option<&Arc>, - node_resolver: Option<&Arc>, -) -> Arc { - Arc::new(CliResolver::new(CliResolverOptions { - node_resolver: node_resolver.cloned(), - npm_resolver: npm_resolver.cloned(), - workspace_resolver: config_data.map(|d| d.resolver.clone()).unwrap_or_else( - || { - Arc::new(WorkspaceResolver::new_raw( - // this is fine because this is only used before initialization - Arc::new(ModuleSpecifier::parse("file:///").unwrap()), - None, - Vec::new(), - Vec::new(), - PackageJsonDepResolution::Disabled, - )) - }, - ), - maybe_vendor_dir: config_data.and_then(|d| d.vendor_dir.as_ref()), - bare_node_builtins_enabled: config_data - .is_some_and(|d| d.unstable.contains("bare-node-builtins")), - sloppy_imports_resolver: config_data - .and_then(|d| d.sloppy_imports_resolver.clone()), - })) + pub fn npm_pkg_req_resolver(&self) -> Option<&Arc> { + self + .services + .npm_pkg_req_resolver + .get_or_init(|| { + let node_resolver = self.node_resolver()?; + let npm_resolver = self.npm_resolver()?; + Some(Arc::new(CliNpmReqResolver::new(NpmReqResolverOptions { + byonm_resolver: (npm_resolver.clone()).into_maybe_byonm(), + fs: CliDenoResolverFs(self.fs.clone()), + in_npm_pkg_checker: self.in_npm_pkg_checker().clone(), + node_resolver: node_resolver.clone(), + npm_req_resolver: npm_resolver.clone().into_npm_req_resolver(), + }))) + }) + .as_ref() + } } #[derive(Debug, Eq, PartialEq)] -- cgit v1.2.3 From c9baf3849fdbe161a9251a712a71e2b91eeabf3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 15 Nov 2024 09:33:03 +0000 Subject: perf: use available system memory for v8 isolate memory limit (#26868) Instead of using the default 1.4Gb limit (which was meant for browser tabs) configure V8 to set the heap limit to the amount of memory available in the system. Closes https://github.com/denoland/deno/issues/23424 Closes https://github.com/denoland/deno/issues/26435 Closes https://github.com/denoland/deno/issues/21226 --- cli/lsp/tsc.rs | 2 ++ 1 file changed, 2 insertions(+) (limited to 'cli/lsp') diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index c9b24176a..48dcb7964 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -34,6 +34,7 @@ use crate::util::path::relative_specifier; use crate::util::path::to_percent_decoded_str; use crate::util::result::InfallibleResultExt; use crate::util::v8::convert; +use crate::worker::create_isolate_create_params; use deno_core::convert::Smi; use deno_core::convert::ToV8; use deno_core::error::StdAnyError; @@ -4760,6 +4761,7 @@ fn run_tsc_thread( specifier_map, request_rx, )], + create_params: create_isolate_create_params(), startup_snapshot: Some(tsc::compiler_snapshot()), inspector: has_inspector_server, ..Default::default() -- cgit v1.2.3 From 3f26310728ef2d56ace7370a555eb9a862295983 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Fri, 15 Nov 2024 14:40:32 +0000 Subject: feat(lsp): auto-imports with @deno-types directives (#26821) Co-authored-by: David Sherret --- cli/lsp/documents.rs | 120 ++++++++++++++++++++++++++++++++------------- cli/lsp/language_server.rs | 31 ++++++------ cli/lsp/resolver.rs | 49 ++++++++++++++++-- cli/lsp/tsc.rs | 92 ++++++++++++++++++++++++---------- 4 files changed, 214 insertions(+), 78 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index b62fa8553..b01544ddf 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -5,6 +5,7 @@ use super::cache::LspCache; use super::config::Config; use super::resolver::LspIsCjsResolver; use super::resolver::LspResolver; +use super::resolver::ScopeDepInfo; use super::resolver::SingleReferrerGraphResolver; use super::testing::TestCollector; use super::testing::TestModule; @@ -38,7 +39,6 @@ use indexmap::IndexSet; use node_resolver::NodeModuleKind; use std::borrow::Cow; use std::collections::BTreeMap; -use std::collections::BTreeSet; use std::collections::HashMap; use std::collections::HashSet; use std::fs; @@ -989,12 +989,7 @@ pub struct Documents { open_docs: HashMap>, /// Documents stored on the file system. file_system_docs: Arc, - /// The npm package requirements found in npm specifiers. - npm_reqs_by_scope: - Arc, BTreeSet>>, - /// Config scopes that contain a node: specifier such that a @types/node - /// package should be injected. - scopes_with_node_specifier: Arc>>, + dep_info_by_scope: Arc, Arc>>, } impl Documents { @@ -1157,17 +1152,20 @@ impl Documents { false } - pub fn npm_reqs_by_scope( + pub fn dep_info_by_scope( &mut self, - ) -> Arc, BTreeSet>> { - self.calculate_npm_reqs_if_dirty(); - self.npm_reqs_by_scope.clone() + ) -> Arc, Arc>> { + self.calculate_dep_info_if_dirty(); + self.dep_info_by_scope.clone() } - pub fn scopes_with_node_specifier( - &self, - ) -> &Arc>> { - &self.scopes_with_node_specifier + pub fn scopes_with_node_specifier(&self) -> HashSet> { + self + .dep_info_by_scope + .iter() + .filter(|(_, i)| i.has_node_specifier) + .map(|(s, _)| s.clone()) + .collect::>() } /// Return a document for the specifier. @@ -1410,34 +1408,46 @@ impl Documents { /// Iterate through the documents, building a map where the key is a unique /// document and the value is a set of specifiers that depend on that /// document. - fn calculate_npm_reqs_if_dirty(&mut self) { - let mut npm_reqs_by_scope: BTreeMap<_, BTreeSet<_>> = Default::default(); - let mut scopes_with_specifier = HashSet::new(); + fn calculate_dep_info_if_dirty(&mut self) { + let mut dep_info_by_scope: BTreeMap<_, ScopeDepInfo> = Default::default(); let is_fs_docs_dirty = self.file_system_docs.set_dirty(false); if !is_fs_docs_dirty && !self.dirty { return; } let mut visit_doc = |doc: &Arc| { let scope = doc.scope(); - let reqs = npm_reqs_by_scope.entry(scope.cloned()).or_default(); + let dep_info = dep_info_by_scope.entry(scope.cloned()).or_default(); for dependency in doc.dependencies().values() { - if let Some(dep) = dependency.get_code() { + let code_specifier = dependency.get_code(); + let type_specifier = dependency.get_type(); + if let Some(dep) = code_specifier { if dep.scheme() == "node" { - scopes_with_specifier.insert(scope.cloned()); + dep_info.has_node_specifier = true; } if let Ok(reference) = NpmPackageReqReference::from_specifier(dep) { - reqs.insert(reference.into_inner().req); + dep_info.npm_reqs.insert(reference.into_inner().req); } } - if let Some(dep) = dependency.get_type() { + if let Some(dep) = type_specifier { if let Ok(reference) = NpmPackageReqReference::from_specifier(dep) { - reqs.insert(reference.into_inner().req); + dep_info.npm_reqs.insert(reference.into_inner().req); + } + } + if dependency.maybe_deno_types_specifier.is_some() { + if let (Some(code_specifier), Some(type_specifier)) = + (code_specifier, type_specifier) + { + if MediaType::from_specifier(type_specifier).is_declaration() { + dep_info + .deno_types_to_code_resolutions + .insert(type_specifier.clone(), code_specifier.clone()); + } } } } if let Some(dep) = doc.maybe_types_dependency().maybe_specifier() { if let Ok(reference) = NpmPackageReqReference::from_specifier(dep) { - reqs.insert(reference.into_inner().req); + dep_info.npm_reqs.insert(reference.into_inner().req); } } }; @@ -1448,14 +1458,49 @@ impl Documents { visit_doc(doc); } - // fill the reqs from the lockfile for (scope, config_data) in self.config.tree.data_by_scope().as_ref() { + let dep_info = dep_info_by_scope.entry(Some(scope.clone())).or_default(); + (|| { + let config_file = config_data.maybe_deno_json()?; + let jsx_config = + config_file.to_maybe_jsx_import_source_config().ok()??; + let type_specifier = jsx_config.default_types_specifier.as_ref()?; + let code_specifier = jsx_config.default_specifier.as_ref()?; + let cli_resolver = self.resolver.as_cli_resolver(Some(scope)); + let range = deno_graph::Range { + specifier: jsx_config.base_url.clone(), + start: deno_graph::Position::zeroed(), + end: deno_graph::Position::zeroed(), + }; + let type_specifier = cli_resolver + .resolve( + type_specifier, + &range, + // todo(dsherret): this is wrong because it doesn't consider CJS referrers + deno_package_json::NodeModuleKind::Esm, + ResolutionMode::Types, + ) + .ok()?; + let code_specifier = cli_resolver + .resolve( + code_specifier, + &range, + // todo(dsherret): this is wrong because it doesn't consider CJS referrers + deno_package_json::NodeModuleKind::Esm, + ResolutionMode::Execution, + ) + .ok()?; + dep_info + .deno_types_to_code_resolutions + .insert(type_specifier, code_specifier); + Some(()) + })(); + // fill the reqs from the lockfile if let Some(lockfile) = config_data.lockfile.as_ref() { - let reqs = npm_reqs_by_scope.entry(Some(scope.clone())).or_default(); let lockfile = lockfile.lock(); for dep_req in lockfile.content.packages.specifiers.keys() { if dep_req.kind == deno_semver::package::PackageKind::Npm { - reqs.insert(dep_req.req.clone()); + dep_info.npm_reqs.insert(dep_req.req.clone()); } } } @@ -1464,15 +1509,22 @@ impl Documents { // Ensure a @types/node package exists when any module uses a node: specifier. // Unlike on the command line, here we just add @types/node to the npm package // requirements since this won't end up in the lockfile. - for scope in &scopes_with_specifier { - let reqs = npm_reqs_by_scope.entry(scope.clone()).or_default(); - if !reqs.iter().any(|r| r.name == "@types/node") { - reqs.insert(PackageReq::from_str("@types/node").unwrap()); + for dep_info in dep_info_by_scope.values_mut() { + if dep_info.has_node_specifier + && !dep_info.npm_reqs.iter().any(|r| r.name == "@types/node") + { + dep_info + .npm_reqs + .insert(PackageReq::from_str("@types/node").unwrap()); } } - self.npm_reqs_by_scope = Arc::new(npm_reqs_by_scope); - self.scopes_with_node_specifier = Arc::new(scopes_with_specifier); + self.dep_info_by_scope = Arc::new( + dep_info_by_scope + .into_iter() + .map(|(s, i)| (s, Arc::new(i))) + .collect(), + ); self.dirty = false; } diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index b2bd72416..e56adafef 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1036,7 +1036,7 @@ impl Inner { // refresh the npm specifiers because it might have discovered // a @types/node package and now's a good time to do that anyway - self.refresh_npm_specifiers().await; + self.refresh_dep_info().await; self.project_changed([], true); } @@ -1082,7 +1082,7 @@ impl Inner { ); if document.is_diagnosable() { self.project_changed([(document.specifier(), ChangeKind::Opened)], false); - self.refresh_npm_specifiers().await; + self.refresh_dep_info().await; self.diagnostics_server.invalidate(&[specifier]); self.send_diagnostics_update(); self.send_testing_update(); @@ -1103,8 +1103,8 @@ impl Inner { Ok(document) => { if document.is_diagnosable() { let old_scopes_with_node_specifier = - self.documents.scopes_with_node_specifier().clone(); - self.refresh_npm_specifiers().await; + self.documents.scopes_with_node_specifier(); + self.refresh_dep_info().await; let mut config_changed = false; if !self .documents @@ -1155,13 +1155,15 @@ impl Inner { })); } - async fn refresh_npm_specifiers(&mut self) { - let package_reqs = self.documents.npm_reqs_by_scope(); + async fn refresh_dep_info(&mut self) { + let dep_info_by_scope = self.documents.dep_info_by_scope(); let resolver = self.resolver.clone(); // spawn due to the lsp's `Send` requirement - spawn(async move { resolver.set_npm_reqs(&package_reqs).await }) - .await - .ok(); + spawn( + async move { resolver.set_dep_info_by_scope(&dep_info_by_scope).await }, + ) + .await + .ok(); } async fn did_close(&mut self, params: DidCloseTextDocumentParams) { @@ -1180,7 +1182,7 @@ impl Inner { .uri_to_specifier(¶ms.text_document.uri, LspUrlKind::File); self.diagnostics_state.clear(&specifier); if self.is_diagnosable(&specifier) { - self.refresh_npm_specifiers().await; + self.refresh_dep_info().await; self.diagnostics_server.invalidate(&[specifier.clone()]); self.send_diagnostics_update(); self.send_testing_update(); @@ -3600,15 +3602,16 @@ impl Inner { if byonm { roots.retain(|s| s.scheme() != "npm"); - } else if let Some(npm_reqs) = self + } else if let Some(dep_info) = self .documents - .npm_reqs_by_scope() + .dep_info_by_scope() .get(&config_data.map(|d| d.scope.as_ref().clone())) { // always include the npm packages since resolution of one npm package // might affect the resolution of other npm packages roots.extend( - npm_reqs + dep_info + .npm_reqs .iter() .map(|req| ModuleSpecifier::parse(&format!("npm:{}", req)).unwrap()), ); @@ -3686,7 +3689,7 @@ impl Inner { async fn post_cache(&mut self) { self.resolver.did_cache(); - self.refresh_npm_specifiers().await; + self.refresh_dep_info().await; self.diagnostics_server.invalidate_all(); self.project_changed([], true); self.ts_server.cleanup_semantic_cache(self.snapshot()).await; diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 399b89638..4ede79922 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -7,6 +7,7 @@ use deno_cache_dir::HttpCache; use deno_config::deno_json::JsxImportSourceConfig; use deno_config::workspace::PackageJsonDepResolution; use deno_config::workspace::WorkspaceResolver; +use deno_core::parking_lot::Mutex; use deno_core::url::Url; use deno_graph::source::ResolutionMode; use deno_graph::GraphImport; @@ -84,6 +85,7 @@ struct LspScopeResolver { pkg_json_resolver: Arc, redirect_resolver: Option>, graph_imports: Arc>, + dep_info: Arc>>, package_json_deps_by_resolution: Arc>, config_data: Option>, } @@ -101,6 +103,7 @@ impl Default for LspScopeResolver { pkg_json_resolver: factory.pkg_json_resolver().clone(), redirect_resolver: None, graph_imports: Default::default(), + dep_info: Default::default(), package_json_deps_by_resolution: Default::default(), config_data: None, } @@ -180,6 +183,15 @@ impl LspScopeResolver { NodeModuleKind::Esm, NodeResolutionMode::Types, ) + .or_else(|_| { + npm_pkg_req_resolver.resolve_req_reference( + &req_ref, + &referrer, + // todo(dsherret): this is wrong because it doesn't consider CJS referrers + NodeModuleKind::Esm, + NodeResolutionMode::Execution, + ) + }) .ok()?, )) .0; @@ -200,6 +212,7 @@ impl LspScopeResolver { pkg_json_resolver, redirect_resolver, graph_imports, + dep_info: Default::default(), package_json_deps_by_resolution, config_data: config_data.cloned(), } @@ -222,6 +235,7 @@ impl LspScopeResolver { redirect_resolver: self.redirect_resolver.clone(), pkg_json_resolver: factory.pkg_json_resolver().clone(), graph_imports: self.graph_imports.clone(), + dep_info: self.dep_info.clone(), package_json_deps_by_resolution: self .package_json_deps_by_resolution .clone(), @@ -288,19 +302,24 @@ impl LspResolver { } } - pub async fn set_npm_reqs( + pub async fn set_dep_info_by_scope( &self, - reqs: &BTreeMap, BTreeSet>, + dep_info_by_scope: &Arc< + BTreeMap, Arc>, + >, ) { for (scope, resolver) in [(None, &self.unscoped)] .into_iter() .chain(self.by_scope.iter().map(|(s, r)| (Some(s), r))) { + let dep_info = dep_info_by_scope.get(&scope.cloned()); + if let Some(dep_info) = dep_info { + *resolver.dep_info.lock() = dep_info.clone(); + } if let Some(npm_resolver) = resolver.npm_resolver.as_ref() { if let Some(npm_resolver) = npm_resolver.as_managed() { - let reqs = reqs - .get(&scope.cloned()) - .map(|reqs| reqs.iter().cloned().collect::>()) + let reqs = dep_info + .map(|i| i.npm_reqs.iter().cloned().collect::>()) .unwrap_or_default(); if let Err(err) = npm_resolver.set_package_reqs(&reqs).await { lsp_warn!("Could not set npm package requirements: {:#}", err); @@ -434,6 +453,19 @@ impl LspResolver { .cloned() } + pub fn deno_types_to_code_resolution( + &self, + specifier: &ModuleSpecifier, + file_referrer: Option<&ModuleSpecifier>, + ) -> Option { + let resolver = self.get_scope_resolver(file_referrer); + let dep_info = resolver.dep_info.lock().clone(); + dep_info + .deno_types_to_code_resolutions + .get(specifier) + .cloned() + } + pub fn in_node_modules(&self, specifier: &ModuleSpecifier) -> bool { fn has_node_modules_dir(specifier: &ModuleSpecifier) -> bool { // consider any /node_modules/ directory as being in the node_modules @@ -538,6 +570,13 @@ impl LspResolver { } } +#[derive(Debug, Default, Clone)] +pub struct ScopeDepInfo { + pub deno_types_to_code_resolutions: HashMap, + pub npm_reqs: BTreeSet, + pub has_node_specifier: bool, +} + #[derive(Default)] struct ResolverFactoryServices { cli_resolver: Deferred>, diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 48dcb7964..fc7ff5794 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -3417,9 +3417,18 @@ fn parse_code_actions( additional_text_edits.extend(change.text_changes.iter().map(|tc| { let mut text_edit = tc.as_text_edit(asset_or_doc.line_index()); if let Some(specifier_rewrite) = &data.specifier_rewrite { - text_edit.new_text = text_edit - .new_text - .replace(&specifier_rewrite.0, &specifier_rewrite.1); + text_edit.new_text = text_edit.new_text.replace( + &specifier_rewrite.old_specifier, + &specifier_rewrite.new_specifier, + ); + if let Some(deno_types_specifier) = + &specifier_rewrite.new_deno_types_specifier + { + text_edit.new_text = format!( + "// @deno-types=\"{}\"\n{}", + deno_types_specifier, &text_edit.new_text + ); + } } text_edit })); @@ -3578,17 +3587,23 @@ impl CompletionEntryDetails { let mut text_edit = original_item.text_edit.clone(); if let Some(specifier_rewrite) = &data.specifier_rewrite { if let Some(text_edit) = &mut text_edit { - match text_edit { - lsp::CompletionTextEdit::Edit(text_edit) => { - text_edit.new_text = text_edit - .new_text - .replace(&specifier_rewrite.0, &specifier_rewrite.1); - } + let new_text = match text_edit { + lsp::CompletionTextEdit::Edit(text_edit) => &mut text_edit.new_text, lsp::CompletionTextEdit::InsertAndReplace(insert_replace_edit) => { - insert_replace_edit.new_text = insert_replace_edit - .new_text - .replace(&specifier_rewrite.0, &specifier_rewrite.1); + &mut insert_replace_edit.new_text } + }; + *new_text = new_text.replace( + &specifier_rewrite.old_specifier, + &specifier_rewrite.new_specifier, + ); + if let Some(deno_types_specifier) = + &specifier_rewrite.new_deno_types_specifier + { + *new_text = format!( + "// @deno-types=\"{}\"\n{}", + deno_types_specifier, new_text + ); } } } @@ -3693,6 +3708,13 @@ impl CompletionInfo { } } +#[derive(Debug, Clone, Deserialize, Serialize)] +pub struct CompletionSpecifierRewrite { + old_specifier: String, + new_specifier: String, + new_deno_types_specifier: Option, +} + #[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct CompletionItemData { @@ -3705,7 +3727,7 @@ pub struct CompletionItemData { /// be rewritten by replacing the first string with the second. Intended for /// auto-import specifiers to be reverse-import-mapped. #[serde(skip_serializing_if = "Option::is_none")] - pub specifier_rewrite: Option<(String, String)>, + pub specifier_rewrite: Option, #[serde(skip_serializing_if = "Option::is_none")] pub data: Option, pub use_code_snippet: bool, @@ -3927,20 +3949,40 @@ impl CompletionEntry { if let Some(source) = &self.source { let mut display_source = source.clone(); if let Some(import_data) = &self.auto_import_data { - if let Some(new_module_specifier) = language_server - .get_ts_response_import_mapper(specifier) + let import_mapper = + language_server.get_ts_response_import_mapper(specifier); + if let Some(mut new_specifier) = import_mapper .check_specifier(&import_data.normalized, specifier) .or_else(|| relative_specifier(specifier, &import_data.normalized)) { - if new_module_specifier.contains("/node_modules/") { + if new_specifier.contains("/node_modules/") { return None; } - display_source.clone_from(&new_module_specifier); - if new_module_specifier != import_data.raw.module_specifier { - specifier_rewrite = Some(( - import_data.raw.module_specifier.clone(), - new_module_specifier, - )); + let mut new_deno_types_specifier = None; + if let Some(code_specifier) = language_server + .resolver + .deno_types_to_code_resolution( + &import_data.normalized, + Some(specifier), + ) + .and_then(|s| { + import_mapper + .check_specifier(&s, specifier) + .or_else(|| relative_specifier(specifier, &s)) + }) + { + new_deno_types_specifier = + Some(std::mem::replace(&mut new_specifier, code_specifier)); + } + display_source.clone_from(&new_specifier); + if new_specifier != import_data.raw.module_specifier + || new_deno_types_specifier.is_some() + { + specifier_rewrite = Some(CompletionSpecifierRewrite { + old_specifier: import_data.raw.module_specifier.clone(), + new_specifier, + new_deno_types_specifier, + }); } } else if source.starts_with(jsr_url().as_str()) { return None; @@ -4246,9 +4288,7 @@ impl TscSpecifierMap { return specifier.to_string(); } let mut specifier = original.to_string(); - if specifier.contains("/node_modules/.deno/") - && !specifier.contains("/node_modules/@types/node/") - { + if !specifier.contains("/node_modules/@types/node/") { // The ts server doesn't give completions from files in // `node_modules/.deno/`. We work around it like this. specifier = specifier.replace("/node_modules/", "/$node_modules/"); @@ -4415,6 +4455,8 @@ fn op_load<'s>( }) }; + lsp_warn!("op_load {} {}", &specifier, maybe_load_response.is_some()); + let serialized = serde_v8::to_v8(scope, maybe_load_response)?; state.performance.measure(mark); -- cgit v1.2.3 From 84e12386480d76e97ad85d9c5314168e7ab03e04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 16 Nov 2024 11:18:17 +0000 Subject: feat(task): support object notation, remove support for JSDocs (#26886) This commit changes three aspects of `deno task`: 1. Tasks can now be written using object notation like so: ```jsonc { "tasks": { "foo": "deno run foo.js", "bar": { "command": "deno run bar.js" } } ``` 2. Support for comments for tasks is now removed. Comments above tasks will no longer be printed when running `deno task`. 3. Tasks written using object notation can have "description" field that replaces support for comments above tasks: ```jsonc { "tasks": { "bar": { "description": "This is a bar task" "command": "deno run bar.js" } } ``` ``` $ deno task Available tasks: - bar // This is a bar task deno run bar.js ``` Pulled most of the changes from https://github.com/denoland/deno/pull/26467 to support "dependencies" in tasks. Additionally some cleanup was performed to make code easier to read. --------- Co-authored-by: David Sherret --- cli/lsp/language_server.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index e56adafef..c93628555 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -3632,9 +3632,8 @@ impl Inner { deno_json_cache: None, pkg_json_cache: None, workspace_cache: None, - config_parse_options: deno_config::deno_json::ConfigParseOptions { - include_task_comments: false, - }, + config_parse_options: + deno_config::deno_json::ConfigParseOptions::default(), additional_config_file_names: &[], discover_pkg_json: !has_flag_env_var("DENO_NO_PACKAGE_JSON"), maybe_vendor_override: if force_global_cache { -- cgit v1.2.3 From c36f877f8d98f9756154e26809c614d6749b3fd1 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Mon, 18 Nov 2024 21:45:34 +0000 Subject: fix(lsp): ignore editor indent settings if deno.json is present (#26912) --- cli/lsp/language_server.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'cli/lsp') diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index c93628555..2ce26c1f2 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1396,9 +1396,11 @@ impl Inner { .fmt_config_for_specifier(&specifier) .options .clone(); - fmt_options.use_tabs = Some(!params.options.insert_spaces); - fmt_options.indent_width = Some(params.options.tab_size as u8); let config_data = self.config.tree.data_for_specifier(&specifier); + if !config_data.is_some_and(|d| d.maybe_deno_json().is_some()) { + fmt_options.use_tabs = Some(!params.options.insert_spaces); + fmt_options.indent_width = Some(params.options.tab_size as u8); + } let unstable_options = UnstableFmtOptions { component: config_data .map(|d| d.unstable.contains("fmt-component")) -- cgit v1.2.3