Fix presentation of purely "additive" replacement suggestion parts #136958

compiler-errors · 2025-02-13T03:02:16Z

#127541 changes replacement suggestions to use the "diff" view always, which I think is really verbose in cases where a replacement snippet is a "superset" of the snippet that is being replaced.

Consider:

LL -     Self::Baz: Clone,
LL +     Self::Baz: Clone, T: std::clone::Clone

In this code, we suggest replacing ", " with ", T: std::clone::Clone". This is a consequence of how the snippet is constructed. I believe that since the string that is being replaced is a subset of the replacement string, it's not providing much value to present this as a diff. Users should be able to clearly understand what's being suggested here using the ~ underline view we've been suggesting for some time now.

Given that this affects ~100 tests out of the ~1000 UI tests affected, I expect this to be a pretty meaningful improvement of the fallout of #127541.

In the last commit, this PR also "trims" replacement parts so that they are turned into their purely additive subset, if possible. See the diff for what this means.

r? estebank

rustbot · 2025-02-13T04:30:56Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

compiler-errors · 2025-02-13T05:52:09Z

src/tools/clippy/tests/ui/implicit_return.stderr

@@ -8,8 +8,7 @@ LL |     true
   = help: to override `-D warnings` add `#[allow(clippy::implicit_return)]`
 help: add `return` as shown
   |
-LL -     true
-LL +     return true
+LL |     return true


this renders strangely since it has no addition +'s, but it's exactly how it rendered before #127541, so it's a preexisting bug in the emitter.

compiler/rustc_errors/src/emitter.rs

estebank · 2025-02-13T20:05:30Z

@bors r+

bors · 2025-02-13T20:05:33Z

📌 Commit 1b5337c has been approved by estebank

It is now in the queue for this repository.

…nt, r=estebank Fix presentation of purely "additive" replacement suggestion parts rust-lang#127541 changes replacement suggestions to use the "diff" view always, which I think is really verbose in cases where a replacement snippet is a "superset" of the snippet that is being replaced. Consider: ``` LL - Self::Baz: Clone, LL + Self::Baz: Clone, T: std::clone::Clone ``` In this code, we suggest replacing `", "` with `", T: std::clone::Clone"`. This is a consequence of how the snippet is constructed. I believe that since the string that is being replaced is a subset of the replacement string, it's not providing much value to present this as a diff. Users should be able to clearly understand what's being suggested here using the `~` underline view we've been suggesting for some time now. Given that this affects ~100 tests out of the ~1000 UI tests affected, I expect this to be a pretty meaningful improvement of the fallout of rust-lang#127541. --- In the last commit, this PR also "trims" replacement parts so that they are turned into their purely additive subset, if possible. See the diff for what this means. --- r? estebank

workingjubilee · 2025-02-14T01:43:15Z

Sequences with #136869 so one must be rebased over the other

@bors r-

workingjubilee · 2025-02-14T08:50:13Z

rebased

workingjubilee · 2025-02-14T08:50:19Z

@bors r=estebank

bors · 2025-02-14T08:50:23Z

📌 Commit 6d71251 has been approved by estebank

It is now in the queue for this repository.

…nt, r=estebank Fix presentation of purely "additive" replacement suggestion parts rust-lang#127541 changes replacement suggestions to use the "diff" view always, which I think is really verbose in cases where a replacement snippet is a "superset" of the snippet that is being replaced. Consider: ``` LL - Self::Baz: Clone, LL + Self::Baz: Clone, T: std::clone::Clone ``` In this code, we suggest replacing `", "` with `", T: std::clone::Clone"`. This is a consequence of how the snippet is constructed. I believe that since the string that is being replaced is a subset of the replacement string, it's not providing much value to present this as a diff. Users should be able to clearly understand what's being suggested here using the `~` underline view we've been suggesting for some time now. Given that this affects ~100 tests out of the ~1000 UI tests affected, I expect this to be a pretty meaningful improvement of the fallout of rust-lang#127541. --- In the last commit, this PR also "trims" replacement parts so that they are turned into their purely additive subset, if possible. See the diff for what this means. --- r? estebank

