diff options
| -rw-r--r-- | cli/args/flags.rs | 4 | ||||
| -rw-r--r-- | cli/lsp/documents.rs | 27 | ||||
| -rw-r--r-- | cli/lsp/language_server.rs | 21 | ||||
| -rw-r--r-- | cli/lsp/logging.rs | 35 | ||||
| -rw-r--r-- | cli/lsp/repl.rs | 3 | ||||
| -rw-r--r-- | cli/lsp/tsc.rs | 4 | ||||
| -rw-r--r-- | cli/tests/integration/repl_tests.rs | 37 | ||||
| -rw-r--r-- | cli/tests/integration/run_tests.rs | 2 | ||||
| -rw-r--r-- | cli/tools/repl/session.rs | 2 | ||||
| -rw-r--r-- | test_util/src/builders.rs | 27 | ||||
| -rw-r--r-- | test_util/src/lib.rs | 24 | ||||
| -rw-r--r-- | test_util/src/pty.rs | 7 |
12 files changed, 124 insertions, 69 deletions
diff --git a/cli/args/flags.rs b/cli/args/flags.rs index f3b92b4fb..7caa12ca8 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -527,7 +527,7 @@ impl Flags { .ok() } Task(_) | Check(_) | Coverage(_) | Cache(_) | Info(_) | Eval(_) - | Test(_) | Bench(_) => std::env::current_dir().ok(), + | Test(_) | Bench(_) | Repl(_) => std::env::current_dir().ok(), _ => None, } } @@ -2245,7 +2245,7 @@ fn lock_arg() -> Arg { Arg::new("lock") .long("lock") .value_name("FILE") - .help("Check the specified lock file. + .help("Check the specified lock file. If value is not provided, defaults to \"deno.lock\" in the current working directory.") .num_args(0..=1) diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index d8a94e538..aa47faf62 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -834,8 +834,6 @@ pub struct Documents { /// A resolver that takes into account currently loaded import map and JSX /// settings. resolver: CliGraphResolver, - /// The npm package requirements found in a package.json file. - npm_package_json_reqs: Arc<Vec<NpmPackageReq>>, /// The npm package requirements found in npm specifiers. npm_specifier_reqs: Arc<Vec<NpmPackageReq>>, /// Gets if any document had a node: specifier such that a @types/node package @@ -856,7 +854,6 @@ impl Documents { resolver_config_hash: 0, imports: Default::default(), resolver: CliGraphResolver::default(), - npm_package_json_reqs: Default::default(), npm_specifier_reqs: Default::default(), has_injected_types_node_package: false, specifier_resolver: Arc::new(SpecifierResolver::new(location)), @@ -994,15 +991,9 @@ impl Documents { } /// Returns a collection of npm package requirements. - pub fn npm_package_reqs(&mut self) -> Vec<NpmPackageReq> { + pub fn npm_package_reqs(&mut self) -> Arc<Vec<NpmPackageReq>> { self.calculate_dependents_if_dirty(); - let mut reqs = Vec::with_capacity( - self.npm_package_json_reqs.len() + self.npm_specifier_reqs.len(), - ); - // resolve the package.json reqs first, then the npm specifiers - reqs.extend(self.npm_package_json_reqs.iter().cloned()); - reqs.extend(self.npm_specifier_reqs.iter().cloned()); - reqs + self.npm_specifier_reqs.clone() } /// Returns if a @types/node package was injected into the npm @@ -1206,20 +1197,6 @@ impl Documents { maybe_jsx_config.as_ref(), maybe_package_json_deps.as_ref(), ); - self.npm_package_json_reqs = Arc::new({ - match &maybe_package_json_deps { - Some(deps) => { - let mut reqs = deps - .values() - .filter_map(|r| r.as_ref().ok()) - .cloned() - .collect::<Vec<_>>(); - reqs.sort(); - reqs - } - None => Vec::new(), - } - }); let deps_installer = PackageJsonDepsInstaller::new( npm_registry_api.clone(), npm_resolution.clone(), diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 372a1489d..2d0bbd140 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -13,7 +13,6 @@ use deno_runtime::deno_node::PackageJson; use deno_runtime::deno_web::BlobStore; use import_map::ImportMap; use log::error; -use log::warn; use serde_json::from_value; use std::collections::HashMap; use std::collections::HashSet; @@ -47,6 +46,7 @@ use super::documents::Documents; use super::documents::DocumentsFilter; use super::documents::LanguageId; use super::logging::lsp_log; +use super::logging::lsp_warn; use super::lsp_custom; use super::parent_process_checker; use super::performance::Performance; @@ -675,7 +675,7 @@ impl Inner { if let Some(ignored_options) = maybe_ignored_options { // TODO(@kitsonk) turn these into diagnostics that can be sent to the // client - warn!("{}", ignored_options); + lsp_warn!("{}", ignored_options); } } @@ -1166,9 +1166,10 @@ impl Inner { LanguageId::Unknown }); if language_id == LanguageId::Unknown { - warn!( + lsp_warn!( "Unsupported language id \"{}\" received for document \"{}\".", - params.text_document.language_id, params.text_document.uri + params.text_document.language_id, + params.text_document.uri ); } let document = self.documents.open( @@ -1209,8 +1210,12 @@ impl Inner { async fn refresh_npm_specifiers(&mut self) { let package_reqs = self.documents.npm_package_reqs(); - if let Err(err) = self.npm_resolver.set_package_reqs(package_reqs).await { - warn!("Could not set npm package requirements. {:#}", err); + if let Err(err) = self + .npm_resolver + .set_package_reqs((*package_reqs).clone()) + .await + { + lsp_warn!("Could not set npm package requirements. {:#}", err); } } @@ -1463,7 +1468,7 @@ impl Inner { Ok(None) => Some(Vec::new()), Err(err) => { // TODO(lucacasonato): handle error properly - warn!("Format error: {:#}", err); + lsp_warn!("Format error: {:#}", err); None } }; @@ -2846,7 +2851,7 @@ impl tower_lsp::LanguageServer for LanguageServer { .register_capability(vec![registration]) .await { - warn!("Client errored on capabilities.\n{:#}", err); + lsp_warn!("Client errored on capabilities.\n{:#}", err); } } diff --git a/cli/lsp/logging.rs b/cli/lsp/logging.rs index 7099b08bc..8b703f2a5 100644 --- a/cli/lsp/logging.rs +++ b/cli/lsp/logging.rs @@ -6,6 +6,8 @@ use std::sync::atomic::Ordering; static LSP_DEBUG_FLAG: AtomicBool = AtomicBool::new(false); static LSP_LOG_LEVEL: AtomicUsize = AtomicUsize::new(log::Level::Info as usize); +static LSP_WARN_LEVEL: AtomicUsize = + AtomicUsize::new(log::Level::Warn as usize); pub fn set_lsp_debug_flag(value: bool) { LSP_DEBUG_FLAG.store(value, Ordering::SeqCst) @@ -15,6 +17,7 @@ pub fn lsp_debug_enabled() -> bool { LSP_DEBUG_FLAG.load(Ordering::SeqCst) } +/// Change the lsp to log at the provided level. pub fn set_lsp_log_level(level: log::Level) { LSP_LOG_LEVEL.store(level as usize, Ordering::SeqCst) } @@ -28,13 +31,40 @@ pub fn lsp_log_level() -> log::Level { } } +/// Change the lsp to warn at the provided level. +pub fn set_lsp_warn_level(level: log::Level) { + LSP_WARN_LEVEL.store(level as usize, Ordering::SeqCst) +} + +pub fn lsp_warn_level() -> log::Level { + let level = LSP_LOG_LEVEL.load(Ordering::SeqCst); + // TODO(bartlomieju): + #[allow(clippy::undocumented_unsafe_blocks)] + unsafe { + std::mem::transmute(level) + } +} + /// Use this macro to do "info" logs in the lsp code. This allows /// for downgrading these logs to another log level in the REPL. macro_rules! lsp_log { ($($arg:tt)+) => ( - let lsp_log_level = crate::lsp::logging::lsp_log_level(); + let lsp_log_level = $crate::lsp::logging::lsp_log_level(); + if lsp_log_level == log::Level::Debug { + $crate::lsp::logging::lsp_debug!($($arg)+) + } else { + log::log!(lsp_log_level, $($arg)+) + } + ) +} + +/// Use this macro to do "warn" logs in the lsp code. This allows +/// for downgrading these logs to another log level in the REPL. +macro_rules! lsp_warn { + ($($arg:tt)+) => ( + let lsp_log_level = $crate::lsp::logging::lsp_warn_level(); if lsp_log_level == log::Level::Debug { - crate::lsp::logging::lsp_debug!($($arg)+) + $crate::lsp::logging::lsp_debug!($($arg)+) } else { log::log!(lsp_log_level, $($arg)+) } @@ -51,3 +81,4 @@ macro_rules! lsp_debug { pub(super) use lsp_debug; pub(super) use lsp_log; +pub(super) use lsp_warn; diff --git a/cli/lsp/repl.rs b/cli/lsp/repl.rs index 41a3f993a..ada8b9404 100644 --- a/cli/lsp/repl.rs +++ b/cli/lsp/repl.rs @@ -53,7 +53,10 @@ pub struct ReplLanguageServer { impl ReplLanguageServer { pub async fn new_initialized() -> Result<ReplLanguageServer, AnyError> { + // downgrade info and warn lsp logging to debug super::logging::set_lsp_log_level(log::Level::Debug); + super::logging::set_lsp_warn_level(log::Level::Debug); + let language_server = super::language_server::LanguageServer::new(Client::new_for_repl()); diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 6ef9b1dc3..316436988 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -20,6 +20,7 @@ use super::urls::LspUrlMap; use super::urls::INVALID_SPECIFIER; use crate::args::TsConfig; +use crate::lsp::logging::lsp_warn; use crate::tsc; use crate::tsc::ResolveArgs; use crate::util::path::relative_specifier; @@ -43,7 +44,6 @@ use deno_core::ModuleSpecifier; use deno_core::OpState; use deno_core::RuntimeOptions; use deno_runtime::tokio_util::create_basic_runtime; -use log::warn; use once_cell::sync::Lazy; use regex::Captures; use regex::Regex; @@ -114,7 +114,7 @@ impl TsServer { } let value = request(&mut ts_runtime, state_snapshot, req, token); if tx.send(value).is_err() { - warn!("Unable to send result to client."); + lsp_warn!("Unable to send result to client."); } } }) diff --git a/cli/tests/integration/repl_tests.rs b/cli/tests/integration/repl_tests.rs index 95a747523..27a9b716c 100644 --- a/cli/tests/integration/repl_tests.rs +++ b/cli/tests/integration/repl_tests.rs @@ -5,6 +5,7 @@ use test_util::assert_contains; use test_util::assert_ends_with; use test_util::assert_not_contains; use util::TempDir; +use util::TestContextBuilder; #[test] fn pty_multiline() { @@ -884,3 +885,39 @@ fn pty_tab_indexable_props() { assert_not_contains!(output, "0", "1", "2"); }); } + +#[test] +fn package_json_uncached_no_error() { + let test_context = TestContextBuilder::for_npm() + .use_temp_cwd() + .use_http_server() + .env("RUST_BACKTRACE", "1") + .build(); + let temp_dir = test_context.temp_dir(); + temp_dir.write( + "package.json", + r#"{ + "dependencies": { + "@denotest/esm-basic": "1.0.0" + } +} +"#, + ); + test_context.new_command().with_pty(|mut console| { + console.write_line("console.log(123 + 456);"); + console.expect("579"); + assert_not_contains!( + console.all_output(), + "Could not set npm package requirements", + ); + + // should support getting the package now though + console + .write_line("import { getValue, setValue } from '@denotest/esm-basic';"); + console.expect("undefined"); + console.write_line("setValue(12 + 30);"); + console.expect("undefined"); + console.write_line("getValue()"); + console.expect("42") + }); +} diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 4504c970d..003bc59fc 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -4248,8 +4248,6 @@ itest!(permission_args_quiet { // Regression test for https://github.com/denoland/deno/issues/16772 #[test] -// todo(dsherret): getting a dns error on windows for some reason -#[cfg(unix)] fn file_fetcher_preserves_permissions() { let _guard = util::http_server(); util::with_pty(&["repl", "--quiet"], |mut console| { diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index 350b38bb9..95233de05 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -106,7 +106,7 @@ pub fn result_to_evaluation_output( match r { Ok(value) => value, Err(err) => { - EvaluationOutput::Error(format!("{} {}", colors::red("error:"), err)) + EvaluationOutput::Error(format!("{} {:#}", colors::red("error:"), err)) } } } diff --git a/test_util/src/builders.rs b/test_util/src/builders.rs index 84befb57a..4997dac2c 100644 --- a/test_util/src/builders.rs +++ b/test_util/src/builders.rs @@ -312,6 +312,14 @@ impl TestCommandBuilder { .collect::<Vec<_>>() } + fn build_envs(&self) -> HashMap<String, String> { + let mut envs = self.context.envs.clone(); + for (key, value) in &self.envs { + envs.insert(key.to_string(), value.to_string()); + } + envs + } + pub fn with_pty(&self, mut action: impl FnMut(Pty)) { if !Pty::is_supported() { return; @@ -319,11 +327,20 @@ impl TestCommandBuilder { let args = self.build_args(); let args = args.iter().map(|s| s.as_str()).collect::<Vec<_>>(); - let mut envs = self.envs.clone(); + let mut envs = self.build_envs(); if !envs.contains_key("NO_COLOR") { // set this by default for pty tests envs.insert("NO_COLOR".to_string(), "1".to_string()); } + + // note(dsherret): for some reason I need to inject the current + // environment here for the pty tests or else I get dns errors + if !self.env_clear { + for (key, value) in std::env::vars() { + envs.entry(key).or_insert(value); + } + } + action(Pty::new( &self.build_command_path(), &args, @@ -361,13 +378,7 @@ impl TestCommandBuilder { command.env_clear(); } command.env("DENO_DIR", self.context.deno_dir.path()); - command.envs({ - let mut envs = self.context.envs.clone(); - for (key, value) in &self.envs { - envs.insert(key.to_string(), value.to_string()); - } - envs - }); + command.envs(self.build_envs()); command.current_dir(cwd); command.stdin(Stdio::piped()); diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs index b38d72cd9..c844e594f 100644 --- a/test_util/src/lib.rs +++ b/test_util/src/lib.rs @@ -2166,24 +2166,12 @@ pub fn pattern_match(pattern: &str, s: &str, wildcard: &str) -> bool { t.1.is_empty() } -pub fn with_pty(deno_args: &[&str], mut action: impl FnMut(Pty)) { - if !Pty::is_supported() { - return; - } - - let deno_dir = new_deno_dir(); - let mut env_vars = std::collections::HashMap::new(); - env_vars.insert("NO_COLOR".to_string(), "1".to_string()); - env_vars.insert( - "DENO_DIR".to_string(), - deno_dir.path().to_string_lossy().to_string(), - ); - action(Pty::new( - &deno_exe_path(), - deno_args, - &testdata_path(), - Some(env_vars), - )) +pub fn with_pty(deno_args: &[&str], action: impl FnMut(Pty)) { + let context = TestContextBuilder::default().build(); + context + .new_command() + .args_vec(deno_args.iter().map(ToString::to_string).collect()) + .with_pty(action); } pub struct WrkOutput { diff --git a/test_util/src/pty.rs b/test_util/src/pty.rs index 80d06881e..2f89e481e 100644 --- a/test_util/src/pty.rs +++ b/test_util/src/pty.rs @@ -1,5 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use std::borrow::Cow; use std::collections::HashMap; use std::collections::HashSet; use std::io::Read; @@ -35,7 +36,7 @@ impl Pty { read_bytes: Vec::new(), last_index: 0, }; - if args[0] == "repl" && !args.contains(&"--quiet") { + if args.is_empty() || args[0] == "repl" && !args.contains(&"--quiet") { // wait for the repl to start up before writing to it pty.expect("exit using ctrl+d, ctrl+c, or close()"); } @@ -151,6 +152,10 @@ impl Pty { }); } + pub fn all_output(&self) -> Cow<str> { + String::from_utf8_lossy(&self.read_bytes) + } + #[track_caller] fn read_until_with_advancing( &mut self, |