From 6f278e5c40d101f0fb8e7b69e28d34b1c766a8fe Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 15 Apr 2024 17:50:52 -0400 Subject: [PATCH] fix(lsp): improved cjs tracking (#23374) Our cjs tracking was a bit broken. It was marking stuff as esm that was actually cjs leading to type checking errors. --- Cargo.lock | 5 +- cli/lsp/documents.rs | 205 +++++++++++------- cli/lsp/tsc.rs | 5 + cli/resolver.rs | 7 + cli/tsc/99_main_compiler.js | 24 +- tests/Cargo.toml | 2 +- tests/integration/lsp_tests.rs | 49 +++++ tests/specs/mod.rs | 31 +-- .../__test__.jsonc | 15 ++ .../main.out | 5 + .../cjs_internal_types_default_export/main.ts | 3 + .../package.json | 5 + .../1.0.0/add.d.ts | 3 + .../1.0.0/index.d.ts | 1 + .../1.0.0/index.js | 1 + .../1.0.0/package.json | 4 + 16 files changed, 263 insertions(+), 102 deletions(-) create mode 100644 tests/specs/npm/cjs_internal_types_default_export/__test__.jsonc create mode 100644 tests/specs/npm/cjs_internal_types_default_export/main.out create mode 100644 tests/specs/npm/cjs_internal_types_default_export/main.ts create mode 100644 tests/specs/npm/cjs_internal_types_default_export/package.json create mode 100644 tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/add.d.ts create mode 100644 tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/index.d.ts create mode 100644 tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/index.js create mode 100644 tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/package.json diff --git a/Cargo.lock b/Cargo.lock index b449f1a285..4715e178e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2641,10 +2641,11 @@ checksum = "c007b1ae3abe1cb6f85a16305acd418b7ca6343b953633fee2b76d8f108b830f" [[package]] name = "file_test_runner" -version = "0.2.0" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e13bacb20a988be248a13adc6011bce718648f78d4684d4dd0444d05256f4a7b" +checksum = "b66e9ef00f9f6b82b030b7a9d659030f73498921d4c021b0772e75dfd7090d80" dependencies = [ + "anyhow", "crossbeam-channel", "deno_terminal", "parking_lot 0.12.1", diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 1dd18f5997..e7ef048cfd 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -12,6 +12,7 @@ use super::tsc::AssetDocument; use crate::args::package_json; use crate::cache::HttpCache; use crate::jsr::JsrCacheResolver; +use crate::lsp::logging::lsp_warn; use crate::npm::CliNpmResolver; use crate::resolver::CliGraphResolver; use crate::resolver::CliGraphResolverOptions; @@ -43,7 +44,6 @@ use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; use indexmap::IndexMap; -use once_cell::sync::Lazy; use package_json::PackageJsonDepsProvider; use std::borrow::Cow; use std::collections::BTreeSet; @@ -57,36 +57,6 @@ use std::sync::atomic::Ordering; use std::sync::Arc; use tower_lsp::lsp_types as lsp; -static JS_HEADERS: Lazy> = Lazy::new(|| { - ([( - "content-type".to_string(), - "application/javascript".to_string(), - )]) - .into_iter() - .collect() -}); - -static JSX_HEADERS: Lazy> = Lazy::new(|| { - ([("content-type".to_string(), "text/jsx".to_string())]) - .into_iter() - .collect() -}); - -static TS_HEADERS: Lazy> = Lazy::new(|| { - ([( - "content-type".to_string(), - "application/typescript".to_string(), - )]) - .into_iter() - .collect() -}); - -static TSX_HEADERS: Lazy> = Lazy::new(|| { - ([("content-type".to_string(), "text/tsx".to_string())]) - .into_iter() - .collect() -}); - pub const DOCUMENT_SCHEMES: [&str; 5] = ["data", "blob", "file", "http", "https"]; @@ -103,18 +73,6 @@ pub enum LanguageId { } impl LanguageId { - pub fn as_media_type(&self) -> MediaType { - match self { - LanguageId::JavaScript => MediaType::JavaScript, - LanguageId::Jsx => MediaType::Jsx, - LanguageId::TypeScript => MediaType::TypeScript, - LanguageId::Tsx => MediaType::Tsx, - LanguageId::Json => MediaType::Json, - LanguageId::JsonC => MediaType::Json, - LanguageId::Markdown | LanguageId::Unknown => MediaType::Unknown, - } - } - pub fn as_extension(&self) -> Option<&'static str> { match self { LanguageId::JavaScript => Some("js"), @@ -128,13 +86,15 @@ impl LanguageId { } } - fn as_headers(&self) -> Option<&HashMap> { + pub fn as_content_type(&self) -> Option<&'static str> { match self { - Self::JavaScript => Some(&JS_HEADERS), - Self::Jsx => Some(&JSX_HEADERS), - Self::TypeScript => Some(&TS_HEADERS), - Self::Tsx => Some(&TSX_HEADERS), - _ => None, + LanguageId::JavaScript => Some("application/javascript"), + LanguageId::Jsx => Some("text/jsx"), + LanguageId::TypeScript => Some("application/typescript"), + LanguageId::Tsx => Some("text/tsx"), + LanguageId::Json | LanguageId::JsonC => Some("application/json"), + LanguageId::Markdown => Some("text/markdown"), + LanguageId::Unknown => None, } } @@ -291,6 +251,7 @@ pub struct Document { // so having a mutex to hold it is ok maybe_navigation_tree: Mutex>>, maybe_parsed_source: Option, + media_type: MediaType, specifier: ModuleSpecifier, text_info: SourceTextInfo, } @@ -302,15 +263,23 @@ impl Document { maybe_headers: Option>, text_info: SourceTextInfo, resolver: &dyn deno_graph::source::Resolver, + maybe_node_resolver: Option<&CliNodeResolver>, npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> Arc { // we only ever do `Document::new` on disk resources that are supposed to // be diagnosable, unlike `Document::open`, so it is safe to unconditionally // parse the module. + let media_type = resolve_media_type( + &specifier, + maybe_headers.as_ref(), + None, + maybe_node_resolver, + ); let (maybe_parsed_source, maybe_module) = parse_and_analyze_module( &specifier, text_info.clone(), maybe_headers.as_ref(), + media_type, resolver, npm_resolver, ); @@ -328,6 +297,7 @@ impl Document { maybe_navigation_tree: Mutex::new(None), maybe_parsed_source: maybe_parsed_source .filter(|_| specifier.scheme() == "file"), + media_type, text_info, specifier, }) @@ -336,12 +306,27 @@ impl Document { fn maybe_with_new_resolver( &self, resolver: &dyn deno_graph::source::Resolver, + maybe_node_resolver: Option<&CliNodeResolver>, npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> Option> { - let parsed_source_result = match &self.maybe_parsed_source { + let mut parsed_source_result = match &self.maybe_parsed_source { Some(parsed_source_result) => parsed_source_result.clone(), None => return None, // nothing to change }; + let media_type = resolve_media_type( + &self.specifier, + self.maybe_headers.as_ref(), + self.maybe_language_id, + maybe_node_resolver, + ); + // reparse if the media type has changed + if let Ok(parsed_source) = &parsed_source_result { + if parsed_source.media_type() != media_type { + parsed_source_result = + parse_source(&self.specifier, self.text_info.clone(), media_type); + } + } + let maybe_module = Some(analyze_module( &self.specifier, &parsed_source_result, @@ -363,27 +348,37 @@ impl Document { maybe_headers: self.maybe_headers.clone(), maybe_language_id: self.maybe_language_id, maybe_lsp_version: self.maybe_lsp_version, + media_type, text_info: self.text_info.clone(), specifier: self.specifier.clone(), })) } + #[allow(clippy::too_many_arguments)] fn open( specifier: ModuleSpecifier, version: i32, language_id: LanguageId, + maybe_headers: Option>, content: Arc, cache: &Arc, resolver: &dyn deno_graph::source::Resolver, + maybe_node_resolver: Option<&CliNodeResolver>, npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> Arc { - let maybe_headers = language_id.as_headers(); let text_info = SourceTextInfo::new(content); + let media_type = resolve_media_type( + &specifier, + None, + Some(language_id), + maybe_node_resolver, + ); let (maybe_parsed_source, maybe_module) = if language_id.is_diagnosable() { parse_and_analyze_module( &specifier, text_info.clone(), - maybe_headers, + maybe_headers.as_ref(), + media_type, resolver, npm_resolver, ) @@ -400,11 +395,12 @@ impl Document { line_index, maybe_language_id: Some(language_id), maybe_lsp_version: Some(version), - maybe_headers: maybe_headers.map(ToOwned::to_owned), + maybe_headers, maybe_module, maybe_navigation_tree: Mutex::new(None), maybe_parsed_source: maybe_parsed_source .filter(|_| specifier.scheme() == "file"), + media_type, text_info, specifier, }) @@ -434,20 +430,18 @@ impl Document { } } let text_info = SourceTextInfo::from_string(content); + let media_type = self.media_type; let (maybe_parsed_source, maybe_module) = if self .maybe_language_id .as_ref() .map(|li| li.is_diagnosable()) .unwrap_or(false) { - let maybe_headers = self - .maybe_language_id - .as_ref() - .and_then(|li| li.as_headers()); parse_and_analyze_module( &self.specifier, text_info.clone(), - maybe_headers, + self.maybe_headers.as_ref(), + media_type, resolver, npm_resolver, ) @@ -477,6 +471,7 @@ impl Document { .filter(|_| self.specifier.scheme() == "file"), maybe_lsp_version: Some(version), maybe_navigation_tree: Mutex::new(None), + media_type, })) } @@ -494,6 +489,7 @@ impl Document { maybe_parsed_source: self.maybe_parsed_source.clone(), maybe_lsp_version: self.maybe_lsp_version, maybe_navigation_tree: Mutex::new(None), + media_type: self.media_type, }) } @@ -554,18 +550,7 @@ impl Document { } pub fn media_type(&self) -> MediaType { - if let Some(Ok(module)) = &self.maybe_module { - return module.media_type; - } - let specifier_media_type = MediaType::from_specifier(&self.specifier); - if specifier_media_type != MediaType::Unknown { - return specifier_media_type; - } - - self - .maybe_language_id - .map(|id| id.as_media_type()) - .unwrap_or(MediaType::Unknown) + self.media_type } pub fn maybe_language_id(&self) -> Option { @@ -629,6 +614,39 @@ impl Document { } } +fn resolve_media_type( + specifier: &ModuleSpecifier, + maybe_headers: Option<&HashMap>, + maybe_language_id: Option, + maybe_node_resolver: Option<&CliNodeResolver>, +) -> MediaType { + if let Some(node_resolver) = maybe_node_resolver { + if node_resolver.in_npm_package(specifier) { + match node_resolver.url_to_node_resolution(specifier.clone()) { + Ok(resolution) => { + let (_, media_type) = + NodeResolution::into_specifier_and_media_type(Some(resolution)); + return media_type; + } + Err(err) => { + lsp_warn!("Node resolution failed for '{}': {}", specifier, err); + } + } + } + } + + if maybe_headers.is_some() { + return MediaType::from_specifier_and_headers(specifier, maybe_headers); + } + + // LanguageId is a subset of MediaType, so get its content type and + // also use the specifier to inform its media type + MediaType::from_specifier_and_content_type( + specifier, + maybe_language_id.and_then(|id| id.as_content_type()), + ) +} + pub fn to_lsp_range(range: &deno_graph::Range) -> lsp::Range { lsp::Range { start: lsp::Position { @@ -712,6 +730,7 @@ impl FileSystemDocuments { cache: &Arc, resolver: &dyn deno_graph::source::Resolver, specifier: &ModuleSpecifier, + maybe_node_resolver: Option<&CliNodeResolver>, npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> Option> { let fs_version = if specifier.scheme() == "data" { @@ -724,7 +743,13 @@ impl FileSystemDocuments { != fs_version { // attempt to update the file on the file system - self.refresh_document(cache, resolver, specifier, npm_resolver) + self.refresh_document( + cache, + resolver, + specifier, + maybe_node_resolver, + npm_resolver, + ) } else { file_system_doc } @@ -737,6 +762,7 @@ impl FileSystemDocuments { cache: &Arc, resolver: &dyn deno_graph::source::Resolver, specifier: &ModuleSpecifier, + maybe_node_resolver: Option<&CliNodeResolver>, npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> Option> { let doc = if specifier.scheme() == "file" { @@ -751,6 +777,7 @@ impl FileSystemDocuments { None, SourceTextInfo::from_string(content), resolver, + maybe_node_resolver, npm_resolver, ) } else if specifier.scheme() == "data" { @@ -764,6 +791,7 @@ impl FileSystemDocuments { None, SourceTextInfo::from_string(source), resolver, + maybe_node_resolver, npm_resolver, ) } else { @@ -791,6 +819,7 @@ impl FileSystemDocuments { maybe_headers, SourceTextInfo::from_string(content), resolver, + maybe_node_resolver, npm_resolver, ) }; @@ -837,6 +866,8 @@ pub struct Documents { /// Any imports to the context supplied by configuration files. This is like /// the imports into the a module graph in CLI. imports: Arc>, + /// Resolver for node_modules. + maybe_node_resolver: Option>, /// A resolver that takes into account currently loaded import map and JSX /// settings. resolver: Arc, @@ -861,6 +892,7 @@ impl Documents { open_docs: HashMap::default(), file_system_docs: Default::default(), imports: Default::default(), + maybe_node_resolver: None, resolver: Arc::new(CliGraphResolver::new(CliGraphResolverOptions { node_resolver: None, npm_resolver: None, @@ -905,9 +937,14 @@ impl Documents { specifier.clone(), version, language_id, + // todo(dsherret): don't we want to pass in the headers from + // the cache for remote modules here in order to get the + // x-typescript-types? + None, content, &self.cache, resolver, + self.maybe_node_resolver.as_deref(), npm_resolver, ); @@ -1102,6 +1139,7 @@ impl Documents { &self.cache, self.get_resolver(), &specifier, + self.maybe_node_resolver.as_deref(), self.get_npm_resolver(), ) } @@ -1281,6 +1319,7 @@ impl Documents { ) { let config_data = config.tree.root_data(); let config_file = config_data.and_then(|d| d.config_file.as_deref()); + self.maybe_node_resolver = node_resolver.clone(); self.resolver = Arc::new(CliGraphResolver::new(CliGraphResolverOptions { node_resolver, npm_resolver, @@ -1352,9 +1391,11 @@ impl Documents { if !config.specifier_enabled(doc.specifier()) { continue; } - if let Some(new_doc) = - doc.maybe_with_new_resolver(resolver, npm_resolver) - { + if let Some(new_doc) = doc.maybe_with_new_resolver( + resolver, + self.maybe_node_resolver.as_deref(), + npm_resolver, + ) { *doc = new_doc; } } @@ -1362,9 +1403,11 @@ impl Documents { if !config.specifier_enabled(doc.specifier()) { continue; } - if let Some(new_doc) = - doc.maybe_with_new_resolver(resolver, npm_resolver) - { + if let Some(new_doc) = doc.maybe_with_new_resolver( + resolver, + self.maybe_node_resolver.as_deref(), + npm_resolver, + ) { *doc.value_mut() = new_doc; } } @@ -1385,6 +1428,7 @@ impl Documents { &self.cache, resolver, specifier, + self.maybe_node_resolver.as_deref(), npm_resolver, ); } @@ -1628,10 +1672,11 @@ fn parse_and_analyze_module( specifier: &ModuleSpecifier, text_info: SourceTextInfo, maybe_headers: Option<&HashMap>, + media_type: MediaType, resolver: &dyn deno_graph::source::Resolver, npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> (Option, Option) { - let parsed_source_result = parse_source(specifier, text_info, maybe_headers); + let parsed_source_result = parse_source(specifier, text_info, media_type); let module_result = analyze_module( specifier, &parsed_source_result, @@ -1645,12 +1690,12 @@ fn parse_and_analyze_module( fn parse_source( specifier: &ModuleSpecifier, text_info: SourceTextInfo, - maybe_headers: Option<&HashMap>, + media_type: MediaType, ) -> ParsedSourceResult { deno_ast::parse_module(deno_ast::ParseParams { specifier: specifier.clone(), text_info, - media_type: MediaType::from_specifier_and_headers(specifier, maybe_headers), + media_type, capture_tokens: true, scope_analysis: true, maybe_syntax: None, diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 2e0726a795..13da932f7f 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -3996,6 +3996,7 @@ struct LoadResponse { data: Arc, script_kind: i32, version: Option, + is_cjs: bool, } #[op2] @@ -4018,6 +4019,10 @@ fn op_load<'s>( data: doc.text(), script_kind: crate::tsc::as_ts_script_kind(doc.media_type()), version: state.script_version(&specifier), + is_cjs: matches!( + doc.media_type(), + MediaType::Cjs | MediaType::Cts | MediaType::Dcts + ), }) }; diff --git a/cli/resolver.rs b/cli/resolver.rs index fc326e1b19..d5a85e0012 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -223,6 +223,13 @@ impl CliNodeResolver { Ok(specifier) } + pub fn url_to_node_resolution( + &self, + specifier: ModuleSpecifier, + ) -> Result { + self.node_resolver.url_to_node_resolution(specifier) + } + fn handle_node_resolve_result( &self, result: Result, AnyError>, diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js index be99628148..b76c95aa50 100644 --- a/cli/tsc/99_main_compiler.js +++ b/cli/tsc/99_main_compiler.js @@ -135,12 +135,16 @@ delete Object.prototype.__proto__; #cache = new Set(); /** @param {[string, ts.Extension]} param */ - add([specifier, ext]) { + maybeAdd([specifier, ext]) { if (ext === ".cjs" || ext === ".d.cts" || ext === ".cts") { this.#cache.add(specifier); } } + add(specifier) { + this.#cache.add(specifier); + } + /** @param specifier {string} */ has(specifier) { return this.#cache.has(specifier); @@ -603,22 +607,30 @@ delete Object.prototype.__proto__; return sourceFile; } - /** @type {{ data: string; scriptKind: ts.ScriptKind; version: string; }} */ + /** @type {{ data: string; scriptKind: ts.ScriptKind; version: string; isCjs: boolean }} */ const fileInfo = ops.op_load(specifier); if (!fileInfo) { return undefined; } - const { data, scriptKind, version } = fileInfo; + let { data, scriptKind, version, isCjs } = fileInfo; assert( data != null, `"data" is unexpectedly null for "${specifier}".`, ); + + // use the cache for non-lsp + if (isCjs == null) { + isCjs = isCjsCache.has(specifier); + } else if (isCjs) { + isCjsCache.add(specifier); + } + sourceFile = ts.createSourceFile( specifier, data, { ...getCreateSourceFileOptions(languageVersion), - impliedNodeFormat: isCjsCache.has(specifier) + impliedNodeFormat: isCjs ? ts.ModuleKind.CommonJS : ts.ModuleKind.ESNext, // no need to parse docs for `deno check` @@ -688,7 +700,7 @@ delete Object.prototype.__proto__; base: containingFilePath, })?.[0]; if (resolved) { - isCjsCache.add(resolved); + isCjsCache.maybeAdd(resolved); return { primary: true, resolvedFileName: resolved[0], @@ -723,7 +735,7 @@ delete Object.prototype.__proto__; if (resolved) { const result = resolved.map((item) => { if (item) { - isCjsCache.add(item); + isCjsCache.maybeAdd(item); const [resolvedFileName, extension] = item; return { resolvedFileName, diff --git a/tests/Cargo.toml b/tests/Cargo.toml index b0f9ff0af1..5552f6f315 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -43,7 +43,7 @@ deno_lockfile.workspace = true deno_terminal.workspace = true deno_tls.workspace = true fastwebsockets = { workspace = true, features = ["upgrade", "unstable-split"] } -file_test_runner = "0.2.0" +file_test_runner = "0.4.0" flaky_test = "=0.1.0" http.workspace = true http-body-util.workspace = true diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index a3d2cf57ec..76d6e22e10 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -12287,3 +12287,52 @@ fn lsp_uses_lockfile_for_npm_initialization() { assert_eq!(skipping_count, 1); client.shutdown(); } + +#[test] +fn lsp_cjs_internal_types_default_export() { + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .add_npm_env_vars() + .env("DENO_FUTURE", "1") + .build(); + let temp_dir = context.temp_dir(); + temp_dir.write("deno.json", r#"{}"#); + temp_dir.write( + "package.json", + r#"{ + "dependencies": { + "@denotest/cjs-internal-types-default-export": "*" + } +}"#, + ); + context.run_npm("install"); + + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + // this was previously being resolved as ESM and not correctly as CJS + let node_modules_index_d_ts = temp_dir.path().join( + "node_modules/@denotest/cjs-internal-types-default-export/index.d.ts", + ); + client.did_open(json!({ + "textDocument": { + "uri": node_modules_index_d_ts.uri_file(), + "languageId": "typescript", + "version": 1, + "text": node_modules_index_d_ts.read_to_string(), + } + })); + let main_url = temp_dir.path().join("main.ts").uri_file(); + let diagnostics = client.did_open( + json!({ + "textDocument": { + "uri": main_url, + "languageId": "typescript", + "version": 1, + "text": "import * as mod from '@denotest/cjs-internal-types-default-export';\nmod.add(1, 2);", + } + }), + ); + // previously, diagnostic about `add` not being callable + assert_eq!(json!(diagnostics.all()), json!([])); +} diff --git a/tests/specs/mod.rs b/tests/specs/mod.rs index 1028519cfa..2040eea627 100644 --- a/tests/specs/mod.rs +++ b/tests/specs/mod.rs @@ -9,6 +9,10 @@ use std::sync::Arc; use deno_core::anyhow::Context; use deno_core::serde_json; +use file_test_runner::collection::collect_tests_or_exit; +use file_test_runner::collection::strategies::TestPerDirectoryCollectionStrategy; +use file_test_runner::collection::CollectOptions; +use file_test_runner::collection::CollectedTest; use serde::Deserialize; use test_util::tests_path; use test_util::PathRef; @@ -69,6 +73,7 @@ struct StepMetaData { pub clean_deno_dir: bool, pub args: VecOrString, pub cwd: Option, + pub command_name: Option, #[serde(default)] pub envs: HashMap, pub output: String, @@ -77,15 +82,13 @@ struct StepMetaData { } pub fn main() { - let root_category = - file_test_runner::collect_tests_or_exit(file_test_runner::CollectOptions { - base: tests_path().join("specs").to_path_buf(), - strategy: file_test_runner::FileCollectionStrategy::TestPerDirectory { - file_name: MANIFEST_FILE_NAME.to_string(), - }, - root_category_name: "specs".to_string(), - filter_override: None, - }); + let root_category = collect_tests_or_exit(CollectOptions { + base: tests_path().join("specs").to_path_buf(), + strategy: Box::new(TestPerDirectoryCollectionStrategy { + file_name: MANIFEST_FILE_NAME.to_string(), + }), + filter_override: None, + }); if root_category.is_empty() { return; // all tests filtered out @@ -111,15 +114,13 @@ pub fn main() { output.extend(panic_output); file_test_runner::TestResult::Failed { output } } + file_test_runner::TestResult::Steps(_) => unreachable!(), } }), ); } -fn run_test( - test: &file_test_runner::CollectedTest, - diagnostic_logger: Rc>>, -) { +fn run_test(test: &CollectedTest, diagnostic_logger: Rc>>) { let metadata_path = PathRef::new(&test.path); let metadata_value = metadata_path.read_jsonc_value(); // checking for "steps" leads to a more targeted error message @@ -182,6 +183,10 @@ fn run_test( Some(cwd) => command.current_dir(cwd), None => command, }; + let command = match &step.command_name { + Some(command_name) => command.name(command_name), + None => command, + }; let output = command.run(); if step.output.ends_with(".out") { let test_output_path = cwd.join(&step.output); diff --git a/tests/specs/npm/cjs_internal_types_default_export/__test__.jsonc b/tests/specs/npm/cjs_internal_types_default_export/__test__.jsonc new file mode 100644 index 0000000000..dc8aabedb9 --- /dev/null +++ b/tests/specs/npm/cjs_internal_types_default_export/__test__.jsonc @@ -0,0 +1,15 @@ +{ + "tempDir": true, + "envs": { + "DENO_FUTURE": "1" + }, + "steps": [{ + "commandName": "npm", + "args": "install", + "output": "[WILDCARD]" + }, { + "args": "check main.ts", + "exitCode": 1, + "output": "main.out" + }] +} diff --git a/tests/specs/npm/cjs_internal_types_default_export/main.out b/tests/specs/npm/cjs_internal_types_default_export/main.out new file mode 100644 index 0000000000..66ec3f37b0 --- /dev/null +++ b/tests/specs/npm/cjs_internal_types_default_export/main.out @@ -0,0 +1,5 @@ +Check file:///[WILDLINE]/main.ts +error: TS2345 [ERROR]: Argument of type 'string' is not assignable to parameter of type 'number'. +add(1, "test"); // should error + ~~~~~~ + at file:///[WILDLINE]/main.ts:3:8 diff --git a/tests/specs/npm/cjs_internal_types_default_export/main.ts b/tests/specs/npm/cjs_internal_types_default_export/main.ts new file mode 100644 index 0000000000..339efcc594 --- /dev/null +++ b/tests/specs/npm/cjs_internal_types_default_export/main.ts @@ -0,0 +1,3 @@ +import { add } from "@denotest/cjs-internal-types-default-export"; + +add(1, "test"); // should error diff --git a/tests/specs/npm/cjs_internal_types_default_export/package.json b/tests/specs/npm/cjs_internal_types_default_export/package.json new file mode 100644 index 0000000000..f97b7b5650 --- /dev/null +++ b/tests/specs/npm/cjs_internal_types_default_export/package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "@denotest/cjs-internal-types-default-export": "*" + } +} diff --git a/tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/add.d.ts b/tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/add.d.ts new file mode 100644 index 0000000000..0b38dc4fc4 --- /dev/null +++ b/tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/add.d.ts @@ -0,0 +1,3 @@ +const _default: (a: number, b: number) => number; + +export default _default; diff --git a/tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/index.d.ts b/tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/index.d.ts new file mode 100644 index 0000000000..bfde9725ef --- /dev/null +++ b/tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/index.d.ts @@ -0,0 +1 @@ +export { default as add } from './add'; \ No newline at end of file diff --git a/tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/index.js b/tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/index.js new file mode 100644 index 0000000000..62c45aa26d --- /dev/null +++ b/tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/index.js @@ -0,0 +1 @@ +module.exports.add = (a, b) => a + b; diff --git a/tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/package.json b/tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/package.json new file mode 100644 index 0000000000..57b3b9e4ab --- /dev/null +++ b/tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/package.json @@ -0,0 +1,4 @@ +{ + "name": "@denotest/cjs-internal-types-default-export", + "version": "1.0.0" +}