refactor: improve graph and tsc_config (#7747)

6 files changed, 147 insertions, 104 deletions

diff --git a/cli/global_state.rs b/cli/global_state.rs
index 3fe755ff1..ec2d78130 100644
--- a/cli/global_state.rs
+++ b/cli/global_state.rs
@@ -21,8 +21,6 @@ use deno_core::error::AnyError;
 use deno_core::ModuleSpecifier;
 use std::cell::RefCell;
 use std::env;
-use std::fs;
-use std::io;
 use std::rc::Rc;
 use std::sync::Arc;
 use std::sync::Mutex;
@@ -134,37 +132,17 @@ impl GlobalState {
       builder.insert(&module_specifier).await?;
       let mut graph = builder.get_graph(&self.lockfile)?;
 
-      // TODO(kitsonk) this needs to move, but CompilerConfig is way too
-      // complicated to use here.
-      let maybe_config = if let Some(path) = self.flags.config_path.clone() {
-        let cwd = std::env::current_dir()?;
-        let config_file = cwd.join(path);
-        let config_path = config_file.canonicalize().map_err(|_| {
-          io::Error::new(
-            io::ErrorKind::InvalidInput,
-            format!(
-              "Could not find the config file: {}",
-              config_file.to_string_lossy()
-            ),
-          )
-        })?;
-        let config_str = fs::read_to_string(config_path)?;
-
-        Some(config_str)
-      } else {
-        None
-      };
-
       let (stats, maybe_ignored_options) =
         graph.transpile(TranspileOptions {
           debug: self.flags.log_level == Some(log::Level::Debug),
-          maybe_config,
+          maybe_config_path: self.flags.config_path.clone(),
         })?;
 
-      debug!("{}", stats);
       if let Some(ignored_options) = maybe_ignored_options {
-        println!("Some compiler options were ignored:\n  {}", ignored_options);
+        eprintln!("{}", ignored_options);
       }
+
+      debug!("{}", stats);
     } else {
       let mut module_graph_loader = ModuleGraphLoader::new(
         self.file_fetcher.clone(),
diff --git a/cli/graph.rs b/cli/graph.rs
index ccc7751e4..9025df185 100644
--- a/cli/graph.rs
+++ b/cli/graph.rs
@@ -14,14 +14,12 @@ use crate::specifier_handler::EmitMap;
 use crate::specifier_handler::EmitType;
 use crate::specifier_handler::FetchFuture;
 use crate::specifier_handler::SpecifierHandler;
-use crate::tsc_config::json_merge;
-use crate::tsc_config::parse_config;
 use crate::tsc_config::IgnoredCompilerOptions;
+use crate::tsc_config::TsConfig;
 use crate::AnyError;
 
 use deno_core::futures::stream::FuturesUnordered;
 use deno_core::futures::stream::StreamExt;
-use deno_core::serde_json;
 use deno_core::serde_json::json;
 use deno_core::ModuleSpecifier;
 use regex::Regex;
@@ -38,8 +36,6 @@ use std::sync::Mutex;
 use std::time::Instant;
 use swc_ecmascript::dep_graph::DependencyKind;
 
-type Result<V> = result::Result<V, AnyError>;
-
 pub type BuildInfoMap = HashMap<EmitType, TextDocument>;
 
 lazy_static! {
@@ -123,7 +119,7 @@ pub trait ModuleProvider {
     &self,
     specifier: &str,
     referrer: &ModuleSpecifier,
-  ) -> Result<(ModuleSpecifier, MediaType)>;
+  ) -> Result<(ModuleSpecifier, MediaType), AnyError>;
 }
 
 /// An enum which represents the parsed out values of references in source code.
@@ -237,7 +233,7 @@ impl Module {
     self.is_hydrated = true;
   }
 
-  pub fn parse(&mut self) -> Result<()> {
+  pub fn parse(&mut self) -> Result<(), AnyError> {
     let parsed_module =
       parse(&self.specifier, &self.source.to_str()?, &self.media_type)?;
 
@@ -318,7 +314,7 @@ impl Module {
     &self,
     specifier: &str,
     maybe_location: Option<Location>,
-  ) -> Result<ModuleSpecifier> {
+  ) -> Result<ModuleSpecifier, AnyError> {
     let maybe_resolve = if let Some(import_map) = self.maybe_import_map.clone()
     {
       import_map
@@ -385,22 +381,10 @@ impl fmt::Display for Stats {
 pub struct TranspileOptions {
   /// If `true` then debug logging will be output from the isolate.
   pub debug: bool,
-  /// A string of configuration data that augments the the default configuration
-  /// passed to the TypeScript compiler.  This is typically the contents of a
-  /// user supplied `tsconfig.json`.
-  pub maybe_config: Option<String>,
-}
-
-/// The transpile options that are significant out of a user provided tsconfig
-/// file, that we want to deserialize out of the final config for a transpile.
-#[derive(Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
-struct TranspileConfigOptions {
-  pub check_js: bool,
-  pub emit_decorator_metadata: bool,
-  pub jsx: String,
-  pub jsx_factory: String,
-  pub jsx_fragment_factory: String,
+  /// An optional string that points to a user supplied TypeScript configuration
+  /// file that augments the the default configuration passed to the TypeScript
+  /// compiler.
+  pub maybe_config_path: Option<String>,
 }
 
 /// A dependency graph of modules, were the modules that have been inserted via
@@ -432,7 +416,7 @@ impl Graph {
 
   /// Update the handler with any modules that are marked as _dirty_ and update
   /// any build info if present.
-  fn flush(&mut self, emit_type: &EmitType) -> Result<()> {
+  fn flush(&mut self, emit_type: &EmitType) -> Result<(), AnyError> {
     let mut handler = self.handler.borrow_mut();
     for (_, module) in self.modules.iter_mut() {
       if module.is_dirty {
@@ -462,7 +446,10 @@ impl Graph {
   /// Verify the subresource integrity of the graph based upon the optional
   /// lockfile, updating the lockfile with any missing resources.  This will
   /// error if any of the resources do not match their lock status.
-  pub fn lock(&self, maybe_lockfile: &Option<Mutex<Lockfile>>) -> Result<()> {
+  pub fn lock(
+    &self,
+    maybe_lockfile: &Option<Mutex<Lockfile>>,
+  ) -> Result<(), AnyError> {
     if let Some(lf) = maybe_lockfile {
       let mut lockfile = lf.lock().unwrap();
       for (ms, module) in self.modules.iter() {
@@ -493,28 +480,22 @@ impl Graph {
   pub fn transpile(
     &mut self,
     options: TranspileOptions,
-  ) -> Result<(Stats, Option<IgnoredCompilerOptions>)> {
+  ) -> Result<(Stats, Option<IgnoredCompilerOptions>), AnyError> {
     let start = Instant::now();
     let emit_type = EmitType::Cli;
-    let mut compiler_options = json!({
+
+    let mut ts_config = TsConfig::new(json!({
       "checkJs": false,
       "emitDecoratorMetadata": false,
       "jsx": "react",
       "jsxFactory": "React.createElement",
       "jsxFragmentFactory": "React.Fragment",
-    });
+    }));
 
-    let maybe_ignored_options = if let Some(config_text) = options.maybe_config
-    {
-      let (user_config, ignored_options) = parse_config(&config_text)?;
-      json_merge(&mut compiler_options, &user_config);
-      ignored_options
-    } else {
-      None
-    };
+    let maybe_ignored_options =
+      ts_config.merge_user_config(options.maybe_config_path)?;
 
-    let compiler_options: TranspileConfigOptions =
-      serde_json::from_value(compiler_options)?;
+    let compiler_options = ts_config.as_transpile_config()?;
     let check_js = compiler_options.check_js;
     let transform_jsx = compiler_options.jsx == "react";
     let emit_options = ast::TranspileOptions {
@@ -581,7 +562,7 @@ impl<'a> ModuleProvider for Graph {
     &self,
     specifier: &str,
     referrer: &ModuleSpecifier,
-  ) -> Result<(ModuleSpecifier, MediaType)> {
+  ) -> Result<(ModuleSpecifier, MediaType), AnyError> {
     if !self.modules.contains_key(referrer) {
       return Err(MissingSpecifier(referrer.to_owned()).into());
     }
@@ -659,7 +640,7 @@ impl GraphBuilder {
 
   /// Request a module to be fetched from the handler and queue up its future
   /// to be awaited to be resolved.
-  fn fetch(&mut self, specifier: &ModuleSpecifier) -> Result<()> {
+  fn fetch(&mut self, specifier: &ModuleSpecifier) -> Result<(), AnyError> {
     if self.fetched.contains(&specifier) {
       return Ok(());
     }
@@ -674,7 +655,7 @@ impl GraphBuilder {
   /// Visit a module that has been fetched, hydrating the module, analyzing its
   /// dependencies if required, fetching those dependencies, and inserting the
   /// module into the graph.
-  fn visit(&mut self, cached_module: CachedModule) -> Result<()> {
+  fn visit(&mut self, cached_module: CachedModule) -> Result<(), AnyError> {
     let specifier = cached_module.specifier.clone();
     let mut module =
       Module::new(specifier.clone(), self.maybe_import_map.clone());
@@ -711,7 +692,10 @@ impl GraphBuilder {
   /// Insert a module into the graph based on a module specifier.  The module
   /// and any dependencies will be fetched from the handler.  The module will
   /// also be treated as a _root_ module in the graph.
-  pub async fn insert(&mut self, specifier: &ModuleSpecifier) -> Result<()> {
+  pub async fn insert(
+    &mut self,
+    specifier: &ModuleSpecifier,
+  ) -> Result<(), AnyError> {
     self.fetch(specifier)?;
 
     loop {
@@ -736,7 +720,7 @@ impl GraphBuilder {
   pub fn get_graph(
     self,
     maybe_lockfile: &Option<Mutex<Lockfile>>,
-  ) -> Result<Graph> {
+  ) -> Result<Graph, AnyError> {
     self.graph.lock(maybe_lockfile)?;
     Ok(self.graph)
   }
@@ -902,7 +886,7 @@ mod tests {
     let c = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap());
     let fixtures = c.join("tests/module_graph");
     let handler = Rc::new(RefCell::new(MockSpecifierHandler {
-      fixtures,
+      fixtures: fixtures.clone(),
       ..MockSpecifierHandler::default()
     }));
     let mut builder = GraphBuilder::new(handler.clone(), None);
@@ -914,21 +898,15 @@ mod tests {
       .await
       .expect("module not inserted");
     let mut graph = builder.get_graph(&None).expect("could not get graph");
-    let config = r#"{
-        "compilerOptions": {
-          "target": "es5",
-          "jsx": "preserve"
-        }
-      }"#;
     let (_, maybe_ignored_options) = graph
       .transpile(TranspileOptions {
         debug: false,
-        maybe_config: Some(config.to_string()),
+        maybe_config_path: Some("tests/module_graph/tsconfig.json".to_string()),
       })
       .unwrap();
     assert_eq!(
-      maybe_ignored_options,
-      Some(IgnoredCompilerOptions(vec!["target".to_string()])),
+      maybe_ignored_options.unwrap().items,
+      vec!["target".to_string()],
       "the 'target' options should have been ignored"
     );
     let h = handler.borrow();
diff --git a/cli/tests/config.ts.out b/cli/tests/config.ts.out
index 8f5cf7e39..9840dba2e 100644
--- a/cli/tests/config.ts.out
+++ b/cli/tests/config.ts.out
@@ -1,4 +1,4 @@
-[WILDCARD]Unsupported compiler options in "[WILDCARD]config.tsconfig.json"
+[WILDCARD]Unsupported compiler options in "[WILDCARD]config.tsconfig.json".
   The following options were ignored:
     module, target
 error: TS2532 [ERROR]: Object is possibly 'undefined'.
diff --git a/cli/tests/module_graph/tsconfig.json b/cli/tests/module_graph/tsconfig.json
new file mode 100644
index 000000000..a4c5f4f33
--- /dev/null
+++ b/cli/tests/module_graph/tsconfig.json
@@ -0,0 +1,6 @@
+{
+  "compilerOptions": {
+    "target": "ES5",
+    "jsx": "preserve"
+  }
+}
diff --git a/cli/tsc.rs b/cli/tsc.rs
index 245247f13..9b944125f 100644
--- a/cli/tsc.rs
+++ b/cli/tsc.rs
@@ -41,7 +41,6 @@ use std::collections::HashSet;
 use std::fs;
 use std::io;
 use std::ops::Deref;
-use std::path::Path;
 use std::path::PathBuf;
 use std::str;
 use std::sync::Arc;
@@ -141,14 +140,9 @@ lazy_static! {
 
 fn warn_ignored_options(
   maybe_ignored_options: Option<tsc_config::IgnoredCompilerOptions>,
-  config_path: &Path,
 ) {
   if let Some(ignored_options) = maybe_ignored_options {
-    eprintln!(
-      "Unsupported compiler options in \"{}\"\n  The following options were ignored:\n    {}",
-      config_path.to_string_lossy(),
-      ignored_options
-    );
+    eprintln!("{}", ignored_options);
   }
 }
 
@@ -210,7 +204,7 @@ impl CompilerConfig {
     let (options, maybe_ignored_options) = if config_str.is_empty() {
       (json!({}), None)
     } else {
-      tsc_config::parse_config(&config_str)?
+      tsc_config::parse_config(&config_str, &config_path)?
     };
 
     // If `checkJs` is set to true in `compilerOptions` then we're gonna be compiling
@@ -526,10 +520,7 @@ impl TsCompiler {
 
     tsc_config::json_merge(&mut compiler_options, &compiler_config.options);
 
-    warn_ignored_options(
-      compiler_config.maybe_ignored_options,
-      compiler_config.path.as_ref().unwrap(),
-    );
+    warn_ignored_options(compiler_config.maybe_ignored_options);
 
     let j = json!({
       "type": CompilerRequestType::Compile,
@@ -646,10 +637,7 @@ impl TsCompiler {
 
     tsc_config::json_merge(&mut compiler_options, &compiler_config.options);
 
-    warn_ignored_options(
-      compiler_config.maybe_ignored_options,
-      compiler_config.path.as_ref().unwrap(),
-    );
+    warn_ignored_options(compiler_config.maybe_ignored_options);
 
     let j = json!({
       "type": CompilerRequestType::Bundle,
diff --git a/cli/tsc_config.rs b/cli/tsc_config.rs
index f748c7a5c..b52ed2abd 100644
--- a/cli/tsc_config.rs
+++ b/cli/tsc_config.rs
@@ -5,19 +5,40 @@ use deno_core::serde_json;
 use deno_core::serde_json::Value;
 use jsonc_parser::JsonValue;
 use serde::Deserialize;
+use serde::Serialize;
+use serde::Serializer;
 use std::collections::HashMap;
 use std::fmt;
+use std::path::Path;
+use std::path::PathBuf;
 use std::str::FromStr;
 
-#[derive(Clone, Debug, PartialEq)]
-pub struct IgnoredCompilerOptions(pub Vec<String>);
+/// The transpile options that are significant out of a user provided tsconfig
+/// file, that we want to deserialize out of the final config for a transpile.
+#[derive(Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct TranspileConfigOptions {
+  pub check_js: bool,
+  pub emit_decorator_metadata: bool,
+  pub jsx: String,
+  pub jsx_factory: String,
+  pub jsx_fragment_factory: String,
+}
+
+/// A structure that represents a set of options that were ignored and the
+/// path those options came from.
+#[derive(Debug, Clone, PartialEq)]
+pub struct IgnoredCompilerOptions {
+  pub items: Vec<String>,
+  pub path: PathBuf,
+}
 
 impl fmt::Display for IgnoredCompilerOptions {
   fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-    let mut codes = self.0.clone();
+    let mut codes = self.items.clone();
     codes.sort();
 
-    write!(f, "{}", codes.join(", "))
+    write!(f, "Unsupported compiler options in \"{}\".\n  The following options were ignored:\n    {}", self.path.to_string_lossy(), codes.join(", "))
   }
 }
 
@@ -149,6 +170,7 @@ pub fn parse_raw_config(config_text: &str) -> Result<Value, AnyError> {
 /// The result also contains any options that were ignored.
 pub fn parse_config(
   config_text: &str,
+  path: &Path,
 ) -> Result<(Value, Option<IgnoredCompilerOptions>), AnyError> {
   assert!(!config_text.is_empty());
   let jsonc = jsonc_parser::parse_to_value(config_text)?.unwrap();
@@ -167,7 +189,10 @@ pub fn parse_config(
   }
   let options_value = serde_json::to_value(compiler_options)?;
   let ignored_options = if !items.is_empty() {
-    Some(IgnoredCompilerOptions(items))
+    Some(IgnoredCompilerOptions {
+      items,
+      path: path.to_path_buf(),
+    })
   } else {
     None
   };
@@ -175,6 +200,70 @@ pub fn parse_config(
   Ok((options_value, ignored_options))
 }
 
+/// A structure for managing the configuration of TypeScript
+#[derive(Debug, Clone)]
+pub struct TsConfig(Value);
+
+impl TsConfig {
+  /// Create a new `TsConfig` with the base being the `value` supplied.
+  pub fn new(value: Value) -> Self {
+    TsConfig(value)
+  }
+
+  /// Take an optional string representing a user provided TypeScript config file
+  /// which was passed in via the `--config` compiler option and merge it with
+  /// the configuration.  Returning the result which optionally contains any
+  /// compiler options that were ignored.
+  ///
+  /// When there are options ignored out of the file, a warning will be written
+  /// to stderr regarding the options that were ignored.
+  pub fn merge_user_config(
+    &mut self,
+    maybe_path: Option<String>,
+  ) -> Result<Option<IgnoredCompilerOptions>, AnyError> {
+    if let Some(path) = maybe_path {
+      let cwd = std::env::current_dir()?;
+      let config_file = cwd.join(path);
+      let config_path = config_file.canonicalize().map_err(|_| {
+        std::io::Error::new(
+          std::io::ErrorKind::InvalidInput,
+          format!(
+            "Could not find the config file: {}",
+            config_file.to_string_lossy()
+          ),
+        )
+      })?;
+      let config_text = std::fs::read_to_string(config_path.clone())?;
+      let (value, maybe_ignored_options) =
+        parse_config(&config_text, &config_path)?;
+      json_merge(&mut self.0, &value);
+
+      Ok(maybe_ignored_options)
+    } else {
+      Ok(None)
+    }
+  }
+
+  /// Return the current configuration as a `TranspileConfigOptions` structure.
+  pub fn as_transpile_config(
+    &self,
+  ) -> Result<TranspileConfigOptions, AnyError> {
+    let options: TranspileConfigOptions =
+      serde_json::from_value(self.0.clone())?;
+    Ok(options)
+  }
+}
+
+impl Serialize for TsConfig {
+  /// Serializes inner hash map which is ordered by the key
+  fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
+  where
+    S: Serializer,
+  {
+    Serialize::serialize(&self.0, serializer)
+  }
+}
+
 #[cfg(test)]
 mod tests {
   use super::*;
@@ -210,15 +299,19 @@ mod tests {
         "strict": true
       }
     }"#;
+    let config_path = PathBuf::from("/deno/tsconfig.json");
     let (options_value, ignored) =
-      parse_config(config_text).expect("error parsing");
+      parse_config(config_text, &config_path).expect("error parsing");
     assert!(options_value.is_object());
     let options = options_value.as_object().unwrap();
     assert!(options.contains_key("strict"));
     assert_eq!(options.len(), 1);
     assert_eq!(
       ignored,
-      Some(IgnoredCompilerOptions(vec!["build".to_string()])),
+      Some(IgnoredCompilerOptions {
+        items: vec!["build".to_string()],
+        path: config_path,
+      }),
     );
   }