From 810474d8b79738c29bd5fe975920169e03ec61b0 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Tue, 18 Jun 2024 23:00:17 +0100 Subject: [PATCH] fix(lsp): use import map from workspace root (#24246) Follow up to #24206 which broke deno_std intellisense. --- cli/lsp/config.rs | 185 +++++++++++++++++---------------- tests/integration/lsp_tests.rs | 62 +++++++++++ 2 files changed, 158 insertions(+), 89 deletions(-) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 1544ebdf34..da5af0c2df 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -1132,7 +1132,7 @@ impl ConfigData { async fn load( config_file_specifier: Option<&ModuleSpecifier>, scope: &ModuleSpecifier, - workspace_root: Option<(&ModuleSpecifier, &ConfigData)>, + workspace_root: Option<&ConfigData>, settings: &Settings, file_fetcher: Option<&Arc>, ) -> Self { @@ -1194,7 +1194,7 @@ impl ConfigData { async fn load_inner( config_file: Option, scope: &ModuleSpecifier, - workspace_root: Option<(&ModuleSpecifier, &ConfigData)>, + workspace_root: Option<&ConfigData>, settings: &Settings, file_fetcher: Option<&Arc>, ) -> Self { @@ -1219,7 +1219,7 @@ impl ConfigData { } let mut fmt_options = None; - if let Some((_, workspace_data)) = workspace_root { + if let Some(workspace_data) = workspace_root { let has_own_fmt_options = config_file .as_ref() .is_some_and(|config_file| config_file.json.fmt.is_some()); @@ -1250,7 +1250,7 @@ impl ConfigData { }); let mut lint_options_rules = None; - if let Some((_, workspace_data)) = workspace_root { + if let Some(workspace_data) = workspace_root { let has_own_lint_options = config_file .as_ref() .is_some_and(|config_file| config_file.json.lint.is_some()); @@ -1406,99 +1406,106 @@ impl ConfigData { let mut import_map_value = None; let mut import_map_specifier = None; let mut import_map_from_settings = false; - if let Some(config_file) = &config_file { - if config_file.is_an_import_map() { - import_map_value = Some(config_file.to_import_map_value_from_imports()); - import_map_specifier = Some(config_file.specifier.clone()); - } else if let Ok(Some(specifier)) = config_file.to_import_map_specifier() - { - import_map_specifier = Some(specifier); - } - } - import_map_specifier = import_map_specifier.or_else(|| { - let import_map_str = settings.import_map.as_ref()?; - let specifier = Url::parse(import_map_str) - .ok() - .or_else(|| workspace_folder?.join(import_map_str).ok())?; - import_map_from_settings = true; - Some(specifier) - }); - if let Some(specifier) = &import_map_specifier { - if let Ok(path) = specifier_to_file_path(specifier) { - watched_files - .entry(specifier.clone()) - .or_insert(ConfigWatchedFileType::ImportMap); - let import_map_canonicalized_specifier = - canonicalize_path_maybe_not_exists(&path) - .ok() - .and_then(|p| ModuleSpecifier::from_file_path(p).ok()); - if let Some(specifier) = import_map_canonicalized_specifier { - watched_files - .entry(specifier) - .or_insert(ConfigWatchedFileType::ImportMap); + if let Some(workspace_data) = workspace_root { + import_map.clone_from(&workspace_data.import_map); + import_map_from_settings = workspace_data.import_map_from_settings; + } else { + if let Some(config_file) = &config_file { + if config_file.is_an_import_map() { + import_map_value = + Some(config_file.to_import_map_value_from_imports()); + import_map_specifier = Some(config_file.specifier.clone()); + } else if let Ok(Some(specifier)) = + config_file.to_import_map_specifier() + { + import_map_specifier = Some(specifier); } } - if import_map_value.is_none() { - if let Some(file_fetcher) = file_fetcher { - // spawn due to the lsp's `Send` requirement - let fetch_result = deno_core::unsync::spawn({ - let file_fetcher = file_fetcher.clone(); - let specifier = specifier.clone(); - async move { - file_fetcher - .fetch(&specifier, &PermissionsContainer::allow_all()) - .await + import_map_specifier = import_map_specifier.or_else(|| { + let import_map_str = settings.import_map.as_ref()?; + let specifier = Url::parse(import_map_str) + .ok() + .or_else(|| workspace_folder?.join(import_map_str).ok())?; + import_map_from_settings = true; + Some(specifier) + }); + if let Some(specifier) = &import_map_specifier { + if let Ok(path) = specifier_to_file_path(specifier) { + watched_files + .entry(specifier.clone()) + .or_insert(ConfigWatchedFileType::ImportMap); + let import_map_canonicalized_specifier = + canonicalize_path_maybe_not_exists(&path) + .ok() + .and_then(|p| ModuleSpecifier::from_file_path(p).ok()); + if let Some(specifier) = import_map_canonicalized_specifier { + watched_files + .entry(specifier) + .or_insert(ConfigWatchedFileType::ImportMap); + } + } + if import_map_value.is_none() { + if let Some(file_fetcher) = file_fetcher { + // spawn due to the lsp's `Send` requirement + let fetch_result = deno_core::unsync::spawn({ + let file_fetcher = file_fetcher.clone(); + let specifier = specifier.clone(); + async move { + file_fetcher + .fetch(&specifier, &PermissionsContainer::allow_all()) + .await + } + }) + .await + .unwrap(); + let value_result = fetch_result.and_then(|f| { + serde_json::from_slice::(&f.source).map_err(|e| e.into()) + }); + match value_result { + Ok(value) => { + import_map_value = Some(value); + } + Err(err) => { + lsp_warn!( + " Couldn't read import map \"{}\": {}", + specifier.as_str(), + err + ); + } } - }) - .await - .unwrap(); - let value_result = fetch_result.and_then(|f| { - serde_json::from_slice::(&f.source).map_err(|e| e.into()) - }); - match value_result { - Ok(value) => { - import_map_value = Some(value); + } + } + } + if let (Some(value), Some(specifier)) = + (import_map_value, import_map_specifier) + { + match import_map::parse_from_value(specifier.clone(), value) { + Ok(result) => { + if config_file.as_ref().map(|c| &c.specifier) == Some(&specifier) { + lsp_log!(" Resolved import map from configuration file"); + } else { + lsp_log!(" Resolved import map: \"{}\"", specifier.as_str()); } - Err(err) => { + if !result.diagnostics.is_empty() { lsp_warn!( - " Couldn't read import map \"{}\": {}", - specifier.as_str(), - err + " Import map diagnostics:\n{}", + result + .diagnostics + .iter() + .map(|d| format!(" - {d}")) + .collect::>() + .join("\n") ); } + import_map = Some(Arc::new(result.import_map)); } - } - } - } - if let (Some(value), Some(specifier)) = - (import_map_value, import_map_specifier) - { - match import_map::parse_from_value(specifier.clone(), value) { - Ok(result) => { - if config_file.as_ref().map(|c| &c.specifier) == Some(&specifier) { - lsp_log!(" Resolved import map from configuration file"); - } else { - lsp_log!(" Resolved import map: \"{}\"", specifier.as_str()); - } - if !result.diagnostics.is_empty() { + Err(err) => { lsp_warn!( - " Import map diagnostics:\n{}", - result - .diagnostics - .iter() - .map(|d| format!(" - {d}")) - .collect::>() - .join("\n") + "Couldn't read import map \"{}\": {}", + specifier.as_str(), + err ); } - import_map = Some(result.import_map); - } - Err(err) => { - lsp_warn!( - "Couldn't read import map \"{}\": {}", - specifier.as_str(), - err - ); } } } @@ -1533,7 +1540,7 @@ impl ConfigData { }) .unwrap_or_default(), ) - } else if let Some((_, workspace_data)) = workspace_root { + } else if let Some(workspace_data) = workspace_root { workspace_data.workspace_members.clone() } else if config_file.as_ref().is_some_and(|c| c.json.name.is_some()) { Arc::new(vec![scope.clone()]) @@ -1555,7 +1562,7 @@ impl ConfigData { lockfile: lockfile.map(Mutex::new).map(Arc::new), package_json: package_json.map(Arc::new), npmrc, - import_map: import_map.map(Arc::new), + import_map, import_map_from_settings, package_config: package_config.map(Arc::new), is_workspace_root, @@ -1740,7 +1747,7 @@ impl ConfigTree { let member_data = ConfigData::load( Some(&config_file_specifier), member_scope, - Some((&scope, &data)), + Some(&data), settings, Some(file_fetcher), ) diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 3692bf7d96..0659bb4392 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -12694,6 +12694,68 @@ fn lsp_deno_json_workspace_lint_config() { client.shutdown(); } +#[test] +fn lsp_deno_json_workspace_import_map() { + let context = TestContextBuilder::new().use_temp_cwd().build(); + let temp_dir = context.temp_dir(); + temp_dir.create_dir_all("project1/project2"); + temp_dir.write( + "project1/deno.json", + json!({ + "workspaces": ["project2"], + "imports": { + "foo": "./foo1.ts", + }, + }) + .to_string(), + ); + temp_dir.write("project1/foo1.ts", ""); + temp_dir.write( + "project1/project2/deno.json", + // Should ignore and inherit import map from `project1/deno.json`. + json!({ + "imports": { + "foo": "./foo2.ts", + }, + }) + .to_string(), + ); + temp_dir.write("project1/project2/foo2.ts", ""); + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + client.did_open(json!({ + "textDocument": { + "uri": temp_dir.uri().join("project1/project2/file.ts").unwrap(), + "languageId": "typescript", + "version": 1, + "text": "import \"foo\";\n", + }, + })); + let res = client.write_request( + "textDocument/hover", + json!({ + "textDocument": { + "uri": temp_dir.uri().join("project1/project2/file.ts").unwrap(), + }, + "position": { "line": 0, "character": 7 }, + }), + ); + assert_eq!( + res, + json!({ + "contents": { + "kind": "markdown", + "value": format!("**Resolved Dependency**\n\n**Code**: file​{}\n", temp_dir.uri().join("project1/foo1.ts").unwrap().as_str().trim_start_matches("file")), + }, + "range": { + "start": { "line": 0, "character": 7 }, + "end": { "line": 0, "character": 12 }, + }, + }) + ); + client.shutdown(); +} + #[test] fn lsp_deno_json_workspace_jsr_resolution() { let context = TestContextBuilder::new().use_temp_cwd().build();