From 641523d55ebeb71fd09d6961dbcae39f7f4b0322 Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Mon, 9 Sep 2024 15:48:16 +0300 Subject: [PATCH 01/25] fix --- Cargo.lock | 1 + cli/args/flags.rs | 9 ++-- cli/graph_util.rs | 2 +- cli/lsp/analysis.rs | 2 +- cli/lsp/cache.rs | 2 +- cli/lsp/completions.rs | 2 +- cli/lsp/config.rs | 2 +- cli/lsp/documents.rs | 2 +- cli/lsp/language_server.rs | 85 +++++++++++++++++----------------- cli/lsp/resolver.rs | 7 ++- cli/lsp/tsc.rs | 2 +- cli/npm/byonm.rs | 2 +- cli/resolver.rs | 15 +++--- runtime/fs_util.rs | 45 ------------------ runtime/permissions/Cargo.toml | 1 + runtime/permissions/lib.rs | 65 +++++++++++++++++++++++++- tests/integration/run_tests.rs | 17 +++++++ 17 files changed, 148 insertions(+), 113 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2fd8a59a293458..215013379007e8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1924,6 +1924,7 @@ dependencies = [ "libc", "log", "once_cell", + "percent-encoding", "serde", "which 4.4.2", "winapi", diff --git a/cli/args/flags.rs b/cli/args/flags.rs index bd92d878f80a57..2e8c06b43a3e61 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -7,10 +7,13 @@ use std::net::SocketAddr; use std::num::NonZeroU32; use std::num::NonZeroU8; use std::num::NonZeroUsize; +use std::panic; use std::path::Path; use std::path::PathBuf; use std::str::FromStr; +use crate::args::resolve_no_prompt; +use crate::util::fs::canonicalize_path; use clap::builder::styling::AnsiColor; use clap::builder::FalseyValueParser; use clap::error::ErrorKind; @@ -34,15 +37,13 @@ use deno_core::url::Url; use deno_graph::GraphKind; use deno_runtime::colors; use deno_runtime::deno_permissions::parse_sys_kind; +use deno_runtime::deno_permissions::specifier_to_file_path; use deno_runtime::deno_permissions::PermissionsOptions; use log::debug; use log::Level; use serde::Deserialize; use serde::Serialize; -use crate::args::resolve_no_prompt; -use crate::util::fs::canonicalize_path; - use super::flags_net; #[derive(Clone, Debug, Default, Eq, PartialEq)] @@ -998,7 +999,7 @@ impl Flags { if module_specifier.scheme() == "file" || module_specifier.scheme() == "npm" { - if let Ok(p) = module_specifier.to_file_path() { + if let Ok(p) = specifier_to_file_path(&module_specifier) { Some(vec![p.parent().unwrap().to_path_buf()]) } else { Some(vec![current_dir.to_path_buf()]) diff --git a/cli/graph_util.rs b/cli/graph_util.rs index d73733123f72ab..f07fa9c3ccb51e 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -24,7 +24,6 @@ use deno_graph::source::LoaderChecksum; use deno_graph::JsrLoadError; use deno_graph::ModuleLoadError; use deno_graph::WorkspaceFastCheckOption; -use deno_runtime::fs_util::specifier_to_file_path; use deno_core::error::custom_error; use deno_core::error::AnyError; @@ -41,6 +40,7 @@ use deno_graph::ResolutionError; use deno_graph::SpecifierError; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node; +use deno_runtime::deno_permissions::specifier_to_file_path; use deno_runtime::deno_permissions::PermissionsContainer; use deno_semver::jsr::JsrDepPackageReq; use deno_semver::package::PackageNv; diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index eeab796bcab736..05347c1c345c72 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -11,7 +11,6 @@ use super::urls::url_to_uri; use crate::args::jsr_url; use crate::tools::lint::CliLinter; use deno_lint::diagnostic::LintDiagnosticRange; -use deno_runtime::fs_util::specifier_to_file_path; use deno_ast::SourceRange; use deno_ast::SourceRangedForSpanned; @@ -25,6 +24,7 @@ use deno_core::serde_json; use deno_core::serde_json::json; use deno_core::ModuleSpecifier; use deno_runtime::deno_node::PathClean; +use deno_runtime::deno_permissions::specifier_to_file_path; use deno_semver::jsr::JsrPackageNvReference; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; diff --git a/cli/lsp/cache.rs b/cli/lsp/cache.rs index d3c05ca91aaba4..3e8a4ade7c11ba 100644 --- a/cli/lsp/cache.rs +++ b/cli/lsp/cache.rs @@ -7,10 +7,10 @@ use crate::cache::LocalLspHttpCache; use crate::lsp::config::Config; use crate::lsp::logging::lsp_log; use crate::lsp::logging::lsp_warn; -use deno_runtime::fs_util::specifier_to_file_path; use deno_core::url::Url; use deno_core::ModuleSpecifier; +use deno_runtime::deno_permissions::specifier_to_file_path; use std::collections::BTreeMap; use std::fs; use std::path::Path; diff --git a/cli/lsp/completions.rs b/cli/lsp/completions.rs index 1e5504d75cf006..c1ee037a830779 100644 --- a/cli/lsp/completions.rs +++ b/cli/lsp/completions.rs @@ -19,7 +19,6 @@ use crate::util::path::relative_specifier; use deno_graph::source::ResolutionMode; use deno_graph::Range; use deno_runtime::deno_node::SUPPORTED_BUILTIN_NODE_MODULES; -use deno_runtime::fs_util::specifier_to_file_path; use deno_ast::LineAndColumnIndex; use deno_ast::SourceTextInfo; @@ -30,6 +29,7 @@ use deno_core::serde::Serialize; use deno_core::serde_json::json; use deno_core::url::Position; use deno_core::ModuleSpecifier; +use deno_runtime::deno_permissions::specifier_to_file_path; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::package::PackageNv; use import_map::ImportMap; diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index c9729a5e5d9b05..4b3d1e12a6bca0 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -37,8 +37,8 @@ use deno_lint::linter::LintConfig as DenoLintConfig; use deno_npm::npm_rc::ResolvedNpmRc; use deno_package_json::PackageJsonCache; use deno_runtime::deno_node::PackageJson; +use deno_runtime::deno_permissions::specifier_to_file_path; use deno_runtime::deno_permissions::PermissionsContainer; -use deno_runtime::fs_util::specifier_to_file_path; use indexmap::IndexSet; use lsp_types::ClientCapabilities; use std::collections::BTreeMap; diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index d6af96b2380daf..403c4a4b95638f 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -11,7 +11,6 @@ use super::tsc; use super::tsc::AssetDocument; use crate::graph_util::CliJsrUrlProvider; -use deno_runtime::fs_util::specifier_to_file_path; use dashmap::DashMap; use deno_ast::swc::visit::VisitWith; @@ -28,6 +27,7 @@ use deno_core::ModuleSpecifier; use deno_graph::source::ResolutionMode; use deno_graph::Resolution; use deno_runtime::deno_node; +use deno_runtime::deno_permissions::specifier_to_file_path; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 2bb13e5e4c5d74..a3e01b590c9d23 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1,47 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use base64::Engine; -use deno_ast::MediaType; -use deno_config::workspace::WorkspaceDirectory; -use deno_config::workspace::WorkspaceDiscoverOptions; -use deno_core::anyhow::anyhow; -use deno_core::error::AnyError; -use deno_core::resolve_url; -use deno_core::serde_json; -use deno_core::serde_json::json; -use deno_core::serde_json::Value; -use deno_core::unsync::spawn; -use deno_core::url; -use deno_core::url::Url; -use deno_core::ModuleSpecifier; -use deno_graph::GraphKind; -use deno_graph::Resolution; -use deno_runtime::deno_tls::rustls::RootCertStore; -use deno_runtime::deno_tls::RootCertStoreProvider; -use deno_semver::jsr::JsrPackageReqReference; -use indexmap::Equivalent; -use indexmap::IndexSet; -use log::error; -use serde::Deserialize; -use serde_json::from_value; -use std::collections::BTreeMap; -use std::collections::BTreeSet; -use std::collections::HashMap; -use std::collections::HashSet; -use std::collections::VecDeque; -use std::env; -use std::fmt::Write as _; -use std::path::PathBuf; -use std::str::FromStr; -use std::sync::Arc; -use tokio::sync::mpsc::unbounded_channel; -use tokio::sync::mpsc::UnboundedReceiver; -use tokio::sync::mpsc::UnboundedSender; -use tower_lsp::jsonrpc::Error as LspError; -use tower_lsp::jsonrpc::Result as LspResult; -use tower_lsp::lsp_types::request::*; -use tower_lsp::lsp_types::*; - use super::analysis::fix_ts_import_changes; use super::analysis::ts_changes_to_edit; use super::analysis::CodeActionCollection; @@ -113,7 +71,48 @@ use crate::util::fs::remove_dir_all_if_exists; use crate::util::path::is_importable_ext; use crate::util::path::to_percent_decoded_str; use crate::util::sync::AsyncFlag; -use deno_runtime::fs_util::specifier_to_file_path; +use base64::Engine; +use deno_ast::MediaType; +use deno_config::workspace::WorkspaceDirectory; +use deno_config::workspace::WorkspaceDiscoverOptions; +use deno_core::anyhow::anyhow; +use deno_core::error::AnyError; +use deno_core::resolve_url; +use deno_core::serde_json; +use deno_core::serde_json::json; +use deno_core::serde_json::Value; +use deno_core::unsync::spawn; +use deno_core::url; +use deno_core::url::Url; +use deno_core::ModuleSpecifier; +use deno_graph::GraphKind; +use deno_graph::Resolution; +use deno_runtime::deno_permissions::specifier_to_file_path; +use deno_runtime::deno_tls::rustls::RootCertStore; +use deno_runtime::deno_tls::RootCertStoreProvider; +use deno_semver::jsr::JsrPackageReqReference; +use indexmap::Equivalent; +use indexmap::IndexSet; +use log::error; +use serde::Deserialize; +use serde_json::from_value; +use std::collections::BTreeMap; +use std::collections::BTreeSet; +use std::collections::HashMap; +use std::collections::HashSet; +use std::collections::VecDeque; +use std::env; +use std::fmt::Write as _; +use std::path::PathBuf; +use std::str::FromStr; +use std::sync::Arc; +use tokio::sync::mpsc::unbounded_channel; +use tokio::sync::mpsc::UnboundedReceiver; +use tokio::sync::mpsc::UnboundedSender; +use tower_lsp::jsonrpc::Error as LspError; +use tower_lsp::jsonrpc::Result as LspResult; +use tower_lsp::lsp_types::request::*; +use tower_lsp::lsp_types::*; struct LspRootCertStoreProvider(RootCertStore); diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 1c3d5c88b25420..bea83db019eb54 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -1,5 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use super::cache::LspCache; +use super::jsr::JsrCacheResolver; use crate::args::create_default_npmrc; use crate::args::CacheSetting; use crate::args::CliLockfile; @@ -36,7 +38,7 @@ use deno_npm::NpmSystemInfo; use deno_runtime::deno_fs; use deno_runtime::deno_node::NodeResolver; use deno_runtime::deno_node::PackageJson; -use deno_runtime::fs_util::specifier_to_file_path; +use deno_runtime::deno_permissions::specifier_to_file_path; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageNv; @@ -53,9 +55,6 @@ use std::collections::HashMap; use std::collections::HashSet; use std::sync::Arc; -use super::cache::LspCache; -use super::jsr::JsrCacheResolver; - #[derive(Debug, Clone)] struct LspScopeResolver { graph_resolver: Arc, diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 28b1cd0b7a9a4f..a40f82d7a56e8f 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -39,7 +39,6 @@ use deno_core::convert::ToV8; use deno_core::error::StdAnyError; use deno_core::futures::stream::FuturesOrdered; use deno_core::futures::StreamExt; -use deno_runtime::fs_util::specifier_to_file_path; use dashmap::DashMap; use deno_ast::MediaType; @@ -63,6 +62,7 @@ use deno_core::ModuleSpecifier; use deno_core::OpState; use deno_core::PollEventLoopOptions; use deno_core::RuntimeOptions; +use deno_runtime::deno_permissions::specifier_to_file_path; use deno_runtime::inspector_server::InspectorServer; use deno_runtime::tokio_util::create_basic_runtime; use indexmap::IndexMap; diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs index 3249b2ed198284..fe1b1db5925f1d 100644 --- a/cli/npm/byonm.rs +++ b/cli/npm/byonm.rs @@ -16,6 +16,7 @@ use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NodeRequireResolver; use deno_runtime::deno_node::NpmProcessStateProvider; use deno_runtime::deno_node::PackageJson; +use deno_runtime::deno_permissions::specifier_to_file_path; use deno_semver::package::PackageReq; use deno_semver::Version; use node_resolver::errors::PackageFolderResolveError; @@ -28,7 +29,6 @@ use node_resolver::NpmResolver; use crate::args::NpmProcessState; use crate::args::NpmProcessStateKind; use crate::util::fs::canonicalize_path_maybe_not_exists_with_fs; -use deno_runtime::fs_util::specifier_to_file_path; use super::managed::normalize_pkg_name_for_node_modules_deno_folder; use super::CliNpmResolver; diff --git a/cli/resolver.rs b/cli/resolver.rs index 5b657b89533d33..97a1adc6c838ad 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -1,5 +1,11 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use crate::args::JsxImportSourceConfig; +use crate::args::DENO_DISABLE_PEDANTIC_NODE_WARNINGS; +use crate::node::CliNodeCodeTranslator; +use crate::npm::CliNpmResolver; +use crate::npm::InnerCliNpmResolverRef; +use crate::util::sync::AtomicFlag; use async_trait::async_trait; use dashmap::DashMap; use dashmap::DashSet; @@ -27,7 +33,7 @@ use deno_runtime::deno_fs; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::is_builtin_node_module; use deno_runtime::deno_node::NodeResolver; -use deno_runtime::fs_util::specifier_to_file_path; +use deno_runtime::deno_permissions::specifier_to_file_path; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; use node_resolver::errors::ClosestPkgJsonError; @@ -47,13 +53,6 @@ use std::path::Path; use std::path::PathBuf; use std::sync::Arc; -use crate::args::JsxImportSourceConfig; -use crate::args::DENO_DISABLE_PEDANTIC_NODE_WARNINGS; -use crate::node::CliNodeCodeTranslator; -use crate::npm::CliNpmResolver; -use crate::npm::InnerCliNpmResolverRef; -use crate::util::sync::AtomicFlag; - pub struct ModuleCodeStringSource { pub code: ModuleSourceCode, pub found_url: ModuleSpecifier, diff --git a/runtime/fs_util.rs b/runtime/fs_util.rs index fe9736038419ad..82c28ab704c934 100644 --- a/runtime/fs_util.rs +++ b/runtime/fs_util.rs @@ -1,8 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use deno_ast::ModuleSpecifier; use deno_core::anyhow::Context; -use deno_core::error::uri_error; use deno_core::error::AnyError; pub use deno_core::normalize_path; use std::path::Path; @@ -20,49 +18,6 @@ pub fn resolve_from_cwd(path: &Path) -> Result { } } -/// Attempts to convert a specifier to a file path. By default, uses the Url -/// crate's `to_file_path()` method, but falls back to try and resolve unix-style -/// paths on Windows. -pub fn specifier_to_file_path( - specifier: &ModuleSpecifier, -) -> Result { - let result = if specifier.scheme() != "file" { - Err(()) - } else if cfg!(windows) { - match specifier.to_file_path() { - Ok(path) => Ok(path), - Err(()) => { - // This might be a unix-style path which is used in the tests even on Windows. - // Attempt to see if we can convert it to a `PathBuf`. This code should be removed - // once/if https://github.com/servo/rust-url/issues/730 is implemented. - if specifier.scheme() == "file" - && specifier.host().is_none() - && specifier.port().is_none() - && specifier.path_segments().is_some() - { - let path_str = specifier.path(); - match String::from_utf8( - percent_encoding::percent_decode(path_str.as_bytes()).collect(), - ) { - Ok(path_str) => Ok(PathBuf::from(path_str)), - Err(_) => Err(()), - } - } else { - Err(()) - } - } - } - } else { - specifier.to_file_path() - }; - match result { - Ok(path) => Ok(path), - Err(()) => Err(uri_error(format!( - "Invalid file path.\n Specifier: {specifier}" - ))), - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/runtime/permissions/Cargo.toml b/runtime/permissions/Cargo.toml index b4357489651737..9821b614842618 100644 --- a/runtime/permissions/Cargo.toml +++ b/runtime/permissions/Cargo.toml @@ -20,6 +20,7 @@ fqdn = "0.3.4" libc.workspace = true log.workspace = true once_cell.workspace = true +percent-encoding = { version = "2.3.1", features = [] } serde.workspace = true which.workspace = true diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index c5cfbff7039291..01d30ae73421ac 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -1587,7 +1587,7 @@ impl Permissions { specifier: &ModuleSpecifier, ) -> Result<(), AnyError> { match specifier.scheme() { - "file" => match specifier.to_file_path() { + "file" => match specifier_to_file_path(specifier) { Ok(path) => self.read.check(&path, Some("import()")), Err(_) => Err(uri_error(format!( "Invalid file path.\n Specifier: {specifier}" @@ -1600,6 +1600,45 @@ impl Permissions { } } +/// Attempts to convert a specifier to a file path. By default, uses the Url +/// crate's `to_file_path()` method, but falls back to try and resolve unix-style +/// paths on Windows. +pub fn specifier_to_file_path( + specifier: &ModuleSpecifier, +) -> Result { + let result = if specifier.scheme() != "file" { + Err(()) + } else if cfg!(windows) { + // This might be a unix-style path which is used in the tests even on Windows. + // Attempt to see if we can convert it to a `PathBuf`. This code should be removed + // once/if https://github.com/servo/rust-url/issues/730 is implemented. + if specifier.scheme() == "file" + && specifier.host().is_none() + && specifier.port().is_none() + && specifier.path_segments().is_some() + { + let path_str = specifier.path(); + let _ = match String::from_utf8( + percent_encoding::percent_decode(path_str.as_bytes()).collect(), + ) { + Ok(path_str) => Ok(PathBuf::from(path_str)), + Err(_) => Err(()), + }; + specifier.to_file_path() + } else { + Err(()) + } + } else { + specifier.to_file_path() + }; + match result { + Ok(path) => Ok(path), + Err(()) => Err(uri_error(format!( + "Invalid file path.\n Specifier: {specifier}" + ))), + } +} + /// Wrapper struct for `Permissions` that can be shared across threads. /// /// We need a way to have internal mutability for permissions as they might get @@ -3674,4 +3713,28 @@ mod tests { assert_eq!(NetDescriptor::parse(input).ok(), *expected, "'{input}'"); } } + + #[cfg(target_os = "windows")] + #[test] + fn test_specifier_to_file_path() { + run_success_test("file:///../../tools/format.js", "../../tools/format.js"); + run_success_test("../../tools/format.js", "../../tools/format.js"); + + assert_no_panic_specifier_to_file_path("file:/"); + assert_no_panic_specifier_to_file_path("file://"); + assert_no_panic_specifier_to_file_path("file:///"); + assert_no_panic_specifier_to_file_path("file://asdf"); + + fn run_success_test(specifier: &str, expected_path: &str) { + let result = + specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()) + .unwrap(); + assert_eq!(result, PathBuf::from(expected_path)); + } + fn assert_no_panic_specifier_to_file_path(specifier: &str) { + // we just want to make sure that specifier to file path doens't panic + let _ = + specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()); + } + } } diff --git a/tests/integration/run_tests.rs b/tests/integration/run_tests.rs index 16faba43880727..e416955d1faf2e 100644 --- a/tests/integration/run_tests.rs +++ b/tests/integration/run_tests.rs @@ -5272,3 +5272,20 @@ fn emit_failed_readonly_file_system() { .run(); output.assert_matches_text("[WILDCARD]Error saving emit data ([WILDLINE]main.ts)[WILDCARD]Skipped emit cache save of [WILDLINE]other.ts[WILDCARD]hi[WILDCARD]"); } + +#[test] +fn handle_invalid_path_error() { + let deno_panicked = |output: &std::process::Output| { + String::from_utf8_lossy(&output.stderr).contains("Deno has panicked") + }; + + // this one used to panic on linux + let deno_cmd = util::deno_cmd_with_deno_dir(&util::new_deno_dir()); + let output = deno_cmd.arg("run").arg("file://asdf").output().unwrap(); + assert!(!deno_panicked(&output)); + + // this one used to panic on windows + let deno_cmd = util::deno_cmd_with_deno_dir(&util::new_deno_dir()); + let output = deno_cmd.arg("run").arg("file:/asdf").output().unwrap(); + assert!(!deno_panicked(&output)); +} From e8f7211c2141cb8ca83a1a6ed9b334ce6cd7017d Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Mon, 9 Sep 2024 16:17:35 +0300 Subject: [PATCH 02/25] fix --- runtime/fs_util.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/runtime/fs_util.rs b/runtime/fs_util.rs index 82c28ab704c934..4dd4af869a6ddc 100644 --- a/runtime/fs_util.rs +++ b/runtime/fs_util.rs @@ -20,6 +20,7 @@ pub fn resolve_from_cwd(path: &Path) -> Result { #[cfg(test)] mod tests { + use deno_core::ModuleSpecifier; use super::*; fn current_dir() -> PathBuf { @@ -82,7 +83,7 @@ mod tests { fn run_success_test(specifier: &str, expected_path: &str) { let result = - specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()) + deno_permissions::specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()) .unwrap(); assert_eq!(result, PathBuf::from(expected_path)); } From 817319e1f37a848dbafc6208a09e547a0bdc7a28 Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Mon, 9 Sep 2024 16:25:22 +0300 Subject: [PATCH 03/25] fix lint --- runtime/fs_util.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/runtime/fs_util.rs b/runtime/fs_util.rs index 4dd4af869a6ddc..7046e047231782 100644 --- a/runtime/fs_util.rs +++ b/runtime/fs_util.rs @@ -20,8 +20,8 @@ pub fn resolve_from_cwd(path: &Path) -> Result { #[cfg(test)] mod tests { - use deno_core::ModuleSpecifier; use super::*; + use deno_core::ModuleSpecifier; fn current_dir() -> PathBuf { #[allow(clippy::disallowed_methods)] @@ -82,9 +82,10 @@ mod tests { ); fn run_success_test(specifier: &str, expected_path: &str) { - let result = - deno_permissions::specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()) - .unwrap(); + let result=deno_permissions::specifier_to_file_path( + &ModuleSpecifier::parse(specifier).unwrap(), + ) + .unwrap(); assert_eq!(result, PathBuf::from(expected_path)); } } From 59987b48490d777286c3ba9ddfb61ec98f562923 Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Mon, 9 Sep 2024 16:30:26 +0300 Subject: [PATCH 04/25] fix fmt --- runtime/fs_util.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/fs_util.rs b/runtime/fs_util.rs index 7046e047231782..24f9834fdf1931 100644 --- a/runtime/fs_util.rs +++ b/runtime/fs_util.rs @@ -82,9 +82,9 @@ mod tests { ); fn run_success_test(specifier: &str, expected_path: &str) { - let result=deno_permissions::specifier_to_file_path( + let result = deno_permissions::specifier_to_file_path( &ModuleSpecifier::parse(specifier).unwrap(), - ) + ) .unwrap(); assert_eq!(result, PathBuf::from(expected_path)); } From ceea4661269db7a5f5882b179bb255afbff3e248 Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Tue, 10 Sep 2024 10:03:58 +0300 Subject: [PATCH 05/25] fix test --- runtime/permissions/lib.rs | 48 +++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index 01d30ae73421ac..365d79e6579855 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -1618,13 +1618,12 @@ pub fn specifier_to_file_path( && specifier.path_segments().is_some() { let path_str = specifier.path(); - let _ = match String::from_utf8( + match String::from_utf8( percent_encoding::percent_decode(path_str.as_bytes()).collect(), ) { - Ok(path_str) => Ok(PathBuf::from(path_str)), + Ok(_) => specifier.to_file_path(), Err(_) => Err(()), - }; - specifier.to_file_path() + } } else { Err(()) } @@ -3717,13 +3716,41 @@ mod tests { #[cfg(target_os = "windows")] #[test] fn test_specifier_to_file_path() { - run_success_test("file:///../../tools/format.js", "../../tools/format.js"); - run_success_test("../../tools/format.js", "../../tools/format.js"); + use std::env; + use std::fs; + use std::fs::File; + use std::io::Write; + use std::path::PathBuf; + + let temp_dir = match env::current_dir() { + Ok(dir) => dir, + Err(e) => { + assert!(false, "{:?}", e); + return; + } + }; + + let temp_file_path = temp_dir.join("test.txt"); + let temp_file_path_str = temp_file_path.to_str().unwrap(); + if let Err(e) = File::create(&temp_file_path) + .and_then(|mut file| writeln!(file, "Temporary test content")) + { + assert!(false, "{:?}", e); + } + run_success_test( + format!("file:/{}", temp_file_path_str).as_str(), + temp_file_path_str, + ); + + if let Err(e) = fs::remove_file(&temp_file_path) { + assert!(false, "{:?}", e); + } + + let failure_tests = vec!["file:/", "file://", "file:///", "file://asdf"]; - assert_no_panic_specifier_to_file_path("file:/"); - assert_no_panic_specifier_to_file_path("file://"); - assert_no_panic_specifier_to_file_path("file:///"); - assert_no_panic_specifier_to_file_path("file://asdf"); + for specifier in failure_tests { + assert_no_panic_specifier_to_file_path(specifier); + } fn run_success_test(specifier: &str, expected_path: &str) { let result = @@ -3732,7 +3759,6 @@ mod tests { assert_eq!(result, PathBuf::from(expected_path)); } fn assert_no_panic_specifier_to_file_path(specifier: &str) { - // we just want to make sure that specifier to file path doens't panic let _ = specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()); } From 6a7b0f081f9a4559d2f2192b7addec5e5146b8f9 Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Tue, 10 Sep 2024 11:16:44 +0300 Subject: [PATCH 06/25] fix lint --- runtime/permissions/lib.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index 365d79e6579855..c499b455520d88 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -3725,25 +3725,26 @@ mod tests { let temp_dir = match env::current_dir() { Ok(dir) => dir, Err(e) => { - assert!(false, "{:?}", e); - return; + panic!("{:?}", e); } }; - let temp_file_path = temp_dir.join("test.txt"); + #[allow(clippy::join_absolute_paths)] + let temp_file_path = temp_dir.join("/test.txt"); let temp_file_path_str = temp_file_path.to_str().unwrap(); if let Err(e) = File::create(&temp_file_path) .and_then(|mut file| writeln!(file, "Temporary test content")) { - assert!(false, "{:?}", e); + panic!("{:?}", e); } + run_success_test( format!("file:/{}", temp_file_path_str).as_str(), temp_file_path_str, ); if let Err(e) = fs::remove_file(&temp_file_path) { - assert!(false, "{:?}", e); + panic!("{:?}", e); } let failure_tests = vec!["file:/", "file://", "file:///", "file://asdf"]; From c4ec979f96ca7647648ac97a5b529be599107ddc Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Tue, 10 Sep 2024 12:39:14 +0300 Subject: [PATCH 07/25] fix test --- runtime/permissions/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index c499b455520d88..b8d0727d9f0a3b 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -3739,7 +3739,7 @@ mod tests { } run_success_test( - format!("file:/{}", temp_file_path_str).as_str(), + format!("file:///{}", temp_file_path_str).as_str(), temp_file_path_str, ); From 50ab8b3a6a5fcae8e4165e2a6125b3652e44625f Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Tue, 10 Sep 2024 14:45:46 +0300 Subject: [PATCH 08/25] fix test --- runtime/permissions/lib.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index b8d0727d9f0a3b..2b50bf6aaadfa9 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -3716,21 +3716,12 @@ mod tests { #[cfg(target_os = "windows")] #[test] fn test_specifier_to_file_path() { - use std::env; use std::fs; use std::fs::File; use std::io::Write; use std::path::PathBuf; - let temp_dir = match env::current_dir() { - Ok(dir) => dir, - Err(e) => { - panic!("{:?}", e); - } - }; - - #[allow(clippy::join_absolute_paths)] - let temp_file_path = temp_dir.join("/test.txt"); + let temp_file_path = PathBuf::from("/test.txt"); let temp_file_path_str = temp_file_path.to_str().unwrap(); if let Err(e) = File::create(&temp_file_path) .and_then(|mut file| writeln!(file, "Temporary test content")) From e8fdc89410bacf0af7b8675399ecd05955169d88 Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Tue, 10 Sep 2024 16:04:31 +0300 Subject: [PATCH 09/25] fix test --- runtime/permissions/lib.rs | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index 2b50bf6aaadfa9..047bb61345b8b4 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -3716,27 +3716,23 @@ mod tests { #[cfg(target_os = "windows")] #[test] fn test_specifier_to_file_path() { - use std::fs; + use std::env; use std::fs::File; use std::io::Write; use std::path::PathBuf; - let temp_file_path = PathBuf::from("/test.txt"); - let temp_file_path_str = temp_file_path.to_str().unwrap(); - if let Err(e) = File::create(&temp_file_path) - .and_then(|mut file| writeln!(file, "Temporary test content")) - { - panic!("{:?}", e); - } - - run_success_test( - format!("file:///{}", temp_file_path_str).as_str(), - temp_file_path_str, - ); - - if let Err(e) = fs::remove_file(&temp_file_path) { - panic!("{:?}", e); - } + let temp_dir = env::current_dir().expect("Failed to get current directory"); + let temp_file_path = temp_dir.join("temp_test_file.js"); + let mut file = + File::create(&temp_file_path).expect("Failed to create temporary file"); + writeln!(file, "console.log('Temporary test file');") + .expect("Failed to write to temporary file"); + let temp_file_path_str = temp_file_path + .to_str() + .expect("Failed to convert path to string"); + let file_url = format!("file:///{}", temp_file_path_str.replace("\\", "/")); + + run_success_test(&file_url, temp_file_path_str); let failure_tests = vec!["file:/", "file://", "file:///", "file://asdf"]; From 364d6cf943dd2cd707f10bdb367abe5ac8945bcf Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Tue, 10 Sep 2024 16:07:58 +0300 Subject: [PATCH 10/25] fix test --- runtime/permissions/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index 047bb61345b8b4..bbf0c6a10e63e9 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -3717,6 +3717,7 @@ mod tests { #[test] fn test_specifier_to_file_path() { use std::env; + use std::fs; use std::fs::File; use std::io::Write; use std::path::PathBuf; @@ -3734,6 +3735,10 @@ mod tests { run_success_test(&file_url, temp_file_path_str); + if let Err(e) = fs::remove_file(&temp_file_path) { + panic!("Failed to remove temporary file: {:?}", e); + } + let failure_tests = vec!["file:/", "file://", "file:///", "file://asdf"]; for specifier in failure_tests { From 8ee44f2a6633a4d574f9395ee960bc5d99e26dde Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Tue, 10 Sep 2024 16:32:21 +0300 Subject: [PATCH 11/25] fix test --- runtime/permissions/lib.rs | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index bbf0c6a10e63e9..6adacf38d57243 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -3716,42 +3716,8 @@ mod tests { #[cfg(target_os = "windows")] #[test] fn test_specifier_to_file_path() { - use std::env; - use std::fs; - use std::fs::File; - use std::io::Write; - use std::path::PathBuf; - - let temp_dir = env::current_dir().expect("Failed to get current directory"); - let temp_file_path = temp_dir.join("temp_test_file.js"); - let mut file = - File::create(&temp_file_path).expect("Failed to create temporary file"); - writeln!(file, "console.log('Temporary test file');") - .expect("Failed to write to temporary file"); - let temp_file_path_str = temp_file_path - .to_str() - .expect("Failed to convert path to string"); - let file_url = format!("file:///{}", temp_file_path_str.replace("\\", "/")); - - run_success_test(&file_url, temp_file_path_str); - - if let Err(e) = fs::remove_file(&temp_file_path) { - panic!("Failed to remove temporary file: {:?}", e); - } - let failure_tests = vec!["file:/", "file://", "file:///", "file://asdf"]; - for specifier in failure_tests { - assert_no_panic_specifier_to_file_path(specifier); - } - - fn run_success_test(specifier: &str, expected_path: &str) { - let result = - specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()) - .unwrap(); - assert_eq!(result, PathBuf::from(expected_path)); - } - fn assert_no_panic_specifier_to_file_path(specifier: &str) { let _ = specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()); } From 6b44a6f2add36cc033fed5652f867974af2325f3 Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Wed, 11 Sep 2024 15:11:12 +0300 Subject: [PATCH 12/25] fix test --- runtime/fs_util.rs | 20 -------------------- runtime/permissions/lib.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/runtime/fs_util.rs b/runtime/fs_util.rs index 24f9834fdf1931..f7c006a919ead6 100644 --- a/runtime/fs_util.rs +++ b/runtime/fs_util.rs @@ -21,7 +21,6 @@ pub fn resolve_from_cwd(path: &Path) -> Result { #[cfg(test)] mod tests { use super::*; - use deno_core::ModuleSpecifier; fn current_dir() -> PathBuf { #[allow(clippy::disallowed_methods)] @@ -70,23 +69,4 @@ mod tests { let absolute_expected = cwd.join(expected); assert_eq!(resolve_from_cwd(expected).unwrap(), absolute_expected); } - - #[test] - fn test_specifier_to_file_path() { - run_success_test("file:///", "/"); - run_success_test("file:///test", "/test"); - run_success_test("file:///dir/test/test.txt", "/dir/test/test.txt"); - run_success_test( - "file:///dir/test%20test/test.txt", - "/dir/test test/test.txt", - ); - - fn run_success_test(specifier: &str, expected_path: &str) { - let result = deno_permissions::specifier_to_file_path( - &ModuleSpecifier::parse(specifier).unwrap(), - ) - .unwrap(); - assert_eq!(result, PathBuf::from(expected_path)); - } - } } diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index 2eb1a837adcc7e..43c63fcb547c09 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -3713,6 +3713,40 @@ mod tests { #[cfg(target_os = "windows")] #[test] fn test_specifier_to_file_path() { + use std::env; + use std::fs; + use std::fs::File; + use std::io::Write; + use std::path::PathBuf; + + let temp_dir = match env::current_dir() { + Ok(dir) => dir, + Err(e) => { + panic!("{:?}", e); + } + }; + + #[allow(clippy::join_absolute_paths)] + let temp_file_path = temp_dir.join("test.txt"); + let temp_file_path_str = temp_file_path.to_str().unwrap(); + if let Err(e) = File::create(&temp_file_path) + .and_then(|mut file| writeln!(file, "Temporary test content")) + { + panic!("{:?}", e); + } + + let result = specifier_to_file_path( + &ModuleSpecifier::parse( + format!("file:///{}", temp_file_path_str).as_str(), + ) + .unwrap(), + ) + .unwrap(); + assert_eq!(result, PathBuf::from(temp_file_path_str)); + + if let Err(e) = fs::remove_file(&temp_file_path) { + panic!("{:?}", e); + } let failure_tests = vec!["file:/", "file://", "file:///", "file://asdf"]; for specifier in failure_tests { let _ = From 08345610a88af17cf98dea39954a5d22f17f31b6 Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Wed, 11 Sep 2024 16:19:56 +0300 Subject: [PATCH 13/25] Trigger Build From 5b71f1ba10b459e15bf300c78a53db827f7b759f Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Thu, 12 Sep 2024 09:54:43 +0300 Subject: [PATCH 14/25] fix test --- cli/lsp/config.rs | 110 ++++++++++++++++++++++++++++++++++--- runtime/permissions/lib.rs | 1 - 2 files changed, 102 insertions(+), 9 deletions(-) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 8f3d33300740fe..945724b27c8763 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -2087,9 +2087,28 @@ mod tests { #[test] fn test_config_specifier_enabled() { - let root_uri = resolve_url("file:///").unwrap(); + use std::fs::File; + use std::fs::{self}; + use std::io::Write; + use tempfile::tempdir; + + let temp_dir = tempdir().unwrap(); + let temp_dir_path = temp_dir.path(); + + let file_a_path = temp_dir_path.join("a.ts"); + + File::create(&file_a_path) + .unwrap() + .write_all(b"Temporary content") + .unwrap(); + + let root_uri = + resolve_url(&format!("file://{}/", temp_dir_path.to_str().unwrap())) + .unwrap(); let mut config = Config::new_with_roots(vec![root_uri]); - let specifier = resolve_url("file:///a.ts").unwrap(); + let specifier = + resolve_url(&format!("file://{}/a.ts", temp_dir_path.to_str().unwrap())) + .unwrap(); assert!(!config.specifier_enabled(&specifier)); config.set_workspace_settings( serde_json::from_value(json!({ @@ -2099,13 +2118,30 @@ mod tests { vec![], ); assert!(config.specifier_enabled(&specifier)); + + fs::remove_file(file_a_path).unwrap(); } #[test] fn test_config_snapshot_specifier_enabled() { - let root_uri = resolve_url("file:///").unwrap(); + use std::fs::File; + use std::fs::{self}; + use tempfile::tempdir; + + let temp_dir = tempdir().unwrap(); + let temp_dir_path = temp_dir.path(); + + let file_a_path = temp_dir_path.join("a.ts"); + + File::create(&file_a_path).unwrap(); + + let root_uri = + resolve_url(&format!("file://{}/", temp_dir_path.to_str().unwrap())) + .unwrap(); let mut config = Config::new_with_roots(vec![root_uri]); - let specifier = resolve_url("file:///a.ts").unwrap(); + let specifier = + resolve_url(&format!("file://{}/a.ts", temp_dir_path.to_str().unwrap())) + .unwrap(); assert!(!config.specifier_enabled(&specifier)); config.set_workspace_settings( serde_json::from_value(json!({ @@ -2115,14 +2151,48 @@ mod tests { vec![], ); assert!(config.specifier_enabled(&specifier)); + + fs::remove_file(file_a_path).unwrap(); } #[test] fn test_config_specifier_enabled_path() { - let root_uri = resolve_url("file:///project/").unwrap(); + use std::fs::File; + use std::fs::{self}; + use std::io::Write; + use tempfile::tempdir; + + let temp_dir = tempdir().unwrap(); + let temp_dir_path = temp_dir.path(); + + let worker_dir = temp_dir_path.join("worker"); + let other_dir = temp_dir_path.join("other"); + fs::create_dir_all(&worker_dir).unwrap(); + fs::create_dir_all(&other_dir).unwrap(); + + let file_a_path = worker_dir.join("a.ts"); + let file_b_path = other_dir.join("b.ts"); + + let mut file_a = File::create(&file_a_path).unwrap(); + writeln!(file_a, "Temporary content for a.ts").unwrap(); + + let mut file_b = File::create(&file_b_path).unwrap(); + writeln!(file_b, "Temporary content for b.ts").unwrap(); + + let root_uri = + resolve_url(&format!("file://{}/", temp_dir_path.to_str().unwrap())) + .unwrap(); let mut config = Config::new_with_roots(vec![root_uri]); - let specifier_a = resolve_url("file:///project/worker/a.ts").unwrap(); - let specifier_b = resolve_url("file:///project/other/b.ts").unwrap(); + let specifier_a = resolve_url(&format!( + "file://{}/worker/a.ts", + temp_dir_path.to_str().unwrap() + )) + .unwrap(); + let specifier_b = resolve_url(&format!( + "file://{}/other/b.ts", + temp_dir_path.to_str().unwrap() + )) + .unwrap(); assert!(!config.specifier_enabled(&specifier_a)); assert!(!config.specifier_enabled(&specifier_b)); let workspace_settings = @@ -2130,11 +2200,31 @@ mod tests { config.set_workspace_settings(workspace_settings, vec![]); assert!(config.specifier_enabled(&specifier_a)); assert!(!config.specifier_enabled(&specifier_b)); + + fs::remove_file(file_a_path).unwrap(); + fs::remove_file(file_b_path).unwrap(); } #[test] fn test_config_specifier_disabled_path() { - let root_uri = resolve_url("file:///root/").unwrap(); + use std::fs::File; + use std::fs::{self}; + use tempfile::tempdir; + + let temp_dir = tempdir().unwrap(); + let temp_dir_path = temp_dir.path(); + + let file_mod1_path = temp_dir_path.join("mod1.ts"); + let file_mod2_path = temp_dir_path.join("mod2.ts"); + let file_mod3_path = temp_dir_path.join("mod3.ts"); + + File::create(&file_mod1_path).unwrap(); + File::create(&file_mod2_path).unwrap(); + File::create(&file_mod3_path).unwrap(); + + let root_uri = + resolve_url(&format!("file://{}/", temp_dir_path.to_str().unwrap())) + .unwrap(); let mut config = Config::new_with_roots(vec![root_uri.clone()]); config.set_workspace_settings( WorkspaceSettings { @@ -2149,6 +2239,10 @@ mod tests { assert!(config.specifier_enabled(&root_uri.join("mod1.ts").unwrap())); assert!(!config.specifier_enabled(&root_uri.join("mod2.ts").unwrap())); assert!(!config.specifier_enabled(&root_uri.join("mod3.ts").unwrap())); + + fs::remove_file(file_mod1_path).unwrap(); + fs::remove_file(file_mod2_path).unwrap(); + fs::remove_file(file_mod3_path).unwrap(); } #[test] diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index 43c63fcb547c09..0c862dcfca2450 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -3710,7 +3710,6 @@ mod tests { } } - #[cfg(target_os = "windows")] #[test] fn test_specifier_to_file_path() { use std::env; From 68a5d1598039e917201506084077f965172c1102 Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Tue, 17 Sep 2024 17:01:43 +0300 Subject: [PATCH 15/25] new fix --- cli/args/flags.rs | 8 ++- cli/args/mod.rs | 11 ++++ cli/lsp/config.rs | 110 +++---------------------------------- runtime/permissions/lib.rs | 99 +++++++++++++++++---------------- 4 files changed, 74 insertions(+), 154 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index a46879352b6854..2a702ad850e1cd 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -1000,8 +1000,12 @@ impl Flags { if module_specifier.scheme() == "file" || module_specifier.scheme() == "npm" { - if let Ok(p) = specifier_to_file_path(&module_specifier) { - Some(vec![p.parent().unwrap().to_path_buf()]) + if let Ok(file_path) = specifier_to_file_path(&module_specifier) { + if let Some(parent_path) = file_path.parent() { + Some(vec![parent_path.to_path_buf()]) + } else { + Some(vec![file_path.to_path_buf()]) + } } else { Some(vec![current_dir.to_path_buf()]) } diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 7d885295e0e191..f86fa412ee872e 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -883,6 +883,17 @@ impl CliOptions { let start_dir = match &flags.config_flag { ConfigFlag::Discover => { if let Some(start_paths) = flags.config_path_args(&initial_cwd) { + for path in &start_paths { + if !path.exists() || !path.is_dir() || path == Path::new("/") { + let path_str = match &flags.subcommand { + DenoSubcommand::Run(run) => &run.script, + _ => path.to_str().unwrap_or("Invalid path"), + }; + return Err(deno_core::error::uri_error(format!( + "Invalid file path.\n Specifier: {path_str}" + ))); + } + } WorkspaceDirectory::discover( WorkspaceDiscoverStart::Paths(&start_paths), &resolve_workspace_discover_options(), diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 945724b27c8763..8f3d33300740fe 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -2087,28 +2087,9 @@ mod tests { #[test] fn test_config_specifier_enabled() { - use std::fs::File; - use std::fs::{self}; - use std::io::Write; - use tempfile::tempdir; - - let temp_dir = tempdir().unwrap(); - let temp_dir_path = temp_dir.path(); - - let file_a_path = temp_dir_path.join("a.ts"); - - File::create(&file_a_path) - .unwrap() - .write_all(b"Temporary content") - .unwrap(); - - let root_uri = - resolve_url(&format!("file://{}/", temp_dir_path.to_str().unwrap())) - .unwrap(); + let root_uri = resolve_url("file:///").unwrap(); let mut config = Config::new_with_roots(vec![root_uri]); - let specifier = - resolve_url(&format!("file://{}/a.ts", temp_dir_path.to_str().unwrap())) - .unwrap(); + let specifier = resolve_url("file:///a.ts").unwrap(); assert!(!config.specifier_enabled(&specifier)); config.set_workspace_settings( serde_json::from_value(json!({ @@ -2118,30 +2099,13 @@ mod tests { vec![], ); assert!(config.specifier_enabled(&specifier)); - - fs::remove_file(file_a_path).unwrap(); } #[test] fn test_config_snapshot_specifier_enabled() { - use std::fs::File; - use std::fs::{self}; - use tempfile::tempdir; - - let temp_dir = tempdir().unwrap(); - let temp_dir_path = temp_dir.path(); - - let file_a_path = temp_dir_path.join("a.ts"); - - File::create(&file_a_path).unwrap(); - - let root_uri = - resolve_url(&format!("file://{}/", temp_dir_path.to_str().unwrap())) - .unwrap(); + let root_uri = resolve_url("file:///").unwrap(); let mut config = Config::new_with_roots(vec![root_uri]); - let specifier = - resolve_url(&format!("file://{}/a.ts", temp_dir_path.to_str().unwrap())) - .unwrap(); + let specifier = resolve_url("file:///a.ts").unwrap(); assert!(!config.specifier_enabled(&specifier)); config.set_workspace_settings( serde_json::from_value(json!({ @@ -2151,48 +2115,14 @@ mod tests { vec![], ); assert!(config.specifier_enabled(&specifier)); - - fs::remove_file(file_a_path).unwrap(); } #[test] fn test_config_specifier_enabled_path() { - use std::fs::File; - use std::fs::{self}; - use std::io::Write; - use tempfile::tempdir; - - let temp_dir = tempdir().unwrap(); - let temp_dir_path = temp_dir.path(); - - let worker_dir = temp_dir_path.join("worker"); - let other_dir = temp_dir_path.join("other"); - fs::create_dir_all(&worker_dir).unwrap(); - fs::create_dir_all(&other_dir).unwrap(); - - let file_a_path = worker_dir.join("a.ts"); - let file_b_path = other_dir.join("b.ts"); - - let mut file_a = File::create(&file_a_path).unwrap(); - writeln!(file_a, "Temporary content for a.ts").unwrap(); - - let mut file_b = File::create(&file_b_path).unwrap(); - writeln!(file_b, "Temporary content for b.ts").unwrap(); - - let root_uri = - resolve_url(&format!("file://{}/", temp_dir_path.to_str().unwrap())) - .unwrap(); + let root_uri = resolve_url("file:///project/").unwrap(); let mut config = Config::new_with_roots(vec![root_uri]); - let specifier_a = resolve_url(&format!( - "file://{}/worker/a.ts", - temp_dir_path.to_str().unwrap() - )) - .unwrap(); - let specifier_b = resolve_url(&format!( - "file://{}/other/b.ts", - temp_dir_path.to_str().unwrap() - )) - .unwrap(); + let specifier_a = resolve_url("file:///project/worker/a.ts").unwrap(); + let specifier_b = resolve_url("file:///project/other/b.ts").unwrap(); assert!(!config.specifier_enabled(&specifier_a)); assert!(!config.specifier_enabled(&specifier_b)); let workspace_settings = @@ -2200,31 +2130,11 @@ mod tests { config.set_workspace_settings(workspace_settings, vec![]); assert!(config.specifier_enabled(&specifier_a)); assert!(!config.specifier_enabled(&specifier_b)); - - fs::remove_file(file_a_path).unwrap(); - fs::remove_file(file_b_path).unwrap(); } #[test] fn test_config_specifier_disabled_path() { - use std::fs::File; - use std::fs::{self}; - use tempfile::tempdir; - - let temp_dir = tempdir().unwrap(); - let temp_dir_path = temp_dir.path(); - - let file_mod1_path = temp_dir_path.join("mod1.ts"); - let file_mod2_path = temp_dir_path.join("mod2.ts"); - let file_mod3_path = temp_dir_path.join("mod3.ts"); - - File::create(&file_mod1_path).unwrap(); - File::create(&file_mod2_path).unwrap(); - File::create(&file_mod3_path).unwrap(); - - let root_uri = - resolve_url(&format!("file://{}/", temp_dir_path.to_str().unwrap())) - .unwrap(); + let root_uri = resolve_url("file:///root/").unwrap(); let mut config = Config::new_with_roots(vec![root_uri.clone()]); config.set_workspace_settings( WorkspaceSettings { @@ -2239,10 +2149,6 @@ mod tests { assert!(config.specifier_enabled(&root_uri.join("mod1.ts").unwrap())); assert!(!config.specifier_enabled(&root_uri.join("mod2.ts").unwrap())); assert!(!config.specifier_enabled(&root_uri.join("mod3.ts").unwrap())); - - fs::remove_file(file_mod1_path).unwrap(); - fs::remove_file(file_mod2_path).unwrap(); - fs::remove_file(file_mod3_path).unwrap(); } #[test] diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index 0c862dcfca2450..181f93dd3dc2a2 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -1609,23 +1609,31 @@ pub fn specifier_to_file_path( let result = if specifier.scheme() != "file" { Err(()) } else if cfg!(windows) { - // This might be a unix-style path which is used in the tests even on Windows. - // Attempt to see if we can convert it to a `PathBuf`. This code should be removed - // once/if https://github.com/servo/rust-url/issues/730 is implemented. - if specifier.scheme() == "file" - && specifier.host().is_none() - && specifier.port().is_none() - && specifier.path_segments().is_some() - { - let path_str = specifier.path(); - match String::from_utf8( - percent_encoding::percent_decode(path_str.as_bytes()).collect(), - ) { - Ok(_) => specifier.to_file_path(), - Err(_) => Err(()), - } - } else { + if specifier.host().is_some() { Err(()) + } else { + match specifier.to_file_path() { + Ok(path) => Ok(path), + Err(()) => { + // This might be a unix-style path which is used in the tests even on Windows. + // Attempt to see if we can convert it to a `PathBuf`. This code should be removed + // once/if https://github.com/servo/rust-url/issues/730 is implemented. + if specifier.scheme() == "file" + && specifier.port().is_none() + && specifier.path_segments().is_some() + { + let path_str = specifier.path(); + match String::from_utf8( + percent_encoding::percent_decode(path_str.as_bytes()).collect(), + ) { + Ok(path_str) => Ok(PathBuf::from(path_str)), + Err(_) => Err(()), + } + } else { + Err(()) + } + } + } } } else { specifier.to_file_path() @@ -3712,44 +3720,35 @@ mod tests { #[test] fn test_specifier_to_file_path() { - use std::env; - use std::fs; - use std::fs::File; - use std::io::Write; - use std::path::PathBuf; - - let temp_dir = match env::current_dir() { - Ok(dir) => dir, - Err(e) => { - panic!("{:?}", e); - } - }; - - #[allow(clippy::join_absolute_paths)] - let temp_file_path = temp_dir.join("test.txt"); - let temp_file_path_str = temp_file_path.to_str().unwrap(); - if let Err(e) = File::create(&temp_file_path) - .and_then(|mut file| writeln!(file, "Temporary test content")) - { - panic!("{:?}", e); - } + run_success_test("file:///", "/"); + run_success_test("file:///test", "/test"); + run_success_test("file:///dir/test/test.txt", "/dir/test/test.txt"); + run_success_test( + "file:///dir/test%20test/test.txt", + "/dir/test test/test.txt", + ); - let result = specifier_to_file_path( - &ModuleSpecifier::parse( - format!("file:///{}", temp_file_path_str).as_str(), - ) - .unwrap(), - ) - .unwrap(); - assert_eq!(result, PathBuf::from(temp_file_path_str)); + assert_no_panic_specifier_to_file_path("file:/"); + assert_no_panic_specifier_to_file_path("file://"); + assert_no_panic_specifier_to_file_path("file://asdf/"); + assert_no_panic_specifier_to_file_path("file://asdf/66666/a.ts"); - if let Err(e) = fs::remove_file(&temp_file_path) { - panic!("{:?}", e); + fn run_success_test(specifier: &str, expected_path: &str) { + let result = + specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()) + .unwrap(); + assert_eq!(result, PathBuf::from(expected_path)); } - let failure_tests = vec!["file:/", "file://", "file:///", "file://asdf"]; - for specifier in failure_tests { - let _ = + fn assert_no_panic_specifier_to_file_path(specifier: &str) { + let result = specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()); + match result { + Ok(_) => (), + Err(err) => assert_eq!( + err.to_string(), + format!("Invalid file path.\n Specifier: {specifier}") + ), + } } } } From 2415d40f74e8177ae209bfb2a464252515d8a972 Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Tue, 17 Sep 2024 17:16:36 +0300 Subject: [PATCH 16/25] resolve conflict --- runtime/permissions/lib.rs | 62 +++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index 6febf2b3d7dc60..c70ce298a7726a 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -1932,7 +1932,7 @@ impl Permissions { specifier: &ModuleSpecifier, ) -> Result<(), AnyError> { match specifier.scheme() { - "file" => match specifier_to_file_path(specifier) { + "file" => match specifier_to_file_path(specifier) { Ok(path) => self.read.check( &PathQueryDescriptor { requested: path.to_string_lossy().into_owned(), @@ -4269,37 +4269,37 @@ mod tests { } } - #[test] - fn test_specifier_to_file_path() { - run_success_test("file:///", "/"); - run_success_test("file:///test", "/test"); - run_success_test("file:///dir/test/test.txt", "/dir/test/test.txt"); - run_success_test( - "file:///dir/test%20test/test.txt", - "/dir/test test/test.txt", - ); + #[test] + fn test_specifier_to_file_path() { + run_success_test("file:///", "/"); + run_success_test("file:///test", "/test"); + run_success_test("file:///dir/test/test.txt", "/dir/test/test.txt"); + run_success_test( + "file:///dir/test%20test/test.txt", + "/dir/test test/test.txt", + ); - assert_no_panic_specifier_to_file_path("file:/"); - assert_no_panic_specifier_to_file_path("file://"); - assert_no_panic_specifier_to_file_path("file://asdf/"); - assert_no_panic_specifier_to_file_path("file://asdf/66666/a.ts"); + assert_no_panic_specifier_to_file_path("file:/"); + assert_no_panic_specifier_to_file_path("file://"); + assert_no_panic_specifier_to_file_path("file://asdf/"); + assert_no_panic_specifier_to_file_path("file://asdf/66666/a.ts"); - fn run_success_test(specifier: &str, expected_path: &str) { - let result = - specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()) - .unwrap(); - assert_eq!(result, PathBuf::from(expected_path)); - } - fn assert_no_panic_specifier_to_file_path(specifier: &str) { - let result = - specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()); - match result { - Ok(_) => (), - Err(err) => assert_eq!( - err.to_string(), - format!("Invalid file path.\n Specifier: {specifier}") - ), - } - } + fn run_success_test(specifier: &str, expected_path: &str) { + let result = + specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()) + .unwrap(); + assert_eq!(result, PathBuf::from(expected_path)); } + fn assert_no_panic_specifier_to_file_path(specifier: &str) { + let result = + specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap()); + match result { + Ok(_) => (), + Err(err) => assert_eq!( + err.to_string(), + format!("Invalid file path.\n Specifier: {specifier}") + ), + } + } + } } From bb0e521eaf3bdcf766fe2d6a7746b25ea8664861 Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Tue, 17 Sep 2024 17:26:23 +0300 Subject: [PATCH 17/25] lint --- cli/graph_util.rs | 1 - cli/lsp/config.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 2f3501ec4262c9..d50f3dc8d1101a 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -42,7 +42,6 @@ use deno_graph::SpecifierError; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node; use deno_runtime::deno_permissions::specifier_to_file_path; -use deno_runtime::deno_permissions::PermissionsContainer; use deno_semver::jsr::JsrDepPackageReq; use deno_semver::package::PackageNv; use deno_semver::Version; diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index bf6435aedae51c..0f03ea4d259d0f 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -38,7 +38,6 @@ use deno_npm::npm_rc::ResolvedNpmRc; use deno_package_json::PackageJsonCache; use deno_runtime::deno_node::PackageJson; use deno_runtime::deno_permissions::specifier_to_file_path; -use deno_runtime::deno_permissions::PermissionsContainer; use indexmap::IndexSet; use lsp_types::ClientCapabilities; use std::collections::BTreeMap; From e9aa1ae7d1750fdbdf73279abb70892adc0f87f0 Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Tue, 17 Sep 2024 17:31:56 +0300 Subject: [PATCH 18/25] Trigger Build From b24f967e72efd2f18b01af0975f0631fa7da3f49 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 17 Sep 2024 18:47:45 +0100 Subject: [PATCH 19/25] fix merge --- cli/lsp/language_server.rs | 43 +------------------------------------- 1 file changed, 1 insertion(+), 42 deletions(-) diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 448ea4c848657f..f703fa49329ef8 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -15,6 +15,7 @@ use deno_core::url::Url; use deno_core::ModuleSpecifier; use deno_graph::GraphKind; use deno_graph::Resolution; +use deno_runtime::deno_permissions::specifier_to_file_path; use deno_runtime::deno_tls::rustls::RootCertStore; use deno_runtime::deno_tls::RootCertStoreProvider; use deno_semver::jsr::JsrPackageReqReference; @@ -112,48 +113,6 @@ use crate::util::fs::remove_dir_all_if_exists; use crate::util::path::is_importable_ext; use crate::util::path::to_percent_decoded_str; use crate::util::sync::AsyncFlag; -use base64::Engine; -use deno_ast::MediaType; -use deno_config::workspace::WorkspaceDirectory; -use deno_config::workspace::WorkspaceDiscoverOptions; -use deno_core::anyhow::anyhow; -use deno_core::error::AnyError; -use deno_core::resolve_url; -use deno_core::serde_json; -use deno_core::serde_json::json; -use deno_core::serde_json::Value; -use deno_core::unsync::spawn; -use deno_core::url; -use deno_core::url::Url; -use deno_core::ModuleSpecifier; -use deno_graph::GraphKind; -use deno_graph::Resolution; -use deno_runtime::deno_permissions::specifier_to_file_path; -use deno_runtime::deno_tls::rustls::RootCertStore; -use deno_runtime::deno_tls::RootCertStoreProvider; -use deno_semver::jsr::JsrPackageReqReference; -use indexmap::Equivalent; -use indexmap::IndexSet; -use log::error; -use serde::Deserialize; -use serde_json::from_value; -use std::collections::BTreeMap; -use std::collections::BTreeSet; -use std::collections::HashMap; -use std::collections::HashSet; -use std::collections::VecDeque; -use std::env; -use std::fmt::Write as _; -use std::path::PathBuf; -use std::str::FromStr; -use std::sync::Arc; -use tokio::sync::mpsc::unbounded_channel; -use tokio::sync::mpsc::UnboundedReceiver; -use tokio::sync::mpsc::UnboundedSender; -use tower_lsp::jsonrpc::Error as LspError; -use tower_lsp::jsonrpc::Result as LspResult; -use tower_lsp::lsp_types::request::*; -use tower_lsp::lsp_types::*; struct LspRootCertStoreProvider(RootCertStore); From 4422694b10a965821accfd3a466a99f0b0eaf12d Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 17 Sep 2024 18:58:24 +0100 Subject: [PATCH 20/25] reverts --- cli/args/flags.rs | 11 +++----- cli/args/mod.rs | 11 -------- cli/graph_util.rs | 2 +- cli/lsp/analysis.rs | 2 +- cli/lsp/cache.rs | 2 +- cli/lsp/completions.rs | 2 +- cli/lsp/config.rs | 2 +- cli/lsp/documents.rs | 2 +- cli/lsp/language_server.rs | 2 +- cli/lsp/resolver.rs | 53 +++++++++++++++++++------------------- cli/lsp/tsc.rs | 2 +- cli/npm/byonm.rs | 2 +- cli/resolver.rs | 15 ++++++----- runtime/fs_util.rs | 4 ++- 14 files changed, 50 insertions(+), 62 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 62b88a68a60f55..960780067cbe91 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -7,7 +7,6 @@ use std::net::SocketAddr; use std::num::NonZeroU32; use std::num::NonZeroU8; use std::num::NonZeroUsize; -use std::panic; use std::path::Path; use std::path::PathBuf; use std::str::FromStr; @@ -36,8 +35,8 @@ use deno_core::resolve_url_or_path; use deno_core::url::Url; use deno_graph::GraphKind; use deno_runtime::deno_permissions::parse_sys_kind; -use deno_runtime::deno_permissions::specifier_to_file_path; use deno_runtime::deno_permissions::PermissionsOptions; +use deno_runtime::fs_util::specifier_to_file_path; use log::debug; use log::Level; use serde::Deserialize; @@ -917,12 +916,8 @@ impl Flags { if module_specifier.scheme() == "file" || module_specifier.scheme() == "npm" { - if let Ok(file_path) = specifier_to_file_path(&module_specifier) { - if let Some(parent_path) = file_path.parent() { - Some(vec![parent_path.to_path_buf()]) - } else { - Some(vec![file_path.to_path_buf()]) - } + if let Ok(p) = specifier_to_file_path(&module_specifier) { + Some(vec![p.parent().unwrap().to_path_buf()]) } else { Some(vec![current_dir.to_path_buf()]) } diff --git a/cli/args/mod.rs b/cli/args/mod.rs index cb00c66f3e021f..db8cf149e61429 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -884,17 +884,6 @@ impl CliOptions { let start_dir = match &flags.config_flag { ConfigFlag::Discover => { if let Some(start_paths) = flags.config_path_args(&initial_cwd) { - for path in &start_paths { - if !path.exists() || !path.is_dir() || path == Path::new("/") { - let path_str = match &flags.subcommand { - DenoSubcommand::Run(run) => &run.script, - _ => path.to_str().unwrap_or("Invalid path"), - }; - return Err(deno_core::error::uri_error(format!( - "Invalid file path.\n Specifier: {path_str}" - ))); - } - } WorkspaceDirectory::discover( WorkspaceDiscoverStart::Paths(&start_paths), &resolve_workspace_discover_options(), diff --git a/cli/graph_util.rs b/cli/graph_util.rs index d50f3dc8d1101a..cd98c3824de371 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -41,7 +41,7 @@ use deno_graph::ResolutionError; use deno_graph::SpecifierError; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node; -use deno_runtime::deno_permissions::specifier_to_file_path; +use deno_runtime::fs_util::specifier_to_file_path; use deno_semver::jsr::JsrDepPackageReq; use deno_semver::package::PackageNv; use deno_semver::Version; diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 05347c1c345c72..fee9c86cbe7ffe 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -24,7 +24,7 @@ use deno_core::serde_json; use deno_core::serde_json::json; use deno_core::ModuleSpecifier; use deno_runtime::deno_node::PathClean; -use deno_runtime::deno_permissions::specifier_to_file_path; +use deno_runtime::fs_util::specifier_to_file_path; use deno_semver::jsr::JsrPackageNvReference; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; diff --git a/cli/lsp/cache.rs b/cli/lsp/cache.rs index 3e8a4ade7c11ba..a32087842d5e04 100644 --- a/cli/lsp/cache.rs +++ b/cli/lsp/cache.rs @@ -10,7 +10,7 @@ use crate::lsp::logging::lsp_warn; use deno_core::url::Url; use deno_core::ModuleSpecifier; -use deno_runtime::deno_permissions::specifier_to_file_path; +use deno_runtime::fs_util::specifier_to_file_path; use std::collections::BTreeMap; use std::fs; use std::path::Path; diff --git a/cli/lsp/completions.rs b/cli/lsp/completions.rs index c1ee037a830779..b3d6fbbd04f655 100644 --- a/cli/lsp/completions.rs +++ b/cli/lsp/completions.rs @@ -29,7 +29,7 @@ use deno_core::serde::Serialize; use deno_core::serde_json::json; use deno_core::url::Position; use deno_core::ModuleSpecifier; -use deno_runtime::deno_permissions::specifier_to_file_path; +use deno_runtime::fs_util::specifier_to_file_path; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::package::PackageNv; use import_map::ImportMap; diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 0f03ea4d259d0f..f69cae43596b8a 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -37,7 +37,7 @@ use deno_lint::linter::LintConfig as DenoLintConfig; use deno_npm::npm_rc::ResolvedNpmRc; use deno_package_json::PackageJsonCache; use deno_runtime::deno_node::PackageJson; -use deno_runtime::deno_permissions::specifier_to_file_path; +use deno_runtime::fs_util::specifier_to_file_path; use indexmap::IndexSet; use lsp_types::ClientCapabilities; use std::collections::BTreeMap; diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 403c4a4b95638f..e96b8831bb960d 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -27,7 +27,7 @@ use deno_core::ModuleSpecifier; use deno_graph::source::ResolutionMode; use deno_graph::Resolution; use deno_runtime::deno_node; -use deno_runtime::deno_permissions::specifier_to_file_path; +use deno_runtime::fs_util::specifier_to_file_path; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index f703fa49329ef8..5f6e380823f6f6 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -15,9 +15,9 @@ use deno_core::url::Url; use deno_core::ModuleSpecifier; use deno_graph::GraphKind; use deno_graph::Resolution; -use deno_runtime::deno_permissions::specifier_to_file_path; use deno_runtime::deno_tls::rustls::RootCertStore; use deno_runtime::deno_tls::RootCertStoreProvider; +use deno_runtime::fs_util::specifier_to_file_path; use deno_semver::jsr::JsrPackageReqReference; use indexmap::Equivalent; use indexmap::IndexSet; diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index bea83db019eb54..2844eb6f98adc8 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -1,30 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use super::cache::LspCache; -use super::jsr::JsrCacheResolver; -use crate::args::create_default_npmrc; -use crate::args::CacheSetting; -use crate::args::CliLockfile; -use crate::args::NpmInstallDepsProvider; -use crate::graph_util::CliJsrUrlProvider; -use crate::http_util::HttpClientProvider; -use crate::lsp::config::Config; -use crate::lsp::config::ConfigData; -use crate::lsp::logging::lsp_warn; -use crate::npm::create_cli_npm_resolver_for_lsp; -use crate::npm::CliNpmResolver; -use crate::npm::CliNpmResolverByonmCreateOptions; -use crate::npm::CliNpmResolverCreateOptions; -use crate::npm::CliNpmResolverManagedCreateOptions; -use crate::npm::CliNpmResolverManagedSnapshotOption; -use crate::npm::ManagedCliNpmResolver; -use crate::resolver::CjsResolutionStore; -use crate::resolver::CliGraphResolver; -use crate::resolver::CliGraphResolverOptions; -use crate::resolver::CliNodeResolver; -use crate::resolver::WorkerCliNpmGraphResolver; -use crate::util::progress_bar::ProgressBar; -use crate::util::progress_bar::ProgressBarStyle; use dashmap::DashMap; use deno_ast::MediaType; use deno_cache_dir::HttpCache; @@ -38,7 +13,7 @@ use deno_npm::NpmSystemInfo; use deno_runtime::deno_fs; use deno_runtime::deno_node::NodeResolver; use deno_runtime::deno_node::PackageJson; -use deno_runtime::deno_permissions::specifier_to_file_path; +use deno_runtime::fs_util::specifier_to_file_path; use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageNv; @@ -55,6 +30,32 @@ use std::collections::HashMap; use std::collections::HashSet; use std::sync::Arc; +use super::cache::LspCache; +use super::jsr::JsrCacheResolver; +use crate::args::create_default_npmrc; +use crate::args::CacheSetting; +use crate::args::CliLockfile; +use crate::args::NpmInstallDepsProvider; +use crate::graph_util::CliJsrUrlProvider; +use crate::http_util::HttpClientProvider; +use crate::lsp::config::Config; +use crate::lsp::config::ConfigData; +use crate::lsp::logging::lsp_warn; +use crate::npm::create_cli_npm_resolver_for_lsp; +use crate::npm::CliNpmResolver; +use crate::npm::CliNpmResolverByonmCreateOptions; +use crate::npm::CliNpmResolverCreateOptions; +use crate::npm::CliNpmResolverManagedCreateOptions; +use crate::npm::CliNpmResolverManagedSnapshotOption; +use crate::npm::ManagedCliNpmResolver; +use crate::resolver::CjsResolutionStore; +use crate::resolver::CliGraphResolver; +use crate::resolver::CliGraphResolverOptions; +use crate::resolver::CliNodeResolver; +use crate::resolver::WorkerCliNpmGraphResolver; +use crate::util::progress_bar::ProgressBar; +use crate::util::progress_bar::ProgressBarStyle; + #[derive(Debug, Clone)] struct LspScopeResolver { graph_resolver: Arc, diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index ee594caa44f1c6..fe3708d3cb5ad3 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -62,7 +62,7 @@ use deno_core::ModuleSpecifier; use deno_core::OpState; use deno_core::PollEventLoopOptions; use deno_core::RuntimeOptions; -use deno_runtime::deno_permissions::specifier_to_file_path; +use deno_runtime::fs_util::specifier_to_file_path; use deno_runtime::inspector_server::InspectorServer; use deno_runtime::tokio_util::create_basic_runtime; use indexmap::IndexMap; diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs index 5c29f314b81754..fc7069e1f8179d 100644 --- a/cli/npm/byonm.rs +++ b/cli/npm/byonm.rs @@ -16,7 +16,7 @@ use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NodeRequireResolver; use deno_runtime::deno_node::NpmProcessStateProvider; use deno_runtime::deno_node::PackageJson; -use deno_runtime::deno_permissions::specifier_to_file_path; +use deno_runtime::fs_util::specifier_to_file_path; use deno_semver::package::PackageReq; use deno_semver::Version; use node_resolver::errors::PackageFolderResolveError; diff --git a/cli/resolver.rs b/cli/resolver.rs index ed9867e247e7a2..07cb6931df5960 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -1,11 +1,5 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use crate::args::JsxImportSourceConfig; -use crate::args::DENO_DISABLE_PEDANTIC_NODE_WARNINGS; -use crate::node::CliNodeCodeTranslator; -use crate::npm::CliNpmResolver; -use crate::npm::InnerCliNpmResolverRef; -use crate::util::sync::AtomicFlag; use async_trait::async_trait; use dashmap::DashMap; use dashmap::DashSet; @@ -33,7 +27,7 @@ use deno_runtime::deno_fs; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::is_builtin_node_module; use deno_runtime::deno_node::NodeResolver; -use deno_runtime::deno_permissions::specifier_to_file_path; +use deno_runtime::fs_util::specifier_to_file_path; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; use node_resolver::errors::ClosestPkgJsonError; @@ -53,6 +47,13 @@ use std::path::Path; use std::path::PathBuf; use std::sync::Arc; +use crate::args::JsxImportSourceConfig; +use crate::args::DENO_DISABLE_PEDANTIC_NODE_WARNINGS; +use crate::node::CliNodeCodeTranslator; +use crate::npm::CliNpmResolver; +use crate::npm::InnerCliNpmResolverRef; +use crate::util::sync::AtomicFlag; + pub struct ModuleCodeStringSource { pub code: ModuleSourceCode, pub found_url: ModuleSpecifier, diff --git a/runtime/fs_util.rs b/runtime/fs_util.rs index f7c006a919ead6..15ebafdf61d44e 100644 --- a/runtime/fs_util.rs +++ b/runtime/fs_util.rs @@ -2,10 +2,12 @@ use deno_core::anyhow::Context; use deno_core::error::AnyError; -pub use deno_core::normalize_path; use std::path::Path; use std::path::PathBuf; +pub use deno_core::normalize_path; +pub use deno_permissions::specifier_to_file_path; + #[inline] pub fn resolve_from_cwd(path: &Path) -> Result { if path.is_absolute() { From 2e24eafbde2f5a057380ce06d230e91de6675fcb Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 17 Sep 2024 19:03:18 +0100 Subject: [PATCH 21/25] another revert --- cli/args/flags.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 960780067cbe91..b3dfa0da1f8ae7 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -11,8 +11,6 @@ use std::path::Path; use std::path::PathBuf; use std::str::FromStr; -use crate::args::resolve_no_prompt; -use crate::util::fs::canonicalize_path; use clap::builder::styling::AnsiColor; use clap::builder::FalseyValueParser; use clap::error::ErrorKind; @@ -42,6 +40,9 @@ use log::Level; use serde::Deserialize; use serde::Serialize; +use crate::args::resolve_no_prompt; +use crate::util::fs::canonicalize_path; + use super::flags_net; #[derive(Clone, Debug, Default, Eq, PartialEq)] From 2390d3e0f42a64cc9442eed76611b88ac99eef6e Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Wed, 18 Sep 2024 12:58:16 +0300 Subject: [PATCH 22/25] fix check_invalid_specifiers test file:/// URL, it should successfully resolve to a valid file path --- runtime/permissions/lib.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index c70ce298a7726a..b5c870a077da22 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -3274,12 +3274,9 @@ mod tests { let mut test_cases = vec![]; - if cfg!(target_os = "windows") { - test_cases.push("file://"); - test_cases.push("file:///"); - } else { - test_cases.push("file://remotehost/"); - } + test_cases.push("file://dir"); + test_cases.push("file://asdf/"); + test_cases.push("file://remotehost/"); for url in test_cases { assert!(perms From 904b630e153dec19da2b3cc5a781811e3a757a5d Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Wed, 18 Sep 2024 15:03:20 +0300 Subject: [PATCH 23/25] fix handle_invalid_path_error --- tests/integration/run_tests.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/integration/run_tests.rs b/tests/integration/run_tests.rs index 54d2f6e19e2657..25133d8b398d30 100644 --- a/tests/integration/run_tests.rs +++ b/tests/integration/run_tests.rs @@ -5128,17 +5128,19 @@ fn emit_failed_readonly_file_system() { #[test] fn handle_invalid_path_error() { - let deno_panicked = |output: &std::process::Output| { - String::from_utf8_lossy(&output.stderr).contains("Deno has panicked") - }; - - // this one used to panic on linux let deno_cmd = util::deno_cmd_with_deno_dir(&util::new_deno_dir()); let output = deno_cmd.arg("run").arg("file://asdf").output().unwrap(); - assert!(!deno_panicked(&output)); + assert!(String::from_utf8_lossy(&output.stderr).contains("Invalid file path.")); + + let deno_cmd = util::deno_cmd_with_deno_dir(&util::new_deno_dir()); + let output = deno_cmd.arg("run").arg("/a/b").output().unwrap(); + assert!(String::from_utf8_lossy(&output.stderr).contains("Module not found")); + + let deno_cmd = util::deno_cmd_with_deno_dir(&util::new_deno_dir()); + let output = deno_cmd.arg("run").arg("//a/b").output().unwrap(); + assert!(String::from_utf8_lossy(&output.stderr).contains("Invalid file path.")); - // this one used to panic on windows let deno_cmd = util::deno_cmd_with_deno_dir(&util::new_deno_dir()); - let output = deno_cmd.arg("run").arg("file:/asdf").output().unwrap(); - assert!(!deno_panicked(&output)); + let output = deno_cmd.arg("run").arg("///a/b").output().unwrap(); + assert!(String::from_utf8_lossy(&output.stderr).contains("Module not found")); } From 459d96b45f02737c319e94e4a6226bd4f6254eb1 Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Wed, 18 Sep 2024 15:11:48 +0300 Subject: [PATCH 24/25] fmt --- tests/integration/run_tests.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/integration/run_tests.rs b/tests/integration/run_tests.rs index 25133d8b398d30..975aff3a48ea16 100644 --- a/tests/integration/run_tests.rs +++ b/tests/integration/run_tests.rs @@ -5130,7 +5130,9 @@ fn emit_failed_readonly_file_system() { fn handle_invalid_path_error() { let deno_cmd = util::deno_cmd_with_deno_dir(&util::new_deno_dir()); let output = deno_cmd.arg("run").arg("file://asdf").output().unwrap(); - assert!(String::from_utf8_lossy(&output.stderr).contains("Invalid file path.")); + assert!( + String::from_utf8_lossy(&output.stderr).contains("Invalid file path.") + ); let deno_cmd = util::deno_cmd_with_deno_dir(&util::new_deno_dir()); let output = deno_cmd.arg("run").arg("/a/b").output().unwrap(); @@ -5138,7 +5140,9 @@ fn handle_invalid_path_error() { let deno_cmd = util::deno_cmd_with_deno_dir(&util::new_deno_dir()); let output = deno_cmd.arg("run").arg("//a/b").output().unwrap(); - assert!(String::from_utf8_lossy(&output.stderr).contains("Invalid file path.")); + assert!( + String::from_utf8_lossy(&output.stderr).contains("Invalid file path.") + ); let deno_cmd = util::deno_cmd_with_deno_dir(&util::new_deno_dir()); let output = deno_cmd.arg("run").arg("///a/b").output().unwrap(); From c31344f72ad7cc858413a7f76666b4222faa078b Mon Sep 17 00:00:00 2001 From: yazan-abdalrahman Date: Wed, 18 Sep 2024 15:27:55 +0300 Subject: [PATCH 25/25] fix --- tests/integration/run_tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/run_tests.rs b/tests/integration/run_tests.rs index 975aff3a48ea16..50f0f58f88a8a1 100644 --- a/tests/integration/run_tests.rs +++ b/tests/integration/run_tests.rs @@ -5126,6 +5126,7 @@ fn emit_failed_readonly_file_system() { output.assert_matches_text("[WILDCARD]Error saving emit data ([WILDLINE]main.ts)[WILDCARD]Skipped emit cache save of [WILDLINE]other.ts[WILDCARD]hi[WILDCARD]"); } +#[cfg(windows)] #[test] fn handle_invalid_path_error() { let deno_cmd = util::deno_cmd_with_deno_dir(&util::new_deno_dir());