Add error module and wrap all valid module errors

Add the `error` module, defining an Error enum using `thiserror`. Create
variants for all of the errors generated by the `valid` module, and
replace its custom `ValidationError` enum. Includes tests for each
variant. Then teach that module to return only our errors.

Note the special treatment for these error conversions:

*   `boon::ValidationError`s use lifetimes to hang on to values from the
    boon compiler and from the JSON that fails validation. This makes it
    quite tricky to use separate from those values. But since, so far,
    we just want to print JSON Schema validation errors, add a `From`
    trait implementation that simply copies its string value into our
    error type. This allows all the lifetime annotations to be removed
    from the `valid` crate.

*   `wax::GlobError` is an enum that wraps the two types that the `wax`
    create generates. To coerce those two types, add a `From` trait for
    each.

While at it, improve the error generation for the custom `license`
format so that the error message generated by boon is more legible and
useful. Previously it just showed the value that failed --- twice ---
but now it also shows the "reason" for the error.

Also: change the behavior of `is_license` and `is_path` to return
`Ok(())` if the value is not a string, following the example of the
format validation functions in boon itself. New tests for the `license`
and `path` custom formats validate the usability of the error messages.

CHANGELOG.md

@@ -7,6 +7,24 @@ All notable changes to this project will be documented in this file. It uses the
   [Semantic Versioning]: https://semver.org/spec/v2.0.0.html
     "Semantic Versioning 2.0.0"
 
+## [v0.5.0] — Unreleased
+
+### ⚡ Improvements
+
+*   Added the [error module], which defines all the errors returned by
+    pgxn_meta.
+*   Changed the errors returned from the [valid module] from boxed
+    [boon] errors with lifetimes to [error module] errors with no lifetimes.
+
+### 📔 Notes
+
+*   Removed the `valid::ValidationError` enum.
+
+  [v0.5.0]: https://github.com/pgxn/meta/compare/v0.4.0...v0.5.0
+  [error module]: https://docs.rs/pgxn_meta/0.5.0/pgxn_meta/error/
+  [valid module]: https://docs.rs/pgxn_meta/0.5.0/pgxn_meta/valid/
+  [boon]: https://github.com/santhosh-tekuri/boon
+
 ## [v0.4.0] — 2024-10-08
 
 The theme of this release is *JSON Web Signatures.*

Cargo.toml

@@ -28,6 +28,7 @@ serde = { version = "1", features = ["derive"] }
 serde_json = "1.0"
 serde_with = { version = "3.9.0", features = ["hex"] }
 spdx = "0.10.6"
+thiserror = "1.0"
 wax = "0.6.0"
 
 [build-dependencies]

src/error/mod.rs

