diff --git a/Cargo.lock b/Cargo.lock index 5e0e2bb42e..1f17b5fabe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1137,9 +1137,9 @@ dependencies = [ [[package]] name = "deno_doc" -version = "0.73.5" +version = "0.74.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "76d4d2960ce5c7af64e57ec4e7fd99023bd1ab664cedc4ec267db1d294b6002a" +checksum = "347a6288b9fa4f1ce2d56f04326bc39e1dec243545a89240efeef402d5fbde6b" dependencies = [ "anyhow", "cfg-if", @@ -1160,9 +1160,9 @@ dependencies = [ [[package]] name = "deno_emit" -version = "0.31.5" +version = "0.32.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b98917905e9be9740722f89c3b2d3a963beaed8a05dce58e642947d0f6aa17d" +checksum = "bba6c66922c3953ba465fac215e42c637cc91119618911c0cd84cb994b25d5b7" dependencies = [ "anyhow", "base64 0.13.1", @@ -1228,9 +1228,9 @@ dependencies = [ [[package]] name = "deno_graph" -version = "0.61.5" +version = "0.62.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "332e6a1930b3266dc848edeaf1426bbbd3ddca25c5e107e70823efb1b3ce68be" +checksum = "2b0c15558cd72b534064bf7ae07e0fc99e0693915a4552f67558cc0b2b09c287" dependencies = [ "anyhow", "async-trait", @@ -2204,9 +2204,9 @@ dependencies = [ [[package]] name = "eszip" -version = "0.55.5" +version = "0.56.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "455c055f28756fc7ba0d64506e6167b91878f3a39854271d7d63b577deb8b45d" +checksum = "f50abfc246ed6d7a5e34c9658cac93f959788058c8455c528d69471ef72d8ebf" dependencies = [ "anyhow", "base64 0.21.4", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 0c9d228c3c..8fc6431de0 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -60,16 +60,16 @@ deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "dep_gra deno_cache_dir = "=0.6.1" deno_config = "=0.6.5" deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] } -deno_doc = { version = "=0.73.5", features = ["html"] } -deno_emit = "=0.31.5" -deno_graph = "=0.61.5" +deno_doc = { version = "=0.74.0", features = ["html"] } +deno_emit = "=0.32.0" +deno_graph = "=0.62.0" deno_lint = { version = "=0.52.2", features = ["docs"] } deno_lockfile.workspace = true deno_npm = "0.15.3" deno_runtime = { workspace = true, features = ["dont_create_runtime_snapshot", "include_js_files_for_snapshotting"] } deno_semver = "0.5.1" deno_task_shell = "=0.14.0" -eszip = "=0.55.5" +eszip = "=0.56.0" napi_sym.workspace = true async-trait.workspace = true diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 9bd3a62d44..1acef2058a 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -455,6 +455,7 @@ pub struct Flags { pub unstable: bool, pub unstable_bare_node_builtins: bool, pub unstable_byonm: bool, + pub unstable_sloppy_imports: bool, pub unstable_workspaces: bool, pub unstable_features: Vec, pub unsafely_ignore_certificate_errors: Option>, @@ -862,6 +863,7 @@ pub fn flags_from_vec(args: Vec) -> clap::error::Result { flags.unstable_bare_node_builtins = matches.get_flag("unstable-bare-node-builtins"); flags.unstable_byonm = matches.get_flag("unstable-byonm"); + flags.unstable_sloppy_imports = matches.get_flag("unstable-sloppy-imports"); flags.unstable_workspaces = matches.get_flag("unstable-workspaces"); if matches.get_flag("quiet") { @@ -981,6 +983,17 @@ fn clap_root() -> Command { .action(ArgAction::SetTrue) .global(true), ) + .arg( + Arg::new("unstable-sloppy-imports") + .long("unstable-sloppy-imports") + .help( + "Enable unstable resolving of specifiers by extension probing, .js to .ts, and directory probing.", + ) + .env("DENO_UNSTABLE_SLOPPY_IMPORTS") + .value_parser(FalseyValueParser::new()) + .action(ArgAction::SetTrue) + .global(true), + ) .arg( Arg::new("unstable-workspaces") .long("unstable-workspaces") diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 3e61b50bdf..033b30d979 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -1342,7 +1342,7 @@ impl CliOptions { || self .maybe_config_file() .as_ref() - .map(|c| c.json.unstable.contains(&"bare-node-builtins".to_string())) + .map(|c| c.has_unstable("bare-node-builtins")) .unwrap_or(false) } @@ -1355,7 +1355,16 @@ impl CliOptions { || self .maybe_config_file() .as_ref() - .map(|c| c.json.unstable.iter().any(|c| c == "byonm")) + .map(|c| c.has_unstable("byonm")) + .unwrap_or(false) + } + + pub fn unstable_sloppy_imports(&self) -> bool { + self.flags.unstable_sloppy_imports + || self + .maybe_config_file() + .as_ref() + .map(|c| c.has_unstable("sloppy-imports")) .unwrap_or(false) } diff --git a/cli/factory.rs b/cli/factory.rs index 78f8f5b866..e4b9c1da85 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -39,6 +39,7 @@ use crate::npm::CliNpmResolverManagedPackageJsonInstallerOption; use crate::npm::CliNpmResolverManagedSnapshotOption; use crate::resolver::CliGraphResolver; use crate::resolver::CliGraphResolverOptions; +use crate::resolver::UnstableSloppyImportsResolver; use crate::standalone::DenoCompileBinaryWriter; use crate::tools::check::TypeChecker; use crate::util::file_watcher::WatcherCommunicator; @@ -381,6 +382,11 @@ impl CliFactory { Ok(Arc::new(CliGraphResolver::new(CliGraphResolverOptions { fs: self.fs().clone(), cjs_resolutions: Some(self.cjs_resolutions().clone()), + sloppy_imports_resolver: if self.options.unstable_sloppy_imports() { + Some(UnstableSloppyImportsResolver::new(self.fs().clone())) + } else { + None + }, node_resolver: Some(self.node_resolver().await?.clone()), npm_resolver: if self.options.no_npm() { None diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 445f269e31..2e7766b9f0 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -857,7 +857,7 @@ mod test { use deno_graph::ResolutionError; use deno_graph::SpecifierError; - use crate::graph_util::get_resolution_error_bare_node_specifier; + use super::*; #[test] fn import_map_node_resolution_error() { diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 1349029c32..4e4e9b3bb5 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -1097,7 +1097,10 @@ impl DenoDiagnostic { changes: Some(HashMap::from([( specifier.clone(), vec![lsp::TextEdit { - new_text: format!("\"{}\"", data.redirect), + new_text: format!( + "\"{}\"", + specifier_text_for_redirected(&data.redirect, specifier) + ), range: diagnostic.range, }], )])), @@ -1193,6 +1196,27 @@ impl DenoDiagnostic { } } +fn specifier_text_for_redirected( + redirect: &lsp::Url, + referrer: &lsp::Url, +) -> String { + if redirect.scheme() == "file" && referrer.scheme() == "file" { + // use a relative specifier when it's going to a file url + match referrer.make_relative(redirect) { + Some(relative) => { + if relative.starts_with('.') { + relative + } else { + format!("./{}", relative) + } + } + None => redirect.to_string(), + } + } else { + redirect.to_string() + } +} + fn diagnose_resolution( snapshot: &language_server::StateSnapshot, dependency_key: &str, @@ -1833,4 +1857,29 @@ let c: number = "a"; ]) ); } + + #[test] + fn test_specifier_text_for_redirected() { + #[track_caller] + fn run_test(specifier: &str, referrer: &str, expected: &str) { + let result = specifier_text_for_redirected( + &ModuleSpecifier::parse(specifier).unwrap(), + &ModuleSpecifier::parse(referrer).unwrap(), + ); + assert_eq!(result, expected); + } + + run_test("file:///a/a.ts", "file:///a/mod.ts", "./a.ts"); + run_test("file:///a/a.ts", "file:///a/sub_dir/mod.ts", "../a.ts"); + run_test( + "file:///a/sub_dir/a.ts", + "file:///a/mod.ts", + "./sub_dir/a.ts", + ); + run_test( + "https://deno.land/x/example/mod.ts", + "file:///a/sub_dir/a.ts", + "https://deno.land/x/example/mod.ts", + ); + } } diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 95e8df917b..6687d2208d 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -20,6 +20,9 @@ use crate::lsp::logging::lsp_warn; use crate::npm::CliNpmResolver; use crate::resolver::CliGraphResolver; use crate::resolver::CliGraphResolverOptions; +use crate::resolver::UnstableSloppyImportsFsEntry; +use crate::resolver::UnstableSloppyImportsResolution; +use crate::resolver::UnstableSloppyImportsResolver; use crate::util::glob; use crate::util::path::specifier_to_file_path; use crate::util::text_encoding; @@ -50,6 +53,7 @@ use indexmap::IndexMap; use lsp::Url; use once_cell::sync::Lazy; use package_json::PackageJsonDepsProvider; +use std::borrow::Cow; use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; @@ -688,12 +692,12 @@ fn recurse_dependents( } #[derive(Debug)] -struct SpecifierResolver { +struct RedirectResolver { cache: Arc, redirects: Mutex>, } -impl SpecifierResolver { +impl RedirectResolver { pub fn new(cache: Arc) -> Self { Self { cache, @@ -887,7 +891,9 @@ pub struct Documents { /// should be injected. has_injected_types_node_package: bool, /// Resolves a specifier to its final redirected to specifier. - specifier_resolver: Arc, + redirect_resolver: Arc, + /// If --unstable-sloppy-imports is enabled. + unstable_sloppy_imports: bool, } impl Documents { @@ -910,10 +916,12 @@ impl Documents { maybe_import_map: None, maybe_vendor_dir: None, bare_node_builtins_enabled: false, + sloppy_imports_resolver: None, })), npm_specifier_reqs: Default::default(), has_injected_types_node_package: false, - specifier_resolver: Arc::new(SpecifierResolver::new(cache)), + redirect_resolver: Arc::new(RedirectResolver::new(cache)), + unstable_sloppy_imports: false, } } @@ -1022,7 +1030,15 @@ impl Documents { ) -> bool { let maybe_specifier = self .get_resolver() - .resolve(specifier, referrer, ResolutionMode::Types) + .resolve( + specifier, + &deno_graph::Range { + specifier: referrer.clone(), + start: deno_graph::Position::zeroed(), + end: deno_graph::Position::zeroed(), + }, + ResolutionMode::Types, + ) .ok(); if let Some(import_specifier) = maybe_specifier { self.exists(&import_specifier) @@ -1031,16 +1047,48 @@ impl Documents { } } - pub fn resolve_redirected( + pub fn resolve_specifier( &self, specifier: &ModuleSpecifier, ) -> Option { - self.specifier_resolver.resolve(specifier) + if self.unstable_sloppy_imports && specifier.scheme() == "file" { + Some( + self + .resolve_unstable_sloppy_import(specifier) + .into_owned_specifier(), + ) + } else { + self.redirect_resolver.resolve(specifier) + } + } + + fn resolve_unstable_sloppy_import<'a>( + &self, + specifier: &'a ModuleSpecifier, + ) -> UnstableSloppyImportsResolution<'a> { + UnstableSloppyImportsResolver::resolve_with_stat_sync(specifier, |path| { + if let Ok(specifier) = ModuleSpecifier::from_file_path(path) { + if self.open_docs.contains_key(&specifier) + || self.cache.contains(&specifier) + { + return Some(UnstableSloppyImportsFsEntry::File); + } + } + path.metadata().ok().and_then(|m| { + if m.is_file() { + Some(UnstableSloppyImportsFsEntry::File) + } else if m.is_dir() { + Some(UnstableSloppyImportsFsEntry::Dir) + } else { + None + } + }) + }) } /// Return `true` if the specifier can be resolved to a document. pub fn exists(&self, specifier: &ModuleSpecifier) -> bool { - let specifier = self.specifier_resolver.resolve(specifier); + let specifier = self.resolve_specifier(specifier); if let Some(specifier) = specifier { if self.open_docs.contains_key(&specifier) { return true; @@ -1069,7 +1117,7 @@ impl Documents { ) -> Vec { self.calculate_dependents_if_dirty(); let mut dependents = HashSet::new(); - if let Some(specifier) = self.specifier_resolver.resolve(specifier) { + if let Some(specifier) = self.resolve_specifier(specifier) { recurse_dependents(&specifier, &self.dependents_map, &mut dependents); dependents.into_iter().collect() } else { @@ -1091,7 +1139,7 @@ impl Documents { /// Return a document for the specifier. pub fn get(&self, original_specifier: &ModuleSpecifier) -> Option { - let specifier = self.specifier_resolver.resolve(original_specifier)?; + let specifier = self.resolve_specifier(original_specifier)?; if let Some(document) = self.open_docs.get(&specifier) { Some(document.clone()) } else { @@ -1221,7 +1269,7 @@ impl Documents { pub fn set_cache(&mut self, cache: Arc) { // TODO update resolved dependencies? self.cache = cache.clone(); - self.specifier_resolver = Arc::new(SpecifierResolver::new(cache)); + self.redirect_resolver = Arc::new(RedirectResolver::new(cache)); self.dirty = true; } @@ -1257,6 +1305,7 @@ impl Documents { maybe_jsx_config: Option<&JsxImportSourceConfig>, maybe_vendor_dir: Option, maybe_package_json_deps: Option<&PackageJsonDeps>, + maybe_unstable_flags: Option<&Vec>, ) -> u64 { let mut hasher = FastInsecureHasher::default(); hasher.write_hashable(document_preload_limit); @@ -1272,6 +1321,7 @@ impl Documents { } hasher.write_hashable(maybe_vendor_dir); hasher.write_hashable(maybe_jsx_config); + hasher.write_hashable(maybe_unstable_flags); if let Some(package_json_deps) = &maybe_package_json_deps { // We need to ensure the hashing is deterministic so explicitly type // this in order to catch if the type of package_json_deps ever changes @@ -1307,11 +1357,13 @@ impl Documents { maybe_jsx_config.as_ref(), options.maybe_config_file.and_then(|c| c.vendor_dir_flag()), maybe_package_json_deps.as_ref(), + options.maybe_config_file.map(|c| &c.json.unstable), ); let deps_provider = Arc::new(PackageJsonDepsProvider::new(maybe_package_json_deps)); + let fs = Arc::new(RealFs); self.resolver = Arc::new(CliGraphResolver::new(CliGraphResolverOptions { - fs: Arc::new(RealFs), + fs: fs.clone(), node_resolver: options.node_resolver, npm_resolver: options.npm_resolver, cjs_resolutions: None, // only used for runtime @@ -1324,14 +1376,15 @@ impl Documents { .as_ref(), bare_node_builtins_enabled: options .maybe_config_file - .map(|config| { - config - .json - .unstable - .contains(&"bare-node-builtins".to_string()) - }) + .map(|config| config.has_unstable("bare-node-builtins")) .unwrap_or(false), + // Don't set this for the LSP because instead we'll use the OpenDocumentsLoader + // because it's much easier and we get diagnostics/quick fixes about a redirected + // specifier for free. + sloppy_imports_resolver: None, })); + self.redirect_resolver = + Arc::new(RedirectResolver::new(self.cache.clone())); self.imports = Arc::new( if let Some(Ok(imports)) = options.maybe_config_file.map(|cf| cf.to_maybe_imports()) @@ -1352,6 +1405,10 @@ impl Documents { IndexMap::new() }, ); + self.unstable_sloppy_imports = options + .maybe_config_file + .map(|c| c.has_unstable("sloppy-imports")) + .unwrap_or(false); // only refresh the dependencies if the underlying configuration has changed if self.resolver_config_hash != new_resolver_config_hash { @@ -1649,6 +1706,7 @@ fn node_resolve_npm_req_ref( pub struct OpenDocumentsGraphLoader<'a> { pub inner_loader: &'a mut dyn deno_graph::source::Loader, pub open_docs: &'a HashMap, + pub unstable_sloppy_imports: bool, } impl<'a> OpenDocumentsGraphLoader<'a> { @@ -1670,6 +1728,28 @@ impl<'a> OpenDocumentsGraphLoader<'a> { } None } + + fn resolve_unstable_sloppy_import<'b>( + &self, + specifier: &'b ModuleSpecifier, + ) -> UnstableSloppyImportsResolution<'b> { + UnstableSloppyImportsResolver::resolve_with_stat_sync(specifier, |path| { + if let Ok(specifier) = ModuleSpecifier::from_file_path(path) { + if self.open_docs.contains_key(&specifier) { + return Some(UnstableSloppyImportsFsEntry::File); + } + } + path.metadata().ok().and_then(|m| { + if m.is_file() { + Some(UnstableSloppyImportsFsEntry::File) + } else if m.is_dir() { + Some(UnstableSloppyImportsFsEntry::Dir) + } else { + None + } + }) + }) + } } impl<'a> deno_graph::source::Loader for OpenDocumentsGraphLoader<'a> { @@ -1683,9 +1763,19 @@ impl<'a> deno_graph::source::Loader for OpenDocumentsGraphLoader<'a> { is_dynamic: bool, cache_setting: deno_graph::source::CacheSetting, ) -> deno_graph::source::LoadFuture { - match self.load_from_docs(specifier) { + let specifier = if self.unstable_sloppy_imports { + self + .resolve_unstable_sloppy_import(specifier) + .into_specifier() + } else { + Cow::Borrowed(specifier) + }; + + match self.load_from_docs(&specifier) { Some(fut) => fut, - None => self.inner_loader.load(specifier, is_dynamic, cache_setting), + None => self + .inner_loader + .load(&specifier, is_dynamic, cache_setting), } } diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 1104acc9c7..92173b8ad2 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -256,6 +256,7 @@ impl LanguageServer { let mut loader = crate::lsp::documents::OpenDocumentsGraphLoader { inner_loader: &mut inner_loader, open_docs: &open_docs, + unstable_sloppy_imports: cli_options.unstable_sloppy_imports(), }; let graph = module_graph_builder .create_graph_with_loader(GraphKind::All, roots.clone(), &mut loader) @@ -1080,7 +1081,7 @@ async fn create_npm_resolver( let is_byonm = std::env::var("DENO_UNSTABLE_BYONM").as_deref() == Ok("1") || maybe_config_file .as_ref() - .map(|c| c.json.unstable.iter().any(|c| c == "byonm")) + .map(|c| c.has_unstable("byonm")) .unwrap_or(false); create_cli_npm_resolver_for_lsp(if is_byonm { CliNpmResolverCreateOptions::Byonm(CliNpmResolverByonmCreateOptions { diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 72dbcc3a8f..b4f4d52c76 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -3977,7 +3977,7 @@ fn op_script_names(state: &mut OpState) -> Vec { ); for specifier in specifiers { if seen.insert(specifier.as_str()) { - if let Some(specifier) = documents.resolve_redirected(specifier) { + if let Some(specifier) = documents.resolve_specifier(specifier) { // only include dependencies we know to exist otherwise typescript will error if documents.exists(&specifier) { result.push(specifier.to_string()); diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 8332ea4f99..92116dc7b9 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -568,7 +568,11 @@ impl ModuleLoader for CliModuleLoader { // support in REPL. This should be fixed. let resolution = self.shared.resolver.resolve( specifier, - &referrer, + &deno_graph::Range { + specifier: referrer.clone(), + start: deno_graph::Position::zeroed(), + end: deno_graph::Position::zeroed(), + }, ResolutionMode::Execution, ); diff --git a/cli/resolver.rs b/cli/resolver.rs index 44de3aa3f8..12b86632ff 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -1,10 +1,12 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use deno_ast::MediaType; use deno_core::anyhow::anyhow; use deno_core::error::AnyError; use deno_core::futures::future; use deno_core::futures::future::LocalBoxFuture; use deno_core::futures::FutureExt; +use deno_core::parking_lot::Mutex; use deno_core::ModuleSpecifier; use deno_graph::source::NpmPackageReqResolution; use deno_graph::source::NpmResolver; @@ -24,16 +26,21 @@ use deno_runtime::permissions::PermissionsContainer; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; use import_map::ImportMap; +use std::borrow::Cow; +use std::collections::HashMap; +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use crate::args::package_json::PackageJsonDeps; use crate::args::JsxImportSourceConfig; use crate::args::PackageJsonDepsProvider; +use crate::graph_util::format_range_with_colors; use crate::module_loader::CjsResolutionStore; use crate::npm::ByonmCliNpmResolver; use crate::npm::CliNpmResolver; use crate::npm::InnerCliNpmResolverRef; +use crate::util::path::specifier_to_file_path; use crate::util::sync::AtomicFlag; /// Result of checking if a specifier is mapped via @@ -110,6 +117,7 @@ impl MappedSpecifierResolver { #[derive(Debug)] pub struct CliGraphResolver { fs: Arc, + sloppy_imports_resolver: Option, mapped_specifier_resolver: MappedSpecifierResolver, maybe_default_jsx_import_source: Option, maybe_jsx_import_source_module: Option, @@ -124,6 +132,7 @@ pub struct CliGraphResolver { pub struct CliGraphResolverOptions<'a> { pub fs: Arc, pub cjs_resolutions: Option>, + pub sloppy_imports_resolver: Option, pub node_resolver: Option>, pub npm_resolver: Option>, pub package_json_deps_provider: Arc, @@ -143,6 +152,7 @@ impl CliGraphResolver { Self { fs: options.fs, cjs_resolutions: options.cjs_resolutions, + sloppy_imports_resolver: options.sloppy_imports_resolver, mapped_specifier_resolver: MappedSpecifierResolver::new( options.maybe_import_map, if is_byonm { @@ -232,7 +242,7 @@ impl Resolver for CliGraphResolver { fn resolve( &self, specifier: &str, - referrer: &ModuleSpecifier, + referrer_range: &deno_graph::Range, mode: ResolutionMode, ) -> Result { fn to_node_mode(mode: ResolutionMode) -> NodeResolutionMode { @@ -242,6 +252,7 @@ impl Resolver for CliGraphResolver { } } + let referrer = &referrer_range.specifier; let result = match self .mapped_specifier_resolver .resolve(specifier, referrer)? @@ -253,10 +264,26 @@ impl Resolver for CliGraphResolver { self.found_package_json_dep_flag.raise(); Ok(specifier) } - MappedResolution::None => deno_graph::resolve_import(specifier, referrer) - .map_err(|err| err.into()), + MappedResolution::None => { + deno_graph::resolve_import(specifier, &referrer_range.specifier) + .map_err(|err| err.into()) + } }; + // do sloppy imports resolution if enabled + let result = + if let Some(sloppy_imports_resolver) = &self.sloppy_imports_resolver { + result.map(|specifier| { + sloppy_imports_resolve( + sloppy_imports_resolver, + specifier, + referrer_range, + ) + }) + } else { + result + }; + // When the user is vendoring, don't allow them to import directly from the vendor/ directory // as it might cause them confusion or duplicate dependencies. Additionally, this folder has // special treatment in the language server so it will definitely cause issues/confusion there @@ -371,6 +398,52 @@ impl Resolver for CliGraphResolver { } } +fn sloppy_imports_resolve( + resolver: &UnstableSloppyImportsResolver, + specifier: ModuleSpecifier, + referrer_range: &deno_graph::Range, +) -> ModuleSpecifier { + let resolution = resolver.resolve(&specifier); + let hint_message = match &resolution { + UnstableSloppyImportsResolution::JsToTs(to_specifier) => { + let from_media_type = MediaType::from_specifier(&specifier); + let to_media_type = MediaType::from_specifier(to_specifier); + format!( + "update {} extension to {}", + from_media_type.as_ts_extension(), + to_media_type.as_ts_extension() + ) + } + UnstableSloppyImportsResolution::NoExtension(to_specifier) => { + let to_media_type = MediaType::from_specifier(to_specifier); + format!("add {} extension", to_media_type.as_ts_extension()) + } + UnstableSloppyImportsResolution::Directory(to_specifier) => { + let file_name = to_specifier + .path() + .rsplit_once('/') + .map(|(_, file_name)| file_name) + .unwrap_or(to_specifier.path()); + format!("specify path to {} file in directory instead", file_name) + } + UnstableSloppyImportsResolution::None(_) => return specifier, + }; + // show a warning when this happens in order to drive + // the user towards correcting these specifiers + log::warn!( + "{} Sloppy import resolution {}\n at {}", + crate::colors::yellow("Warning"), + crate::colors::gray(format!("(hint: {})", hint_message)), + if referrer_range.end == deno_graph::Position::zeroed() { + // not worth showing the range in this case + crate::colors::cyan(referrer_range.specifier.as_str()).to_string() + } else { + format_range_with_colors(referrer_range) + }, + ); + resolution.into_owned_specifier() +} + fn resolve_package_json_dep( specifier: &str, deps: &PackageJsonDeps, @@ -467,10 +540,208 @@ impl NpmResolver for CliGraphResolver { } } +#[derive(Debug)] +struct UnstableSloppyImportsStatCache { + fs: Arc, + cache: Mutex>>, +} + +impl UnstableSloppyImportsStatCache { + pub fn new(fs: Arc) -> Self { + Self { + fs, + cache: Default::default(), + } + } + + pub fn stat_sync(&self, path: &Path) -> Option { + // there will only ever be one thread in here at a + // time, so it's ok to hold the lock for so long + let mut cache = self.cache.lock(); + if let Some(entry) = cache.get(path) { + return *entry; + } + + let entry = self.fs.stat_sync(path).ok().and_then(|stat| { + if stat.is_file { + Some(UnstableSloppyImportsFsEntry::File) + } else if stat.is_directory { + Some(UnstableSloppyImportsFsEntry::Dir) + } else { + None + } + }); + cache.insert(path.to_owned(), entry); + entry + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum UnstableSloppyImportsFsEntry { + File, + Dir, +} + +#[derive(Debug, PartialEq, Eq)] +pub enum UnstableSloppyImportsResolution<'a> { + /// No sloppy resolution was found. + None(&'a ModuleSpecifier), + /// Ex. `./file.js` to `./file.ts` + JsToTs(ModuleSpecifier), + /// Ex. `./file` to `./file.ts` + NoExtension(ModuleSpecifier), + /// Ex. `./dir` to `./dir/index.ts` + Directory(ModuleSpecifier), +} + +impl<'a> UnstableSloppyImportsResolution<'a> { + pub fn into_specifier(self) -> Cow<'a, ModuleSpecifier> { + match self { + Self::None(specifier) => Cow::Borrowed(specifier), + Self::JsToTs(specifier) => Cow::Owned(specifier), + Self::NoExtension(specifier) => Cow::Owned(specifier), + Self::Directory(specifier) => Cow::Owned(specifier), + } + } + + pub fn into_owned_specifier(self) -> ModuleSpecifier { + match self { + Self::None(specifier) => specifier.clone(), + Self::JsToTs(specifier) => specifier, + Self::NoExtension(specifier) => specifier, + Self::Directory(specifier) => specifier, + } + } +} + +#[derive(Debug)] +pub struct UnstableSloppyImportsResolver { + stat_cache: UnstableSloppyImportsStatCache, +} + +impl UnstableSloppyImportsResolver { + pub fn new(fs: Arc) -> Self { + Self { + stat_cache: UnstableSloppyImportsStatCache::new(fs), + } + } + + pub fn resolve_with_stat_sync( + specifier: &ModuleSpecifier, + stat_sync: impl Fn(&Path) -> Option, + ) -> UnstableSloppyImportsResolution { + if specifier.scheme() != "file" { + return UnstableSloppyImportsResolution::None(specifier); + } + + let Ok(path) = specifier_to_file_path(specifier) else { + return UnstableSloppyImportsResolution::None(specifier); + }; + let mut is_dir_resolution = false; + let mut is_no_ext_resolution = false; + let probe_paths = match (stat_sync)(&path) { + Some(UnstableSloppyImportsFsEntry::File) => { + return UnstableSloppyImportsResolution::None(specifier); + } + Some(UnstableSloppyImportsFsEntry::Dir) => { + is_dir_resolution = true; + // try to resolve at the index file + vec![ + path.join("index.ts"), + path.join("index.js"), + path.join("index.mts"), + path.join("index.mjs"), + path.join("index.tsx"), + path.join("index.jsx"), + ] + } + None => { + let media_type = MediaType::from_specifier(specifier); + let probe_media_type_types = match media_type { + MediaType::JavaScript => vec![MediaType::TypeScript, MediaType::Tsx], + MediaType::Jsx => vec![MediaType::Tsx], + MediaType::Mjs => vec![MediaType::Mts], + MediaType::Cjs => vec![MediaType::Cts], + MediaType::TypeScript + | MediaType::Mts + | MediaType::Cts + | MediaType::Dts + | MediaType::Dmts + | MediaType::Dcts + | MediaType::Tsx + | MediaType::Json + | MediaType::Wasm + | MediaType::TsBuildInfo + | MediaType::SourceMap => { + return UnstableSloppyImportsResolution::None(specifier) + } + MediaType::Unknown => { + is_no_ext_resolution = true; + vec![ + MediaType::TypeScript, + MediaType::JavaScript, + MediaType::Tsx, + MediaType::Jsx, + MediaType::Mts, + MediaType::Mjs, + ] + } + }; + let old_path_str = path.to_string_lossy(); + let old_path_str = match media_type { + MediaType::Unknown => old_path_str, + _ => match old_path_str.strip_suffix(media_type.as_ts_extension()) { + Some(s) => Cow::Borrowed(s), + None => return UnstableSloppyImportsResolution::None(specifier), + }, + }; + probe_media_type_types + .into_iter() + .map(|media_type| { + PathBuf::from(format!( + "{}{}", + old_path_str, + media_type.as_ts_extension() + )) + }) + .collect::>() + } + }; + + for probe_path in probe_paths { + if (stat_sync)(&probe_path) == Some(UnstableSloppyImportsFsEntry::File) { + if let Ok(specifier) = ModuleSpecifier::from_file_path(probe_path) { + if is_dir_resolution { + return UnstableSloppyImportsResolution::Directory(specifier); + } else if is_no_ext_resolution { + return UnstableSloppyImportsResolution::NoExtension(specifier); + } else { + return UnstableSloppyImportsResolution::JsToTs(specifier); + } + } + } + } + + UnstableSloppyImportsResolution::None(specifier) + } + + pub fn resolve<'a>( + &self, + specifier: &'a ModuleSpecifier, + ) -> UnstableSloppyImportsResolution<'a> { + Self::resolve_with_stat_sync(specifier, |path| { + self.stat_cache.stat_sync(path) + }) + } +} + #[cfg(test)] mod test { use std::collections::BTreeMap; + use deno_runtime::deno_fs::RealFs; + use test_util::TestContext; + use super::*; #[test] @@ -532,4 +803,86 @@ mod test { // non-existent bare specifier assert_eq!(resolve("non-existent", &deps).unwrap(), None); } + + #[test] + fn test_unstable_sloppy_imports() { + fn resolve(specifier: &ModuleSpecifier) -> UnstableSloppyImportsResolution { + UnstableSloppyImportsResolver::resolve_with_stat_sync(specifier, |path| { + RealFs.stat_sync(path).ok().and_then(|stat| { + if stat.is_file { + Some(UnstableSloppyImportsFsEntry::File) + } else if stat.is_directory { + Some(UnstableSloppyImportsFsEntry::Dir) + } else { + None + } + }) + }) + } + + let context = TestContext::default(); + let temp_dir = context.temp_dir().path(); + + // scenarios like resolving ./example.js to ./example.ts + for (ext_from, ext_to) in [("js", "ts"), ("js", "tsx"), ("mjs", "mts")] { + let ts_file = temp_dir.join(format!("file.{}", ext_to)); + ts_file.write(""); + let ts_file_uri = ts_file.uri_file(); + assert_eq!( + resolve(&ts_file.uri_file()), + UnstableSloppyImportsResolution::None(&ts_file_uri), + ); + assert_eq!( + resolve( + &temp_dir + .uri_dir() + .join(&format!("file.{}", ext_from)) + .unwrap() + ), + UnstableSloppyImportsResolution::JsToTs(ts_file.uri_file()), + ); + ts_file.remove_file(); + } + + // no extension scenarios + for ext in ["js", "ts", "js", "tsx", "jsx", "mjs", "mts"] { + let file = temp_dir.join(format!("file.{}", ext)); + file.write(""); + assert_eq!( + resolve( + &temp_dir + .uri_dir() + .join("file") // no ext + .unwrap() + ), + UnstableSloppyImportsResolution::NoExtension(file.uri_file()), + ); + file.remove_file(); + } + + // .ts and .js exists, .js specified (goes to specified) + { + let ts_file = temp_dir.join("file.ts"); + ts_file.write(""); + let js_file = temp_dir.join("file.js"); + js_file.write(""); + let js_file_uri = js_file.uri_file(); + assert_eq!( + resolve(&js_file.uri_file()), + UnstableSloppyImportsResolution::None(&js_file_uri), + ); + } + + // resolving a directory to an index file + { + let routes_dir = temp_dir.join("routes"); + routes_dir.create_dir_all(); + let index_file = routes_dir.join("index.ts"); + index_file.write(""); + assert_eq!( + resolve(&routes_dir.uri_file()), + UnstableSloppyImportsResolution::Directory(index_file.uri_file()), + ); + } + } } diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 6dfb0a29a4..711d0bcd88 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -1095,8 +1095,7 @@ fn lsp_import_attributes() { "only": ["quickfix"] } }), - ) - ; + ); assert_eq!( res, json!([{ @@ -10506,3 +10505,166 @@ fn lsp_jupyter_byonm_diagnostics() { ); client.shutdown(); } + +#[test] +fn lsp_sloppy_imports_warn() { + let context = TestContextBuilder::new().use_temp_cwd().build(); + let temp_dir = context.temp_dir(); + let temp_dir = temp_dir.path(); + temp_dir + .join("deno.json") + .write(r#"{ "unstable": ["sloppy-imports"] }"#); + // should work when exists on the fs and when doesn't + temp_dir.join("a.ts").write("export class A {}"); + let mut client = context.new_lsp_command().build(); + client.initialize(|builder| { + builder.set_root_uri(temp_dir.uri_dir()); + }); + client.did_open(json!({ + "textDocument": { + "uri": temp_dir.join("b.ts").uri_file(), + "languageId": "typescript", + "version": 1, + "text": "export class B {}", + }, + })); + let diagnostics = client.did_open(json!({ + "textDocument": { + "uri": temp_dir.join("file.ts").uri_file(), + "languageId": "typescript", + "version": 1, + "text": "import * as a from './a';\nimport * as b from './b.js';\nconsole.log(a)\nconsole.log(b);\n", + }, + })); + assert_eq!( + diagnostics.messages_with_source("deno"), + lsp::PublishDiagnosticsParams { + uri: temp_dir.join("file.ts").uri_file(), + diagnostics: vec![ + lsp::Diagnostic { + range: lsp::Range { + start: lsp::Position { + line: 0, + character: 19 + }, + end: lsp::Position { + line: 0, + character: 24 + } + }, + severity: Some(lsp::DiagnosticSeverity::INFORMATION), + code: Some(lsp::NumberOrString::String("redirect".to_string())), + source: Some("deno".to_string()), + message: format!( + "The import of \"{}\" was redirected to \"{}\".", + temp_dir.join("a").uri_file(), + temp_dir.join("a.ts").uri_file() + ), + data: Some(json!({ + "specifier": temp_dir.join("a").uri_file(), + "redirect": temp_dir.join("a.ts").uri_file() + })), + ..Default::default() + }, + lsp::Diagnostic { + range: lsp::Range { + start: lsp::Position { + line: 1, + character: 19 + }, + end: lsp::Position { + line: 1, + character: 27 + } + }, + severity: Some(lsp::DiagnosticSeverity::INFORMATION), + code: Some(lsp::NumberOrString::String("redirect".to_string())), + source: Some("deno".to_string()), + message: format!( + "The import of \"{}\" was redirected to \"{}\".", + temp_dir.join("b.js").uri_file(), + temp_dir.join("b.ts").uri_file() + ), + data: Some(json!({ + "specifier": temp_dir.join("b.js").uri_file(), + "redirect": temp_dir.join("b.ts").uri_file() + })), + ..Default::default() + } + ], + version: Some(1), + } + ); + + let res = client.write_request( + "textDocument/codeAction", + json!({ + "textDocument": { + "uri": temp_dir.join("file.ts").uri_file() + }, + "range": { + "start": { "line": 0, "character": 19 }, + "end": { "line": 0, "character": 24 } + }, + "context": { + "diagnostics": [{ + "range": { + "start": { "line": 0, "character": 19 }, + "end": { "line": 0, "character": 24 } + }, + "severity": 3, + "code": "redirect", + "source": "deno", + "message": format!( + "The import of \"{}\" was redirected to \"{}\".", + temp_dir.join("a").uri_file(), + temp_dir.join("a.ts").uri_file() + ), + "data": { + "specifier": temp_dir.join("a").uri_file(), + "redirect": temp_dir.join("a.ts").uri_file() + }, + }], + "only": ["quickfix"] + } + }), + ); + assert_eq!( + res, + json!([{ + "title": "Update specifier to its redirected specifier.", + "kind": "quickfix", + "diagnostics": [{ + "range": { + "start": { "line": 0, "character": 19 }, + "end": { "line": 0, "character": 24 } + }, + "severity": 3, + "code": "redirect", + "source": "deno", + "message": format!( + "The import of \"{}\" was redirected to \"{}\".", + temp_dir.join("a").uri_file(), + temp_dir.join("a.ts").uri_file() + ), + "data": { + "specifier": temp_dir.join("a").uri_file(), + "redirect": temp_dir.join("a.ts").uri_file() + }, + }], + "edit": { + "changes": { + temp_dir.join("file.ts").uri_file(): [{ + "range": { + "start": { "line": 0, "character": 19 }, + "end": { "line": 0, "character": 24 } + }, + "newText": "\"./a.ts\"" + }] + } + } + }]) + ); + + client.shutdown(); +} diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index b3bd98098c..4e0bbdfd20 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -4735,3 +4735,71 @@ itest!(unsafe_proto_flag { http_server: false, exit_code: 0, }); + +#[test] +fn test_unstable_sloppy_imports() { + let context = TestContextBuilder::new().use_temp_cwd().build(); + let temp_dir = context.temp_dir(); + temp_dir.write("deno.json", r#"{ "unstable": ["sloppy-imports"] }"#); + temp_dir.write("a.ts", "export class A {}"); + temp_dir.write("b.js", "export class B {}"); + temp_dir.write("c.mts", "export class C {}"); + temp_dir.write("d.mjs", "export class D {}"); + temp_dir.write("e.tsx", "export class E {}"); + temp_dir.write("f.jsx", "export class F {}"); + let dir = temp_dir.path().join("dir"); + dir.create_dir_all(); + dir.join("index.tsx").write("export class G {}"); + temp_dir.write( + "main.ts", + r#"import * as a from "./a.js"; +import * as b from "./b"; +import * as c from "./c"; +import * as d from "./d"; +import * as e from "./e"; +import * as e2 from "./e.js"; +import * as f from "./f"; +import * as g from "./dir"; +console.log(a.A); +console.log(b.B); +console.log(c.C); +console.log(d.D); +console.log(e.E); +console.log(e2.E); +console.log(f.F); +console.log(g.G); +"#, + ); + + context + .new_command() + .args("run main.ts") + .run() + .assert_matches_text( + "Warning Sloppy import resolution (hint: update .js extension to .ts) + at file:///[WILDCARD]/main.ts:1:20 +Warning Sloppy import resolution (hint: add .js extension) + at file:///[WILDCARD]/main.ts:2:20 +Warning Sloppy import resolution (hint: add .mts extension) + at file:///[WILDCARD]/main.ts:3:20 +Warning Sloppy import resolution (hint: add .mjs extension) + at file:///[WILDCARD]/main.ts:4:20 +Warning Sloppy import resolution (hint: add .tsx extension) + at file:///[WILDCARD]/main.ts:5:20 +Warning Sloppy import resolution (hint: update .js extension to .tsx) + at file:///[WILDCARD]/main.ts:6:21 +Warning Sloppy import resolution (hint: add .jsx extension) + at file:///[WILDCARD]/main.ts:7:20 +Warning Sloppy import resolution (hint: specify path to index.tsx file in directory instead) + at file:///[WILDCARD]/main.ts:8:20 +[class A] +[class B] +[class C] +[class D] +[class E] +[class E] +[class F] +[class G] +", + ); +} diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index 0a0dd66484..22dd30caf5 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -657,13 +657,18 @@ impl ReplSession { let mut collector = ImportCollector::new(); program.visit_with(&mut collector); + let referrer_range = deno_graph::Range { + specifier: self.referrer.clone(), + start: deno_graph::Position::zeroed(), + end: deno_graph::Position::zeroed(), + }; let resolved_imports = collector .imports .iter() .flat_map(|i| { self .resolver - .resolve(i, &self.referrer, ResolutionMode::Execution) + .resolve(i, &referrer_range, ResolutionMode::Execution) .ok() .or_else(|| ModuleSpecifier::parse(i).ok()) }) diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs index dd7c053c17..80dda86cc3 100644 --- a/cli/tools/vendor/build.rs +++ b/cli/tools/vendor/build.rs @@ -114,7 +114,11 @@ pub async fn build< if let Some(specifier_text) = jsx_import_source.maybe_specifier_text() { if let Ok(specifier) = resolver.resolve( &specifier_text, - &jsx_import_source.base_url, + &deno_graph::Range { + specifier: jsx_import_source.base_url.clone(), + start: deno_graph::Position::zeroed(), + end: deno_graph::Position::zeroed(), + }, ResolutionMode::Execution, ) { entry_points.push(specifier); diff --git a/cli/tools/vendor/import_map.rs b/cli/tools/vendor/import_map.rs index b5893e9f54..5d565014a7 100644 --- a/cli/tools/vendor/import_map.rs +++ b/cli/tools/vendor/import_map.rs @@ -216,7 +216,11 @@ pub fn build_import_map( if let Some(specifier_text) = jsx_import_source.maybe_specifier_text() { if let Ok(resolved_url) = resolver.resolve( &specifier_text, - &jsx_import_source.base_url, + &deno_graph::Range { + specifier: jsx_import_source.base_url.clone(), + start: deno_graph::Position::zeroed(), + end: deno_graph::Position::zeroed(), + }, ResolutionMode::Execution, ) { builder.imports.add(specifier_text, &resolved_url); diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs index 94e692a928..f013ad5835 100644 --- a/cli/tools/vendor/test.rs +++ b/cli/tools/vendor/test.rs @@ -300,6 +300,7 @@ fn build_resolver( node_resolver: None, npm_resolver: None, cjs_resolutions: None, + sloppy_imports_resolver: None, package_json_deps_provider: Default::default(), maybe_jsx_import_source_config, maybe_import_map: original_import_map.map(Arc::new),