diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index d189ab2534b03c..4890221d9e05c5 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -228,6 +228,7 @@ pub struct TsResponseImportMapper<'a> { documents: &'a Documents, maybe_import_map: Option<&'a ImportMap>, resolver: &'a LspResolver, + file_referrer: ModuleSpecifier, } impl<'a> TsResponseImportMapper<'a> { @@ -235,11 +236,13 @@ impl<'a> TsResponseImportMapper<'a> { documents: &'a Documents, maybe_import_map: Option<&'a ImportMap>, resolver: &'a LspResolver, + file_referrer: &ModuleSpecifier, ) -> Self { Self { documents, maybe_import_map, resolver, + file_referrer: file_referrer.clone(), } } @@ -260,8 +263,6 @@ impl<'a> TsResponseImportMapper<'a> { } } - let file_referrer = self.documents.get_file_referrer(referrer); - if let Some(jsr_path) = specifier.as_str().strip_prefix(jsr_url().as_str()) { let mut segments = jsr_path.split('/'); @@ -276,7 +277,7 @@ impl<'a> TsResponseImportMapper<'a> { let export = self.resolver.jsr_lookup_export_for_path( &nv, &path, - file_referrer.as_deref(), + Some(&self.file_referrer), )?; let sub_path = (export != ".").then_some(export); let mut req = None; @@ -302,7 +303,7 @@ impl<'a> TsResponseImportMapper<'a> { req = req.or_else(|| { self .resolver - .jsr_lookup_req_for_nv(&nv, file_referrer.as_deref()) + .jsr_lookup_req_for_nv(&nv, Some(&self.file_referrer)) }); let spec_str = if let Some(req) = req { let req_ref = PackageReqReference { req, sub_path }; @@ -332,7 +333,7 @@ impl<'a> TsResponseImportMapper<'a> { if let Some(npm_resolver) = self .resolver - .maybe_managed_npm_resolver(file_referrer.as_deref()) + .maybe_managed_npm_resolver(Some(&self.file_referrer)) { if npm_resolver.in_npm_package(specifier) { if let Ok(Some(pkg_id)) = @@ -468,6 +469,26 @@ impl<'a> TsResponseImportMapper<'a> { } None } + + pub fn is_valid_import( + &self, + specifier_text: &str, + referrer: &ModuleSpecifier, + ) -> bool { + self + .resolver + .as_graph_resolver(Some(&self.file_referrer)) + .resolve( + specifier_text, + &deno_graph::Range { + specifier: referrer.clone(), + start: deno_graph::Position::zeroed(), + end: deno_graph::Position::zeroed(), + }, + deno_graph::source::ResolutionMode::Types, + ) + .is_ok() + } } fn try_reverse_map_package_json_exports( @@ -580,7 +601,7 @@ fn fix_ts_import_action( referrer: &ModuleSpecifier, action: &tsc::CodeFixAction, import_mapper: &TsResponseImportMapper, -) -> Result { +) -> Result, AnyError> { if matches!( action.fix_name.as_str(), "import" | "fixMissingFunctionDeclaration" @@ -623,19 +644,21 @@ fn fix_ts_import_action( }) .collect(); - return Ok(tsc::CodeFixAction { + 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); } } } - Ok(action.clone()) + Ok(Some(action.clone())) } /// Determines if two TypeScript diagnostic codes are effectively equivalent. @@ -976,11 +999,14 @@ impl CodeActionCollection { "The action returned from TypeScript is unsupported.", )); } - let action = fix_ts_import_action( + let Some(action) = fix_ts_import_action( specifier, action, &language_server.get_ts_response_import_mapper(specifier), - )?; + )? + else { + return Ok(()); + }; let edit = ts_changes_to_edit(&action.changes, language_server)?; let code_action = lsp::CodeAction { title: action.description.clone(), diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 90a2579f4d7c12..8269dc8515ccf8 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1918,6 +1918,7 @@ impl Inner { // as the import map is an implementation detail .and_then(|d| d.resolver.maybe_import_map()), self.resolver.as_ref(), + file_referrer, ) } diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index fc543759478b0a..19ea89a54bff76 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -8504,6 +8504,74 @@ fn lsp_completions_auto_import_and_quick_fix_with_import_map() { client.shutdown(); } +// Regression test for https://github.com/denoland/deno/issues/25775. +#[test] +fn lsp_quick_fix_missing_import_exclude_bare_node_builtins() { + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .add_npm_env_vars() + .build(); + let temp_dir = context.temp_dir(); + temp_dir.write( + "package.json", + json!({ + "dependencies": { + "@types/node": "*", + }, + }) + .to_string(), + ); + context.run_npm("install"); + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + let diagnostics = client.did_open(json!({ + "textDocument": { + "uri": temp_dir.url().join("file.ts").unwrap(), + "languageId": "typescript", + "version": 1, + // Include node:buffer import to ensure @types/node is in the dep graph. + "text": "import \"node:buffer\";\nassert();\n", + }, + })); + let diagnostic = diagnostics + .all() + .into_iter() + .find(|d| d.message == "Cannot find name 'assert'.") + .unwrap(); + let res = client.write_request( + "textDocument/codeAction", + json!({ + "textDocument": { + "uri": temp_dir.url().join("file.ts").unwrap(), + }, + "range": { + "start": { "line": 1, "character": 0 }, + "end": { "line": 1, "character": 6 }, + }, + "context": { + "diagnostics": [&diagnostic], + "only": ["quickfix"], + }, + }), + ); + let code_actions = + serde_json::from_value::>(res).unwrap(); + let titles = code_actions + .iter() + .map(|a| a.title.clone()) + .collect::>(); + assert_eq!( + json!(titles), + json!([ + "Add import from \"node:assert\"", + "Add import from \"node:console\"", + "Add missing function declaration 'assert'", + ]), + ); + client.shutdown(); +} + #[test] fn lsp_completions_snippet() { let context = TestContextBuilder::new().use_temp_cwd().build();