fix(lsp): respect DENO_FUTURE for BYONM config (#23207)

cli/lsp/config.rs

@@ -1145,6 +1145,7 @@ pub struct ConfigData {
   pub lint_options: Arc<LintOptions>,
   pub lint_rules: Arc<ConfiguredRules>,
   pub ts_config: Arc<LspTsConfig>,
+  pub byonm: bool,
   pub node_modules_dir: Option<PathBuf>,
   pub vendor_dir: Option<PathBuf>,
   pub lockfile: Option<Arc<Mutex<Lockfile>>>,
@@ -1267,8 +1268,6 @@ impl ConfigData {
     let lint_rules =
       get_configured_rules(lint_options.rules.clone(), config_file.as_ref());
     let ts_config = LspTsConfig::new(config_file.as_ref());
-    let node_modules_dir =
-      config_file.as_ref().and_then(resolve_node_modules_dir);
     let vendor_dir = config_file.as_ref().and_then(|c| c.vendor_dir_path());
 
     // Load lockfile
@@ -1327,6 +1326,23 @@ impl ConfigData {
         }
       }
     }
+    let byonm = std::env::var("DENO_UNSTABLE_BYONM").is_ok()
+      || config_file
+        .as_ref()
+        .map(|c| c.has_unstable("byonm"))
+        .unwrap_or(false)
+      || (std::env::var("DENO_FUTURE").is_ok()
+        && package_json.is_some()
+        && config_file
+          .as_ref()
+          .map(|c| c.json.node_modules_dir.is_none())
+          .unwrap_or(true));
+    if byonm {
+      lsp_log!("  Enabled 'bring your own node_modules'.");
+    }
+    let node_modules_dir = config_file
+      .as_ref()
+      .and_then(|c| resolve_node_modules_dir(c, byonm));
 
     // Load import map
     let mut import_map = None;
@@ -1427,6 +1443,7 @@ impl ConfigData {
       lint_options: Arc::new(lint_options),
       lint_rules: Arc::new(lint_rules),
       ts_config: Arc::new(ts_config),
+      byonm,
       node_modules_dir,
       vendor_dir,
       lockfile: lockfile.map(Mutex::new).map(Arc::new),
@@ -1648,7 +1665,10 @@ fn resolve_lockfile_from_config(config_file: &ConfigFile) -> Option<Lockfile> {
   resolve_lockfile_from_path(lockfile_path)
 }
 
-fn resolve_node_modules_dir(config_file: &ConfigFile) -> Option<PathBuf> {
+fn resolve_node_modules_dir(
+  config_file: &ConfigFile,
+  byonm: bool,
+) -> Option<PathBuf> {
   // For the language server, require an explicit opt-in via the
   // `nodeModulesDir: true` setting in the deno.json file. This is to
   // reduce the chance of modifying someone's node_modules directory
@@ -1657,7 +1677,8 @@ fn resolve_node_modules_dir(config_file: &ConfigFile) -> Option<PathBuf> {
   if explicitly_disabled {
     return None;
   }
-  let enabled = config_file.json.node_modules_dir == Some(true)
+  let enabled = byonm
+    || config_file.json.node_modules_dir == Some(true)
     || config_file.json.vendor == Some(true);
   if !enabled {
     return None;

cli/lsp/language_server.rs

@@ -4,7 +4,6 @@ use base64::Engine;
 use deno_ast::MediaType;
 use deno_core::anyhow::anyhow;
 use deno_core::error::AnyError;
-use deno_core::parking_lot::Mutex;
 use deno_core::resolve_url;
 use deno_core::serde_json;
 use deno_core::serde_json::json;
@@ -14,7 +13,6 @@ use deno_core::url;
 use deno_core::ModuleSpecifier;
 use deno_graph::GraphKind;
 use deno_graph::Resolution;
-use deno_lockfile::Lockfile;
 use deno_npm::NpmSystemInfo;
 use deno_runtime::deno_fs;
 use deno_runtime::deno_node::NodeResolver;
@@ -54,6 +52,7 @@ use super::client::Client;
 use super::code_lens;
 use super::completions;
 use super::config::Config;
+use super::config::ConfigData;
 use super::config::ConfigSnapshot;
 use super::config::UpdateImportsOnFileMoveEnabled;
 use super::config::WorkspaceSettings;
@@ -92,7 +91,6 @@ use crate::args::get_root_cert_store;
 use crate::args::CaData;
 use crate::args::CacheSetting;
 use crate::args::CliOptions;
-use crate::args::ConfigFile;
 use crate::args::Flags;
 use crate::cache::DenoDir;
 use crate::cache::FastInsecureHasher;
@@ -152,10 +150,9 @@ struct LspNpmConfigHash(u64);
 impl LspNpmConfigHash {
   pub fn from_inner(inner: &Inner) -> Self {
     let config_data = inner.config.tree.root_data();
-    let node_modules_dir = config_data
-      .as_ref()
-      .and_then(|d| d.node_modules_dir.as_ref());
-    let lockfile = config_data.as_ref().and_then(|d| d.lockfile.as_ref());
+    let node_modules_dir =
+      config_data.and_then(|d| d.node_modules_dir.as_ref());
+    let lockfile = config_data.and_then(|d| d.lockfile.as_ref());
     let mut hasher = FastInsecureHasher::new();
     hasher.write_hashable(node_modules_dir);
     hasher.write_hashable(&inner.maybe_global_cache_path);
@@ -792,11 +789,7 @@ impl Inner {
       &deno_dir,
       &self.initial_cwd,
       &self.http_client,
-      config_data.as_ref().and_then(|d| d.config_file.as_deref()),
-      config_data.as_ref().and_then(|d| d.lockfile.as_ref()),
-      config_data
-        .as_ref()
-        .and_then(|d| d.node_modules_dir.clone()),
+      config_data,
     )
     .await;
     let node_resolver = Arc::new(NodeResolver::new(
@@ -854,16 +847,10 @@ async fn create_npm_resolver(
   deno_dir: &DenoDir,
   initial_cwd: &Path,
   http_client: &Arc<HttpClient>,
-  maybe_config_file: Option<&ConfigFile>,
-  maybe_lockfile: Option<&Arc<Mutex<Lockfile>>>,
-  maybe_node_modules_dir_path: Option<PathBuf>,
+  config_data: Option<&ConfigData>,
 ) -> Arc<dyn CliNpmResolver> {
-  let is_byonm = std::env::var("DENO_UNSTABLE_BYONM").as_deref() == Ok("1")
-    || maybe_config_file
-      .as_ref()
-      .map(|c| c.has_unstable("byonm"))
-      .unwrap_or(false);
-  create_cli_npm_resolver_for_lsp(if is_byonm {
+  let byonm = config_data.map(|d| d.byonm).unwrap_or(false);
+  create_cli_npm_resolver_for_lsp(if byonm {
     CliNpmResolverCreateOptions::Byonm(CliNpmResolverByonmCreateOptions {
       fs: Arc::new(deno_fs::RealFs),
       root_node_modules_dir: initial_cwd.join("node_modules"),
@@ -871,7 +858,7 @@ async fn create_npm_resolver(
   } else {
     CliNpmResolverCreateOptions::Managed(CliNpmResolverManagedCreateOptions {
       http_client: http_client.clone(),
-      snapshot: match maybe_lockfile {
+      snapshot: match config_data.and_then(|d| d.lockfile.as_ref()) {
         Some(lockfile) => {
           CliNpmResolverManagedSnapshotOption::ResolveFromLockfile(
             lockfile.clone(),
@@ -890,7 +877,8 @@ async fn create_npm_resolver(
       // the user is typing.
       cache_setting: CacheSetting::Only,
       text_only_progress_bar: ProgressBar::new(ProgressBarStyle::TextOnly),
-      maybe_node_modules_path: maybe_node_modules_dir_path,
+      maybe_node_modules_path: config_data
+        .and_then(|d| d.node_modules_dir.clone()),
       // do not install while resolving in the lsp—leave that to the cache command
       package_json_installer:
         CliNpmResolverManagedPackageJsonInstallerOption::NoInstall,

tests/integration/lsp_tests.rs

@@ -11786,6 +11786,56 @@ fn lsp_jupyter_byonm_diagnostics() {
   client.shutdown();
 }
 
+#[test]
+fn lsp_deno_future_env_byonm() {
+  let context = TestContextBuilder::for_npm()
+    .env("DENO_FUTURE", "1")
+    .use_temp_cwd()
+    .build();
+  let temp_dir = context.temp_dir();
+  temp_dir.path().join("package.json").write_json(&json!({
+    "dependencies": {
+      "@denotest/esm-basic": "*",
+    },
+  }));
+  context.run_npm("install");
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+  let diagnostics = client.did_open(json!({
+    "textDocument": {
+      "uri": temp_dir.uri().join("file.ts").unwrap(),
+      "languageId": "typescript",
+      "version": 1,
+      "text": r#"
+        import "npm:chalk";
+        import "@denotest/esm-basic";
+      "#,
+    },
+  }));
+  assert_eq!(
+    json!(diagnostics.all()),
+    json!([
+      {
+        "range": {
+          "start": {
+            "line": 1,
+            "character": 15,
+          },
+          "end": {
+            "line": 1,
+            "character": 26,
+          },
+        },
+        "severity": 1,
+        "code": "resolver-error",
+        "source": "deno",
+        "message": format!("Could not find a matching package for 'npm:chalk' in '{}'. You must specify this as a package.json dependency when the node_modules folder is not managed by Deno.", temp_dir.path().join("package.json")),
+      },
+    ])
+  );
+  client.shutdown();
+}
+
 #[test]
 fn lsp_sloppy_imports_warn() {
   let context = TestContextBuilder::new().use_temp_cwd().build();

tests/util/server/src/builders.rs

@@ -292,9 +292,13 @@ impl TestContext {
   }
 
   pub fn new_lsp_command(&self) -> LspClientBuilder {
-    LspClientBuilder::new_with_dir(self.deno_dir.clone())
+    let mut builder = LspClientBuilder::new_with_dir(self.deno_dir.clone())
       .deno_exe(&self.deno_exe)
-      .set_root_dir(self.temp_dir.path().clone())
+      .set_root_dir(self.temp_dir.path().clone());
+    for (key, value) in &self.envs {
+      builder = builder.env(key, value);
+    }
+    builder
   }
 
   pub fn run_npm(&self, args: impl AsRef<str>) {

tests/util/server/src/lsp.rs

@@ -33,7 +33,10 @@ use serde::Serialize;
 use serde_json::json;
 use serde_json::to_value;
 use serde_json::Value;
+use std::collections::HashMap;
 use std::collections::HashSet;
+use std::ffi::OsStr;
+use std::ffi::OsString;
 use std::io;
 use std::io::BufRead;
 use std::io::BufReader;
@@ -465,6 +468,7 @@ pub struct LspClientBuilder {
   root_dir: PathRef,
   use_diagnostic_sync: bool,
   deno_dir: TempDir,
+  envs: HashMap<OsString, OsString>,
 }
 
 impl LspClientBuilder {
@@ -481,6 +485,7 @@ impl LspClientBuilder {
       root_dir: deno_dir.path().clone(),
       use_diagnostic_sync: true,
       deno_dir,
+      envs: Default::default(),
     }
   }
 
@@ -514,6 +519,17 @@ impl LspClientBuilder {
     self
   }
 
+  pub fn env(
+    mut self,
+    key: impl AsRef<OsStr>,
+    value: impl AsRef<OsStr>,
+  ) -> Self {
+    self
+      .envs
+      .insert(key.as_ref().to_owned(), value.as_ref().to_owned());
+    self
+  }
+
   pub fn build(&self) -> LspClient {
     self.build_result().unwrap()
   }
@@ -534,6 +550,9 @@ impl LspClientBuilder {
       .arg("lsp")
       .stdin(Stdio::piped())
       .stdout(Stdio::piped());
+    for (key, value) in &self.envs {
+      command.env(key, value);
+    }
     if self.capture_stderr {
       command.stderr(Stdio::piped());
     } else if !self.print_stderr {