…kingjubilee Rollup of 10 pull requests Successful merges: - rust-lang#135778 (account for `c_enum_min_bits` in `multiple-reprs` UI test) - rust-lang#136052 (Correct comment for FreeBSD and DragonFly BSD in unix/thread) - rust-lang#136886 (Remove the common prelude module) - rust-lang#136938 (Remove `:` from `stack-protector-heuristics-effect.rs` Filecheck Pattern) - rust-lang#136956 (add vendor directory to .gitignore) - rust-lang#136958 (Fix presentation of purely "additive" replacement suggestion parts) - rust-lang#136967 (Use `slice::fill` in `io::Repeat` implementation) - rust-lang#136976 (alloc boxed: docs: use MaybeUninit::write instead of as_mut_ptr) - rust-lang#137007 (Emit MIR for each bit with on `dont_reset_cast_kind_without_updating_operand`) - rust-lang#137008 (Move code into `rustc_mir_transform`) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#135778 (account for `c_enum_min_bits` in `multiple-reprs` UI test) - rust-lang#136052 (Correct comment for FreeBSD and DragonFly BSD in unix/thread) - rust-lang#136886 (Remove the common prelude module) - rust-lang#136956 (add vendor directory to .gitignore) - rust-lang#136958 (Fix presentation of purely "additive" replacement suggestion parts) - rust-lang#136967 (Use `slice::fill` in `io::Repeat` implementation) - rust-lang#136976 (alloc boxed: docs: use MaybeUninit::write instead of as_mut_ptr) - rust-lang#137007 (Emit MIR for each bit with on `dont_reset_cast_kind_without_updating_operand`) - rust-lang#137008 (Move code into `rustc_mir_transform`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#136958 - compiler-errors:additive-replacmeent, r=estebank Fix presentation of purely "additive" replacement suggestion parts rust-lang#127541 changes replacement suggestions to use the "diff" view always, which I think is really verbose in cases where a replacement snippet is a "superset" of the snippet that is being replaced. Consider: ``` LL - Self::Baz: Clone, LL + Self::Baz: Clone, T: std::clone::Clone ``` In this code, we suggest replacing `", "` with `", T: std::clone::Clone"`. This is a consequence of how the snippet is constructed. I believe that since the string that is being replaced is a subset of the replacement string, it's not providing much value to present this as a diff. Users should be able to clearly understand what's being suggested here using the `~` underline view we've been suggesting for some time now. Given that this affects ~100 tests out of the ~1000 UI tests affected, I expect this to be a pretty meaningful improvement of the fallout of rust-lang#127541. --- In the last commit, this PR also "trims" replacement parts so that they are turned into their purely additive subset, if possible. See the diff for what this means. --- r? estebank

More sophisticated span trimming for suggestions Previously rust-lang#136958 only cared about prefixes or suffixes. Now it detects more cases where a suggestion is "sandwiched" by unchanged code on the left or the right. Would be cool if we could detect several insertions, like `ACE` going to `ABCDE`, extracting `B` and `D`, but that seems unwieldy. r? `@estebank`

More sophisticated span trimming for suggestions Previously rust-lang#136958 only cared about prefixes or suffixes. Now it detects more cases where a suggestion is "sandwiched" by unchanged code on the left or the right. Would be cool if we could detect several insertions, like `ACE` going to `ABCDE`, extracting `B` and `D`, but that seems unwieldy. r? ``@estebank``

More sophisticated span trimming for suggestions Previously rust-lang#136958 only cared about prefixes or suffixes. Now it detects more cases where a suggestion is "sandwiched" by unchanged code on the left or the right. Would be cool if we could detect several insertions, like `ACE` going to `ABCDE`, extracting `B` and `D`, but that seems unwieldy. r? `@estebank`

