Add new methods to get error details

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Added new methods to get error details: `Error::is_parse`, `Error::is_io` and `Error::file`.
+
 ## [0.3.3] - 2023-04-27
 
 ### Changed

protox/src/compile/mod.rs

@@ -134,7 +134,7 @@ impl Compiler {
 
         if let Some(parsed_file) = self.files.get_mut(&name) {
             if is_resolved {
-                check_shadow(parsed_file.file.path(), path)?;
+                check_shadow(&name, parsed_file.file.path(), path)?;
             }
             parsed_file.is_root = true;
             return Ok(self);
@@ -150,7 +150,7 @@ impl Compiler {
             }
         })?;
         if is_resolved {
-            check_shadow(file.path(), path)?;
+            check_shadow(&name, file.path(), path)?;
         }
 
         let mut import_stack = vec![name.clone()];
@@ -261,7 +261,10 @@ impl Compiler {
             }
             write!(&mut cycle, "{}", file_name).unwrap();
 
-            return Err(Error::from_kind(ErrorKind::CircularImport { cycle }));
+            return Err(Error::from_kind(ErrorKind::CircularImport {
+                name: file_name.to_owned(),
+                cycle,
+            }));
         }
 
         if self.files.contains_key(file_name) {

protox/src/compile/tests.rs

@@ -659,7 +659,8 @@ fn shadow_file_rel() {
         let err = compiler.open_file("foo.proto").unwrap_err();
 
         match err.kind() {
-            ErrorKind::FileShadowed { path, shadow } => {
+            ErrorKind::FileShadowed { name, path, shadow } => {
+                assert_eq!(name, "foo.proto");
                 assert_eq!(path, Path::new("foo.proto"));
                 assert_eq!(shadow, &Path::new("include").join("foo.proto"));
             }
@@ -684,7 +685,8 @@ fn shadow_file_rel_subdir() {
             .unwrap_err();
 
         match err.kind() {
-            ErrorKind::FileShadowed { path, shadow } => {
+            ErrorKind::FileShadowed { name, path, shadow } => {
+                assert_eq!(name, "foo.proto");
                 assert_eq!(path, &Path::new("include2").join("foo.proto"));
                 assert_eq!(shadow, &Path::new("include1").join("foo.proto"));
             }
@@ -707,7 +709,8 @@ fn shadow_file_abs() {
         .unwrap_err();
 
     match err.kind() {
-        ErrorKind::FileShadowed { path, shadow } => {
+        ErrorKind::FileShadowed { name, path, shadow } => {
+            assert_eq!(name, "foo.proto");
             assert_eq!(path, &dir.path().join("foo.proto"));
             assert_eq!(shadow, &dir.path().join("include").join("foo.proto"));
         }
@@ -732,7 +735,8 @@ fn shadow_file_abs_subdir() {
         .unwrap_err();
 
     match err.kind() {
-        ErrorKind::FileShadowed { path, shadow } => {
+        ErrorKind::FileShadowed { name, path, shadow } => {
+            assert_eq!(name, "foo.proto");
             assert_eq!(path, &dir.path().join("include2").join("foo.proto"));
             assert_eq!(shadow, &dir.path().join("include1").join("foo.proto"));
         }
@@ -783,7 +787,8 @@ fn shadow_already_imported_file() {
         .unwrap_err();
 
     match err.kind() {
-        ErrorKind::FileShadowed { path, shadow } => {
+        ErrorKind::FileShadowed { name, path, shadow } => {
+            assert_eq!(name, "foo.proto");
             assert_eq!(path, &dir.path().join("include2").join("foo.proto"));
             assert_eq!(shadow, &dir.path().join("include1").join("foo.proto"));
         }
@@ -880,7 +885,8 @@ fn import_cycle() {
     let err = compiler.open_file("root.proto").unwrap_err();
 
     match err.kind() {
-        ErrorKind::CircularImport { cycle } => {
+        ErrorKind::CircularImport { name, cycle } => {
+            assert_eq!(name, "root.proto");
             assert_eq!(cycle, "root.proto -> dep.proto -> dep2.proto -> root.proto")
         }
         kind => panic!("unexpected error: {}", kind),
@@ -891,13 +897,35 @@ fn import_cycle() {
 fn import_cycle_short() {
     let dir = TempDir::new().unwrap();
 
+    std::fs::write(dir.path().join("root.proto"), "import 'dep.proto';").unwrap();
+    std::fs::write(dir.path().join("dep.proto"), "import 'dep.proto';").unwrap();
+
+    let mut compiler = Compiler::new([dir.path()]).unwrap();
+    let err = compiler.open_file("root.proto").unwrap_err();
+
+    match err.kind() {
+        ErrorKind::CircularImport { name, cycle } => {
+            assert_eq!(name, "dep.proto");
+            assert_eq!(cycle, "root.proto -> dep.proto -> dep.proto")
+        }
+        kind => panic!("unexpected error: {}", kind),
+    }
+}
+
+#[test]
+fn import_cycle_nested() {
+    let dir = TempDir::new().unwrap();
+
     std::fs::write(dir.path().join("root.proto"), "import 'root.proto';").unwrap();
 
     let mut compiler = Compiler::new([dir.path().to_owned(), dir.path().join("include")]).unwrap();
     let err = compiler.open_file("root.proto").unwrap_err();
 
     match err.kind() {
-        ErrorKind::CircularImport { cycle } => assert_eq!(cycle, "root.proto -> root.proto"),
+        ErrorKind::CircularImport { name, cycle } => {
+            assert_eq!(name, "root.proto");
+            assert_eq!(cycle, "root.proto -> root.proto")
+        }
         kind => panic!("unexpected error: {}", kind),
     }
 }

protox/src/error.rs

@@ -23,6 +23,7 @@ pub(crate) enum ErrorKind {
     Check { err: DescriptorError },
     #[error("error opening file '{path}'")]
     OpenFile {
+        name: String,
         path: PathBuf,
         #[source]
         err: io::Error,
@@ -33,12 +34,16 @@ pub(crate) enum ErrorKind {
     #[error("import '{name}' not found")]
     ImportNotFound { name: String },
     #[error("import cycle detected: {cycle}")]
-    CircularImport { cycle: String },
+    CircularImport { name: String, cycle: String },
     #[error("file '{path}' is not in any include path")]
     FileNotIncluded { path: PathBuf },
     #[error("path '{path}' is shadowed by '{shadow}' in the include paths")]
     #[diagnostic(help("either pass '{}' as the input file, or re-order the include paths so that '{}' comes first", shadow.display(), path.display()))]
-    FileShadowed { path: PathBuf, shadow: PathBuf },
+    FileShadowed {
+        name: String,
+        path: PathBuf,
+        shadow: PathBuf,
+    },
     #[error(transparent)]
     Custom(Box<dyn std::error::Error + Send + Sync>),
 }
@@ -61,6 +66,21 @@ impl Error {
         })
     }
 
+    /// The file in which this error occurred, if available.
+    pub fn file(&self) -> Option<&str> {
+        match &*self.kind {
+            ErrorKind::Parse { err } => Some(err.file()),
+            ErrorKind::Check { err } => err.file(),
+            ErrorKind::OpenFile { name, .. }
+            | ErrorKind::FileTooLarge { name }
+            | ErrorKind::ImportNotFound { name }
+            | ErrorKind::CircularImport { name, .. }
+            | ErrorKind::FileShadowed { name, .. } => Some(name),
+            ErrorKind::FileNotIncluded { .. } => None,
+            ErrorKind::Custom(_) => None,
+        }
+    }
+
     pub(crate) fn from_kind(kind: ErrorKind) -> Self {
         Error {
             kind: Box::new(kind),
@@ -76,6 +96,23 @@ impl Error {
     pub fn is_file_not_found(&self) -> bool {
         matches!(&*self.kind, ErrorKind::ImportNotFound { .. })
     }
+
+    /// Returns true if this error is caused by an invalid protobuf source file.
+    pub fn is_parse(&self) -> bool {
+        matches!(
+            &*self.kind,
+            ErrorKind::Parse { .. } | ErrorKind::FileTooLarge { .. }
+        )
+    }
+
+    /// Returns true if this error is caused by an IO error while opening a file.
+    pub fn is_io(&self) -> bool {
+        match &*self.kind {
+            ErrorKind::OpenFile { .. } => true,
+            ErrorKind::Custom(err) if err.downcast_ref::<io::Error>().is_some() => true,
+            _ => false,
+        }
+    }
 }
 
 impl From<DescriptorError> for Error {
@@ -90,6 +127,12 @@ impl From<ParseError> for Error {
     }
 }
 
+impl From<io::Error> for Error {
+    fn from(err: io::Error) -> Self {
+        Error::new(err)
+    }
+}
+
 impl fmt::Debug for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match &*self.kind {
@@ -107,14 +150,29 @@ impl fmt::Debug for Error {
 }
 
 #[test]
-fn fmt_debug() {
+fn fmt_debug_io() {
     let err = Error::from_kind(ErrorKind::OpenFile {
+        name: "file.proto".into(),
         path: "path/to/file.proto".into(),
         err: io::Error::new(io::ErrorKind::Other, "io error"),
     });
 
+    assert!(err.is_io());
+    assert_eq!(err.file(), Some("file.proto"));
     assert_eq!(
         format!("{:?}", err),
         "error opening file 'path/to/file.proto': io error"
     );
 }
+
+#[test]
+fn fmt_debug_parse() {
+    let err = Error::from(protox_parse::parse("file.proto", "invalid").unwrap_err());
+
+    assert!(err.is_parse());
+    assert_eq!(err.file(), Some("file.proto"));
+    assert_eq!(
+        format!("{:?}", err),
+        "file.proto:1:1: expected 'enum', 'extend', 'import', 'message', 'option', 'service', 'package' or ';', but found 'invalid'"
+    );
+}

protox/src/file/include.rs

@@ -94,11 +94,16 @@ pub(crate) fn path_to_file_name(path: &Path) -> Option<String> {
     }
 }
 
-pub(crate) fn check_shadow(actual_path: Option<&Path>, expected_path: &Path) -> Result<(), Error> {
+pub(crate) fn check_shadow(
+    file: &str,
+    actual_path: Option<&Path>,
+    expected_path: &Path,
+) -> Result<(), Error> {
     // actual_path is expected to be an include path concatenated with `expected_path`
     if let Some(actual_path) = actual_path {
         if !path_eq(actual_path, expected_path) {
             return Err(Error::from_kind(ErrorKind::FileShadowed {
+                name: file.to_string(),
                 path: expected_path.to_owned(),
                 shadow: actual_path.to_owned(),
             }));

protox/src/file/mod.rs

@@ -115,6 +115,7 @@ impl File {
                 Error::file_not_found(name)
             } else {
                 Error::from_kind(ErrorKind::OpenFile {
+                    name: name.to_owned(),
                     path: path.to_owned(),
                     err,
                 })

protox/tests/compiler.rs

@@ -1,4 +1,4 @@
-use std::fs;
+use std::{fs, io};
 
 use insta::assert_yaml_snapshot;
 use miette::{Diagnostic, JSONReportHandler};
@@ -19,7 +19,10 @@ struct TestFileResolver {
 impl FileResolver for TestFileResolver {
     fn open_file(&self, name: &str) -> Result<File, Error> {
         if name == "customerror.proto" {
-            return Err(Error::new("failed to load file!"));
+            return Err(Error::new(io::Error::new(
+                io::ErrorKind::Other,
+                "failed to load file!",
+            )));
         }
 
         for file in self.files {
@@ -307,6 +310,8 @@ fn error_fmt_debug() {
     let import_err = check(&[("root.proto", "import 'notfound.proto';")]).unwrap_err();
     let open_err = check(&[("root.proto", "import 'customerror.proto';")]).unwrap_err();
 
+    assert!(parse_err.is_parse());
+    assert_eq!(parse_err.file(), Some("root.proto"));
     assert_eq!(
         parse_err.to_string(),
         "expected an identifier, but found '{'"
@@ -316,18 +321,27 @@ fn error_fmt_debug() {
         "root.proto:1:9: expected an identifier, but found '{'"
     );
 
+    assert!(!check_err.is_io() && !check_err.is_parse());
+    assert_eq!(check_err.file(), Some("root.proto"));
     assert_eq!(check_err.to_string(), "name 'Foo' is defined twice");
     assert_eq!(
         format!("{:?}", check_err),
         "root.proto:1:24: name 'Foo' is defined twice"
     );
 
+    assert!(import_err.is_file_not_found());
+    assert_eq!(import_err.file(), Some("notfound.proto"));
     assert_eq!(import_err.to_string(), "import 'notfound.proto' not found");
     assert_eq!(
         format!("{:?}", import_err),
         "import 'notfound.proto' not found"
     );
 
+    assert!(open_err.is_io());
+    assert!(open_err.file().is_none());
     assert_eq!(open_err.to_string(), "failed to load file!");
-    assert_eq!(format!("{:?}", open_err), "\"failed to load file!\"");
+    assert_eq!(
+        format!("{:?}", open_err),
+        "Custom { kind: Other, error: \"failed to load file!\" }"
+    );
 }