diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 8533ea00f3..97096a7996 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -23,6 +23,7 @@ use deno_graph::ModuleGraph; use deno_graph::ModuleGraphError; use deno_graph::ModuleLoadError; use deno_graph::ResolutionError; +use deno_graph::SpecifierError; use deno_graph::WorkspaceFastCheckOption; use deno_graph::source::Loader; use deno_graph::source::ResolveError; @@ -43,6 +44,7 @@ use deno_resolver::npm::DenoInNpmPackageChecker; use deno_runtime::deno_permissions::PermissionsContainer; use deno_semver::SmallStackString; use deno_semver::jsr::JsrDepPackageReq; +use import_map::ImportMapErrorKind; use indexmap::IndexMap; use node_resolver::errors::NodeJsErrorCode; use sys_traits::FsMetadata; @@ -74,6 +76,7 @@ use crate::util::progress_bar::ProgressBar; pub struct GraphValidOptions<'a> { pub check_js: CheckJsOption<'a>, pub kind: GraphKind, + pub will_type_check: bool, /// Whether to exit the process for integrity check errors such as /// lockfile checksum mismatches and JSR integrity failures. /// Otherwise, surfaces integrity errors as errors. @@ -106,6 +109,7 @@ pub fn graph_valid( GraphWalkErrorsOptions { check_js: options.check_js, kind: options.kind, + will_type_check: options.will_type_check, allow_unknown_media_types: options.allow_unknown_media_types, allow_unknown_jsr_exports: options.allow_unknown_jsr_exports, }, @@ -129,6 +133,7 @@ pub fn graph_valid( pub struct GraphWalkErrorsOptions<'a> { pub check_js: CheckJsOption<'a>, pub kind: GraphKind, + pub will_type_check: bool, pub allow_unknown_media_types: bool, pub allow_unknown_jsr_exports: bool, } @@ -145,6 +150,7 @@ pub fn graph_walk_errors<'a>( sys: &CliSys, graph_kind: GraphKind, allow_unknown_media_types: bool, + will_type_check: bool, error: &ModuleGraphError, ) -> bool { if (graph_kind == GraphKind::TypesOnly || allow_unknown_media_types) @@ -157,8 +163,7 @@ pub fn graph_walk_errors<'a>( } // surface these as typescript diagnostics instead - graph_kind.include_types() - && has_module_graph_error_for_tsc_diagnostic(sys, error) + will_type_check && has_module_graph_error_for_tsc_diagnostic(sys, error) } graph @@ -177,6 +182,7 @@ pub fn graph_walk_errors<'a>( sys, graph.graph_kind(), options.allow_unknown_media_types, + options.will_type_check, &error, ) { log::debug!("Ignoring: {}", error); @@ -269,14 +275,16 @@ pub fn module_error_for_tsc_diagnostic<'a>( } } -pub struct ModuleNotFoundNodeResolutionErrorRef<'a> { +#[derive(Debug)] +pub struct ResolutionErrorRef<'a> { pub specifier: &'a str, - pub maybe_range: Option<&'a deno_graph::Range>, + pub range: &'a deno_graph::Range, + pub is_module_not_found: bool, } pub fn resolution_error_for_tsc_diagnostic( error: &ResolutionError, -) -> Option> { +) -> Option> { fn is_module_not_found_code(code: NodeJsErrorCode) -> bool { match code { NodeJsErrorCode::ERR_INVALID_MODULE_SPECIFIER @@ -295,28 +303,63 @@ pub fn resolution_error_for_tsc_diagnostic( } match error { + ResolutionError::InvalidDowngrade { .. } + | ResolutionError::InvalidJsrHttpsTypesImport { .. } + | ResolutionError::InvalidLocalImport { .. } => None, + ResolutionError::InvalidSpecifier { error, range } => match error { + SpecifierError::InvalidUrl(..) => None, + SpecifierError::ImportPrefixMissing { specifier, .. } => { + Some(ResolutionErrorRef { + specifier, + range, + is_module_not_found: false, + }) + } + }, ResolutionError::ResolverError { error, specifier, range, } => match error.as_ref() { + ResolveError::Specifier(error) => match error { + SpecifierError::InvalidUrl(..) => None, + SpecifierError::ImportPrefixMissing { specifier, .. } => { + Some(ResolutionErrorRef { + specifier, + range, + is_module_not_found: false, + }) + } + }, + ResolveError::ImportMap(error) => match error.as_kind() { + ImportMapErrorKind::JsonParse(_) + | ImportMapErrorKind::ImportMapNotObject + | ImportMapErrorKind::ImportsFieldNotObject + | ImportMapErrorKind::ScopesFieldNotObject + | ImportMapErrorKind::ScopePrefixNotObject(_) + | ImportMapErrorKind::BlockedByNullEntry(_) + | ImportMapErrorKind::SpecifierResolutionFailure { .. } + | ImportMapErrorKind::SpecifierBacktracksAbovePrefix { .. } => None, + ImportMapErrorKind::UnmappedBareSpecifier(specifier, _) => { + Some(ResolutionErrorRef { + specifier, + range, + is_module_not_found: false, + }) + } + }, ResolveError::Other(error) => { let is_module_not_found_error = downcast_ref_deno_resolve_error(error) .and_then(|err| err.maybe_node_code()) .map(is_module_not_found_code) .unwrap_or(false); - if is_module_not_found_error { - Some(ModuleNotFoundNodeResolutionErrorRef { - specifier, - maybe_range: Some(range), - }) - } else { - None - } + is_module_not_found_error.then(|| ResolutionErrorRef { + specifier, + range, + is_module_not_found: true, + }) } - ResolveError::Specifier(_) | ResolveError::ImportMap(_) => None, }, - _ => None, } } @@ -951,16 +994,18 @@ impl ModuleGraphBuilder { allow_unknown_media_types: bool, allow_unknown_jsr_exports: bool, ) -> Result<(), JsErrorBox> { + let will_type_check = self.cli_options.type_check_mode().is_true(); graph_valid( graph, &self.sys, roots, GraphValidOptions { - kind: if self.cli_options.type_check_mode().is_true() { + kind: if will_type_check { GraphKind::All } else { GraphKind::CodeOnly }, + will_type_check, check_js: CheckJsOption::Custom( self.compiler_options_resolver.as_ref(), ), diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index aafdd3bb86..6d60ad621a 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -409,6 +409,7 @@ impl LanguageServer { &roots, graph_util::GraphValidOptions { kind: GraphKind::All, + will_type_check: true, check_js: CheckJsOption::False, exit_integrity_errors: false, allow_unknown_media_types: true, diff --git a/cli/tools/doc.rs b/cli/tools/doc.rs index 95c0f50b17..4d0455c82f 100644 --- a/cli/tools/doc.rs +++ b/cli/tools/doc.rs @@ -160,6 +160,7 @@ pub async fn doc( GraphWalkErrorsOptions { check_js: CheckJsOption::False, kind: GraphKind::TypesOnly, + will_type_check: false, allow_unknown_media_types: false, allow_unknown_jsr_exports: false, }, diff --git a/cli/tsc/diagnostics.rs b/cli/tsc/diagnostics.rs index 1d724804f3..0c3ff37a8d 100644 --- a/cli/tsc/diagnostics.rs +++ b/cli/tsc/diagnostics.rs @@ -10,8 +10,12 @@ use deno_core::serde::Serialize; use deno_core::serde::Serializer; use deno_core::sourcemap::SourceMap; use deno_graph::ModuleGraph; +use deno_graph::ResolutionError; +use deno_resolver::graph::enhanced_resolution_error_message; use deno_terminal::colors; +use crate::graph_util::resolution_error_for_tsc_diagnostic; + const MAX_SOURCE_LINE_LENGTH: usize = 150; #[derive(Clone, Debug, Eq, PartialEq)] @@ -186,6 +190,34 @@ impl Diagnostic { } } + pub fn maybe_from_resolution_error(error: &ResolutionError) -> Option { + let error_ref = resolution_error_for_tsc_diagnostic(error)?; + if error_ref.is_module_not_found { + return Some(Self::from_missing_error( + error_ref.specifier, + Some(error_ref.range), + None, + )); + } + Some(Self { + category: DiagnosticCategory::Error, + code: 2307, + start: Some(Position::from_deno_graph(error_ref.range.range.start)), + end: Some(Position::from_deno_graph(error_ref.range.range.end)), + original_source_start: None, // will be applied later + message_text: Some(enhanced_resolution_error_message(error)), + message_chain: None, + source: None, + source_line: None, + file_name: Some(error_ref.range.specifier.to_string()), + related_information: None, + reports_deprecated: None, + reports_unnecessary: None, + other: Default::default(), + missing_specifier: Some(error_ref.specifier.to_string()), + }) + } + /// If this diagnostic should be included when it comes from a remote module. pub fn include_when_remote(&self) -> bool { /// TS6133: value is declared but its value is never read (noUnusedParameters and noUnusedLocals) diff --git a/cli/type_checker.rs b/cli/type_checker.rs index dfe679fbce..afdb391667 100644 --- a/cli/type_checker.rs +++ b/cli/type_checker.rs @@ -36,7 +36,6 @@ use crate::cache::TypeCheckCache; use crate::graph_util::BuildFastCheckGraphOptions; use crate::graph_util::ModuleGraphBuilder; use crate::graph_util::module_error_for_tsc_diagnostic; -use crate::graph_util::resolution_error_for_tsc_diagnostic; use crate::node::CliNodeResolver; use crate::npm::CliNpmResolver; use crate::sys::CliSys; @@ -792,7 +791,9 @@ impl<'a> GraphWalker<'a> { } } } - + if dep.is_dynamic { + continue; + } // only surface the code error if there's no type let dep_to_check_error = if dep.maybe_type.is_none() { &dep.maybe_code @@ -801,16 +802,10 @@ impl<'a> GraphWalker<'a> { }; if let deno_graph::Resolution::Err(resolution_error) = dep_to_check_error - && let Some(err) = - resolution_error_for_tsc_diagnostic(resolution_error) + && let Some(diagnostic) = + tsc::Diagnostic::maybe_from_resolution_error(resolution_error) { - self - .missing_diagnostics - .push(tsc::Diagnostic::from_missing_error( - err.specifier, - err.maybe_range, - None, - )); + self.missing_diagnostics.push(diagnostic); } } } diff --git a/tests/registry/jsr/@denotest/multiple-exports/0.7.0_meta.json b/tests/registry/jsr/@denotest/multiple-exports/0.7.0_meta.json index 2897812168..d9f58b9a61 100644 --- a/tests/registry/jsr/@denotest/multiple-exports/0.7.0_meta.json +++ b/tests/registry/jsr/@denotest/multiple-exports/0.7.0_meta.json @@ -2,7 +2,6 @@ "exports": { "./add": "./add.ts", "./subtract": "./subtract.ts", - "./data-json": "./data.json", - "./multiply": "./multiply.ts" + "./data-json": "./data.json" } } diff --git a/tests/registry/jsr/@denotest/multiple-exports/0.7.1_meta.json b/tests/registry/jsr/@denotest/multiple-exports/0.7.1_meta.json index d9f58b9a61..2897812168 100644 --- a/tests/registry/jsr/@denotest/multiple-exports/0.7.1_meta.json +++ b/tests/registry/jsr/@denotest/multiple-exports/0.7.1_meta.json @@ -2,6 +2,7 @@ "exports": { "./add": "./add.ts", "./subtract": "./subtract.ts", - "./data-json": "./data.json" + "./data-json": "./data.json", + "./multiply": "./multiply.ts" } } diff --git a/tests/specs/check/ambient_modules/__test__.jsonc b/tests/specs/check/ambient_modules/__test__.jsonc index d70bcfc401..8fe741849b 100644 --- a/tests/specs/check/ambient_modules/__test__.jsonc +++ b/tests/specs/check/ambient_modules/__test__.jsonc @@ -14,9 +14,5 @@ "args": "run bar.ts", "output": "run.out", "exitCode": 1 - }, { - "args": "check bare_specifier.ts", - "output": "bare_specifier.out", - "exitCode": 1 }] } diff --git a/tests/specs/check/ambient_modules/bare_specifier.out b/tests/specs/check/ambient_modules/bare_specifier.out deleted file mode 100644 index 43533e8270..0000000000 --- a/tests/specs/check/ambient_modules/bare_specifier.out +++ /dev/null @@ -1,3 +0,0 @@ -error: Relative import path "foo" not prefixed with / or ./ or ../ - hint: If you want to use the npm package, try running `deno add npm:foo` - at file:///[WILDLINE]bare_specifier.ts:1:8 diff --git a/tests/specs/check/ambient_modules/bare_specifier.ts b/tests/specs/check/ambient_modules/bare_specifier.ts deleted file mode 100644 index c0748305d5..0000000000 --- a/tests/specs/check/ambient_modules/bare_specifier.ts +++ /dev/null @@ -1 +0,0 @@ -import "foo"; diff --git a/tests/specs/check/ambient_modules/foo.ts b/tests/specs/check/ambient_modules/foo.ts index 0a23bd1fd0..ee2b17aae6 100644 --- a/tests/specs/check/ambient_modules/foo.ts +++ b/tests/specs/check/ambient_modules/foo.ts @@ -1,4 +1,5 @@ /// +import type {} from "bare.svg"; import logo from "/logo.svg"; import "./global.css"; import styles from "./styles.module.css"; diff --git a/tests/specs/check/bare_specifier_not_found/check_import_map.out b/tests/specs/check/bare_specifier_not_found/check_import_map.out index 94637153c8..890000918d 100644 --- a/tests/specs/check/bare_specifier_not_found/check_import_map.out +++ b/tests/specs/check/bare_specifier_not_found/check_import_map.out @@ -1,3 +1,5 @@ -error: Relative import path "@scope/BAZ" not prefixed with / or ./ or ../ and not in import map from "file:///[WILDLINE]/main.ts" +[WILDCARD]TS2307 [ERROR]: Relative import path "@scope/BAZ" not prefixed with / or ./ or ../ and not in import map from "file:///[WILDLINE]/main.ts" hint: If you want to use a JSR or npm package, try running `deno add jsr:@scope/BAZ` or `deno add npm:@scope/BAZ` at file:///[WILDLINE] + +error: Type checking failed. diff --git a/tests/specs/check/bare_specifier_not_found/check_no_import_map.out b/tests/specs/check/bare_specifier_not_found/check_no_import_map.out index 0d2617ac4d..b700ad9309 100644 --- a/tests/specs/check/bare_specifier_not_found/check_no_import_map.out +++ b/tests/specs/check/bare_specifier_not_found/check_no_import_map.out @@ -1,3 +1,5 @@ -error: Relative import path "@scope/BAZ" not prefixed with / or ./ or ../ +[WILDCARD]TS2307 [ERROR]: Relative import path "@scope/BAZ" not prefixed with / or ./ or ../ hint: If you want to use a JSR or npm package, try running `deno add jsr:@scope/BAZ` or `deno add npm:@scope/BAZ` at file:///[WILDLINE] + +error: Type checking failed. diff --git a/tests/specs/check/with_bare_import/095_cache_with_bare_import.ts.out b/tests/specs/check/with_bare_import/095_cache_with_bare_import.ts.out index 70b7d6643f..619eda9370 100644 --- a/tests/specs/check/with_bare_import/095_cache_with_bare_import.ts.out +++ b/tests/specs/check/with_bare_import/095_cache_with_bare_import.ts.out @@ -1,3 +1,5 @@ -[WILDCARD]error: Relative import path "foo" not prefixed with / or ./ or ../ +[WILDCARD]TS2307 [ERROR]: Relative import path "foo" not prefixed with / or ./ or ../ hint: If you want to use the npm package, try running `deno add npm:foo` at file:///[WILDCARD]/095_cache_with_bare_import.ts:[WILDCARD] + +error: Type checking failed. diff --git a/tests/specs/install/jsr_exports_uses_locked/deno.lock b/tests/specs/install/jsr_exports_uses_locked/deno.lock index 977ce7f105..ebc7612ddb 100644 --- a/tests/specs/install/jsr_exports_uses_locked/deno.lock +++ b/tests/specs/install/jsr_exports_uses_locked/deno.lock @@ -10,7 +10,7 @@ "integrity": "3b2e675c1ad7fba2a45bc251992e01aff08a3c974ac09079b11e6a5b95d4bfcb" }, "@denotest/multiple-exports@0.7.0": { - "integrity": "e2dff9c6b3b083d80b869f14e12be35178baeb065e08a1fb48d30dc6afbd43a8", + "integrity": "efe9748a0c0939c7ac245fee04acc0c42bd7a61874ff71a360c4543e4f5f6b36", "dependencies": [ "jsr:@denotest/add", "jsr:@denotest/subtract" diff --git a/tests/specs/install/jsr_exports_uses_locked/install.out b/tests/specs/install/jsr_exports_uses_locked/install.out index c3128e0617..b4e8b640f6 100644 --- a/tests/specs/install/jsr_exports_uses_locked/install.out +++ b/tests/specs/install/jsr_exports_uses_locked/install.out @@ -3,7 +3,6 @@ Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.0_meta.json Download http://127.0.0.1:4250/@denotest/multiple-exports/meta.json Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.0/add.ts Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.0/subtract.ts -Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.0/multiply.ts Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.0/data.json Download http://127.0.0.1:4250/@denotest/add/meta.json Download http://127.0.0.1:4250/@denotest/subtract/meta.json diff --git a/tests/specs/install/jsr_exports_uses_locked_multiple_in_lockfile/deno.lock b/tests/specs/install/jsr_exports_uses_locked_multiple_in_lockfile/deno.lock index 2c32c5bce1..dbaf6fd7cd 100644 --- a/tests/specs/install/jsr_exports_uses_locked_multiple_in_lockfile/deno.lock +++ b/tests/specs/install/jsr_exports_uses_locked_multiple_in_lockfile/deno.lock @@ -11,15 +11,15 @@ "integrity": "3b2e675c1ad7fba2a45bc251992e01aff08a3c974ac09079b11e6a5b95d4bfcb" }, "@denotest/multiple-exports@0.7.0": { - "integrity": "e2dff9c6b3b083d80b869f14e12be35178baeb065e08a1fb48d30dc6afbd43a8", + "integrity": "efe9748a0c0939c7ac245fee04acc0c42bd7a61874ff71a360c4543e4f5f6b36", "dependencies": [ "jsr:@denotest/add", "jsr:@denotest/subtract" ] }, "@denotest/multiple-exports@0.7.1": { - "integrity": "1554a62f7434dbbeebc5dfc8eb0c690fb1eddb721579a12e918b4c4027b77f02", - "dependencies": [ + "integrity": "84cf61094aa4b553522a114637fbc08f6cc9cd007892b7b111e8919e1b3ade63", + "dependencies": [ "jsr:@denotest/add", "jsr:@denotest/subtract" ] diff --git a/tests/specs/install/jsr_exports_uses_locked_multiple_in_lockfile/install.out b/tests/specs/install/jsr_exports_uses_locked_multiple_in_lockfile/install.out index 617ae89895..ce44e83f10 100644 --- a/tests/specs/install/jsr_exports_uses_locked_multiple_in_lockfile/install.out +++ b/tests/specs/install/jsr_exports_uses_locked_multiple_in_lockfile/install.out @@ -3,6 +3,7 @@ Download http://127.0.0.1:4250/@denotest/multiple-exports/meta.json Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.1/add.ts Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.1/data.json Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.1/subtract.ts +Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.1/multiply.ts Download http://127.0.0.1:4250/@denotest/multiple-exports/0.7.1_meta.json Download http://127.0.0.1:4250/@denotest/add/meta.json Download http://127.0.0.1:4250/@denotest/subtract/meta.json diff --git a/tests/specs/run/error_type_definitions/error_type_definitions.ts.out b/tests/specs/run/error_type_definitions/error_type_definitions.ts.out index b2cb723e2e..262a81aa78 100644 --- a/tests/specs/run/error_type_definitions/error_type_definitions.ts.out +++ b/tests/specs/run/error_type_definitions/error_type_definitions.ts.out @@ -1,3 +1,8 @@ -[WILDCARD]error: Failed resolving types. Relative import path "baz" not prefixed with / or ./ or ../ +[WILDCARD]TS2307 [ERROR]: Relative import path "baz" not prefixed with / or ./ or ../ hint: If you want to use the npm package, try running `deno add npm:baz` at [WILDCARD]/type_definitions/bar.d.ts:[WILDCARD] + +error: Type checking failed. + + info: The program failed type-checking, but it still might work correctly. + hint: Re-run with --no-check to skip type-checking.