rustfmt
rustfmt copied to clipboard
Don't drop a DiagnosticBuilder (#5413)
I've implemented this fix under the assumption that the error here was not being dropped (causing the panic) intentionally because the rest of the function returns an Err with a message rather than panicking.
I appreciate you opening a PR, and I've not looked at the diff in detail, but please note that there's a lot of subtle complexity with the parser internals. rustc_parse only emits certain classes of errors when elements are dropped, so even seemingly benign changes or ostensible fixes can actually have negative impacts on diagnostics in other scenarios.
Understood. This is my first time contributing to this project so I don't know my way around it well. If there's any diagnostic messages broken by my change I'd be interested in trying to fix those.
No worries, and I meant what I said, definitely appreciate you jumping in! If you're interested in working on rustfmt in general we'd be happy to point to some things that might be fun to work on.
We'll definitely take a look at this too (at some point), it's just one of the most fiddly parts of the codebase where it's really easy to introduce unintended side effects and fairly difficult to catch them, so it tends to require more manual intervention and testing than the small diffs would usually imply in most other contexts