refactor: nicer warning display (#21547)

4 files changed, 92 insertions, 23 deletions

diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs
index 202d66ee9..fa9fbe33f 100644
--- a/cli/tools/registry/mod.rs
+++ b/cli/tools/registry/mod.rs
@@ -53,6 +53,7 @@ struct PreparedPublishPackage {
   version: String,
   tarball_hash: String,
   tarball: Bytes,
+  diagnostics: Vec<String>,
 }
 
 #[derive(serde::Deserialize)]
@@ -100,8 +101,9 @@ async fn prepare_publish(
 
   let unfurler = ImportMapUnfurler::new(import_map);
 
-  let tarball = tar::create_gzipped_tarball(directory_path, unfurler)
-    .context("Failed to create a tarball")?;
+  let (tarball, diagnostics) =
+    tar::create_gzipped_tarball(directory_path, unfurler)
+      .context("Failed to create a tarball")?;
 
   let tarball_hash_bytes: Vec<u8> =
     sha2::Sha256::digest(&tarball).iter().cloned().collect();
@@ -116,6 +118,7 @@ async fn prepare_publish(
     version: version.to_string(),
     tarball_hash,
     tarball,
+    diagnostics,
   })
 }
 
@@ -221,6 +224,47 @@ struct OidcTokenResponse {
   value: String,
 }
 
