Improving error messages #1129
Improving error messages #1129
Conversation
Signed-off-by: Aaron Eline <aeline+github@amazon.com>
Signed-off-by: Aaron Eline <aeline+github@amazon.com>
aaronjeline
force-pushed
the
aeline/fix_errors
branch
from
35abe79 to
149f0b6
Compare
Signed-off-by: Aaron Eline <aeline+github@amazon.com>
Signed-off-by: Aaron Eline <aeline+github@amazon.com>
Signed-off-by: Aaron Eline <aeline+github@amazon.com>
|
Semver checks expected to fail |
|
It seems this PR overlaps with #1130. I'll close that one in favor this PR. |
Signed-off-by: Aaron Eline <aeline+github@amazon.com>
| @@ -201,6 +204,15 @@ impl Display for ParseErrors { | |||
| } | |||
| } | |||
|
|
|||
| impl IntoIterator for ParseErrors { | |||
| type Item = ParseError; | |||
| type IntoIter = Chain<Once<ParseError>, vec::IntoIter<ParseError>>; | |||
There was a problem hiding this comment.
What's an idiomatic type we should use here? The current one seems overly complicated.
There was a problem hiding this comment.
Eh it's a combination of types that are all in the stdlib. Unless we want to re-implement the underlying iterators this seems like the best.
There was a problem hiding this comment.
Another option is type IntoIter = <NonEmpty as IntoIterator>::IntoIter;
There was a problem hiding this comment.
If this is part of our public API then I think we should expose a nicer iterator type. In other parts of our API we use a wrapper struct that holds the actual iter in a private field.
| @@ -250,14 +262,19 @@ impl Diagnostic for ParseErrors { | |||
| } | |||
| } | |||
|
|
|||
| /// Collection of [`ToJsonSchemaError`] | |||
| /// This collection is guaranteed (by construction) to have at least one error. | |||
There was a problem hiding this comment.
Nit: remove this and change the above line to Non-empty collection of ....
| #[error(transparent)] | ||
| #[diagnostic(transparent)] | ||
| DuplicateContext(DuplicateContext), | ||
| /// Error raised when a `principal` or `resource` is declared twice |
There was a problem hiding this comment.
Nit: twice to multiple times.
Description of changes
Cleans up two remaining error tasks
EntityUid::from_jsonreturns a concrete type instead of an existentialCedarSchemaError::Parseis more transparentIssue #, if available
#745
Checklist for requesting a review
The change in this PR is (choose one, and delete the other options):
cedar-policy(e.g., changes to the signature of an existing API).I confirm that this PR (choose one, and delete the other options):
I confirm that
cedar-spec(choose one, and delete the other options):