@@ -0,0 +1,60 @@
+//! PGXN Meta Errors.
+
+#[cfg(test)]
+mod tests;
+
+/// Build errors.
+#[derive(thiserror::Error, Debug)]
+pub enum Error {
+    /// License Error.
+    #[error("{}", .0.reason)]
+    License(#[from] spdx::error::ParseError),
+
+    /// Validator cannot determine the version of the meta spec.
+    #[error("Cannot determine meta-spec version")]
+    UnknownSpec,
+
+    /// A schema file has no `$id` property.
+    #[error("No $id found in schema")]
+    UnknownSchemaId,
+
+    /// JSON Schema compile error.
+    #[error(transparent)]
+    #[allow(clippy::enum_variant_names)]
+    CompileError(#[from] boon::CompileError),
+
+    /// JSON Schema validation error.
+    #[error("{0}")]
+    #[allow(clippy::enum_variant_names)]
+    ValidationError(String),
+
+    /// Serde JSON error.
+    #[error(transparent)]
+    Serde(#[from] serde_json::Error),
+
+    /// IO error.
+    #[error(transparent)]
+    Io(#[from] std::io::Error),
+
+    /// Glob build error.
+    #[error(transparent)]
+    Glob(#[from] wax::GlobError),
+}
+
+impl<'s, 'v> From<boon::ValidationError<'s, 'v>> for Error {
+    fn from(value: boon::ValidationError<'s, 'v>) -> Self {
+        Self::ValidationError(value.to_string())
+    }
+}
+
+impl From<wax::BuildError> for Error {
+    fn from(value: wax::BuildError) -> Self {
+        wax::GlobError::Build(value).into()
+    }
+}
+
+impl From<wax::WalkError> for Error {
+    fn from(value: wax::WalkError) -> Self {
+        wax::GlobError::Walk(value).into()
+    }
+}

src/error/tests.rs

@@ -0,0 +1,91 @@
+use super::*;
+use serde_json::json;
+
+#[test]
+fn spdx() {
+    let parse_error = spdx::Expression::parse("not a license").unwrap_err();
+    let exp = parse_error.reason.to_string();
+    let not_exp = parse_error.to_string();
+    let err: Error = parse_error.into();
+    assert!(matches!(err, Error::License { .. }));
+    assert_eq!(exp, err.to_string());
+    assert_ne!(not_exp, err.to_string());
+}
+
+#[test]
+fn unknown_spec() {
+    assert_eq!(
+        Error::UnknownSpec.to_string(),
+        "Cannot determine meta-spec version"
+    )
+}
+
+#[test]
+fn unknown_schema_id() {
+    assert_eq!(Error::UnknownSchemaId.to_string(), "No $id found in schema")
+}
+
+#[test]
+fn compile() {
+    let mut c = boon::Compiler::new();
+    c.add_resource("foo", json!("not a schema")).unwrap();
+    let mut s = boon::Schemas::new();
+    let compile_err = c.compile("foo", &mut s).unwrap_err();
+    let exp = compile_err.to_string();
+    let err: Error = compile_err.into();
+    assert!(matches!(err, Error::CompileError { .. }));
+    assert_eq!(exp, err.to_string(),);
+}
+
+#[test]
+fn validation() {
+    let mut c = boon::Compiler::new();
+    c.add_resource("foo", json!({"type": "object"})).unwrap();
+    let mut s = boon::Schemas::new();
+    let idx = c.compile("foo", &mut s).unwrap();
+    let json = json!([]);
+    let valid_err = s.validate(&json, idx).unwrap_err();
+    let exp = valid_err.to_string();
+    let err: Error = valid_err.into();
+    assert!(matches!(err, Error::ValidationError { .. }));
+    assert_eq!(exp, err.to_string());
+}
+
+#[test]
+fn serde() {
+    let serde_err = serde_json::from_str::<String>("[]").unwrap_err();
+    let exp = serde_err.to_string();
+    let err: Error = serde_err.into();
+    assert!(matches!(err, Error::Serde { .. }));
+    assert_eq!(exp, err.to_string());
+}
+
+#[test]
+fn io() {
+    use std::io;
+    let io_error = io::Error::new(io::ErrorKind::Other, "oh no!");
+    let exp = io_error.to_string();
+    let err: Error = io_error.into();
+    assert!(matches!(err, Error::Io { .. }));
+    assert_eq!(exp, err.to_string());
+}
+
+#[test]
+fn glob() {
+    let build_err = wax::Glob::new("[].json").unwrap_err();
+    let exp = build_err.to_string();
+    let err: Error = build_err.into();
+    assert!(matches!(err, Error::Glob { .. }));
+    assert_eq!(exp, err.to_string());
+
+    let glob = wax::Glob::new("*.json").unwrap();
+    for path in glob.walk("nonesuch 😇") {
+        // Would be nice to just fetch the first, but should always be an
+        // error anyway.
+        let walk_err = path.unwrap_err();
+        let exp = walk_err.to_string();
+        let err: Error = walk_err.into();
+        assert!(matches!(err, Error::Glob { .. }));
+        assert_eq!(exp, err.to_string());
+    }
+}

src/lib.rs

@@ -12,6 +12,7 @@ files. It supports both the [v1] and [v2] specs.
 */
 
 pub mod dist;
+pub mod error;
 pub mod release;
 mod util; // private utilities
 pub mod valid;

src/valid/compiler/mod.rs

@@ -1,9 +1,8 @@
-use relative_path::{Component, RelativePath};
+use crate::error::Error;
 /// Public but undocumented and un-exported module that creates a
 /// boon::Compiler for use in validation and Tests.
-use std::error::Error;
-
 use boon::Compiler;
+use relative_path::{Component, RelativePath};
 use serde_json::Value;
 
 /// new returns a new boon::Compiler with the schema files loaded from `dir`
@@ -18,7 +17,7 @@ pub fn new() -> Compiler {
             let schema: Value = serde_json::from_str(line).unwrap();
             let id = &schema["$id"]
                 .as_str()
-                .ok_or(super::ValidationError::UnknownID)
+                .ok_or(Error::UnknownSchemaId)
                 .unwrap();
             compiler.add_resource(id, schema.to_owned()).unwrap();
         }
@@ -44,27 +43,23 @@ pub fn spec_compiler() -> Compiler {
 }
 
 /// Returns an error if v is not a valid path.
-fn is_path(v: &Value) -> Result<(), Box<dyn Error>> {
-    let Value::String(s) = v else {
-        Err("not a string")?
-    };
+fn is_path(v: &Value) -> Result<(), Box<dyn std::error::Error>> {
+    let Value::String(s) = v else { return Ok(()) };
 
     let path = RelativePath::new(s);
     for c in path.components() {
         if c == Component::ParentDir {
-            Err("parent dir")?
+            Err("references parent dir")?
         };
     }
 
     Ok(())
 }
 
-/// Returns an error if vi is not a valid SPDX license expression.
-fn is_license(v: &Value) -> Result<(), Box<dyn Error>> {
-    let Value::String(s) = v else {
-        Err("not a string")?
-    };
-    _ = spdx::Expression::parse(s)?;
+/// Returns an error if v is not a valid SPDX license expression.
+fn is_license(v: &Value) -> Result<(), Box<dyn std::error::Error>> {
+    let Value::String(s) = v else { return Ok(()) };
+    _ = spdx::Expression::parse(s).map_err(crate::error::Error::License)?;
     Ok(())
 }
 
@@ -96,17 +91,17 @@ mod tests {
         }
 
         // Test invalid paths.
-        for invalid in [
-            json!("../outside/path"),
-            json!("thing/../other"),
-            json!({}),
-            json!([]),
-            json!(true),
-            json!(null),
-            json!(42),
+        for (name, invalid, err) in [
+            ("parent", json!("../outside/path"), "references parent dir"),
+            (
+                "sub parent",
+                json!("thing/../other"),
+                "references parent dir",
+            ),
         ] {
-            if is_path(&invalid).is_ok() {
-                panic!("{} unexpectedly passed!", invalid)
+            match is_path(&invalid) {
+                Ok(_) => panic!("{name} unexpectedly passed!"),
+                Err(e) => assert_eq!(err, e.to_string(), "{name}"),
             }
         }
     }
@@ -137,24 +132,89 @@ mod tests {
         }
 
         // Test invalid licenses.
-        for invalid_license in [
-            json!(""),
-            json!(null),
-            json!("0"),
-            json!(0),
-            json!("\n\t"),
-            json!("()"),
-            json!("AND"),
-            json!("OR"),
+        for (name, invalid_license, reason) in [
+            ("empty string", json!(""), spdx::error::Reason::Empty),
+            ("zero", json!("0"), spdx::error::Reason::UnknownTerm),
+            ("control chars", json!("\n\t"), spdx::error::Reason::Empty),
+            (
+                "parens",
+                json!("()"),
+                spdx::error::Reason::Unexpected(&["<license>", "("]),
+            ),
+            (
+                "and",
+                json!("AND"),
+                spdx::error::Reason::Unexpected(&["<license>", "("]),
+            ),
+            (
+                "or",
+                json!("OR"),
+                spdx::error::Reason::Unexpected(&["<license>", "("]),
+            ),
+        ] {
+            match is_license(&invalid_license) {
+                Ok(_) => panic!("{name} unexpectedly passed!"),
+                Err(e) => assert_eq!(reason.to_string(), e.to_string(), "{name}"),
+            }
+        }
+    }
+
+    #[test]
+    fn test_spec_compiler() -> Result<(), Error> {
+        let mut c = spec_compiler();
+        let id = "format";
+        // Compile simple schema to validate license and path.
+        c.add_resource(
+            id,
+            json!({
+                "type": "object",
+                "properties": {
+                    "path": {
+                        "type": "string",
+                        "format": "path",
+                    },
+                    "license": {
+                        "type": "string",
+                        "format": "license",
+                    }
+                }
+            }),
+        )?;
+
+        let mut schemas = Schemas::new();
+        let idx = c.compile(id, &mut schemas)?;
+
+        for (name, json, err) in [
+            (
+                "empty license",
+                json!({"license": ""}),
+                "at '/license': '' is not valid license: empty expression",
+            ),
+            (
+                "zero license",
+                json!({"license": "0"}),
+                "at '/license': '0' is not valid license: unknown term",
+            ),
+            (
+                "parent path",
+                json!({"path": "../foo"}),
+                "'../foo' is not valid path: references parent dir",
+            ),
         ] {
-            if is_license(&invalid_license).is_ok() {
-                panic!("{} unexpectedly passed!", invalid_license)
+            match schemas.validate(&json, idx) {
+                Ok(_) => panic!("{name} unexpectedly succeeded"),
+                Err(e) => {
+                    println!("{e}");
+                    assert!(e.to_string().contains(err), "{name}")
+                }
             }
         }
+
+        Ok(())
     }
 
     #[test]
-    fn test_new() -> Result<(), Box<dyn Error>> {
+    fn test_new() -> Result<(), Error> {
         let mut compiler = new();
 
         for tc in [("v1", "widget.json"), ("v2", "typical-sql.json")] {