diff --git a/Cargo.lock b/Cargo.lock index a5a58f26c..ad976f023 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -170,6 +170,7 @@ dependencies = [ "deno_semver", "futures", "indexmap", + "monch", "once_cell", "parking_lot", "pretty_assertions", diff --git a/js/test.ts b/js/test.ts index 54380a69a..9267d2e2a 100644 --- a/js/test.ts +++ b/js/test.ts @@ -204,6 +204,21 @@ Deno.test({ }, }, }, + "imports": [ + { + "range": { + "end": { + "character": 27, + "line": 0, + }, + "start": { + "character": 19, + "line": 0, + }, + }, + "specifier": "./b.js", + }, + ], }, ], "kind": "esm", @@ -282,6 +297,21 @@ Deno.test({ }, }, }, + "imports": [ + { + "range": { + "end": { + "character": 32, + "line": 0, + }, + "start": { + "character": 19, + "line": 0, + }, + }, + "specifier": "./deno.json", + }, + ], "assertionType": "json", }, ], @@ -377,6 +407,21 @@ Deno.test({ }, }, }, + "imports": [ + { + "range": { + "end": { + "character": 27, + "line": 0, + }, + "start": { + "character": 19, + "line": 0, + }, + }, + "specifier": "./b.js", + }, + ], }, ], "kind": "esm", @@ -473,6 +518,21 @@ Deno.test({ }, }, }, + "imports": [ + { + "range": { + "end": { + "character": 32, + "line": 0, + }, + "start": { + "character": 20, + "line": 0, + }, + }, + "specifier": "builtin:fs", + }, + ], }, { "specifier": "https://example.com/bundle", @@ -489,6 +549,21 @@ Deno.test({ }, }, }, + "imports": [ + { + "range": { + "end": { + "character": 62, + "line": 1, + }, + "start": { + "character": 34, + "line": 1, + }, + }, + "specifier": "https://example.com/bundle", + }, + ], }, ], "kind": "esm", @@ -560,6 +635,21 @@ Deno.test({ "end": { "line": 2, "character": 34 }, }, }, + "imports": [ + { + "range": { + "end": { + "character": 34, + "line": 2, + }, + "start": { + "character": 26, + "line": 2, + }, + }, + "specifier": "./a.ts", + }, + ], }, { "specifier": "./b.ts", "code": { @@ -569,6 +659,21 @@ Deno.test({ "end": { "line": 3, "character": 35 }, }, }, + "imports": [ + { + "range": { + "end": { + "character": 35, + "line": 3, + }, + "start": { + "character": 27, + "line": 3, + }, + }, + "specifier": "./b.ts", + }, + ], }, { "specifier": "./c.ts", "code": { @@ -578,6 +683,21 @@ Deno.test({ "end": { "line": 4, "character": 34 }, }, }, + "imports": [ + { + "range": { + "end": { + "character": 34, + "line": 4, + }, + "start": { + "character": 26, + "line": 4, + }, + }, + "specifier": "./c.ts", + }, + ], }, { "specifier": "./d.ts", "code": { @@ -588,6 +708,22 @@ Deno.test({ }, }, "isDynamic": true, + "imports": [ + { + "isDynamic": true, + "range": { + "end": { + "character": 39, + "line": 5, + }, + "start": { + "character": 31, + "line": 5, + }, + }, + "specifier": "./d.ts", + }, + ], }], "typesDependency": { "specifier": "./test01.d.ts", @@ -723,6 +859,21 @@ Deno.test({ }, }, }, + "imports": [ + { + "range": { + "end": { + "character": 32, + "line": 1, + }, + "start": { + "character": 22, + "line": 1, + }, + }, + "specifier": "./a.json", + }, + ], "assertionType": "json", }, { @@ -741,6 +892,22 @@ Deno.test({ }, }, "isDynamic": true, + "imports": [ + { + "isDynamic": true, + "range": { + "end": { + "character": 31, + "line": 2, + }, + "start": { + "character": 21, + "line": 2, + }, + }, + "specifier": "./b.json", + }, + ], "assertionType": "json", }, ], @@ -780,6 +947,22 @@ Deno.test({ }, }, }, + "imports": [ + { + "kind": "tsReferencePath", + "range": { + "end": { + "character": 38, + "line": 1, + }, + "start": { + "character": 28, + "line": 1, + }, + }, + "specifier": "./a.d.ts", + }, + ], }, { "specifier": "./b.d.ts", @@ -796,6 +979,22 @@ Deno.test({ }, }, }, + "imports": [ + { + "kind": "tsReferenceTypes", + "range": { + "end": { + "character": 39, + "line": 2, + }, + "start": { + "character": 29, + "line": 2, + }, + }, + "specifier": "./b.d.ts", + }, + ], }, ], "kind": "esm", diff --git a/js/types.d.ts b/js/types.d.ts index 5b30a776f..f2c43ed0e 100644 --- a/js/types.d.ts +++ b/js/types.d.ts @@ -91,20 +91,32 @@ export interface TypesDependencyJson { dependency: ResolvedDependency; } -/** The kind of module. - * - * For asserted modules, the value of the `asserted` property is set to the - * `type` value of the assertion. - * - * Dependency analysis is not performed for asserted or Script modules - * currently. Synthetic modules were injected into the graph with their own - * dependencies provided. */ +/** The kind of module. */ export type ModuleKind = | "asserted" | "esm" | "npm" | "external"; +export type ImportKind = + | "tsType" + | "tsReferencePath" + | "tsReferenceTypes" + | "jsxImportSource" + | "jsDoc"; + +export type ImportAssertions = "unknown" | Record; + +export interface ImportJson { + specifier: string; + /** Absent if standard ES import/export. */ + kind?: ImportKind; + range: RangeJson; + isDynamic?: true; + assertions?: ImportAssertions; + errors?: string[]; +} + export interface DependencyJson { /** The string specifier that was used for the dependency. */ specifier: string; @@ -117,6 +129,8 @@ export interface DependencyJson { /** A flag indicating if the dependency was dynamic. (e.g. * `await import("mod.ts")`) */ isDynamic?: true; + imports?: ImportJson[]; + // TODO(nayeemrmn): Legacy field, remove for 2.0. Asserts are per-import. assertionType?: string; } diff --git a/src/analyzer.rs b/src/analyzer.rs index 006612624..c66a0328d 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -128,6 +128,7 @@ impl DependencyKind { } #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] pub enum ImportAssertion { /// The value of this assertion could not be statically analyzed. Unknown, @@ -147,6 +148,7 @@ impl ImportAssertion { } #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] pub enum ImportAssertions { /// There was no import assertions object literal. None, @@ -188,6 +190,17 @@ impl ImportAssertions { _ => None, } } + + /// Check if a assertion key is known to be absent (not set to a value and not + /// undeterminable). This is significant because it is the precise case where + /// `type: "javascript"` is asserted. + pub fn is_known_none(&self, key: &str) -> bool { + match self { + ImportAssertions::None => true, + ImportAssertions::Known(map) => !map.contains_key(key), + ImportAssertions::Unknown => false, + } + } } #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] diff --git a/src/graph.rs b/src/graph.rs index 9ce1bad03..4100d090f 100644 --- a/src/graph.rs +++ b/src/graph.rs @@ -31,6 +31,7 @@ use futures::stream::FuturesUnordered; use futures::stream::StreamExt; use futures::FutureExt; use indexmap::IndexMap; +use once_cell::sync::Lazy; use serde::ser::SerializeSeq; use serde::ser::SerializeStruct; use serde::Deserialize; @@ -44,6 +45,13 @@ use std::collections::VecDeque; use std::fmt; use std::sync::Arc; +static SUPPORTED_ASSERT_TYPES: Lazy> = + Lazy::new(|| { + let mut map = HashMap::new(); + map.insert("json".to_string(), MediaType::Json); + map + }); + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Hash)] pub struct Position { /// The 0-indexed line index. @@ -139,17 +147,6 @@ pub enum ModuleError { MissingDynamic(ModuleSpecifier, Range), ParseErr(ModuleSpecifier, deno_ast::Diagnostic), UnsupportedMediaType(ModuleSpecifier, MediaType, Option), - InvalidTypeAssertion { - specifier: ModuleSpecifier, - range: Range, - actual_media_type: MediaType, - expected_media_type: MediaType, - }, - UnsupportedImportAssertionType { - specifier: ModuleSpecifier, - range: Range, - kind: String, - }, } impl ModuleError { @@ -159,9 +156,7 @@ impl ModuleError { | Self::ParseErr(s, _) | Self::UnsupportedMediaType(s, _, _) | Self::Missing(s, _) - | Self::MissingDynamic(s, _) - | Self::InvalidTypeAssertion { specifier: s, .. } - | Self::UnsupportedImportAssertionType { specifier: s, .. } => s, + | Self::MissingDynamic(s, _) => s, } } @@ -174,8 +169,6 @@ impl ModuleError { maybe_referrer.as_ref() } Self::ParseErr(_, _) => None, - Self::InvalidTypeAssertion { range, .. } => Some(range), - Self::UnsupportedImportAssertionType { range, .. } => Some(range), } } } @@ -187,9 +180,7 @@ impl std::error::Error for ModuleError { Self::Missing(_, _) | Self::MissingDynamic(_, _) | Self::ParseErr(_, _) - | Self::UnsupportedMediaType(_, _, _) - | Self::InvalidTypeAssertion { .. } - | Self::UnsupportedImportAssertionType { .. } => None, + | Self::UnsupportedMediaType(_, _, _) => None, } } } @@ -199,24 +190,19 @@ impl fmt::Display for ModuleError { match self { Self::LoadingErr(_, _, err) => err.fmt(f), Self::ParseErr(_, diagnostic) => write!(f, "The module's source code could not be parsed: {diagnostic}"), - Self::UnsupportedMediaType(specifier, MediaType::Json, ..) => write!(f, "Expected a JavaScript or TypeScript module, but identified a Json module. Consider importing Json modules with an import assertion with the type of \"json\".\n Specifier: {specifier}"), Self::UnsupportedMediaType(specifier, media_type, ..) => write!(f, "Expected a JavaScript or TypeScript module, but identified a {media_type} module. Importing these types of modules is currently not supported.\n Specifier: {specifier}"), Self::Missing(specifier, _) => write!(f, "Module not found \"{specifier}\"."), Self::MissingDynamic(specifier, _) => write!(f, "Dynamic import not found \"{specifier}\"."), - Self::InvalidTypeAssertion { specifier, actual_media_type: MediaType::Json, expected_media_type, .. } => - write!(f, "Expected a {expected_media_type} module, but identified a Json module. Consider importing Json modules with an import assertion with the type of \"json\".\n Specifier: {specifier}"), - Self::InvalidTypeAssertion { specifier, actual_media_type, expected_media_type, .. } => - write!(f, "Expected a {expected_media_type} module, but identified a {actual_media_type} module.\n Specifier: {specifier}"), - Self::UnsupportedImportAssertionType { specifier, kind, .. } => - write!(f, "The import assertion type of \"{kind}\" is unsupported.\n Specifier: {specifier}"), } } } #[derive(Debug, Clone)] +#[allow(clippy::large_enum_variant)] pub enum ModuleGraphError { ModuleError(ModuleError), ResolutionError(ResolutionError), + ImportError(ImportError), } impl ModuleGraphError { @@ -236,6 +222,7 @@ impl ModuleGraphError { match self { Self::ModuleError(err) => err.maybe_referrer(), Self::ResolutionError(err) => Some(err.range()), + Self::ImportError(_) => None, } } } @@ -245,6 +232,7 @@ impl std::error::Error for ModuleGraphError { match self { Self::ModuleError(ref err) => Some(err), Self::ResolutionError(ref err) => Some(err), + Self::ImportError(ref err) => Some(err), } } } @@ -254,6 +242,7 @@ impl fmt::Display for ModuleGraphError { match self { Self::ModuleError(err) => err.fmt(f), Self::ResolutionError(err) => err.fmt(f), + Self::ImportError(err) => err.fmt(f), } } } @@ -471,6 +460,43 @@ fn is_false(v: &bool) -> bool { !v } +#[derive(Clone, Debug)] +#[cfg_attr(test, derive(Eq, PartialEq))] +pub enum ImportError { + TypeAssertionFailed { + specifier: ModuleSpecifier, + range: Range, + actual_media_type: MediaType, + expected_media_type: MediaType, + }, + TypeAssertionUnsupported { + specifier: String, + range: Range, + assert_type: String, + }, +} + +impl std::error::Error for ImportError {} + +impl fmt::Display for ImportError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match &self { + ImportError::TypeAssertionFailed { specifier, actual_media_type: MediaType::Json, expected_media_type, .. } => write!(f, "Expected a {expected_media_type} module, but identified a Json module. Consider importing Json modules with an import assertion with the type of \"json\".\n Specifier: {specifier}"), + ImportError::TypeAssertionFailed { specifier, actual_media_type, expected_media_type, .. } => write!(f, "Expected a {expected_media_type} module, but identified a {actual_media_type} module.\n Specifier: {specifier}"), + ImportError::TypeAssertionUnsupported { assert_type, .. } => write!(f, "The import assertion type of \"{assert_type}\" is unsupported."), + } + } +} + +impl Serialize for ImportError { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&self.to_string()) + } +} + #[derive(Clone, Copy, Debug, Serialize, Eq, PartialEq)] #[serde(rename_all = "camelCase")] pub enum ImportKind { @@ -515,10 +541,12 @@ pub struct Import { #[serde(skip_serializing_if = "is_false")] pub is_dynamic: bool, // Don't include assertions in `deno info --json`, since they may be unstable: - // https://github.com/denoland/deno/issues/17944. Assertion error strings - // eventually will be included in a separate `Import::errors`, however. + // https://github.com/denoland/deno/issues/17944. #[serde(skip_serializing)] pub assertions: ImportAssertions, + // Note: This is only populated in a final pass at the end of graph building. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub errors: Vec, } #[derive(Debug, Default, Clone, Serialize)] @@ -530,11 +558,9 @@ pub struct Dependency { pub maybe_type: Resolution, #[serde(skip_serializing_if = "is_false")] pub is_dynamic: bool, - #[serde(rename = "assertionType", skip_serializing_if = "Option::is_none")] - pub maybe_assert_type: Option, - // TODO(nayeemrmn): Replace `maybe_assert_type` with this in the serialization - // for 2.0. - #[serde(skip_serializing)] + /// List of imports which refer to this dependency. Doesn't include those + /// which only supplement types to the main import (i.e. @deno-types). + #[serde(skip_serializing_if = "Vec::is_empty")] pub imports: Vec, } @@ -797,7 +823,6 @@ impl GraphImport { is_dynamic: false, maybe_code: Resolution::None, maybe_type, - maybe_assert_type: None, imports: vec![], }, ) @@ -1138,6 +1163,15 @@ impl<'a> Iterator for ModuleGraphErrorIterator<'a> { } } } + for import in &dep.imports { + if follow_dynamic || !import.is_dynamic { + if let Some(error) = import.errors.first() { + self + .next_errors + .push(ModuleGraphError::ImportError(error.clone())); + } + } + } } } ModuleEntryRef::Err(error) => { @@ -1146,7 +1180,7 @@ impl<'a> Iterator for ModuleGraphErrorIterator<'a> { let should_ignore = follow_dynamic && matches!( error, - ModuleGraphError::ModuleError(ModuleError::Missing { .. }) + ModuleGraphError::ModuleError(ModuleError::Missing(..)) ); if !should_ignore { self.next_errors.push(error.clone()); @@ -1558,26 +1592,15 @@ pub(crate) fn parse_module( specifier: &ModuleSpecifier, maybe_headers: Option<&HashMap>, content: Arc, - maybe_assert_type: Option, maybe_referrer: Option, maybe_resolver: Option<&dyn Resolver>, module_analyzer: &dyn ModuleAnalyzer, is_root: bool, - is_dynamic_branch: bool, ) -> Result { let media_type = MediaType::from_specifier_and_headers(specifier, maybe_headers); - // here we check any media types that should have assertions made against them - // if they aren't the root and add them to the graph, otherwise we continue - if media_type == MediaType::Json - && (is_root - || is_dynamic_branch - || matches!( - maybe_assert_type.as_ref().map(|t| t.kind.as_str()), - Some("json") - )) - { + if media_type == MediaType::Json { return Ok(Module::Json(JsonModule { maybe_cache_info: None, source: content, @@ -1586,27 +1609,6 @@ pub(crate) fn parse_module( })); } - if let Some(assert_type) = maybe_assert_type { - if assert_type.kind == "json" { - return Err(ModuleGraphError::ModuleError( - ModuleError::InvalidTypeAssertion { - specifier: specifier.clone(), - range: assert_type.range, - actual_media_type: media_type, - expected_media_type: MediaType::Json, - }, - )); - } else { - return Err(ModuleGraphError::ModuleError( - ModuleError::UnsupportedImportAssertionType { - specifier: specifier.clone(), - range: assert_type.range, - kind: assert_type.kind, - }, - )); - } - } - // Here we check for known ES Modules that we will analyze the dependencies of match &media_type { MediaType::JavaScript @@ -1700,6 +1702,7 @@ pub(crate) fn parse_esm_module_from_module_info( range, is_dynamic: false, assertions: Default::default(), + errors: Default::default(), }); } TypeScriptReference::Types(specifier) => { @@ -1726,6 +1729,7 @@ pub(crate) fn parse_esm_module_from_module_info( range, is_dynamic: false, assertions: Default::default(), + errors: Default::default(), }); } } @@ -1776,6 +1780,7 @@ pub(crate) fn parse_esm_module_from_module_info( range, is_dynamic: false, assertions: Default::default(), + errors: Default::default(), }); } } @@ -1797,6 +1802,7 @@ pub(crate) fn parse_esm_module_from_module_info( range, is_dynamic: false, assertions: Default::default(), + errors: Default::default(), }); } @@ -1864,11 +1870,6 @@ pub(crate) fn parse_esm_module_from_module_info( .dependencies .entry(desc.specifier.to_string()) .or_default(); - // TODO(nayeemrmn): Import assertions should be visited and checked for - // every import, not one per specifier. - if dep.maybe_assert_type.is_none() { - dep.maybe_assert_type = desc.import_assertions.get("type").cloned(); - } let range = Range::from_position_range( module.specifier.clone(), desc.specifier_range.clone(), @@ -1888,6 +1889,7 @@ pub(crate) fn parse_esm_module_from_module_info( range, is_dynamic: desc.is_dynamic, assertions: desc.import_assertions.clone(), + errors: vec![], }); } else { if dep.maybe_code.is_none() { @@ -1908,6 +1910,7 @@ pub(crate) fn parse_esm_module_from_module_info( range, is_dynamic: desc.is_dynamic, assertions: desc.import_assertions.clone(), + errors: vec![], }); } if dep.maybe_type.is_none() { @@ -1938,6 +1941,53 @@ fn is_untyped(media_type: &MediaType) -> bool { ) } +/// Check import assertions. +fn check_import_assertions( + import: &Import, + maybe_module_metadata: Option<&ModuleMetadata>, +) -> Vec { + if !matches!(import.kind, ImportKind::Es) { + return vec![]; + } + let mut errors = vec![]; + if let Some(assert_type) = import.assertions.get("type") { + match SUPPORTED_ASSERT_TYPES.get(assert_type) { + Some(&expected_media_type) => { + if let Some(module_metadata) = maybe_module_metadata { + if module_metadata.media_type != expected_media_type { + errors.push(ImportError::TypeAssertionFailed { + specifier: module_metadata.specifier.clone(), + range: import.range.clone(), + actual_media_type: module_metadata.media_type, + expected_media_type, + }) + } + } + } + None => errors.push(ImportError::TypeAssertionUnsupported { + specifier: import.specifier.clone(), + range: import.range.clone(), + assert_type: assert_type.clone(), + }), + } + } else if import.assertions.is_known_none("type") { + if let Some(module_metadata) = maybe_module_metadata { + if SUPPORTED_ASSERT_TYPES + .values() + .any(|t| t == &module_metadata.media_type) + { + errors.push(ImportError::TypeAssertionFailed { + specifier: module_metadata.specifier.clone(), + range: import.range.clone(), + actual_media_type: module_metadata.media_type, + expected_media_type: MediaType::JavaScript, + }) + } + } + } + errors +} + /// The kind of module graph. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum GraphKind { @@ -1967,11 +2017,12 @@ impl Default for GraphKind { type LoadWithSpecifierFuture = LocalBoxFuture<'static, (ModuleSpecifier, Option, LoadResult)>; -#[derive(PartialEq, Eq, Hash)] -pub(crate) struct AssertTypeWithRange { - range: Range, - /// The kind of assertion (ex. "json"). - kind: String, +/// Metadata for modules which should suffice to check assertions on importers +/// of that module. +#[derive(Clone, Debug)] +struct ModuleMetadata { + specifier: ModuleSpecifier, + media_type: MediaType, } struct Builder<'a, 'graph> { @@ -1986,10 +2037,7 @@ struct Builder<'a, 'graph> { FuturesUnordered)>>, pending_npm_specifiers: Vec<(ModuleSpecifier, NpmPackageReqReference, Option)>, - pending_specifiers: - HashMap>>, - dynamic_branches: - HashMap)>, + dynamic_branches: HashMap, module_analyzer: &'a dyn ModuleAnalyzer, reporter: Option<&'a dyn Reporter>, } @@ -2019,7 +2067,6 @@ impl<'a, 'graph> Builder<'a, 'graph> { requested_npm_registry_info_loads: Default::default(), pending_npm_registry_info_loads: Default::default(), pending_npm_specifiers: Default::default(), - pending_specifiers: Default::default(), dynamic_branches: Default::default(), } .fill(roots, imports) @@ -2041,7 +2088,7 @@ impl<'a, 'graph> Builder<'a, 'graph> { .collect::>(); self.graph.roots.extend(roots.clone()); for root in roots { - self.load(&root, None, self.in_dynamic_branch, None); + self.load(&root, None, self.in_dynamic_branch); } // Process any imports that are being added to the graph. @@ -2055,7 +2102,6 @@ impl<'a, 'graph> Builder<'a, 'graph> { &resolved.specifier, Some(&resolved.range), self.in_dynamic_branch, - None, ); } } @@ -2065,16 +2111,7 @@ impl<'a, 'graph> Builder<'a, 'graph> { loop { let specifier = match self.pending.next().await { Some((specifier, maybe_referrer, Ok(Some(response)))) => { - let assert_types = - self.pending_specifiers.remove(&specifier).unwrap(); - for maybe_assert_type in assert_types { - self.visit( - &specifier, - &response, - maybe_assert_type, - maybe_referrer.clone(), - ) - } + self.visit(&specifier, &response, maybe_referrer.clone()); Some(specifier) } Some((specifier, maybe_range, Ok(None))) => { @@ -2116,11 +2153,9 @@ impl<'a, 'graph> Builder<'a, 'graph> { // visiting a dynamic branch. if !self.in_dynamic_branch { self.in_dynamic_branch = true; - for (specifier, (range, maybe_assert_type)) in - std::mem::take(&mut self.dynamic_branches) - { + for (specifier, range) in std::mem::take(&mut self.dynamic_branches) { if !self.graph.module_slots.contains_key(&specifier) { - self.load(&specifier, Some(&range), true, maybe_assert_type); + self.load(&specifier, Some(&range), true); } } } else { @@ -2129,7 +2164,28 @@ impl<'a, 'graph> Builder<'a, 'graph> { } } - // Enrich with cache info from the loader + // Extract module metadata in a separate pass. Needed to avoid borrow issue. + let mut module_metadata = HashMap::new(); + for (specifier, slot) in &self.graph.module_slots { + match slot { + ModuleSlot::Module(Module::Esm(module)) => module_metadata.insert( + specifier.clone(), + ModuleMetadata { + specifier: module.specifier.clone(), + media_type: module.media_type, + }, + ), + ModuleSlot::Module(Module::Json(module)) => module_metadata.insert( + specifier.clone(), + ModuleMetadata { + specifier: module.specifier.clone(), + media_type: MediaType::Json, + }, + ), + _ => None, + }; + } + for slot in self.graph.module_slots.values_mut() { if let ModuleSlot::Module(ref mut module) = slot { match module { @@ -2140,6 +2196,18 @@ impl<'a, 'graph> Builder<'a, 'graph> { Module::Esm(module) => { module.maybe_cache_info = self.loader.get_cache_info(&module.specifier); + + // Check import assertions + for dep in module.dependencies.values_mut() { + let module_metadata = dep + .maybe_code + .maybe_specifier() + .and_then(|s| module_metadata.get(s)); + for import in &mut dep.imports { + import.errors = + check_import_assertions(import, module_metadata); + } + } } Module::External(_) | Module::Npm(_) | Module::Node(_) => {} } @@ -2294,14 +2362,8 @@ impl<'a, 'graph> Builder<'a, 'graph> { specifier: &ModuleSpecifier, maybe_range: Option<&Range>, is_dynamic: bool, - maybe_assert_type: Option, ) { let specifier = self.graph.redirects.get(specifier).unwrap_or(specifier); - self - .pending_specifiers - .entry(specifier.clone()) - .or_default() - .insert(maybe_assert_type); if self.graph.module_slots.contains_key(specifier) { return; } @@ -2401,7 +2463,6 @@ impl<'a, 'graph> Builder<'a, 'graph> { &mut self, requested_specifier: &ModuleSpecifier, response: &LoadResponse, - maybe_assert_type: Option, maybe_referrer: Option, ) { let (specifier, module_slot) = match response { @@ -2425,7 +2486,6 @@ impl<'a, 'graph> Builder<'a, 'graph> { specifier, maybe_headers.as_ref(), content.clone(), - maybe_assert_type, maybe_referrer, ), ) @@ -2443,7 +2503,6 @@ impl<'a, 'graph> Builder<'a, 'graph> { specifier: &ModuleSpecifier, maybe_headers: Option<&HashMap>, content: Arc, - maybe_assert_type: Option, maybe_referrer: Option, ) -> ModuleSlot { use std::borrow::BorrowMut; @@ -2453,12 +2512,10 @@ impl<'a, 'graph> Builder<'a, 'graph> { specifier, maybe_headers, content, - maybe_assert_type, maybe_referrer, self.resolver, self.module_analyzer, is_root, - self.in_dynamic_branch, ) { Ok(module) => ModuleSlot::Module(module), Err(err) => ModuleSlot::Err(err), @@ -2477,25 +2534,12 @@ impl<'a, 'graph> Builder<'a, 'graph> { if let Resolution::Ok(resolved) = &dep.maybe_code { let specifier = &resolved.specifier; let range = &resolved.range; - let maybe_assert_type_with_range = dep - .maybe_assert_type - .as_ref() - .map(|assert_type| AssertTypeWithRange { - range: range.clone(), - kind: assert_type.clone(), - }); if dep.is_dynamic && !self.in_dynamic_branch { - self.dynamic_branches.insert( - specifier.clone(), - (range.clone(), maybe_assert_type_with_range), - ); + self + .dynamic_branches + .insert(specifier.clone(), range.clone()); } else { - self.load( - specifier, - Some(range), - self.in_dynamic_branch, - maybe_assert_type_with_range, - ); + self.load(specifier, Some(range), self.in_dynamic_branch); } } } else { @@ -2509,25 +2553,12 @@ impl<'a, 'graph> Builder<'a, 'graph> { if let Resolution::Ok(resolved) = &dep.maybe_type { let specifier = &resolved.specifier; let range = &resolved.range; - let maybe_assert_type_with_range = dep - .maybe_assert_type - .as_ref() - .map(|assert_type| AssertTypeWithRange { - range: range.clone(), - kind: assert_type.clone(), - }); if dep.is_dynamic && !self.in_dynamic_branch { - self.dynamic_branches.insert( - specifier.clone(), - (range.clone(), maybe_assert_type_with_range), - ); + self + .dynamic_branches + .insert(specifier.clone(), range.clone()); } else { - self.load( - specifier, - Some(range), - self.in_dynamic_branch, - maybe_assert_type_with_range, - ); + self.load(specifier, Some(range), self.in_dynamic_branch); } } } else { @@ -2545,7 +2576,7 @@ impl<'a, 'graph> Builder<'a, 'graph> { .as_ref() .map(|d| &d.dependency) { - self.load(&resolved.specifier, Some(&resolved.range), false, None); + self.load(&resolved.specifier, Some(&resolved.range), false); } } else { module.maybe_types_dependency = None; @@ -2588,15 +2619,24 @@ where S: Serializer, { #[derive(Serialize)] + #[serde(rename_all = "camelCase")] struct DependencyWithSpecifier<'a> { specifier: &'a str, + // TODO(nayeemrmn): Legacy field, remove for 2.0. Asserts are per-import. + #[serde(skip_serializing_if = "Option::is_none")] + assertion_type: Option<&'a String>, #[serde(flatten)] dependency: &'a Dependency, } let mut seq = serializer.serialize_seq(Some(dependencies.len()))?; for (specifier, dependency) in dependencies { + let assertion_type = dependency + .imports + .iter() + .find_map(|i| i.assertions.get("type")); seq.serialize_element(&DependencyWithSpecifier { specifier, + assertion_type, dependency, })? } @@ -2668,10 +2708,8 @@ mod tests { content.into(), None, None, - None, &module_analyzer, true, - false, ) .unwrap(); let module = module.esm().unwrap(); @@ -3204,6 +3242,7 @@ mod tests { }, is_dynamic: false, assertions: ImportAssertions::None, + errors: vec![], }, Import { specifier: "file:///bar.ts".to_string(), @@ -3221,6 +3260,7 @@ mod tests { }, is_dynamic: false, assertions: ImportAssertions::None, + errors: vec![], }, Import { specifier: "file:///bar.ts".to_string(), @@ -3238,6 +3278,7 @@ mod tests { }, is_dynamic: false, assertions: ImportAssertions::None, + errors: vec![], }, Import { specifier: "file:///bar.ts".to_string(), @@ -3255,6 +3296,7 @@ mod tests { }, is_dynamic: true, assertions: ImportAssertions::None, + errors: vec![], }, Import { specifier: "file:///bar.ts".to_string(), @@ -3272,6 +3314,7 @@ mod tests { }, is_dynamic: true, assertions: ImportAssertions::Unknown, + errors: vec![], }, Import { specifier: "file:///bar.ts".to_string(), @@ -3289,6 +3332,7 @@ mod tests { }, is_dynamic: false, assertions: ImportAssertions::None, + errors: vec![], }, ] ); @@ -3313,7 +3357,139 @@ mod tests { "type".to_string(), ImportAssertion::Known("json".to_string()) )])), + errors: vec![], },] ); } + + #[tokio::test] + async fn import_assertion_errors() { + struct TestLoader; + impl Loader for TestLoader { + fn load( + &mut self, + specifier: &ModuleSpecifier, + is_dynamic: bool, + ) -> LoadFuture { + let specifier = specifier.clone(); + match specifier.as_str() { + "file:///foo1.js" => Box::pin(async move { + Ok(Some(LoadResponse::Module { + specifier: specifier.clone(), + maybe_headers: None, + content: " + import 'file:///bar.json' assert { type: 'json' }; + import 'file:///bar.json'; // error + " + .into(), + })) + }), + "file:///foo2.js" => Box::pin(async move { + Ok(Some(LoadResponse::Module { + specifier: specifier.clone(), + maybe_headers: None, + content: " + import 'file:///bar.json' assert { type: 'json' }; + import 'file:///bar.json' assert { type: 'invalid' }; // error + " + .into(), + })) + }), + "file:///foo3.js" => Box::pin(async move { + Ok(Some(LoadResponse::Module { + specifier: specifier.clone(), + maybe_headers: None, + content: " + import 'file:///foo2.js' assert { type: 'json' }; // error + import 'file:///foo2.js'; + " + .into(), + })) + }), + "file:///foo4.js" => Box::pin(async move { + Ok(Some(LoadResponse::Module { + specifier: specifier.clone(), + maybe_headers: None, + content: " + import 'file:///bar.json' assert { type: 'json' }; + await import('file:///bar.json'); // ignored error + " + .into(), + })) + }), + "file:///bar.json" => { + assert!(!is_dynamic); + Box::pin(async move { + Ok(Some(LoadResponse::Module { + specifier: specifier.clone(), + maybe_headers: Some(HashMap::from_iter(vec![( + "Content-Type".to_string(), + "application/json".to_string(), + )])), + content: "{}".into(), + })) + }) + } + _ => unreachable!(), + } + } + } + let mut loader = TestLoader; + let mut graph = ModuleGraph::new(GraphKind::All); + graph + .build( + vec![ + Url::parse("file:///foo1.js").unwrap(), + Url::parse("file:///foo2.js").unwrap(), + Url::parse("file:///foo3.js").unwrap(), + Url::parse("file:///foo4.js").unwrap(), + ], + &mut loader, + Default::default(), + ) + .await; + assert_eq!( + graph + .walk( + &[Url::parse("file:///foo1.js").unwrap()], + WalkOptions::default(), + ) + .validate() + .unwrap_err() + .to_string(), + "Expected a JavaScript module, but identified a Json module. Consider importing Json modules with an import assertion with the type of \"json\".\n Specifier: file:///bar.json" + ); + assert_eq!( + graph + .walk( + &[Url::parse("file:///foo2.js").unwrap()], + WalkOptions::default(), + ) + .validate() + .unwrap_err() + .to_string(), + "The import assertion type of \"invalid\" is unsupported." + ); + assert_eq!( + graph + .walk( + &[Url::parse("file:///foo3.js").unwrap()], + WalkOptions::default(), + ) + .validate() + .unwrap_err() + .to_string(), + "Expected a Json module, but identified a JavaScript module.\n Specifier: file:///foo2.js" + ); + graph + .walk( + &[Url::parse("file:///foo4.js").unwrap()], + WalkOptions { + follow_dynamic: false, + ..Default::default() + }, + ) + .validate() + .unwrap(); + } } diff --git a/src/lib.rs b/src/lib.rs index 62bb27028..d0f741a8d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -83,11 +83,9 @@ pub fn parse_module( maybe_headers, content, None, - None, maybe_resolver, module_analyzer, true, - false, ) } @@ -115,6 +113,7 @@ mod tests { use super::*; use pretty_assertions::assert_eq; use serde_json::json; + use serde_json::Value; use source::tests::MockResolver; use source::CacheInfo; use source::MemoryLoader; @@ -131,6 +130,21 @@ mod tests { MemoryLoader::new(sources, cache_info) } + /// The `dependency.imports` field is a bit verbose for most tests. + fn graph_to_json_without_imports(graph: &ModuleGraph) -> Value { + let mut value = json!(graph); + for module in value.get_mut("modules").unwrap().as_array_mut().unwrap() { + let deps = match module.get_mut("dependencies") { + None => continue, + Some(v) => v.as_array_mut().unwrap(), + }; + for dep in deps { + dep.as_object_mut().unwrap().remove("imports"); + } + } + value + } + #[tokio::test] async fn test_build_graph() { let mut loader = setup( @@ -369,7 +383,7 @@ mod tests { ) .await; assert_eq!( - json!(graph), + graph_to_json_without_imports(&graph), json!({ "roots": [ "file:///a/test.json" @@ -387,87 +401,6 @@ mod tests { ); } - #[tokio::test] - async fn test_build_graph_dynamic_json_ignores_assert() { - let mut loader = setup( - vec![ - ( - "file:///a/test.js", - Source::Module { - specifier: "file:///a/test.js", - maybe_headers: None, - content: r#" - const a = await import("./a.json"); - "#, - }, - ), - ( - "file:///a/a.json", - Source::Module { - specifier: "file:///a/a.json", - maybe_headers: None, - content: r#"{"a":"b"}"#, - }, - ), - ], - vec![], - ); - let roots = vec![ModuleSpecifier::parse("file:///a/test.js").unwrap()]; - let mut graph = ModuleGraph::default(); - graph - .build( - roots.clone(), - &mut loader, - BuildOptions { - is_dynamic: true, - ..Default::default() - }, - ) - .await; - assert_eq!( - json!(graph), - json!({ - "roots": [ - "file:///a/test.js", - ], - "modules": [ - { - "kind": "asserted", - "size": 9, - "mediaType": "Json", - "specifier": "file:///a/a.json" - }, - { - "dependencies": [ - { - "specifier": "./a.json", - "code": { - "specifier": "file:///a/a.json", - "span": { - "start": { - "line": 1, - "character": 31 - }, - "end": { - "line": 1, - "character": 41 - } - } - }, - "isDynamic": true - } - ], - "kind": "esm", - "mediaType": "JavaScript", - "size": 53, - "specifier": "file:///a/test.js" - } - ], - "redirects": {} - }) - ); - } - #[tokio::test] async fn test_valid_type_missing() { let mut loader = setup( @@ -714,7 +647,7 @@ console.log(a); ) .await; assert_eq!( - json!(graph), + graph_to_json_without_imports(&graph), json!({ "roots": ["file:///a/test01.ts"], "modules": [ @@ -842,7 +775,7 @@ console.log(a); ) .await; assert_eq!( - json!(graph), + graph_to_json_without_imports(&graph), json!({ "roots": ["file:///a/test01.ts"], "modules": [ @@ -1068,7 +1001,7 @@ console.log(a); ) .await; assert_eq!( - json!(graph), + graph_to_json_without_imports(&graph), json!({ "roots": [ "file:///a/test01.tsx" @@ -1146,52 +1079,6 @@ console.log(a); } } - #[tokio::test] - async fn test_unsupported_media_type() { - let mut loader = setup( - vec![ - ( - "file:///a/test.ts", - Source::Module { - specifier: "file:///a/test.ts", - maybe_headers: None, - content: r#"import "./test.json";"#, - }, - ), - ( - "file:///a/test.json", - Source::Module { - specifier: "file:///a/test.json", - maybe_headers: None, - content: r#"{"hello":"world"}"#, - }, - ), - ], - vec![], - ); - let root_specifier = - ModuleSpecifier::parse("file:///a/test.ts").expect("bad url"); - let mut graph = ModuleGraph::default(); - graph - .build( - vec![root_specifier.clone()], - &mut loader, - Default::default(), - ) - .await; - let result = graph.valid(); - assert!(result.is_err()); - let err = result.unwrap_err(); - assert!(matches!( - err, - ModuleGraphError::ModuleError(ModuleError::UnsupportedMediaType( - _, - MediaType::Json, - _ - )), - )); - } - #[tokio::test] async fn test_root_is_extensionless() { let mut loader = setup( @@ -1262,7 +1149,7 @@ console.log(a); ) .await; assert_eq!( - json!(graph), + graph_to_json_without_imports(&graph), json!({ "roots": [ "file:///a.ts" @@ -1352,7 +1239,7 @@ export function a(a) { .build(vec![root.clone()], &mut loader, Default::default()) .await; assert_eq!( - json!(graph), + graph_to_json_without_imports(&graph), json!({ "roots": [ "file:///a/test.js" @@ -1816,6 +1703,21 @@ export function a(a) { } } }, + "imports": [ + { + "specifier": "./a.json", + "range": { + "start": { + "line": 1, + "character": 26, + }, + "end": { + "line": 1, + "character": 36, + }, + }, + }, + ], "assertionType": "json" }, { @@ -1834,6 +1736,22 @@ export function a(a) { } }, "isDynamic": true, + "imports": [ + { + "specifier": "./b.json", + "range": { + "start": { + "line": 2, + "character": 35, + }, + "end": { + "line": 2, + "character": 45, + }, + }, + "isDynamic": true, + }, + ], "assertionType": "json" }, { @@ -1851,6 +1769,21 @@ export function a(a) { } } }, + "imports": [ + { + "specifier": "./c.json", + "range": { + "start": { + "line": 3, + "character": 31, + }, + "end": { + "line": 3, + "character": 41, + }, + }, + }, + ], "assertionType": "json" }, { @@ -1868,7 +1801,23 @@ export function a(a) { } } }, - "isDynamic": true + "isDynamic": true, + "imports": [ + { + "specifier": "./d.json", + "range": { + "start": { + "line": 5, + "character": 35, + }, + "end": { + "line": 5, + "character": 45, + }, + }, + "isDynamic": true, + }, + ], } ], "kind": "esm", @@ -1919,7 +1868,7 @@ export function a(a) { ) .await; assert_eq!( - json!(graph), + graph_to_json_without_imports(&graph), json!({ "roots": [ "file:///a/test01.ts" @@ -2041,8 +1990,10 @@ export function a(a) { ], "modules": [ { + "kind": "asserted", + "size": 9, + "mediaType": "Json", "specifier": "file:///a/a.json", - "error": "Expected a JavaScript or TypeScript module, but identified a Json module. Consider importing Json modules with an import assertion with the type of \"json\".\n Specifier: file:///a/a.json" }, { "kind": "asserted", @@ -2051,12 +2002,16 @@ export function a(a) { "specifier": "file:///a/b.json" }, { + "kind": "esm", + "size": 21, + "mediaType": "JavaScript", "specifier": "file:///a/c.js", - "error": "Expected a Json module, but identified a JavaScript module.\n Specifier: file:///a/c.js" }, { + "kind": "asserted", + "size": 9, + "mediaType": "Json", "specifier": "file:///a/d.json", - "error": "The import assertion type of \"css\" is unsupported.\n Specifier: file:///a/d.json" }, { "specifier": "file:///a/e.wasm", @@ -2078,7 +2033,23 @@ export function a(a) { "character": 36 } } - } + }, + "imports": [ + { + "specifier": "./a.json", + "range": { + "start": { + "line": 1, + "character": 26, + }, + "end": { + "line": 1, + "character": 36, + }, + }, + "errors": ["Expected a JavaScript module, but identified a Json module. Consider importing Json modules with an import assertion with the type of \"json\".\n Specifier: file:///a/a.json"], + }, + ], }, { "specifier": "./b.json", @@ -2095,6 +2066,21 @@ export function a(a) { } } }, + "imports": [ + { + "specifier": "./b.json", + "range": { + "start": { + "line": 2, + "character": 26, + }, + "end": { + "line": 2, + "character": 36, + }, + }, + }, + ], "assertionType": "json" }, { @@ -2112,6 +2098,22 @@ export function a(a) { } } }, + "imports": [ + { + "specifier": "./c.js", + "range": { + "start": { + "line": 3, + "character": 26, + }, + "end": { + "line": 3, + "character": 34, + }, + }, + "errors": ["Expected a Json module, but identified a JavaScript module.\n Specifier: file:///a/c.js"], + }, + ], "assertionType": "json" }, { @@ -2129,6 +2131,22 @@ export function a(a) { } } }, + "imports": [ + { + "specifier": "./d.json", + "range": { + "start": { + "line": 4, + "character": 26, + }, + "end": { + "line": 4, + "character": 36, + }, + }, + "errors": ["The import assertion type of \"css\" is unsupported."], + }, + ], "assertionType": "css" }, { @@ -2145,7 +2163,22 @@ export function a(a) { "character": 36 } } - } + }, + "imports": [ + { + "specifier": "./e.wasm", + "range": { + "start": { + "line": 5, + "character": 26, + }, + "end": { + "line": 5, + "character": 36, + }, + }, + }, + ], } ], "mediaType": "TypeScript", @@ -2380,7 +2413,7 @@ export function a(a) { ) .await; assert_eq!( - json!(graph), + graph_to_json_without_imports(&graph), json!({ "roots": [ "file:///a/test01.ts" @@ -2607,7 +2640,7 @@ export function a(a) { ) .await; assert_eq!( - json!(graph), + graph_to_json_without_imports(&graph), json!({ "roots": [ "file:///a/test01.ts" @@ -2754,7 +2787,7 @@ export function a(a) { ) .await; assert_eq!( - json!(graph), + graph_to_json_without_imports(&graph), json!({ "roots": [ "file:///a/test01.ts" @@ -2837,69 +2870,6 @@ export function a(a) { assert_eq!(actual.media_type, MediaType::TypeScript); } - #[test] - fn test_parse_module_import_assertions() { - let specifier = ModuleSpecifier::parse("file:///a/test01.ts").unwrap(); - let actual = parse_module( - &specifier, - None, - r#" - import a from "./a.json" assert { type: "json" }; - await import("./b.json", { assert: { type: "json" } }); - "# - .into(), - None, - None, - ) - .unwrap(); - assert_eq!( - json!(actual), - json!({ - "dependencies": [ - { - "specifier": "./a.json", - "code": { - "specifier": "file:///a/a.json", - "span": { - "start": { - "line": 1, - "character": 18 - }, - "end": { - "line": 1, - "character": 28 - } - } - }, - "assertionType": "json" - }, - { - "specifier": "./b.json", - "code": { - "specifier": "file:///a/b.json", - "span": { - "start": { - "line": 2, - "character": 17 - }, - "end": { - "line": 2, - "character": 27 - } - } - }, - "isDynamic": true, - "assertionType": "json" - } - ], - "kind": "esm", - "mediaType": "TypeScript", - "size": 119, - "specifier": "file:///a/test01.ts" - }) - ); - } - #[test] fn test_parse_module_jsx_import_source() { let specifier = ModuleSpecifier::parse("file:///a/test01.tsx").unwrap(); @@ -3034,7 +3004,23 @@ export function a(a) { "character": 32, } } - } + }, + "imports": [ + { + "specifier": "./types.d.ts", + "kind": "jsDoc", + "range": { + "start": { + "line": 4, + "character": 18, + }, + "end": { + "line": 4, + "character": 32, + }, + }, + }, + ], }, { "specifier": "./other.ts", @@ -3050,7 +3036,23 @@ export function a(a) { "character": 31 } } - } + }, + "imports": [ + { + "specifier": "./other.ts", + "kind": "jsDoc", + "range": { + "start": { + "line": 5, + "character": 19, + }, + "end": { + "line": 5, + "character": 31, + }, + }, + }, + ], } ], "kind": "esm",