summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNayeem Rahman <nayeemrmn99@gmail.com>2024-04-05 16:18:48 +0100
committerGitHub <noreply@github.com>2024-04-05 16:18:48 +0100
commit61f1b8e8dc20846093a8b24a8f511a09bbf09919 (patch)
tree15556a7212f9603616f046533385e6e130680ec3
parent049e703331409db8c4b4e2cc7d969f471c229df3 (diff)
fix(lsp): respect DENO_FUTURE for BYONM config (#23207)
-rw-r--r--cli/lsp/config.rs29
-rw-r--r--cli/lsp/language_server.rs34
-rw-r--r--tests/integration/lsp_tests.rs50
-rw-r--r--tests/util/server/src/builders.rs8
-rw-r--r--tests/util/server/src/lsp.rs19
5 files changed, 111 insertions, 29 deletions
diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs
index 1a707c44c..362b029e9 100644
--- a/cli/lsp/config.rs
+++ b/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;
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index 031b53e7d..17145f32c 100644
--- a/cli/lsp/language_server.rs
+++ b/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,
diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs
index 9348d625c..862de41f6 100644
--- a/tests/integration/lsp_tests.rs
+++ b/tests/integration/lsp_tests.rs
@@ -11787,6 +11787,56 @@ fn lsp_jupyter_byonm_diagnostics() {
}
#[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();
let temp_dir = context.temp_dir();
diff --git a/tests/util/server/src/builders.rs b/tests/util/server/src/builders.rs
index 8c93ceeb0..9490c4c44 100644
--- a/tests/util/server/src/builders.rs
+++ b/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>) {
diff --git a/tests/util/server/src/lsp.rs b/tests/util/server/src/lsp.rs
index cc6808390..be455b463 100644
--- a/tests/util/server/src/lsp.rs
+++ b/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 {