Consolidate warnings, suppression and deduplication#246
Merged
Conversation
b9e1d7f to
505e4d8
Compare
cli: Remove sample warning
505e4d8 to
584251b
Compare
error: Move W04 to `Warnings` error: Move W05 to `Warnings` error: Move W06 to `Warnings` error: Move W07 to `Warnings` error: Move W08 to `Warnings` error: Move W09 to `Warnings` error: Move W10 to `Warnings`
error: Move W11 to `Warnings` error: Move W12 to `Warnings` error: Move W13 to `Warnings` error: Move W14 to `Warnings` error: Deduplicate W15/W01 warnings error: Move W15 to `Warnings`
error: Move W17 to `Warnings`
error: Move W19 to `Warnings` error: Move W20 to `Warnings` error: Move W21 to `Warnings` error: Move W22 to `Warnings` error: Move W23 to `Warnings` error: Move W24 to `Warnings` error: Move W30 to `Warnings` error: Move W31 to `Warnings` error: Move W32 to `Warnings` error: Move uncoded Warning to `Warnings`
e6decd3 to
b8a2940
Compare
b8a2940 to
b7601c1
Compare
The `fancy` feature is mainly used for code annotation, that we don't use at the moment. Since we implement our own warning formatter, we don't actually require this feature
micprog
reviewed
Jan 16, 2026
Member
micprog
left a comment
There was a problem hiding this comment.
I added some minor things, but overall LGTM! I really like the updated output and the cleaner handling of suppressing warnings!
There are some warnings that should actually be help information for an error, which are currently noted as TODOs. Thanks for doing this, I think it's best addressed in an upcoming PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Warning collection
This PR collects the current warnings system into a monolithic
diagnostic::Warningsenum, which has the following benefits:Suppression handling improvements
Additionally, the current warning and error suppression system has been improved a bit. A global resp. static
Diagnosticshandler now takes care of suppression. It is initialized in the beginning with the user defined suppressed warnings. This cleans up the code sincesuppress_warningsdoes not need to be passed around anymore, and whether a warning is suppressed is checked automatically behind the scenes when a Warning is emitted. Further, theDiagnostichandler keeps track of emitted warnings and deduplicates them to avoid redundant warnings. It not only differentiates between the type of warning but also it's content (e.g.SomeWarningAbougPkgforpkg1is treaded as different thanSomeWarningAboutPkgforpkg2).Warning formatting
The new formatting of warnings and help messages is shown below:
(in reality the warnings are also colored properly)
New crates adoption
Two new crates were adopted to reduce boilerplate code and derive warning/help messages, error codes and severity from procedural macros:
mietteThis crate uses the
#[diagnostic]macro to implement traits for the specified enumeration variants that allows it to callhelp(),code()andseverity()on an enum variant.miettecan do a lot more and is mostly used for nice annotations of source code. In the case of Bender this is less relevant since it is not a compiler that reads source code. It could in theory be used to annotate wrongBender.ymlfiles, but this is a bit overkill at the moment, and our current YAML parser does not return positional information of fields in the source.thiserrorThe second crate uses the
#[error]macro to derive theDisplaytrait for the warning message. At the moment, we are not usingthiserrorto its full extend. It also implements automaticErrorconversion and chaining, which will be implemented in a separate PR to make the review easier.TODO:
miette, but suppression is impacted potentially.