diff --git a/cli/factory.rs b/cli/factory.rs index 6a99bb2da9..dbf9bd95ba 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -38,6 +38,7 @@ use crate::npm::NpmPackageFsResolver; use crate::npm::NpmResolution; use crate::npm::PackageJsonDepsInstaller; use crate::resolver::CliGraphResolver; +use crate::resolver::CliGraphResolverOptions; use crate::standalone::DenoCompileBinaryWriter; use crate::tools::check::TypeChecker; use crate::util::progress_bar::ProgressBar; @@ -424,13 +425,18 @@ impl CliFactory { .resolver .get_or_try_init_async(async { Ok(Arc::new(CliGraphResolver::new( - self.options.to_maybe_jsx_import_source_config()?, - self.maybe_import_map().await?.clone(), - self.options.no_npm(), self.npm_api()?.clone(), self.npm_resolution().await?.clone(), self.package_json_deps_provider().clone(), self.package_json_deps_installer().await?.clone(), + CliGraphResolverOptions { + maybe_jsx_import_source_config: self + .options + .to_maybe_jsx_import_source_config()?, + maybe_import_map: self.maybe_import_map().await?.clone(), + maybe_vendor_dir: self.options.vendor_dir_path(), + no_npm: self.options.no_npm(), + }, ))) }) .await diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 439cf547d6..8227d2e4c2 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -21,6 +21,7 @@ use crate::npm::CliNpmRegistryApi; use crate::npm::NpmResolution; use crate::npm::PackageJsonDepsInstaller; use crate::resolver::CliGraphResolver; +use crate::resolver::CliGraphResolverOptions; use crate::util::glob; use crate::util::path::specifier_to_file_path; use crate::util::text_encoding; @@ -1241,13 +1242,19 @@ impl Documents { Arc::new(PackageJsonDepsProvider::new(maybe_package_json_deps)); let deps_installer = Arc::new(PackageJsonDepsInstaller::no_op()); self.resolver = Arc::new(CliGraphResolver::new( - maybe_jsx_config, - options.maybe_import_map, - false, options.npm_registry_api, options.npm_resolution, deps_provider, deps_installer, + CliGraphResolverOptions { + maybe_jsx_import_source_config: maybe_jsx_config, + maybe_import_map: options.maybe_import_map, + maybe_vendor_dir: options + .maybe_config_file + .and_then(|c| c.vendor_dir_path()) + .as_ref(), + no_npm: false, + }, )); self.imports = Arc::new( if let Some(Ok(imports)) = diff --git a/cli/resolver.rs b/cli/resolver.rs index 6fa8eaabef..f78f31e8dd 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -1,6 +1,7 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use deno_core::anyhow::anyhow; +use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_core::futures::future; use deno_core::futures::future::LocalBoxFuture; @@ -16,6 +17,7 @@ use deno_npm::registry::NpmRegistryApi; use deno_runtime::deno_node::is_builtin_node_module; use deno_semver::npm::NpmPackageReq; use import_map::ImportMap; +use std::path::PathBuf; use std::sync::Arc; use crate::args::package_json::PackageJsonDeps; @@ -102,6 +104,7 @@ pub struct CliGraphResolver { mapped_specifier_resolver: MappedSpecifierResolver, maybe_default_jsx_import_source: Option, maybe_jsx_import_source_module: Option, + maybe_vendor_specifier: Option, no_npm: bool, npm_registry_api: Arc, npm_resolution: Arc, @@ -125,8 +128,9 @@ impl Default for CliGraphResolver { maybe_import_map: Default::default(), package_json_deps_provider: Default::default(), }, - maybe_default_jsx_import_source: Default::default(), - maybe_jsx_import_source_module: Default::default(), + maybe_default_jsx_import_source: None, + maybe_jsx_import_source_module: None, + maybe_vendor_specifier: None, no_npm: false, npm_registry_api, npm_resolution, @@ -137,27 +141,37 @@ impl Default for CliGraphResolver { } } +pub struct CliGraphResolverOptions<'a> { + pub maybe_jsx_import_source_config: Option, + pub maybe_import_map: Option>, + pub maybe_vendor_dir: Option<&'a PathBuf>, + pub no_npm: bool, +} + impl CliGraphResolver { pub fn new( - maybe_jsx_import_source_config: Option, - maybe_import_map: Option>, - no_npm: bool, npm_registry_api: Arc, npm_resolution: Arc, package_json_deps_provider: Arc, package_json_deps_installer: Arc, + options: CliGraphResolverOptions, ) -> Self { Self { mapped_specifier_resolver: MappedSpecifierResolver { - maybe_import_map, + maybe_import_map: options.maybe_import_map, package_json_deps_provider, }, - maybe_default_jsx_import_source: maybe_jsx_import_source_config + maybe_default_jsx_import_source: options + .maybe_jsx_import_source_config .as_ref() .and_then(|c| c.default_specifier.clone()), - maybe_jsx_import_source_module: maybe_jsx_import_source_config + maybe_jsx_import_source_module: options + .maybe_jsx_import_source_config .map(|c| c.module), - no_npm, + maybe_vendor_specifier: options + .maybe_vendor_dir + .and_then(|v| ModuleSpecifier::from_directory_path(v).ok()), + no_npm: options.no_npm, npm_registry_api, npm_resolution, package_json_deps_installer, @@ -219,7 +233,7 @@ impl Resolver for CliGraphResolver { referrer: &ModuleSpecifier, ) -> Result { use MappedResolution::*; - match self + let result = match self .mapped_specifier_resolver .resolve(specifier, referrer)? { @@ -232,7 +246,21 @@ impl Resolver for CliGraphResolver { } None => deno_graph::resolve_import(specifier, referrer) .map_err(|err| err.into()), + }; + + // 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 + // if they do this. + if let Some(vendor_specifier) = &self.maybe_vendor_specifier { + if let Ok(specifier) = &result { + if specifier.as_str().starts_with(vendor_specifier.as_str()) { + bail!("Importing from the vendor directory is not permitted. Use a remote specifier instead or disable vendoring."); + } + } } + + result } } diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index d540c5f379..d2cb368063 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -8958,5 +8958,47 @@ fn lsp_vendor_dir() { ); assert_eq!(diagnostics.all().len(), 2); + // now try doing a relative import into the vendor directory + client.write_notification( + "textDocument/didChange", + json!({ + "textDocument": { + "uri": local_file_uri, + "version": 2 + }, + "contentChanges": [ + { + "range": { + "start": { "line": 0, "character": 0 }, + "end": { "line": 2, "character": 0 }, + }, + "text": "import { returnsHi } from './vendor/subdir/mod1.ts';\nconst test: string = returnsHi();\nconsole.log(test);" + } + ] + }), + ); + + let diagnostics = client.read_diagnostics(); + + assert_eq!( + json!( + diagnostics + .messages_with_file_and_source(local_file_uri.as_str(), "deno") + .diagnostics + ), + json!([ + { + "range": { + "start": { "line": 0, "character": 26 }, + "end": { "line": 0, "character": 51 } + }, + "severity": 1, + "code": "resolver-error", + "source": "deno", + "message": "Importing from the vendor directory is not permitted. Use a remote specifier instead or disable vendoring." + } + ]), + ); + client.shutdown(); } diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 2ac6ed9851..0c39c2b724 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -4580,4 +4580,17 @@ console.log(returnsHi());"#, .args("run --vendor http://localhost:4545/subdir/CAPITALS/hello_there.ts") .run() .assert_matches_text("hello there\n"); + + // now try importing directly from the vendor folder + temp_dir.write( + "main.ts", + r#"import { returnsHi } from './vendor/http_localhost_4545/subdir/mod1.ts'; +console.log(returnsHi());"#, + ); + deno_run_cmd + .run() + .assert_matches_text("error: Importing from the vendor directory is not permitted. Use a remote specifier instead or disable vendoring. + at [WILDCARD]/main.ts:1:27 +") + .assert_exit_code(1); } diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs index 0bf6f84f39..c00cc654d7 100644 --- a/cli/tools/vendor/test.rs +++ b/cli/tools/vendor/test.rs @@ -26,6 +26,7 @@ use crate::cache::ParsedSourceCache; use crate::npm::CliNpmRegistryApi; use crate::npm::NpmResolution; use crate::resolver::CliGraphResolver; +use crate::resolver::CliGraphResolverOptions; use super::build::VendorEnvironment; @@ -290,7 +291,7 @@ impl VendorTestBuilder { } fn build_resolver( - jsx_import_source_config: Option, + maybe_jsx_import_source_config: Option, original_import_map: Option, ) -> CliGraphResolver { let npm_registry_api = Arc::new(CliNpmRegistryApi::new_uninitialized()); @@ -300,13 +301,16 @@ fn build_resolver( None, )); CliGraphResolver::new( - jsx_import_source_config, - original_import_map.map(Arc::new), - false, npm_registry_api, npm_resolution, Default::default(), Default::default(), + CliGraphResolverOptions { + maybe_jsx_import_source_config, + maybe_import_map: original_import_map.map(Arc::new), + maybe_vendor_dir: None, + no_npm: false, + }, ) } diff --git a/test_util/src/lsp.rs b/test_util/src/lsp.rs index 949dd25d61..8a1f24b328 100644 --- a/test_util/src/lsp.rs +++ b/test_util/src/lsp.rs @@ -998,6 +998,7 @@ impl CollectedDiagnostics { .unwrap() } + #[track_caller] pub fn messages_with_file_and_source( &self, specifier: &str,