+/// Prints diagnostics like so:
+/// ```
+///
+/// Warning
+/// ├╌ Dynamic import was not analyzable...
+/// ├╌╌ at file:///dev/foo/bar/foo.ts:4:5
+/// |
+/// ├╌ Dynamic import was not analyzable...
+/// ├╌╌ at file:///dev/foo/bar/foo.ts:4:5
+/// |
+/// ├╌ Dynamic import was not analyzable...
+/// └╌╌ at file:///dev/foo/bar/foo.ts:4:5
+///
+/// ```
+fn print_diagnostics(diagnostics: Vec<String>) {
+  if !diagnostics.is_empty() {
+    let len = diagnostics.len();
+    log::warn!("");
+    log::warn!("{}", crate::colors::yellow("Warning"));
+    for (i, diagnostic) in diagnostics.iter().enumerate() {
+      let last_diagnostic = i == len - 1;
+      let lines = diagnostic.split('\n').collect::<Vec<_>>();
+      let lines_len = lines.len();
+      if i != 0 {
+        log::warn!("|");
+      }
+      for (j, line) in lines.iter().enumerate() {
+        let last_line = j == lines_len - 1;
+        if j == 0 {
+          log::warn!("├╌ {}", line);
+        } else if last_line && last_diagnostic {
+          log::warn!("└╌╌ {}", line);
+        } else {
+          log::warn!("├╌╌ {}", line);
+        }
+      }
+    }
+    log::warn!("");
+  }
+}
+
 async fn perform_publish(
   http_client: &Arc<HttpClient>,
   packages: Vec<PreparedPublishPackage>,
@@ -229,6 +273,12 @@ async fn perform_publish(
   let client = http_client.client()?;
   let registry_url = deno_registry_api_url().to_string();
 
+  let diagnostics = packages
+    .iter()
+    .flat_map(|p| p.diagnostics.clone())
+    .collect::<Vec<_>>();
+  print_diagnostics(diagnostics);
+
   let permissions = packages
     .iter()
     .map(|package| Permission::VersionPublish {
@@ -267,6 +317,8 @@ async fn perform_publish(
         println!(" @{}/{}", packages[0].scope, packages[0].package);
       }
 
+      // ASCII code for the bell character.
+      print!("\x07");
       println!("{}", colors::gray("Waiting..."));
 
       let interval = std::time::Duration::from_secs(auth.poll_interval);
diff --git a/cli/tools/registry/tar.rs b/cli/tools/registry/tar.rs
index 61532917a..418a5b0fd 100644
--- a/cli/tools/registry/tar.rs
+++ b/cli/tools/registry/tar.rs
@@ -16,11 +16,12 @@ pub fn create_gzipped_tarball(
   // TODO(bartlomieju): this is too specific, factor it out into a callback that
   // returns data
   unfurler: ImportMapUnfurler,
-) -> Result<Bytes, AnyError> {
+) -> Result<(Bytes, Vec<String>), AnyError> {
   let mut tar = TarGzArchive::new();
   let dir = dir
     .canonicalize()
     .map_err(|_| anyhow::anyhow!("Unable to canonicalize path {:?}", dir))?;
+  let mut diagnostics = vec![];
 
   for entry in walkdir::WalkDir::new(&dir).follow_links(false) {
     let entry = entry?;
@@ -37,9 +38,11 @@ pub fn create_gzipped_tarball(
       })?;
       let data = std::fs::read(entry.path())
         .with_context(|| format!("Unable to read file {:?}", entry.path()))?;
-      let content = unfurler
+      let (content, unfurl_diagnostics) = unfurler
         .unfurl(&url, data)
         .with_context(|| format!("Unable to unfurl file {:?}", entry.path()))?;
+
+      diagnostics.extend_from_slice(&unfurl_diagnostics);
       tar
         .add_file(relative_path.to_string(), &content)
         .with_context(|| {
@@ -48,12 +51,13 @@ pub fn create_gzipped_tarball(
     } else if entry.file_type().is_dir() {
       // skip
     } else {
-      log::warn!("Unsupported file type at path {:?}", entry.path());
+      diagnostics
+        .push(format!("Unsupported file type at path {:?}", entry.path()));
     }
   }
 
   let v = tar.finish().context("Unable to finish tarball")?;
-  Ok(Bytes::from(v))
+  Ok((Bytes::from(v), diagnostics))
 }
 
 struct TarGzArchive {
diff --git a/cli/util/import_map.rs b/cli/util/import_map.rs
index b4689064d..cd44d8eab 100644
--- a/cli/util/import_map.rs
+++ b/cli/util/import_map.rs
@@ -25,7 +25,8 @@ impl<'a> ImportMapUnfurler<'a> {
     &self,
     url: &ModuleSpecifier,
     data: Vec<u8>,
-  ) -> Result<Vec<u8>, AnyError> {
+  ) -> Result<(Vec<u8>, Vec<String>), AnyError> {
+    let mut diagnostics = vec![];
     let media_type = MediaType::from_specifier(url);
 
     match media_type {
@@ -48,7 +49,7 @@ impl<'a> ImportMapUnfurler<'a> {
       | MediaType::Wasm
       | MediaType::TsBuildInfo => {
         // not unfurlable data
-        return Ok(data);
+        return Ok((data, diagnostics));
       }
     }
 
@@ -94,14 +95,14 @@ impl<'a> ImportMapUnfurler<'a> {
           );
 
           if !success {
-            log::warn!(
-              "{} Dynamic import was not analyzable and won't use the import map once published.\n    at {}",
-              crate::colors::yellow("Warning"),
-              format_range_with_colors(&deno_graph::Range {
-                specifier: url.clone(),
-                start: dep.range.start.clone(),
-                end: dep.range.end.clone(),
-              })
+            diagnostics.push(
+              format!("Dynamic import was not analyzable and won't use the import map once published.\n    at {}",
+                format_range_with_colors(&deno_graph::Range {
+                  specifier: url.clone(),
+                  start: dep.range.start.clone(),
+                  end: dep.range.end.clone(),
+                })
+              )
             );
           }
         }
@@ -132,13 +133,14 @@ impl<'a> ImportMapUnfurler<'a> {
         &mut text_changes,
       );
     }
-    Ok(
+    Ok((
       deno_ast::apply_text_changes(
         parsed_source.text_info().text_str(),
         text_changes,
       )
       .into_bytes(),
-    )
+      diagnostics,
+    ))
   }
 
   #[cfg(test)]
@@ -146,10 +148,10 @@ impl<'a> ImportMapUnfurler<'a> {
     &self,
     url: &ModuleSpecifier,
     data: Vec<u8>,
-  ) -> Result<String, AnyError> {
-    let data = self.unfurl(url, data)?;
+  ) -> Result<(String, Vec<String>), AnyError> {
+    let (data, diagnostics) = self.unfurl(url, data)?;
     let content = String::from_utf8(data)?;
-    Ok(content)
+    Ok((content, diagnostics))
   }
 }
 
@@ -284,9 +286,12 @@ const test5 = await import(`lib${expr}`);
 const test6 = await import(`${expr}`);
 "#;
       let specifier = ModuleSpecifier::parse("file:///dev/mod.ts").unwrap();
-      let unfurled_source = unfurler
+      let (unfurled_source, d) = unfurler
         .unfurl_to_string(&specifier, source_code.as_bytes().to_vec())
         .unwrap();
+      assert_eq!(d.len(), 2);
+      assert!(d[0].starts_with("Dynamic import was not analyzable and won't use the import map once published."));
+      assert!(d[1].starts_with("Dynamic import was not analyzable and won't use the import map once published."));
       let expected_source = r#"import express from "npm:express@5";"
 import foo from "./lib/foo.ts";
 import bar from "./lib/bar.ts";
@@ -311,9 +316,10 @@ import bar from "lib/bar.ts";
 import fizz from "fizz";
 "#;
       let specifier = ModuleSpecifier::parse("file:///dev/mod").unwrap();
-      let unfurled_source = unfurler
+      let (unfurled_source, d) = unfurler
         .unfurl_to_string(&specifier, source_code.as_bytes().to_vec())
         .unwrap();
+      assert!(d.is_empty());
       assert_eq!(unfurled_source, source_code);
     }
   }
diff --git a/runtime/colors.rs b/runtime/colors.rs
index 8d915b571..8790eccd1 100644
--- a/runtime/colors.rs
+++ b/runtime/colors.rs
@@ -108,6 +108,13 @@ pub fn cyan<S: AsRef<str>>(s: S) -> impl fmt::Display {
   style_spec.set_fg(Some(Cyan));
   style(s, style_spec)
 }
+
+pub fn cyan_with_underline<S: AsRef<str>>(s: S) -> impl fmt::Display {
+  let mut style_spec = ColorSpec::new();
+  style_spec.set_fg(Some(Cyan)).set_underline(true);
+  style(s, style_spec)
+}
+
 pub fn cyan_bold<S: AsRef<str>>(s: S) -> impl fmt::Display {
   let mut style_spec = ColorSpec::new();
   style_spec.set_fg(Some(Cyan)).set_bold(true);