Skip to content

Commit

Permalink
Make ValidationError's string representation more informative
Browse files Browse the repository at this point in the history
* Added a source_type and message field to ValidationError.

- ValidationError should have more detail.
- ValidationError::message should maybe have a String so people can use
format! to generate the messages.

* Forgot the changes in lib.

* Made the ValidationError message for Ensure closures lowercase and
remove the period.

* Debug nad Display impls for ValidationError use the message field.

* `ValidationError::message` retyped from `&str` to `String`.

- String allows for error messages constructed using format! and such
macros.

* ConstructionError includes the input value in the error mesasge.

- The message field is removed as there is no need for custom error
  messages yet.
- The type alias is passed given to the Error so it can print it.

* Restored the assert_matches in the tests.

* Made the ConstructionError::guarded_type_name a &'static str.

* Renamed ConstructionError back to ValidationError.

- There was a plan to have two errors, one for Construction and another
for Mutation. This is not going ahead due to it requiring major arch
changes.

* Fixed a sloppy rename.

* Minor fixes

* Add comment about test `test_deps`

* Check if `define!` works with non-clonable types

* Removed value from ValidationError.

- The value would force the users type to implement `Clone`.

* Reinstated value to `ValidationError` but as a `String`.

- This allows the error to store a value without a generic `T: Clone +
Debug`

* Remove junk file

* Fix comment

Co-authored-by: Alex Ryapolov <ryapolov@pm.me>
  • Loading branch information
bheylin and teenjuna committed Jul 9, 2021
1 parent 2d593a4 commit 427c71e
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 34 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ let mut u = Username::new(" valid name \n\n").unwrap();
assert_eq!(u.get(), "valid name"); // now we're talking!

// This also works for mutations:
assert!(matches!(u.try_mutate(|u| *u = " ".to_owned()), Err(prae::ValidationError)));
assert!(matches!(u.try_mutate(|u| *u = " ".to_owned()), Err(prae::ValidationError { .. })))
```

Now our `Username` trims provided value automatically.
Expand Down
1 change: 1 addition & 0 deletions prae/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ prae_macro = { version = "0.2", path = "../prae_macro" }
serde = { version = "1.0", optional = true }

[dev-dependencies]
assert_matches = "1.5"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
33 changes: 21 additions & 12 deletions prae/src/core.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,32 @@
use core::hash::Hash;
use std::fmt;
use std::ops::{Deref, Index};
use std::{error::Error, fmt};

/// Used for [`define!`](prae_macro::define) macro with `ensure` keyword.
#[derive(Clone, Debug)]
pub struct ValidationError {
/// The name of the type where this error originated.
type_name: &'static str,
/// Stringified value that caused the error.
value: String,
}

/// Default validation error. It is used for [`define!`](prae_macro::define) macro with `ensure`
/// keyword.
#[derive(PartialEq)]
pub struct ValidationError;

impl std::error::Error for ValidationError {}

impl fmt::Debug for ValidationError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
writeln!(f, "provided value is not valid")
impl ValidationError {
/// Create a new error with the input value that failed.
pub fn new(type_name: &'static str, value: String) -> Self {
ValidationError { type_name, value }
}
}

impl Error for ValidationError {}

impl fmt::Display for ValidationError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
writeln!(f, "provided value is not valid")
write!(
f,
"failed to create {} from value {}: provided value is invalid",
self.type_name, self.value,
)
}
}

Expand Down
5 changes: 4 additions & 1 deletion prae/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
//! assert_eq!(u.get(), "valid name"); // now we're talking!
//!
//! // This also works for mutations:
//! assert!(matches!(u.try_mutate(|u| *u = " ".to_owned()), Err(prae::ValidationError)));
//! assert!(matches!(u.try_mutate(|u| *u = " ".to_owned()), Err(prae::ValidationError { .. })));
//! ```
//! Now our `Username` trims provided value automatically.
//!
Expand Down Expand Up @@ -77,8 +77,11 @@ mod core;
pub use crate::core::*;
pub use prae_macro::define;

// We need this to silince the unused_crate_dependencies warning.
// See: https://github.com/rust-lang/rust/issues/57274
#[cfg(test)]
mod test_deps {
use assert_matches as _;
use serde as _;
use serde_json as _;
}
15 changes: 10 additions & 5 deletions prae/tests/adjust_ensure.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use assert_matches::assert_matches;

use prae;

