Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong coloring on "consider removing this method call" #134485

Closed
est31 opened this issue Dec 18, 2024 · 3 comments · Fixed by #134664
Closed

Wrong coloring on "consider removing this method call" #134485

est31 opened this issue Dec 18, 2024 · 3 comments · Fixed by #134664
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@est31
Copy link
Member

est31 commented Dec 18, 2024

Consider this code:

use std::collections::{HashMap, HashSet};
fn foo() -> Vec<(bool, HashSet<u8>)> {
	let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
	hm.into_iter()
		.map(|(is_true, ts)| {
			ts.into_iter()
				.map(|t| (is_true, t))
				.flatten()
		})
		.flatten()
		.collect()
}

It gives:

error[E0277]: `(bool, HashSet<u8>)` is not an iterator
    --> a.rs:8:6
     |
8    |                 .flatten()
     |                  ^^^^^^^ `(bool, HashSet<u8>)` is not an iterator
     |
     = help: the trait `Iterator` is not implemented for `(bool, HashSet<u8>)`
     = note: required for `(bool, HashSet<u8>)` to implement `IntoIterator`
note: required by a bound in `flatten`
    --> src/rust/library/core/src/iter/traits/iterator.rs:1517:21
     |
1514 |     fn flatten(self) -> Flatten<Self>
     |        ------- required by a bound in this associated function
...
1517 |         Self::Item: IntoIterator,
     |                     ^^^^^^^^^^^^ required by this bound in `Iterator::flatten`
help: consider removing this method call, as the receiver has type `std::vec::IntoIter<HashSet<u8>>` and `std::vec::IntoIter<HashSet<u8>>: Iterator` trivially holds
     |
6    -             ts.into_iter()
7    -                 .map(|t| (is_true, t))
6    +             ts.into_iter()
     |

error[E0277]: `(bool, HashSet<u8>)` is not an iterator
   --> a.rs:4:2
    |
4   | /     hm.into_iter()
5   | |         .map(|(is_true, ts)| {
6   | |             ts.into_iter()
7   | |                 .map(|t| (is_true, t))
8   | |                 .flatten()
9   | |         })
    | |__________^ `(bool, HashSet<u8>)` is not an iterator
    |
    = help: the trait `Iterator` is not implemented for `(bool, HashSet<u8>)`
    = note: required for `(bool, HashSet<u8>)` to implement `IntoIterator`
note: required by a bound in `Flatten`
   --> src/rust/library/core/src/iter/adapters/flatten.rs:254:38
    |
