refactor(core): remove "resolve_url_or_path_deprecated" (#18174)

Remove remaining usages of "resolve_url_or_path_deprecated" in favor
of "resolve_url_or_path" with explicit calls to
"std::env::current_dir()".

Towards landing https://github.com/denoland/deno/pull/15454

cli/proc_state.rs

@@ -495,13 +495,18 @@ impl ProcState {
     referrer: &str,
     permissions: &mut PermissionsContainer,
   ) -> Result<ModuleSpecifier, AnyError> {
-    if let Ok(referrer) = deno_core::resolve_url_or_path_deprecated(referrer) {
+    // TODO(bartlomieju): ideally we shouldn't need to call `current_dir()` on each
-      if self.npm_resolver.in_npm_package(&referrer) {
+    // call - maybe it should be caller's responsibility to pass it as an arg?
+    let cwd = std::env::current_dir().context("Unable to get CWD")?;
+    let referrer_result = deno_core::resolve_url_or_path(referrer, &cwd);
+
+    if let Ok(referrer) = referrer_result.as_ref() {
+      if self.npm_resolver.in_npm_package(referrer) {
         // we're in an npm package, so use node resolution
         return self
           .handle_node_resolve_result(node::node_resolve(
             specifier,
-            &referrer,
+            referrer,
             NodeResolutionMode::Execution,
             &self.npm_resolver,
             permissions,
@@ -512,7 +517,7 @@ impl ProcState {
       }
 
       let graph = self.graph_container.graph();
-      let maybe_resolved = match graph.get(&referrer) {
+      let maybe_resolved = match graph.get(referrer) {
         Some(Module::Esm(module)) => {
           module.dependencies.get(specifier).map(|d| &d.maybe_code)
         }
@@ -565,9 +570,9 @@ impl ProcState {
     // but sadly that's not the case due to missing APIs in V8.
     let is_repl = matches!(self.options.sub_command(), DenoSubcommand::Repl(_));
     let referrer = if referrer.is_empty() && is_repl {
-      deno_core::resolve_path("./$deno$repl.ts", self.options.initial_cwd())?
+      deno_core::resolve_path("./$deno$repl.ts", &cwd)?
     } else {
-      deno_core::resolve_url_or_path_deprecated(referrer)?
+      referrer_result?
     };
 
     // FIXME(bartlomieju): this is another hack way to provide NPM specifier

cli/standalone.rs

@@ -140,9 +140,12 @@ impl ModuleLoader for EmbeddedModuleLoader {
     // Try to follow redirects when resolving.
     let referrer = match self.eszip.get_module(referrer) {
       Some(eszip::Module { ref specifier, .. }) => {
-        deno_core::resolve_url_or_path_deprecated(specifier)?
+        ModuleSpecifier::parse(specifier)?
+      }
+      None => {
+        let cwd = std::env::current_dir().context("Unable to get CWD")?;
+        deno_core::resolve_url_or_path(referrer, &cwd)?
       }
-      None => deno_core::resolve_url_or_path_deprecated(referrer)?,
     };
 
     self.maybe_import_map_resolver.as_ref().map_or_else(

cli/tsc/mod.rs

@@ -13,7 +13,7 @@ use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
 use deno_core::located_script_name;
 use deno_core::op;
-use deno_core::resolve_url_or_path_deprecated;
+use deno_core::resolve_url_or_path;
 use deno_core::serde::Deserialize;
 use deno_core::serde::Deserializer;
 use deno_core::serde::Serialize;
@@ -39,6 +39,7 @@ use once_cell::sync::Lazy;
 use std::borrow::Cow;
 use std::collections::HashMap;
 use std::fmt;
+use std::path::Path;
 use std::path::PathBuf;
 use std::sync::Arc;
 
@@ -378,6 +379,7 @@ struct State {
   maybe_npm_resolver: Option<NpmPackageResolver>,
   remapped_specifiers: HashMap<String, ModuleSpecifier>,
   root_map: HashMap<String, ModuleSpecifier>,
+  current_dir: PathBuf,
 }
 
 impl State {
@@ -388,6 +390,7 @@ impl State {
     maybe_tsbuildinfo: Option<String>,
     root_map: HashMap<String, ModuleSpecifier>,
     remapped_specifiers: HashMap<String, ModuleSpecifier>,
+    current_dir: PathBuf,
   ) -> Self {
     State {
       hash_data,
@@ -397,12 +400,16 @@ impl State {
       maybe_response: None,
       remapped_specifiers,
       root_map,
+      current_dir,
     }
   }
 }
 
-fn normalize_specifier(specifier: &str) -> Result<ModuleSpecifier, AnyError> {
+fn normalize_specifier(
-  resolve_url_or_path_deprecated(specifier).map_err(|err| err.into())
+  specifier: &str,
+  current_dir: &Path,
+) -> Result<ModuleSpecifier, AnyError> {
+  resolve_url_or_path(specifier, current_dir).map_err(|err| err.into())
 }
 
 #[derive(Debug, Deserialize)]
@@ -481,7 +488,7 @@ fn op_load(state: &mut OpState, args: Value) -> Result<Value, AnyError> {
   let state = state.borrow_mut::<State>();
   let v: LoadArgs = serde_json::from_value(args)
     .context("Invalid request from JavaScript for \"op_load\".")?;
-  let specifier = normalize_specifier(&v.specifier)
+  let specifier = normalize_specifier(&v.specifier, &state.current_dir)
     .context("Error converting a string module specifier for \"op_load\".")?;
   let mut hash: Option<String> = None;
   let mut media_type = MediaType::Unknown;
@@ -584,7 +591,7 @@ fn op_resolve(
   } else if let Some(remapped_base) = state.root_map.get(&args.base) {
     remapped_base.clone()
   } else {
-    normalize_specifier(&args.base).context(
+    normalize_specifier(&args.base, &state.current_dir).context(
       "Error converting a string module specifier for \"op_resolve\".",
     )?
   };
@@ -831,6 +838,9 @@ pub fn exec(request: Request) -> Result<Response, AnyError> {
           request.maybe_tsbuildinfo.clone(),
           root_map.clone(),
           remapped_specifiers.clone(),
+          std::env::current_dir()
+            .context("Unable to get CWD")
+            .unwrap(),
         ));
       })
       .build()],
@@ -943,6 +953,9 @@ mod tests {
       maybe_tsbuildinfo,
       HashMap::new(),
       HashMap::new(),
+      std::env::current_dir()
+        .context("Unable to get CWD")
+        .unwrap(),
     );
     let mut op_state = OpState::new(1);
     op_state.put(state);

core/lib.rs

@@ -72,7 +72,6 @@ pub use crate::module_specifier::resolve_import;
 pub use crate::module_specifier::resolve_path;
 pub use crate::module_specifier::resolve_url;
 pub use crate::module_specifier::resolve_url_or_path;
-pub use crate::module_specifier::resolve_url_or_path_deprecated;
 pub use crate::module_specifier::ModuleResolutionError;
 pub use crate::module_specifier::ModuleSpecifier;
 pub use crate::module_specifier::DUMMY_SPECIFIER;

core/module_specifier.rs

@@ -117,24 +117,6 @@ pub fn resolve_url(
   Url::parse(url_str).map_err(ModuleResolutionError::InvalidUrl)
 }
 
-/// Takes a string representing either an absolute URL or a file path,
-/// as it may be passed to deno as a command line argument.
-/// The string is interpreted as a URL if it starts with a valid URI scheme,
-/// e.g. 'http:' or 'file:' or 'git+ssh:'. If not, it's interpreted as a
-/// file path; if it is a relative path it's resolved relative to the current
-/// working directory.
-pub fn resolve_url_or_path_deprecated(
-  specifier: &str,
-) -> Result<ModuleSpecifier, ModuleResolutionError> {
-  if specifier_has_uri_scheme(specifier) {
-    resolve_url(specifier)
-  } else {
-    let cwd = current_dir()
-      .map_err(|_| ModuleResolutionError::InvalidPath(specifier.into()))?;
-    resolve_path(specifier, &cwd)
-  }
-}
-
 /// Takes a string representing either an absolute URL or a file path,
 /// as it may be passed to deno as a command line argument.
 /// The string is interpreted as a URL if it starts with a valid URI scheme,
@@ -361,7 +343,7 @@ mod tests {
   }
 
   #[test]
-  fn test_resolve_url_or_path_deprecated() {
+  fn test_resolve_url_or_path() {
     // Absolute URL.
     let mut tests: Vec<(&str, String)> = vec![
       (
@@ -457,9 +439,7 @@ mod tests {
     }
 
     for (specifier, expected_url) in tests {
-      let url = resolve_url_or_path_deprecated(specifier)
+      let url = resolve_url_or_path(specifier, &cwd).unwrap().to_string();
-        .unwrap()
-        .to_string();
       assert_eq!(url, expected_url);
     }
   }
@@ -479,7 +459,8 @@ mod tests {
     }
 
     for (specifier, expected_err) in tests {
-      let err = resolve_url_or_path_deprecated(specifier).unwrap_err();
+      let err =
+        resolve_url_or_path(specifier, &PathBuf::from("/")).unwrap_err();
       assert_eq!(err, expected_err);
     }
   }

core/ops_builtin_v8.rs

@@ -6,13 +6,14 @@ use crate::error::range_error;
 use crate::error::type_error;
 use crate::error::JsError;
 use crate::ops_builtin::WasmStreamingResource;
-use crate::resolve_url_or_path_deprecated;
+use crate::resolve_url_or_path;
 use crate::serde_v8::from_v8;
 use crate::source_map::apply_source_map as apply_source_map_;
 use crate::JsRealm;
 use crate::JsRuntime;
 use crate::OpDecl;
 use crate::ZeroCopyBuf;
+use anyhow::Context;
 use anyhow::Error;
 use deno_ops::op;
 use serde::Deserialize;
@@ -165,7 +166,12 @@ fn op_eval_context<'a>(
   let source = v8::Local::<v8::String>::try_from(source.v8_value)
     .map_err(|_| type_error("Invalid source"))?;
   let specifier = match specifier {
-    Some(s) => resolve_url_or_path_deprecated(&s)?.to_string(),
+    Some(s) => {
+      // TODO(bartlomieju): ideally we shouldn't need to call `current_dir()` on each
+      // call - maybe it should be caller's responsibility to pass fully resolved URL?
+      let cwd = std::env::current_dir().context("Unable to get CWD")?;
+      resolve_url_or_path(&s, &cwd)?.to_string()
+    }
     None => crate::DUMMY_SPECIFIER.to_string(),
   };
   let specifier = v8::String::new(tc_scope, &specifier).unwrap();