From 2f29673fb215885dd6b4a0f71382e05fff2f7957 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Mon, 18 Apr 2022 15:52:26 +0100 Subject: [PATCH] fix(cli/emit): Check JS roots with // @ts-check (#14090) --- cli/emit.rs | 43 +++++++++++++------ cli/graph_util.rs | 22 ++++++++++ cli/tests/integration/run_tests.rs | 7 +++ cli/tests/testdata/js_root_with_ts_check.js | 5 +++ .../testdata/js_root_with_ts_check.js.out | 4 ++ 5 files changed, 68 insertions(+), 13 deletions(-) create mode 100644 cli/tests/testdata/js_root_with_ts_check.js create mode 100644 cli/tests/testdata/js_root_with_ts_check.js.out diff --git a/cli/emit.rs b/cli/emit.rs index 337895a105..b6eef0b88d 100644 --- a/cli/emit.rs +++ b/cli/emit.rs @@ -279,12 +279,21 @@ fn get_tsc_roots( .entries() .into_iter() .filter_map(|(specifier, module_entry)| match module_entry { - ModuleEntry::Module { media_type, .. } => match &media_type { + ModuleEntry::Module { + media_type, + ts_check, + .. + } => match &media_type { MediaType::TypeScript | MediaType::Tsx | MediaType::Mts | MediaType::Cts | MediaType::Jsx => Some((specifier.clone(), *media_type)), + MediaType::JavaScript | MediaType::Mjs | MediaType::Cjs + if check_js || *ts_check => + { + Some((specifier.clone(), *media_type)) + } _ => None, }, _ => None, @@ -461,20 +470,25 @@ pub fn check_and_maybe_emit( // resolve it via the graph. let graph_data = graph_data.read(); let specifier = graph_data.follow_redirect(&specifiers[0]); - let (source_bytes, media_type) = match graph_data.get(&specifier) { - Some(ModuleEntry::Module { - code, media_type, .. - }) => (code.as_bytes(), *media_type), - _ => { - log::debug!("skipping emit for {}", specifier); - continue; - } - }; + let (source_bytes, media_type, ts_check) = + match graph_data.get(&specifier) { + Some(ModuleEntry::Module { + code, + media_type, + ts_check, + .. + }) => (code.as_bytes(), *media_type, *ts_check), + _ => { + log::debug!("skipping emit for {}", specifier); + continue; + } + }; // Sometimes if `tsc` sees a CommonJS file or a JSON module, it will // _helpfully_ output it, which we don't really want to do unless // someone has enabled check_js. if matches!(media_type, MediaType::Json) || (!check_js + && !ts_check && matches!( media_type, MediaType::JavaScript | MediaType::Cjs | MediaType::Mjs @@ -862,10 +876,13 @@ fn valid_emit( reload_exclusions: &HashSet, ) -> bool { let config_bytes = ts_config.as_bytes(); - let emit_js = ts_config.get_check_js(); + let check_js = ts_config.get_check_js(); for (specifier, module_entry) in graph_data.entries() { if let ModuleEntry::Module { - code, media_type, .. + code, + media_type, + ts_check, + .. } = module_entry { match media_type { @@ -875,7 +892,7 @@ fn valid_emit( | MediaType::Tsx | MediaType::Jsx => {} MediaType::JavaScript | MediaType::Mjs | MediaType::Cjs => { - if !emit_js { + if !check_js && !ts_check { continue; } } diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 11678574c3..f2063108f4 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -13,12 +13,18 @@ use deno_graph::ModuleGraphError; use deno_graph::ModuleKind; use deno_graph::Range; use deno_graph::Resolved; +use once_cell::sync::Lazy; +use regex::Regex; use std::collections::BTreeMap; use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; use std::sync::Arc; +/// Matches the `@ts-check` pragma. +static TS_CHECK_RE: Lazy = + Lazy::new(|| Regex::new(r#"(?i)^\s*@ts-check(?:\s+|$)"#).unwrap()); + pub fn contains_specifier( v: &[(ModuleSpecifier, ModuleKind)], specifier: &ModuleSpecifier, @@ -33,6 +39,8 @@ pub enum ModuleEntry { code: Arc, dependencies: BTreeMap, media_type: MediaType, + /// Whether or not this is a JS/JSX module with a `@ts-check` directive. + ts_check: bool, /// A set of type libs that the module has passed a type check with this /// session. This would consist of window, worker or both. checked_libs: HashSet, @@ -122,9 +130,23 @@ impl GraphData { } } } + let ts_check = match &media_type { + MediaType::JavaScript + | MediaType::Mjs + | MediaType::Cjs + | MediaType::Jsx => { + let parsed_source = module.maybe_parsed_source.as_ref().unwrap(); + parsed_source + .get_leading_comments() + .iter() + .any(|c| TS_CHECK_RE.is_match(&c.text)) + } + _ => false, + }; let module_entry = ModuleEntry::Module { code, dependencies: module.dependencies.clone(), + ts_check, media_type, checked_libs: Default::default(), maybe_types, diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index b918e403df..0953e72395 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -2726,3 +2726,10 @@ itest!(complex_error { output: "complex_error.ts.out", exit_code: 1, }); + +// Regression test for https://github.com/denoland/deno/issues/12143. +itest!(js_root_with_ts_check { + args: "run --quiet js_root_with_ts_check.js", + output: "js_root_with_ts_check.js.out", + exit_code: 1, +}); diff --git a/cli/tests/testdata/js_root_with_ts_check.js b/cli/tests/testdata/js_root_with_ts_check.js new file mode 100644 index 0000000000..adca847ee5 --- /dev/null +++ b/cli/tests/testdata/js_root_with_ts_check.js @@ -0,0 +1,5 @@ +// @ts-check + +/** @type {number} */ +const a = ""; +console.log(a); diff --git a/cli/tests/testdata/js_root_with_ts_check.js.out b/cli/tests/testdata/js_root_with_ts_check.js.out new file mode 100644 index 0000000000..34e2fa61ea --- /dev/null +++ b/cli/tests/testdata/js_root_with_ts_check.js.out @@ -0,0 +1,4 @@ +error: TS2322 [ERROR]: Type 'string' is not assignable to type 'number'. +const a = ""; + ^ + at [WILDCARD]