Make parse error suggestions verbose and fix spans #127407

estebank · 2024-07-06T03:09:21Z

Go over all structured parser suggestions and make them verbose style.

When suggesting to add or remove delimiters, turn them into multiple suggestion parts.

rustbot · 2024-07-06T03:09:28Z

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

oli-obk · 2024-07-08T08:21:51Z

Go over all structured parser suggestions and make them verbose style.

why? Most of these seemed fine to me

When suggesting to add or remove delimiters, turn them into multiple suggestion parts.

For these It makes sense

tests/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr

tests/ui/did_you_mean/issue-41679-tilde-bitwise-negation-attempt.stderr

tests/ui/enum/nested-enum.stderr

tests/ui/generics/issue-95208-ignore-qself.stderr

tests/ui/generics/single-colon-path-not-const-generics.stderr

tests/ui/impl-trait/extra-impl-in-trait-impl.stderr

tests/ui/macros/missing-bang-in-decl.stderr

bors · 2024-07-10T21:22:05Z

☔ The latest upstream changes (presumably #127419) made this pull request unmergeable. Please resolve the merge conflicts.

Go over all structured parser suggestions and make them verbose style. When suggesting to add or remove delimiters, turn them into multiple suggestion parts.

oli-obk · 2024-07-12T08:26:31Z

As discussed in our 1:1, this needs a motivation and some links to annotate snippets discussions

estebank · 2024-07-12T16:26:31Z

The initial impetus to start this work is to facilitate the migration to annotate-snippets, which doesn't yet support suggestions, but @Muscraft is working on. Sadly the conversations were 1:1, but the gist of them is:

a precondition for migrating is minimal changes to the output, we'll accept small divergences as long as they are reasonable, but we're aiming for 100% mimicry
annotate-snippets is unlikely to ever support "inline suggestions"
rustc is the one dealing with json output where these are relevant, so rustc can continue transforming suggestions into labels for annotate-snippets' sake
Minimizing how many of the inline suggestions exist will help with the migration
The inline suggestions were the first ones to be implemented; over time we supported multiple suggestions, suggestions with multiple parts, and rendering these suggestions with underlines for addition and replacements and a diff format for removals. I am of the opinion that the "verbose" renderings are easier to understand for people for any case beyond "add an x here" or remove this x`", particularly because suggestions where the code is similar in both cases it is easier to see the difference when presented side by side (people are already used to the patch format that diff tools use), color further aids with this. For TTS users, none of the available outputs are good.
Making suggestions verbose is dredging up several suggestions that were incorrectly written (bad spans, using them as span_help, bad code being suggested) that were harder to spot with the inline suggestions; there are also many cases where the suggestion message is something non-descriptive like "try:", which also needs to be changed (editors can show the message without the code, so the message should be enough for someone that understands what's going on to fix their own code)

The additional commits after the one you initially reviewed are addressing your review comments.

oli-obk · 2024-07-15T10:20:31Z

@bors r+

bors · 2024-07-15T10:20:33Z

📌 Commit 71f16bd has been approved by oli-obk

It is now in the queue for this repository.

…-obk Make parse error suggestions verbose and fix spans Go over all structured parser suggestions and make them verbose style. When suggesting to add or remove delimiters, turn them into multiple suggestion parts.

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#124921 (offset_from: always allow pointers to point to the same address) - rust-lang#127407 (Make parse error suggestions verbose and fix spans) - rust-lang#127675 (Remove invalid help diagnostics for const pointer) - rust-lang#127684 (consolidate miri-unleashed tests for mutable refs into one file) - rust-lang#127758 (coverage: Restrict `ExpressionUsed` simplification to `Code` mappings) r? `@ghost` `@rustbot` modify labels: rollup

…-obk Make parse error suggestions verbose and fix spans Go over all structured parser suggestions and make them verbose style. When suggesting to add or remove delimiters, turn them into multiple suggestion parts.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#124921 (offset_from: always allow pointers to point to the same address) - rust-lang#127407 (Make parse error suggestions verbose and fix spans) - rust-lang#127684 (consolidate miri-unleashed tests for mutable refs into one file) - rust-lang#127729 (Stop using the `gen` identifier in the compiler) - rust-lang#127736 (Add myself to the review rotation) - rust-lang#127758 (coverage: Restrict `ExpressionUsed` simplification to `Code` mappings) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#127407 - estebank:parser-suggestions, r=oli-obk Make parse error suggestions verbose and fix spans Go over all structured parser suggestions and make them verbose style. When suggesting to add or remove delimiters, turn them into multiple suggestion parts.

estebank · 2024-07-16T16:45:18Z

tests/ui/parser/bad-char-literals.stderr

+help: escape the character
+   |
+LL |     '\r';
+   |      ++


I'm noticing that the renderer considers any whitespace as "not there" for the purposes of deciding whether this is a replacement or addition (this is normally done to have better output), but it has the side effect of rendering not quite correctly in the face of suggestions to replace zero-width whitespace characters.

rustbot assigned oli-obk Jul 6, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 6, 2024