From e54684864dee1c708325570eaaff2fce0f928387 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 6 Feb 2024 19:21:26 -0500 Subject: [PATCH] fix(publish): handle diagnostic outside graph (#22310) Hacky quick fix. The real fix is a lot more work to do (move the `SourceTextInfo` into all the diagnostics in order to make this less error pone). I've already started on it, but it will require a lot of downstream create changes. Closes #22288 --- cli/diagnostics.rs | 27 ++++++++++++++++++++++++-- cli/tests/integration/publish_tests.rs | 26 +++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/cli/diagnostics.rs b/cli/diagnostics.rs index 0a674a2c48..f6f68e1d02 100644 --- a/cli/diagnostics.rs +++ b/cli/diagnostics.rs @@ -25,13 +25,36 @@ pub trait SourceTextStore { pub struct SourceTextParsedSourceStore<'a>(pub LazyGraphSourceParser<'a>); +impl<'a> SourceTextParsedSourceStore<'a> { + pub fn get_source_text_from_store( + &self, + specifier: &ModuleSpecifier, + ) -> Option> { + let parsed_source = self.0.get_or_parse_source(specifier).ok()??; + Some(Cow::Owned(parsed_source.text_info().clone())) + } +} + impl SourceTextStore for SourceTextParsedSourceStore<'_> { fn get_source_text<'a>( &'a self, specifier: &ModuleSpecifier, ) -> Option> { - let parsed_source = self.0.get_or_parse_source(specifier).ok()??; - Some(Cow::Owned(parsed_source.text_info().clone())) + match self.get_source_text_from_store(specifier) { + Some(text_info) => Some(text_info), + None => { + // todo(#22117): this is extremely hacky and bad because the file + // may have changed by the time we get here. Instead of doing this, + // we should store the text info in the diagnostics + if specifier.scheme() == "file" { + let path = specifier.to_file_path().ok()?; + let text = std::fs::read_to_string(path).ok()?; + Some(Cow::Owned(SourceTextInfo::new(text.into()))) + } else { + None + } + } + } } } diff --git a/cli/tests/integration/publish_tests.rs b/cli/tests/integration/publish_tests.rs index b15ca5f89f..28c57e010a 100644 --- a/cli/tests/integration/publish_tests.rs +++ b/cli/tests/integration/publish_tests.rs @@ -97,6 +97,32 @@ fn publish_non_exported_files_using_import_map() { .any(|l| l.contains("Unfurling") && l.ends_with("other.ts"))); } +#[test] +fn publish_warning_not_in_graph() { + let context = publish_context_builder().build(); + let temp_dir = context.temp_dir().path(); + temp_dir.join("deno.json").write_json(&json!({ + "name": "@foo/bar", + "version": "1.0.0", + "exports": "./mod.ts", + })); + // file not in the graph that uses a non-analyzable dynamic import (cause a diagnostic) + let other_ts = temp_dir.join("_other.ts"); + other_ts + .write("const nonAnalyzable = './_other.ts'; await import(nonAnalyzable);"); + let mod_ts = temp_dir.join("mod.ts"); + mod_ts.write( + "export function test(a: number, b: number): number { return a + b; }", + ); + context + .new_command() + .args("publish --token 'sadfasdf'") + .run() + .assert_matches_text( + "[WILDCARD]unable to analyze dynamic import[WILDCARD]", + ); +} + itest!(javascript_missing_decl_file { args: "publish --token 'sadfasdf'", output: "publish/javascript_missing_decl_file.out",