Handle Insert-only suggestions

[killercup]

What?

There are lints that give suggestions that only insert new snippets, but don't replace anything. An example I've found in rust-lang/rust#50713 was the missing-comma-in-match lint.

Examples for "insert only"

Let's look at this example code:

fn main() {
    match &Some(3) {
        &None => 1
        &Some(2) => { 3 }
        _ => 2
    };
}

Rustc warns us that

error: expected one of `,`, `.`, `?`, `}`, or an operator, found `=>`
 --> src/main.rs:4:18
  |
3 |         &None => 1
  |                   - help: missing a comma here to end this `match` arm
4 |         &Some(2) => { 3 }
  |                  ^^ expected one of `,`, `.`, `?`, `}`, or an operator here

The interesting part in the JSON diagnostics output is this:

{
  "message": "expected one of `,`, `.`, `?`, `}`, or an operator, found `=>`",
  "children": [{
    "message": "missing a comma here to end this `match` arm",
    "spans": [{
      "byte_start": 51,
      "byte_end": 51,
      "suggested_replacement": ","
    }]
  }]
}

Note how the start and end byte index are the same here, which is the \n in this case.

Full JSON output

{
  "message": "expected one of `,`, `.`, `?`, `}`, or an operator, found `=>`",
  "code": null,
  "level": "error",
  "spans": [
    {
      "file_name": "./tests/everything/missing-comma.rs",
      "byte_start": 69,
      "byte_end": 71,
      "line_start": 4,
      "line_end": 4,
      "column_start": 18,
      "column_end": 20,
      "is_primary": true,
      "text": [
        {
          "text": "        &Some(2) => { 3 }",
          "highlight_start": 18,
          "highlight_end": 20
        }
      ],
      "label": "expected one of `,`, `.`, `?`, `}`, or an operator here",
      "suggested_replacement": null,
      "expansion": null
    }
  ],
  "children": [
    {
      "message": "missing a comma here to end this `match` arm",
      "code": null,
      "level": "help",
      "spans": [
        {
          "file_name": "./tests/everything/missing-comma.rs",
          "byte_start": 51,
          "byte_end": 51,
          "line_start": 3,
          "line_end": 3,
          "column_start": 19,
          "column_end": 19,
          "is_primary": true,
          "text": [
            {
              "text": "        &None => 1",
              "highlight_start": 19,
              "highlight_end": 19
            }
          ],
          "label": null,
          "suggested_replacement": ",",
          "suggestion_applicability": "Unspecified",
          "expansion": null
        }
      ],
      "children": [],
      "rendered": null
    }
  ],
  "rendered": "error: expected one of `,`, `.`, `?`, `}`, or an operator, found `=>`\n --> ./tests/everything/missing-comma.rs:4:18\n  |\n3 |         &None => 1\n  |                   - help: missing a comma here to end this `match` arm\n4 |         &Some(2) => { 3 }\n  |                  ^^ expected one of `,`, `.`, `?`, `}`, or an operator here\n\n"
}
{
  "message": "aborting due to previous error",
  "code": null,
  "level": "error",
  "spans": [],
  "children": [],
  "rendered": "error: aborting due to previous error\n\n"
}

Detect "insert only"

Now, I was wondering: Is the byte_start == byte_end a good enough indicator for "insert only"? Or is it also used for "replace the character at position X"? I've come up with the following example that triggers a "replaces only one char" suggestion:

fn main() {
    let x = 42;
}

rendered:

warning: unused variable: `x`
 --> src/main.rs:2:9
  |
2 |     let x = 42;
  |         ^ help: consider using `_x` instead
  |
  = note: #[warn(unused_variables)] on by default

or, in (reduced) JSON:

{
  "message": "unused variable: `x`",
  "children": [{
      "message": "consider using `_x` instead",
      "spans": [{
        "byte_start": 20,
        "byte_end": 21,
        "suggested_replacement": "_x",
      }]
  }]
}

So, usually the byte range seems to span at least one character. Which is great -- because rustfix uses inclusive ranges internally and it totally depends on this behavior right now:

https://github.com/rust-lang-nursery/rustfix/blob/ebca9b67a1af4acd3196d22a2fcfc51f1edea778/src/lib.rs#L199-L202

Indeed, the original suggestion even makes rustfix bail out of fixing it:

https://github.com/rust-lang-nursery/rustfix/blob/ebca9b67a1af4acd3196d22a2fcfc51f1edea778/src/replace.rs#L64-L69

How to fix this

• Replace the ensure! with a let insert_only = from > up_to_and_including;
• Special-case based on insert_only where we split a chunk into two where the inserted new chunk has a "useful" index
• Ensure this works by adding tests that trigger this
• Decide if we want to bail out when appending twice after the same index

[yaahc]

I'd like to try my hand at this issue.

[killercup]

Gladly! Ping me here, on gitter, irc.mozilla.org, or discord if you need any help!

[killercup]

Fixed in #107