254 | pub struct Flatten<I: Iterator<Item: IntoIterator>> {
    |                                      ^^^^^^^^^^^^ required by this bound in `Flatten`

error[E0599]: the method `collect` exists for struct `Flatten<Map<IntoIter<bool, Vec<HashSet<u8>>>, ...>>`, but its trait bounds were not satisfied
   --> a.rs:11:4
    |
4   | /     hm.into_iter()
5   | |         .map(|(is_true, ts)| {
6   | |             ts.into_iter()
7   | |                 .map(|t| (is_true, t))
...   |
10  | |         .flatten()
11  | |         .collect()
    | |         -^^^^^^^ method cannot be called due to unsatisfied trait bounds
    | |_________|
    |
    |
   ::: src/rust/library/core/src/iter/adapters/flatten.rs:254:1
    |
254 |   pub struct Flatten<I: Iterator<Item: IntoIterator>> {
    |   --------------------------------------------------- doesn't satisfy `<_ as IntoIterator>::IntoIter = _`, `<_ as IntoIterator>::Item = _`, `_: IntoIterator` or `_: Iterator`
    |
    = note: the full type name has been written to 'a.long-type-10511036807638515192.txt'
    = note: consider using `--verbose` to print the full type name to the console
    = note: the following trait bounds were not satisfied:
            `<Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@a.rs:7:10: 7:13}>> as IntoIterator>::IntoIter = _`
            which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@a.rs:5:8: 5:23}>>: Iterator`
            `<Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@a.rs:7:10: 7:13}>> as IntoIterator>::Item = _`
            which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@a.rs:5:8: 5:23}>>: Iterator`
            `Flatten<Map<std::vec::IntoIter<HashSet<u8>>, {closure@a.rs:7:10: 7:13}>>: IntoIterator`
            which is required by `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@a.rs:5:8: 5:23}>>: Iterator`
            `Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@a.rs:5:8: 5:23}>>: Iterator`
            which is required by `&mut Flatten<Map<std::collections::hash_map::IntoIter<bool, Vec<HashSet<u8>>>, {closure@a.rs:5:8: 5:23}>>: Iterator`

My bug report is about the coloring in this particular suggestion:

Image

removing the red parts will obviously make the situation worse and cause parsing errors.

Even if you remove the entire map function call, it won't fix all the errors, what is needed is the removal of flatten. I want to make this particular issue about the wrong coloring of that red part: there is some mistake about it.

Appears on latest nightly rustc 1.85.0-nightly (a4cb3c831 2024-12-17) as well as beta rustc 1.84.0-beta.4 (202008a1b 2024-12-07).

@est31 est31 added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 18, 2024
@est31 est31 changed the title wrong coloring of suggestion Wrong suggestion coloring on "consider removing this method call" Dec 18, 2024
@est31 est31 changed the title Wrong suggestion coloring on "consider removing this method call" Wrong coloring on "consider removing this method call" Dec 18, 2024
@ionicmc-rs
Copy link
Contributor

why didnt you just give the part that had the incorrect color... but anyways thats kinda weird

@jieyouxu jieyouxu added D-confusing Diagnostics: Confusing error or lint that should be reworked. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` labels Dec 19, 2024
@jieyouxu
Copy link
Member

I suspect it's the suggestion using some replacement span which is not quite accurate to the syntax.

@estebank
Copy link
Contributor

The issue is that the highlighting logic doesn't account for "the removed snippet is multiline". We need to look for newlines in the part when the "after" code is an empty string. I'm looking at a way to fix it at the moment.

Thank you for the report! I've noticed some highlights being off, but never managed to capture a good repro when encountering them. This is very useful!

@estebank estebank self-assigned this Dec 22, 2024
estebank added a commit to estebank/rust that referenced this issue Dec 22, 2024
When highlighting the removed parts of a suggestion, properly account for spans that cover more than one line.

Fix rust-lang#134485.
estebank added a commit to estebank/rust that referenced this issue Dec 22, 2024
When highlighting the removed parts of a suggestion, properly account for spans that cover more than one line.

Fix rust-lang#134485.
estebank added a commit to estebank/rust that referenced this issue Dec 22, 2024
When highlighting the removed parts of a suggestion, properly account for spans that cover more than one line.

Fix rust-lang#134485.
estebank added a commit to estebank/rust that referenced this issue Dec 25, 2024
When highlighting the removed parts of a suggestion, properly account for spans that cover more than one line.

Fix rust-lang#134485.
estebank added a commit to estebank/rust that referenced this issue Dec 26, 2024
When highlighting the removed parts of a suggestion, properly account for spans that cover more than one line.

Fix rust-lang#134485.
jieyouxu added a commit to jieyouxu/rust that referenced this issue Dec 26, 2024
…ouxu

Account for removal of multiline span in suggestion

When highlighting the removed parts of a suggestion, properly account for spans that cover more than one line.

Fix rust-lang#134485.

![Screenshot of the highlighted output](https://github.com/user-attachments/assets/18bcd9bc-3bec-4b79-a9d7-e4ea4e6289ad)
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Dec 26, 2024
…ouxu

Account for removal of multiline span in suggestion

When highlighting the removed parts of a suggestion, properly account for spans that cover more than one line.

Fix rust-lang#134485.

![Screenshot of the highlighted output](https://github.com/user-attachments/assets/18bcd9bc-3bec-4b79-a9d7-e4ea4e6289ad)
@bors bors closed this as completed in 12d66d9 Dec 26, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Dec 26, 2024
Rollup merge of rust-lang#134664 - estebank:sugg-highlighting, r=jieyouxu

Account for removal of multiline span in suggestion

When highlighting the removed parts of a suggestion, properly account for spans that cover more than one line.

Fix rust-lang#134485.

![Screenshot of the highlighted output](https://github.com/user-attachments/assets/18bcd9bc-3bec-4b79-a9d7-e4ea4e6289ad)
poliorcetics pushed a commit to poliorcetics/rust that referenced this issue Dec 28, 2024
…ouxu

Account for removal of multiline span in suggestion

When highlighting the removed parts of a suggestion, properly account for spans that cover more than one line.

Fix rust-lang#134485.

![Screenshot of the highlighted output](https://github.com/user-attachments/assets/18bcd9bc-3bec-4b79-a9d7-e4ea4e6289ad)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants