fix(unstable): disable importing from the vendor directory (#20067)

Some people might get think they need to import from this directory,
which could cause confusion and duplicate dependencies. Additionally,
the `vendor` directory has special behaviour in the language server, so
importing from the folder will definitely cause confusion and issues
there.

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

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)) =

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<String>,
   maybe_jsx_import_source_module: Option<String>,
+  maybe_vendor_specifier: Option<ModuleSpecifier>,
   no_npm: bool,
   npm_registry_api: Arc<CliNpmRegistryApi>,
   npm_resolution: Arc<NpmResolution>,
@@ -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<JsxImportSourceConfig>,
+  pub maybe_import_map: Option<Arc<ImportMap>>,
+  pub maybe_vendor_dir: Option<&'a PathBuf>,
+  pub no_npm: bool,
+}
+
 impl CliGraphResolver {
   pub fn new(
-    maybe_jsx_import_source_config: Option<JsxImportSourceConfig>,
-    maybe_import_map: Option<Arc<ImportMap>>,
-    no_npm: bool,
     npm_registry_api: Arc<CliNpmRegistryApi>,
     npm_resolution: Arc<NpmResolution>,
     package_json_deps_provider: Arc<PackageJsonDepsProvider>,
     package_json_deps_installer: Arc<PackageJsonDepsInstaller>,
+    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<ModuleSpecifier, AnyError> {
     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
   }
 }
 

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();
 }

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);
 }

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<JsxImportSourceConfig>,
+  maybe_jsx_import_source_config: Option<JsxImportSourceConfig>,
   original_import_map: Option<ImportMap>,
 ) -> 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,
+    },
   )
 }
 

test_util/src/lsp.rs

@@ -998,6 +998,7 @@ impl CollectedDiagnostics {
       .unwrap()
   }
 
+  #[track_caller]
   pub fn messages_with_file_and_source(
     &self,
     specifier: &str,