rustbot assigned estebank Feb 13, 2025

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. labels Feb 13, 2025

compiler-errors changed the title ~~Use underline suggestions for purely 'additive' replacements~~ Fix presentation of purely "additive" replacement suggestion parts Feb 13, 2025

This comment has been minimized.

Sign in to view

compiler-errors force-pushed the additive-replacmeent branch from 469191f to 1b5337c Compare February 13, 2025 04:30

compiler-errors commented Feb 13, 2025

View reviewed changes

estebank approved these changes Feb 13, 2025

View reviewed changes

compiler/rustc_errors/src/emitter.rs Show resolved Hide resolved

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 13, 2025

workingjubilee mentioned this pull request Feb 14, 2025

Rollup of 12 pull requests #136999

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 14, 2025

workingjubilee closed this Feb 14, 2025

workingjubilee reopened this Feb 14, 2025

This comment has been minimized.

Sign in to view

workingjubilee added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Feb 14, 2025

compiler-errors added 3 commits February 14, 2025 00:27

Use underline suggestions for purely 'additive' replacements

b480a92

Consider add-prefix replacements too

f6406df

Trim suggestion parts to the subset that is purely additive

6d71251

workingjubilee force-pushed the additive-replacmeent branch from 1b5337c to 6d71251 Compare February 14, 2025 08:49

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 14, 2025

workingjubilee mentioned this pull request Feb 14, 2025

Rollup of 10 pull requests #137019

Closed

matthiaskrgr mentioned this pull request Feb 14, 2025

Rollup of 9 pull requests #137030

Merged

bors merged commit 49fb61c into rust-lang:master Feb 14, 2025
6 checks passed

rustbot added this to the 1.86.0 milestone Feb 14, 2025

zachs18 mentioned this pull request Feb 18, 2025

Suggest replacing . with :: in more error diagnostics. #136344

Merged

estebank mentioned this pull request Feb 20, 2025

Make some suggestions "verbose" #137343

Open

compiler-errors mentioned this pull request Feb 20, 2025

More sophisticated span trimming for suggestions #137348

Merged

fmease mentioned this pull request Feb 25, 2025

fix: overflowing bin hex #136424

Merged

compiler-errors mentioned this pull request Jun 3, 2025

Tracking Issue for improving suggestion rendering in diagnostics #141973

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix presentation of purely "additive" replacement suggestion parts #136958

Fix presentation of purely "additive" replacement suggestion parts #136958

Uh oh!

compiler-errors commented Feb 13, 2025 •

edited

Loading

Uh oh!

This comment has been minimized.

rustbot commented Feb 13, 2025

Uh oh!

compiler-errors Feb 13, 2025

Uh oh!

Uh oh!

estebank commented Feb 13, 2025

Uh oh!

bors commented Feb 13, 2025

Uh oh!

workingjubilee commented Feb 14, 2025

Uh oh!

This comment has been minimized.

workingjubilee commented Feb 14, 2025

Uh oh!

workingjubilee commented Feb 14, 2025

Uh oh!

bors commented Feb 14, 2025

Uh oh!

Uh oh!

Uh oh!

Fix presentation of purely "additive" replacement suggestion parts #136958

Fix presentation of purely "additive" replacement suggestion parts #136958

Uh oh!

Conversation

compiler-errors commented Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

rustbot commented Feb 13, 2025

Uh oh!

compiler-errors Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

estebank commented Feb 13, 2025

Uh oh!

bors commented Feb 13, 2025

Uh oh!

workingjubilee commented Feb 14, 2025

Uh oh!

This comment has been minimized.

workingjubilee commented Feb 14, 2025

Uh oh!

workingjubilee commented Feb 14, 2025

Uh oh!

bors commented Feb 14, 2025

Uh oh!

Uh oh!

Uh oh!

compiler-errors commented Feb 13, 2025 •

edited

Loading