[wit-parser] Migrate to structured errors in the AST/package parser#2465
[wit-parser] Migrate to structured errors in the AST/package parser#2465PhoebeSzmucer wants to merge 6 commits intobytecodealliance:mainfrom
Conversation
| } | ||
|
|
||
| #[derive(Debug)] | ||
| pub enum Error { |
There was a problem hiding this comment.
Thsi went away now in favor of just using PackageParseError
|
|
||
| #[non_exhaustive] | ||
| #[derive(Debug, PartialEq, Eq)] | ||
| pub enum PackageParseErrorKind { |
There was a problem hiding this comment.
Note - I named it "package parse", because in the future we might want to expose a dedicated error for the "file parse" stage (a single WIT package can consist of many files).
Parsing is also usually thought of as converting a single file into an AST, so I think mentioning that this is about parsing a whole package makes things a bit clearer.
| /// | ||
| /// On failure returns `Err((self, e))` so the caller can use the source | ||
| /// map for error formatting if needed. | ||
| pub fn parse(self) -> Result<UnresolvedPackageGroup, (Self, PackageParseErrors)> { |
There was a problem hiding this comment.
Note that we're now returning the source map as well. Otherwise the consumers would need to clone the source map if they want to use it for resolving spans (since parse consumes self).
| /// Runs `f` and, on error, attempts to add source highlighting to resolver | ||
| /// error types that still use `anyhow`. Only needed until the resolver is | ||
| /// migrated to structured errors. | ||
| pub(crate) fn rewrite_error<F, T>(&self, f: F) -> anyhow::Result<T> |
There was a problem hiding this comment.
This will go away once the resolver is also migrated to structured errors.
| @@ -1,4 +1,4 @@ | |||
| name `nonexistent` is not defined | |||
| name `nonexistent` does not exist | |||
There was a problem hiding this comment.
Previously we were doing a mix of "does not exist" and "is not defined". I'm going to standarize on "does not exist".
Once the structured error migration is done we might want to audit all error messages and see if anything could be better (now that we can track rich metadata easily).
PhoebeSzmucer
requested review from
alexcrichton
and removed request for
a team
| } | ||
|
|
||
| #[derive(Debug, PartialEq, Eq)] | ||
| pub struct PackageParseErrors(Box<PackageParseErrorKind>); |
There was a problem hiding this comment.
Happy to change this to a struct with #[repr(transparent)] if that's better (is there a practical difference between them?)
1 participant
This PR replaces
anyhowerror handling in the AST/parser laye rwith structuredPackageParseErrorsandPackageParseErrorKindtypes. Errors now carry span information directly, enabling better error diagnostics and better integration with downstream tools that use wit-parser programatically.Part of a larger change: #2460 (see that link for API discussion).