Fix build error due to removed method in libsyntax

[plumenator]

No description provided.

[plumenator]

Please ignore this for now. Looks like I underestimated the change required

[mcarton]

Hum, it looks like since rust-lang/rust#34965, there is no way to create a MultiSpan with several primary spans (it's just 0 or 1, despite primary_spans being Vec<_>).
Cc @jonathandturner, cc @Manishearth

[sophiajt]

@mcarton - Ha, I hadn't noticed that, I just saw that they weren't being used in the compiler. Hmm...

Out of curiosity, how is clippy using multiple primary spans? In the compiler, there are places where multiple primary spans are used, but they generally get their own snippets, so they get turned into SubDiagnostic(s).

I ask because I was hoping to simplify things a bit further.

[mcarton]

@jonathandturner We use them in one function, which builds a suggestion from several snippets (eg. for for a in b we might suggest to change to for c in d).

Do you know another way to build that SubDiagnostic? 😄

[sophiajt]

@mcarton - Ah, so you SubDiagnostic there, too. I guess I was more wondering if it needs to all be primary spans or if you could use secondary spans for part of the message.

[oli-obk]

any update on what to do about this? We could just move to showing two suggestions (and thus two helps)

[mcarton]

@oli-obk no, it's not two suggestions. It's one suggestion made from two different changes. If something must be changed in Clippy, we should play with spans and snippets.
I'd still prefer that we get back support from rustc for something rustc supports anyway.

[Manishearth]

Temporarily we can just get two suggestions and then re-add the support, no?

[mcarton]

Two suggestions wouldn't make sense. If we're to remove that temporarily, just don't make a suggestion.

[mcarton]

As for why I used that feature (it's from my recent big-PR of suggestions improvement I think) while it wasn't used anywhere in rustc itself: it is less error-prone than making spans and cut-and-paste bits of code to make a suggestion. It is also well supported in the two different human-readable error formats and the JSON error format (and for tools such as rustfix, using that feature would be more convenient than having one big suggestion).
If that feature is to be removed from rustc for some reason, there is much more work to do than just removing MultiSpan::from_spans.
I did not had time to make use that feature much in Clippy but I was planning to improve suggestions more.

[sophiajt]

@mcarton - What do you mean by "If that feature is to be removed from rustc for some reason"?

FWIW - these changes shouldn't have any impact on human-readable or JSON errors.

[mcarton]

@jonathandturner we can't create a suggestion from multiple spans anymore, which is less error prone than getting snippets from different ast nodes and trying to build a big suggestions with format! (we mostly do that, but we lose comments, often miss a ‘;’, usually don't include code blocks because they are too big, etc., so I was going to change that).

For example, currently we suggest to change for i in 0..vec.len() to for item in &vec with two spans: i → item and 0..vec.len() → &vec.
Before that we used to suggest to change for i in 0..vec.len() /* some comment here */ { whatever() } to for i in 0..vec.len() { .. } [sic], loosing both the comment (because it was not included in the iteratee span) and the block (because then rustc would think all of it had changed and made a huge error message).

In the JSON format the suggestion for that looks like:

        {
            "message": "consider using an iterator",
            "code": null,
            "level": "help",
            "spans": [
                {
                    "file_name": "tests/compile-fail/for_loop.rs",
                    "byte_start": 3019,
                    "byte_end": 3020,
                    "line_start": 99,
                    "line_end": 99,
                    "column_start": 9,
                    "column_end": 10,
                    "is_primary": true,
                    "text": [
                        {
                            "text": "    for i in 0..vec.len() {",
                            "highlight_start": 9,
                            "highlight_end": 10
                        }
                    ],
                    "label": null,
                    "suggested_replacement": "<item>",
                    "expansion": null
                },
                {
                    "file_name": "tests/compile-fail/for_loop.rs",
                    "byte_start": 3024,
                    "byte_end": 3036,
                    "line_start": 99,
                    "line_end": 99,
                    "column_start": 14,
                    "column_end": 26,
                    "is_primary": true,
                    "text": [
                        {
                            "text": "    for i in 0..vec.len() {",
                            "highlight_start": 14,
                            "highlight_end": 26
                        }
                    ],
                    "label": null,
                    "suggested_replacement": "&vec",
                    "expansion": null
                }
            ],
            "children": [

            ],
            "rendered": "    for <item> in &vec {"
        }

Everything seems to indicate this is not an accidental feature rustc had (even though rustc does not use it; but it also uses actual suggestions like 3 times, most of them are just suggested in an English sentence rather than code-replacement) so I was happy to use it.

[sophiajt]

I mentioned this earlier but maybe it got lost in the shuffle.

pub struct MultiSpan {
    primary_spans: Vec<Span>,
    span_labels: Vec<(Span, String)>,
}

There are other spans associated with errors. I was asking earlier if you need to always be using the primary_spans or if you could use the secondary spans. Secondary spans are mentioned in span_labels but are not one of the ones given in primary_spans. It's the difference between ^^^ and --- in the new error format, where ^^^ is primary and --- is secondary. Can you use the other spans instead?

[mcarton]

No.

Anyway, labels don't make sense for a suggestion, and there is no ^^^ vs --- difference.
Maybe suggestions would need their own MultiSpan type.

[sophiajt]

Ahh, I hadn't seen that assert! Yeah, good point.

Maybe suggestions do want their own type. For now, might as well put the methods back into MultiSpan until we come up with a better solution. If you decide to go that route, feel free to r? me.