prae::define! {
Expand All @@ -8,7 +10,10 @@ prae::define! {

#[test]
fn construction_fails_for_invalid_data() {
assert_eq!(Username::new(" ").unwrap_err(), prae::ValidationError);
assert_matches!(
Username::new(" ").unwrap_err(),
prae::ValidationError { .. }
)
}

#[test]
Expand All @@ -20,10 +25,10 @@ fn construction_succeeds_for_valid_data() {
#[test]
fn mutation_fails_for_invalid_data() {
let mut un = Username::new("user").unwrap();
assert_eq!(
assert_matches!(
un.try_mutate(|u| *u = " ".to_owned()).unwrap_err(),
prae::ValidationError
);
prae::ValidationError { .. }
)
}

#[test]
Expand All @@ -38,4 +43,4 @@ fn mutation_succeeds_for_valid_data() {
let mut un = Username::new("user").unwrap();
assert!(un.try_mutate(|u| *u = " new user ".to_owned()).is_ok());
assert_eq!(un.get(), "new user");
}
}
10 changes: 6 additions & 4 deletions prae/tests/adjust_validate.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use assert_matches::assert_matches;

use prae;

#[derive(Debug, PartialEq)]
#[derive(Debug)]
pub struct UsernameError;

prae::define! {
Expand All @@ -17,7 +19,7 @@ prae::define! {

#[test]
fn construction_fails_for_invalid_data() {
assert_eq!(Username::new(" ").unwrap_err(), UsernameError {});
assert_matches!(Username::new(" ").unwrap_err(), UsernameError {});
}

#[test]
Expand All @@ -29,7 +31,7 @@ fn construction_succeeds_for_valid_data() {
#[test]
fn mutation_fails_for_invalid_data() {
let mut un = Username::new("user").unwrap();
assert_eq!(
assert_matches!(
un.try_mutate(|u| *u = " ".to_owned()).unwrap_err(),
UsernameError {}
);
Expand All @@ -47,4 +49,4 @@ fn mutation_succeeds_for_valid_data() {
let mut un = Username::new("user").unwrap();
assert!(un.try_mutate(|u| *u = " new user ".to_owned()).is_ok());
assert_eq!(un.get(), "new user");
}
}
25 changes: 20 additions & 5 deletions prae/tests/ensure.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use assert_matches::assert_matches;

use prae;

prae::define! {
Expand All @@ -7,7 +9,20 @@ prae::define! {

#[test]
fn construction_fails_for_invalid_data() {
assert_eq!(Username::new("").unwrap_err(), prae::ValidationError);
assert_matches!(
Username::new("").unwrap_err(),
prae::ValidationError { .. }
)
}

#[test]
fn error_formats_correctly() {
let error = Username::new("").unwrap_err();
let message = format!("{}", error);
assert_eq!(
message,
"failed to create Username from value \"\": provided value is invalid"
);
}

#[test]
Expand All @@ -19,10 +34,10 @@ fn construction_succeeds_for_valid_data() {
#[test]
fn mutation_fails_for_invalid_data() {
let mut un = Username::new("user").unwrap();
assert_eq!(
assert_matches!(
un.try_mutate(|u| *u = "".to_owned()).unwrap_err(),
prae::ValidationError
);
prae::ValidationError { .. }
)
}

#[test]
Expand All @@ -37,4 +52,4 @@ fn mutation_succeeds_for_valid_data() {
let mut un = Username::new("user").unwrap();
assert!(un.try_mutate(|u| *u = " new user ".to_owned()).is_ok());
assert_eq!(un.get(), " new user ");
}
}
11 changes: 11 additions & 0 deletions prae/tests/non_clone.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#[derive(Debug)]
struct User {
name: String,
}

prae::define! {
ValidUser: User
ensure |u| !u.name.is_empty()
}


3 changes: 2 additions & 1 deletion prae/tests/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod adjust_ensure;
mod adjust_validate;
mod ensure;
mod validate;
mod non_clone;
mod validate;
10 changes: 6 additions & 4 deletions prae/tests/validate.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use assert_matches::assert_matches;

use prae;

#[derive(Debug, PartialEq)]
#[derive(Debug)]
pub struct UsernameError;

prae::define! {
Expand All @@ -16,7 +18,7 @@ prae::define! {

#[test]
fn construction_fails_for_invalid_data() {
assert_eq!(Username::new("").unwrap_err(), UsernameError {});
assert_matches!(Username::new("").unwrap_err(), UsernameError {});
}

#[test]
Expand All @@ -28,7 +30,7 @@ fn construction_succeeds_for_valid_data() {
#[test]
fn mutation_fails_for_invalid_data() {
let mut un = Username::new("user").unwrap();
assert_eq!(
assert_matches!(
un.try_mutate(|u| *u = "".to_owned()).unwrap_err(),
UsernameError {}
);
Expand All @@ -46,4 +48,4 @@ fn mutation_succeeds_for_valid_data() {
let mut un = Username::new("user").unwrap();
assert!(un.try_mutate(|u| *u = " new user ".to_owned()).is_ok());
assert_eq!(un.get(), " new user ");
}
}
2 changes: 1 addition & 1 deletion prae_macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub fn define(input: TokenStream) -> TokenStream {
GuardClosure::Ensure(EnsureClosure(closure)) => quote! {
fn validate(v: &Self::Target) -> Option<prae::ValidationError> {
let f: fn(&Self::Target) -> bool = #closure;
if f(v) { None } else { Some(prae::ValidationError) }
if f(v) { None } else { Some(prae::ValidationError::new(stringify!(#ident), format!("{:?}", v))) }
}
},
GuardClosure::Validate(ValidateClosure(closure, err_ty)) => quote! {
Expand Down

0 comments on commit 427c71e

Please sign in to comment.