better error messages for 'relative import path not prefixed with / o…

…r ./ or ../' (denoland#3405)
gcollazo · Nov 26, 2019 · 9712e0c · 9712e0c
1 parent 2a34814
commit 9712e0c
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 18 deletions.
diff --git a/cli/deno_error.rs b/cli/deno_error.rs
@@ -140,7 +140,7 @@ impl GetErrorKind for ModuleResolutionError {
  match self {
  InvalidUrl(ref err) | InvalidBaseUrl(ref err) => err.kind(),
  InvalidPath(_) => ErrorKind::InvalidPath,
- ImportPrefixMissing(_) => ErrorKind::ImportPrefixMissing,
+ ImportPrefixMissing(_, _) => ErrorKind::ImportPrefixMissing,
  }
  }
 }

diff --git a/cli/tests/error_011_bad_module_specifier.ts.out b/cli/tests/error_011_bad_module_specifier.ts.out
@@ -1,4 +1,4 @@
-[WILDCARD]error: Uncaught ImportPrefixMissing: relative import path "bad-module.ts" not prefixed with / or ./ or ../
+[WILDCARD]error: Uncaught ImportPrefixMissing: relative import path "bad-module.ts" not prefixed with / or ./ or ../ Imported from "[WILDCARD]/error_011_bad_module_specifier.ts"
 [WILDCARD]dispatch_json.ts:[WILDCARD]
  at DenoError ([WILDCARD]errors.ts:[WILDCARD])
  at unwrapResponse ([WILDCARD]dispatch_json.ts:[WILDCARD])

diff --git a/cli/tests/error_012_bad_dynamic_import_specifier.ts.out b/cli/tests/error_012_bad_dynamic_import_specifier.ts.out
@@ -1,4 +1,4 @@
-[WILDCARD]error: Uncaught ImportPrefixMissing: relative import path "bad-module.ts" not prefixed with / or ./ or ../
+[WILDCARD]error: Uncaught ImportPrefixMissing: relative import path "bad-module.ts" not prefixed with / or ./ or ../ Imported from "[WILDCARD]/error_012_bad_dynamic_import_specifier.ts"
 [WILDCARD]dispatch_json.ts:[WILDCARD]
  at DenoError ([WILDCARD]errors.ts:[WILDCARD])
  at unwrapResponse ([WILDCARD]dispatch_json.ts:[WILDCARD])

diff --git a/cli/tests/error_014_catch_dynamic_import_error.js.out b/cli/tests/error_014_catch_dynamic_import_error.js.out
@@ -1,8 +1,8 @@
 Caught direct dynamic import error.
-TypeError: relative import path "does not exist" not prefixed with / or ./ or ../
+TypeError: relative import path "does not exist" not prefixed with / or ./ or ../ Imported from "[WILDCARD]/error_014_catch_dynamic_import_error.js"
 
 Caught indirect direct dynamic import error.
-TypeError: relative import path "does not exist either" not prefixed with / or ./ or ../
+TypeError: relative import path "does not exist either" not prefixed with / or ./ or ../ Imported from "[WILDCARD]/indirect_import_error.js"
 
 Caught error thrown by dynamically imported module.
 Error: An error

diff --git a/cli/tests/error_type_definitions.ts.out b/cli/tests/error_type_definitions.ts.out
@@ -1,4 +1,4 @@
-[WILDCARD]error: Uncaught ImportPrefixMissing: relative import path "baz" not prefixed with / or ./ or ../
+[WILDCARD]error: Uncaught ImportPrefixMissing: relative import path "baz" not prefixed with / or ./ or ../ Imported from "[WILDCARD]/type_definitions/bar.d.ts"
 [WILDCARD]dispatch_json.ts:[WILDCARD]
  at DenoError ([WILDCARD]errors.ts:[WILDCARD])
  at unwrapResponse ([WILDCARD]dispatch_json.ts:[WILDCARD])

diff --git a/core/module_specifier.rs b/core/module_specifier.rs
@@ -11,7 +11,7 @@ pub enum ModuleResolutionError {
  InvalidUrl(ParseError),
  InvalidBaseUrl(ParseError),
  InvalidPath(PathBuf),
- ImportPrefixMissing(String),
+ ImportPrefixMissing(String, Option<String>),
 }
 use ModuleResolutionError::*;
 
@@ -32,11 +32,19 @@ impl fmt::Display for ModuleResolutionError {
  write!(f, "invalid base URL for relative import: {}", err)
  }
  InvalidPath(ref path) => write!(f, "invalid module path: {:?}", path),
- ImportPrefixMissing(ref specifier) => write!(
- f,
- "relative import path \"{}\" not prefixed with / or ./ or ../",
- specifier
- ),
+ ImportPrefixMissing(ref specifier, ref maybe_referrer) => {
+ let msg = format!(
+ "relative import path \"{}\" not prefixed with / or ./ or ../",
+ specifier
+ );
+ let msg = if let Some(referrer) = maybe_referrer {
+ format!("{} Imported from \"{}\"", msg, referrer)
+ } else {
+ msg
+ };
+
+ write!(f, "{}", msg)
+ }
  }
  }
 }
@@ -78,7 +86,12 @@ impl ModuleSpecifier {
  || specifier.starts_with("./")
  || specifier.starts_with("../")) =>
  {
- return Err(ImportPrefixMissing(specifier.to_string()))
+ let maybe_referrer = if base.is_empty() {
+ None
+ } else {
+ Some(base.to_string())
+ };
+ return Err(ImportPrefixMissing(specifier.to_string(), maybe_referrer));
  }
 
  // 3. Return the result of applying the URL parser to specifier with base
@@ -282,27 +295,42 @@ mod tests {
  (
  "005_more_imports.ts",
  "http:https://deno.land/core/tests/006_url_imports.ts",
- ImportPrefixMissing("005_more_imports.ts".to_string()),
+ ImportPrefixMissing(
+ "005_more_imports.ts".to_string(),
+ Some("http:https://deno.land/core/tests/006_url_imports.ts".to_string()),
+ ),
  ),
  (
  ".tomato",
  "http:https://deno.land/core/tests/006_url_imports.ts",
- ImportPrefixMissing(".tomato".to_string()),
+ ImportPrefixMissing(
+ ".tomato".to_string(),
+ Some("http:https://deno.land/core/tests/006_url_imports.ts".to_string()),
+ ),
  ),
  (
  "..zucchini.mjs",
  "http:https://deno.land/core/tests/006_url_imports.ts",
- ImportPrefixMissing("..zucchini.mjs".to_string()),
+ ImportPrefixMissing(
+ "..zucchini.mjs".to_string(),
+ Some("http:https://deno.land/core/tests/006_url_imports.ts".to_string()),
+ ),
  ),
  (
  r".\yam.es",
  "http:https://deno.land/core/tests/006_url_imports.ts",
- ImportPrefixMissing(r".\yam.es".to_string()),
+ ImportPrefixMissing(
+ r".\yam.es".to_string(),
+ Some("http:https://deno.land/core/tests/006_url_imports.ts".to_string()),
+ ),
  ),
  (
  r"..\yam.es",
  "http:https://deno.land/core/tests/006_url_imports.ts",
- ImportPrefixMissing(r"..\yam.es".to_string()),
+ ImportPrefixMissing(
+ r"..\yam.es".to_string(),
+ Some("http:https://deno.land/core/tests/006_url_imports.ts".to_string()),
+ ),
  ),
  (
  "https://eggplant:b/c",