fix(lsp): formatting should error on certain additional swc diagnostics (#12491)

7 files changed, 198 insertions, 171 deletions

diff --git a/cli/Cargo.toml b/cli/Cargo.toml
index 5eaa13ed5..d667edfab 100644
--- a/cli/Cargo.toml
+++ b/cli/Cargo.toml
@@ -39,11 +39,11 @@ winapi = "0.3.9"
 winres = "0.1.11"
 
 [dependencies]
-deno_ast = { version = "0.3.0", features = ["bundler", "codegen", "dep_graph", "module_specifier", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] }
+deno_ast = { version = "0.4.1", features = ["bundler", "codegen", "dep_graph", "module_specifier", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] }
 deno_core = { version = "0.104.0", path = "../core" }
-deno_doc = "0.16.0"
-deno_graph = "0.7.0"
-deno_lint = { version = "0.17.0", features = ["docs"] }
+deno_doc = "0.17.1"
+deno_graph = "0.8.2"
+deno_lint = { version = "0.18.1", features = ["docs"] }
 deno_runtime = { version = "0.30.0", path = "../runtime" }
 deno_tls = { version = "0.9.0", path = "../ext/tls" }
 
@@ -54,7 +54,7 @@ data-url = "0.1.0"
 dissimilar = "1.0.2"
 dprint-plugin-json = "0.13.0"
 dprint-plugin-markdown = "0.10.0"
-dprint-plugin-typescript = "0.57.4"
+dprint-plugin-typescript = "0.58.1"
 encoding_rs = "0.8.28"
 env_logger = "0.8.4"
 fancy-regex = "0.7.1"
diff --git a/cli/ast/mod.rs b/cli/ast/mod.rs
index f0ad36885..894a42d4e 100644
--- a/cli/ast/mod.rs
+++ b/cli/ast/mod.rs
@@ -24,7 +24,7 @@ use deno_ast::swc::transforms::hygiene;
 use deno_ast::swc::transforms::pass::Optional;
 use deno_ast::swc::transforms::proposals;
 use deno_ast::swc::transforms::react;
-use deno_ast::swc::transforms::resolver::ts_resolver;
+use deno_ast::swc::transforms::resolver_with_mark;
 use deno_ast::swc::transforms::typescript;
 use deno_ast::swc::visit::FoldWith;
 use deno_ast::Diagnostic;
@@ -182,6 +182,8 @@ fn strip_config_from_emit_options(
   options: &EmitOptions,
 ) -> typescript::strip::Config {
   typescript::strip::Config {
+    pragma: Some(options.jsx_factory.clone()),
+    pragma_frag: Some(options.jsx_fragment_factory.clone()),
     import_not_used_as_values: match options.imports_not_used_as_values {
       ImportsNotUsedAsValues::Remove => {
         typescript::strip::ImportsNotUsedAsValues::Remove
@@ -219,42 +221,14 @@ pub fn transpile(
   let globals = Globals::new();
   deno_ast::swc::common::GLOBALS.set(&globals, || {
     let top_level_mark = Mark::fresh(Mark::root());
-    let jsx_pass = chain!(
-      ts_resolver(top_level_mark),
-      react::react(
-        source_map.clone(),
-        Some(&comments),
-        react::Options {
-          pragma: options.jsx_factory.clone(),
-          pragma_frag: options.jsx_fragment_factory.clone(),
-          // this will use `Object.assign()` instead of the `_extends` helper
-          // when spreading props.
-          use_builtins: true,
-          ..Default::default()
-        },
-        top_level_mark,
-      ),
-    );
-    let mut passes = chain!(
-      Optional::new(jsx_pass, options.transform_jsx),
-      Optional::new(transforms::DownlevelImportsFolder, options.repl_imports),
-      Optional::new(transforms::StripExportsFolder, options.repl_imports),
-      proposals::decorators::decorators(proposals::decorators::Config {
-        legacy: true,
-        emit_metadata: options.emit_metadata
-      }),
-      helpers::inject_helpers(),
-      typescript::strip::strip_with_config(strip_config_from_emit_options(
-        options
-      )),
-      fixer(Some(&comments)),
-      hygiene(),
+    let module = fold_program(
+      program,
+      options,
+      source_map.clone(),
+      &comments,
+      top_level_mark,
     );
 
-    let program = helpers::HELPERS.set(&helpers::Helpers::new(false), || {
-      program.fold_with(&mut passes)
-    });
-
     let mut src_map_buf = vec![];
     let mut buf = vec![];
     {
@@ -271,7 +245,7 @@ pub fn transpile(
         cm: source_map.clone(),
         wr: writer,
       };
-      program.emit_with(&mut emitter)?;
+      module.emit_with(&mut emitter)?;
     }
     let mut src = String::from_utf8(buf)?;
     let mut map: Option<String> = None;
@@ -299,8 +273,7 @@ pub fn transpile_module(
   specifier: &ModuleSpecifier,
   source: &str,
   media_type: MediaType,
-  emit_options: &EmitOptions,
-  globals: &Globals,
+  options: &EmitOptions,
   cm: Rc<SourceMap>,
 ) -> Result<(Rc<deno_ast::swc::common::SourceFile>, Module), AnyError> {
   let source = strip_bom(source);
@@ -314,52 +287,85 @@ pub fn transpile_module(
   let module = parser.parse_module().map_err(|err| {
     let location = cm.lookup_char_pos(err.span().lo);
     Diagnostic {
+      specifier: specifier.to_string(),
+      span: err.span(),
       display_position: LineAndColumnDisplay {
         line_number: location.line,
         column_number: location.col_display + 1,
       },
-      specifier: specifier.to_string(),
-      message: err.into_kind().msg().to_string(),
+      kind: err.into_kind(),
     }
   })?;
 
-  deno_ast::swc::common::GLOBALS.set(globals, || {
-    let top_level_mark = Mark::fresh(Mark::root());
-    let jsx_pass = chain!(
-      ts_resolver(top_level_mark),
-      react::react(
-        cm,
-        Some(&comments),
-        react::Options {
-          pragma: emit_options.jsx_factory.clone(),
-          pragma_frag: emit_options.jsx_fragment_factory.clone(),
-          // this will use `Object.assign()` instead of the `_extends` helper
-          // when spreading props.
-          use_builtins: true,
-          ..Default::default()
-        },
-        top_level_mark,
-      ),
-    );
-    let mut passes = chain!(
-      Optional::new(jsx_pass, emit_options.transform_jsx),
-      proposals::decorators::decorators(proposals::decorators::Config {
-        legacy: true,
-        emit_metadata: emit_options.emit_metadata
-      }),
-      helpers::inject_helpers(),
+  let top_level_mark = Mark::fresh(Mark::root());
+  let program = fold_program(
+    Program::Module(module),
+    options,
+    cm,
+    &comments,
+    top_level_mark,
+  );
+  let module = match program {
+    Program::Module(module) => module,
+    _ => unreachable!(),
+  };
+
+  Ok((source_file, module))
+}
+
+fn fold_program(
+  program: Program,
+  options: &EmitOptions,
+  source_map: Rc<SourceMap>,
+  comments: &SingleThreadedComments,
+  top_level_mark: Mark,
+) -> Program {
+  let jsx_pass = chain!(
+    resolver_with_mark(top_level_mark),
+    react::react(
+      source_map.clone(),
+      Some(comments),
+      react::Options {
+        pragma: options.jsx_factory.clone(),
+        pragma_frag: options.jsx_fragment_factory.clone(),
+        // this will use `Object.assign()` instead of the `_extends` helper
+        // when spreading props.
+        use_builtins: true,
+        ..Default::default()
+      },
+      top_level_mark,
+    ),
+  );
+  let mut passes = chain!(
+    Optional::new(transforms::DownlevelImportsFolder, options.repl_imports),
+    Optional::new(transforms::StripExportsFolder, options.repl_imports),
+    proposals::decorators::decorators(proposals::decorators::Config {
+      legacy: true,
+      emit_metadata: options.emit_metadata
+    }),
+    helpers::inject_helpers(),
+    Optional::new(
       typescript::strip::strip_with_config(strip_config_from_emit_options(
-        emit_options
+        options
       )),
-      fixer(Some(&comments)),
-      hygiene(),
-    );
-
-    let module = helpers::HELPERS.set(&helpers::Helpers::new(false), || {
-      module.fold_with(&mut passes)
-    });
-
-    Ok((source_file, module))
+      !options.transform_jsx
+    ),
+    Optional::new(
+      typescript::strip::strip_with_jsx(
+        source_map,
+        strip_config_from_emit_options(options),
+        comments,
+        top_level_mark
+      ),
+      options.transform_jsx
+    ),
+    Optional::new(jsx_pass, options.transform_jsx),
+    fixer(Some(comments)),
+    hygiene(),
+  );
+
+  helpers::HELPERS.set(&helpers::Helpers::new(false), || {
+    program.fold_with(&mut passes)
   })
 }
 
@@ -435,6 +441,37 @@ mod tests {
   }
 
   #[test]
+  fn test_transpile_jsx_pragma() {
+    let specifier = resolve_url_or_path("https://deno.land/x/mod.ts")
+      .expect("could not resolve specifier");
+    let source = r#"
+/** @jsx h */
+/** @jsxFrag Fragment */
+import { h, Fragment } from "https://deno.land/x/mod.ts";
+
+function App() {
+  return (
+    <div><></></div>
+  );
+}"#;
+    let module = parse_module(ParseParams {
+      specifier: specifier.as_str().to_string(),
+      source: SourceTextInfo::from_string(source.to_string()),
+      media_type: deno_ast::MediaType::Jsx,
+      capture_tokens: false,
+      maybe_syntax: None,
+      scope_analysis: true,
+    })
+    .unwrap();
+    let (code, _) = transpile(&module, &EmitOptions::default()).unwrap();
+    let expected = r#"/** @jsx h */ /** @jsxFrag Fragment */ import { h, Fragment } from "https://deno.land/x/mod.ts";
+function App() {
+    return(/*#__PURE__*/ h("div", null, /*#__PURE__*/ h(Fragment, null)));
+}"#;
+    assert_eq!(&code[..expected.len()], expected);
+  }
+
+  #[test]
   fn test_transpile_decorators() {
     let specifier = resolve_url_or_path("https://deno.land/x/mod.ts")
       .expect("could not resolve specifier");
diff --git a/cli/emit.rs b/cli/emit.rs
index 472648bca..7b25713fa 100644
--- a/cli/emit.rs
+++ b/cli/emit.rs
@@ -439,7 +439,6 @@ pub(crate) struct BundleOptions {
 struct BundleLoader<'a> {
   cm: Rc<swc::common::SourceMap>,
   emit_options: &'a ast::EmitOptions,
-  globals: &'a deno_ast::swc::common::Globals,
   graph: &'a ModuleGraph,
 }
 
@@ -456,7 +455,6 @@ impl swc::bundler::Load for BundleLoader<'_> {
             &m.source,
             m.media_type,
             self.emit_options,
-            self.globals,
             self.cm.clone(),
           )?;
           Ok(swc::bundler::ModuleData {
@@ -518,81 +516,82 @@ pub(crate) fn bundle(
   graph: &ModuleGraph,
   options: BundleOptions,
 ) -> Result<(String, Option<String>), AnyError> {
-  let emit_options: ast::EmitOptions = options.ts_config.into();
-
-  let cm = Rc::new(swc::common::SourceMap::new(
-    swc::common::FilePathMapping::empty(),
-  ));
   let globals = swc::common::Globals::new();
-  let loader = BundleLoader {
-    graph,
-    emit_options: &emit_options,
-    globals: &globals,
-    cm: cm.clone(),
-  };
-  let resolver = BundleResolver(graph);
-  let config = swc::bundler::Config {
-    module: options.bundle_type.into(),
-    ..Default::default()
-  };
-  // This hook will rewrite the `import.meta` when bundling to give a consistent
-  // behavior between bundled and unbundled code.
-  let hook = Box::new(ast::BundleHook);
-  let bundler = swc::bundler::Bundler::new(
-    &globals,
-    cm.clone(),
-    loader,
-    resolver,
-    config,
-    hook,
-  );
-  let mut entries = HashMap::new();
-  entries.insert(
-    "bundle".to_string(),
-    swc::common::FileName::Url(graph.roots[0].clone()),
-  );
-  let output = bundler
-    .bundle(entries)
-    .context("Unable to output during bundling.")?;
-  let mut buf = Vec::new();
-  let mut srcmap = Vec::new();
-  {
-    let cfg = swc::codegen::Config { minify: false };
-    let wr = Box::new(swc::codegen::text_writer::JsWriter::new(
-      cm.clone(),
-      "\n",
-      &mut buf,
-      Some(&mut srcmap),
+  deno_ast::swc::common::GLOBALS.set(&globals, || {
+    let emit_options: ast::EmitOptions = options.ts_config.into();
+
+    let cm = Rc::new(swc::common::SourceMap::new(
+      swc::common::FilePathMapping::empty(),
     ));
-    let mut emitter = swc::codegen::Emitter {
-      cfg,
+    let loader = BundleLoader {
+      graph,
+      emit_options: &emit_options,
       cm: cm.clone(),
-      comments: None,
-      wr,
     };
-    emitter
-      .emit_module(&output[0].module)
-      .context("Unable to emit during bundling.")?;
-  }
-  let mut code =
-    String::from_utf8(buf).context("Emitted code is an invalid string.")?;
-  let mut maybe_map: Option<String> = None;
-  {
+    let resolver = BundleResolver(graph);
+    let config = swc::bundler::Config {
+      module: options.bundle_type.into(),
+      ..Default::default()
+    };
+    // This hook will rewrite the `import.meta` when bundling to give a consistent
+    // behavior between bundled and unbundled code.
+    let hook = Box::new(ast::BundleHook);
+    let mut bundler = swc::bundler::Bundler::new(
+      &globals,
+      cm.clone(),
+      loader,
+      resolver,
+      config,
+      hook,
+    );
+    let mut entries = HashMap::new();
+    entries.insert(
+      "bundle".to_string(),
+      swc::common::FileName::Url(graph.roots[0].clone()),
+    );
+    let output = bundler
+      .bundle(entries)
+      .context("Unable to output during bundling.")?;
     let mut buf = Vec::new();
-    cm.build_source_map_from(&mut srcmap, None)
-      .to_writer(&mut buf)?;
-    if emit_options.inline_source_map {
-      let encoded_map = format!(
-        "//# sourceMappingURL=data:application/json;base64,{}\n",
-        base64::encode(buf)
-      );
-      code.push_str(&encoded_map);
-    } else if emit_options.source_map {
-      maybe_map = Some(String::from_utf8(buf)?);
+    let mut srcmap = Vec::new();
+    {
+      let cfg = swc::codegen::Config { minify: false };
+      let wr = Box::new(swc::codegen::text_writer::JsWriter::new(
+        cm.clone(),
+        "\n",
+        &mut buf,
+        Some(&mut srcmap),
+      ));
+      let mut emitter = swc::codegen::Emitter {
+        cfg,
+        cm: cm.clone(),
+        comments: None,
+        wr,
+      };
+      emitter
+        .emit_module(&output[0].module)
+        .context("Unable to emit during bundling.")?;
+    }
+    let mut code =
+      String::from_utf8(buf).context("Emitted code is an invalid string.")?;
+    let mut maybe_map: Option<String> = None;
+    {
+      let mut buf = Vec::new();
+      cm.build_source_map_from(&mut srcmap, None)
+        .to_writer(&mut buf)?;
+      if emit_options.inline_source_map {
+        let encoded_map = format!(
+          "//# sourceMappingURL=data:application/json;base64,{}\n",
+          base64::encode(buf)
+        );
+        code.push_str(&encoded_map);
+      } else if emit_options.source_map {
+        maybe_map = Some(String::from_utf8(buf)?);
+      }
     }
-  }
 
-  Ok((code, maybe_map))
+    Ok((code, maybe_map))
+  })
 }
 
 pub(crate) struct EmitOptions {
@@ -735,7 +734,7 @@ impl fmt::Display for GraphError {
           ResolutionError::InvalidDowngrade(_, _)
             | ResolutionError::InvalidLocalImport(_, _)
         ) {
-          write!(f, "{}", err.to_string_with_span())
+          write!(f, "{}", err.to_string_with_range())
         } else {
           self.0.fmt(f)
         }
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index f92e974e8..c5de984ad 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -62,7 +62,7 @@ use crate::file_fetcher::get_source_from_data_url;
 use crate::fs_util;
 use crate::logger;
 use crate::tools::fmt::format_file;
-use crate::tools::fmt::format_parsed_module;
+use crate::tools::fmt::format_parsed_source;
 
 pub const REGISTRIES_PATH: &str = "registries";
 const SOURCES_PATH: &str = "deps";
@@ -1086,7 +1086,7 @@ impl Inner {
     let text_edits = tokio::task::spawn_blocking(move || {
       let format_result = match source.module() {
         Some(Ok(parsed_module)) => {
-          Ok(format_parsed_module(parsed_module, fmt_options))
+          format_parsed_source(parsed_module, fmt_options)
         }
         Some(Err(err)) => Err(err.to_string()),
         None => {
diff --git a/cli/proc_state.rs b/cli/proc_state.rs
index a633e3f9d..b7956d085 100644
--- a/cli/proc_state.rs
+++ b/cli/proc_state.rs
@@ -60,10 +60,10 @@ struct GraphData {
   // resolution map so that those errors can be surfaced at the appropriate time
   resolution_map:
     HashMap<ModuleSpecifier, HashMap<String, deno_graph::Resolved>>,
-  // in some cases we want to provide the span where the resolution error
+  // in some cases we want to provide the range where the resolution error
   // occurred but need to surface it on load, but on load we don't know who the
   // referrer and span was, so we need to cache those
-  resolved_map: HashMap<ModuleSpecifier, deno_graph::Span>,
+  resolved_map: HashMap<ModuleSpecifier, deno_graph::Range>,
   // deno_graph detects all sorts of issues at build time (prepare_module_load)
   // but if they are errors at that stage, the don't cause the correct behaviors
   // so we cache the error and then surface it when appropriate (e.g. load)
@@ -469,7 +469,7 @@ impl ProcState {
           Some(Err(err)) => {
             return Err(custom_error(
               "TypeError",
-              format!("{}\n", err.to_string_with_span()),
+              format!("{}\n", err.to_string_with_range()),
             ))
           }
           _ => (),
diff --git a/cli/tools/fmt.rs b/cli/tools/fmt.rs
index 5d33240a5..9ac9557cd 100644
--- a/cli/tools/fmt.rs
+++ b/cli/tools/fmt.rs
@@ -220,24 +220,15 @@ pub fn format_file(
   }
 }
 
-pub fn format_parsed_module(
+pub fn format_parsed_source(
   parsed_source: &ParsedSource,
   fmt_options: FmtOptionsConfig,
-) -> String {
-  dprint_plugin_typescript::format_parsed_file(
-    &dprint_plugin_typescript::SourceFileInfo {
-      is_jsx: matches!(
-        parsed_source.media_type(),
-        deno_ast::MediaType::Jsx | deno_ast::MediaType::Tsx
-      ),
-      info: parsed_source.source(),
-      leading_comments: parsed_source.comments().leading_map(),
-      trailing_comments: parsed_source.comments().trailing_map(),
-      module: parsed_source.module(),
-      tokens: parsed_source.tokens(),
-    },
+) -> Result<String, String> {
+  dprint_plugin_typescript::format_parsed_source(
+    parsed_source,
     &get_resolved_typescript_config(&fmt_options),
   )
+  .map_err(|e| e.to_string())
 }
 
 async fn check_source_files(
diff --git a/cli/tools/repl.rs b/cli/tools/repl.rs
index 7bad940d4..f3ba626af 100644
--- a/cli/tools/repl.rs
+++ b/cli/tools/repl.rs
@@ -540,7 +540,7 @@ impl ReplSession {
           Some(diagnostic) => Ok(EvaluationOutput::Error(format!(
             "{}: {} at {}:{}",
             colors::red("parse error"),
-            diagnostic.message,
+            diagnostic.message(),
             diagnostic.display_position.line_number,
             diagnostic.display_position.column_number,
           ))),