Skip to content

Conversation

@p-lindberg
Copy link

This PR suggests a refactor of enum ReportKind<'a> into trait ReportKind in order to get rid of the lifetime parameter. The default kinds Error, Warning and Advice are instead represented using empty structs that implement the trait. Custom report kinds can now instead be defined by implementing the ReportKind trait for new types (with or without lifetime parameters).

The motivation for this change is that the lifetime on Report, which stems specifically from ReportKind::Custom, only serves a purpose in the specific case where a custom report kind is used with a borrowed string, while being superfluous in other cases. If you're not using custom report kinds and want to be able to return reports from functions or store them in structs, you have to stick with Report<'static> everywhere, which feels unnecessary.

Bonus: a custom report kind should now only be colored if the Config has colors enabled. I assume this was a bug.

Note that this PR contains breaking changes to the public API, specifically the change of ReportKind from enum to trait, and the introduction of default trait implementations replacing the enum variants. If this is a concern, it might be possible to keep the old enum and make function signatures backwards compatible using generics, but I haven't looked into it.

…, avoiding the extra lifetime to support custom report kinds.
@zesterer
Copy link
Owner

zesterer commented Oct 28, 2025

Apologies that I didn't get to this earlier, GitHub has a habit of swallowing notifications.

While I don't think that this is inherently a bad idea, I think it'll cause users a lot of frustration in practice. A lot of compilers tend to push all of their errors/warnings to a vector somewhere, but the introduction of a type parameter based on the report kind means that this can't happen. Code that might previously have remained identical aside from a small 'is this an error or a warning?' test now needs to be duplicated entirely to make the type-checker happy.

Given that report generation tends not to happen in hot codepaths, I think changing the &'a str to a String would also be a satisfying solution though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants