{"action":"opened","issue":{"url":"https://api.github.com/repos/rust-lang-nursery/rustfix/issues/103","repository_url":"https://api.github.com/repos/rust-lang-nursery/rustfix","labels_url":"https://api.github.com/repos/rust-lang-nursery/rustfix/issues/103/labels{/name}","comments_url":"https://api.github.com/repos/rust-lang-nursery/rustfix/issues/103/comments","events_url":"https://api.github.com/repos/rust-lang-nursery/rustfix/issues/103/events","html_url":"https://github.com/rust-lang-nursery/rustfix/issues/103","id":322714718,"number":103,"title":"Handle Insert-only suggestions","user":{"login":"killercup","id":20063,"avatar_url":"https://avatars1.githubusercontent.com/u/20063?v=4","gravatar_id":"","url":"https://api.github.com/users/killercup","html_url":"https://github.com/killercup","followers_url":"https://api.github.com/users/killercup/followers","following_url":"https://api.github.com/users/killercup/following{/other_user}","gists_url":"https://api.github.com/users/killercup/gists{/gist_id}","starred_url":"https://api.github.com/users/killercup/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/killercup/subscriptions","organizations_url":"https://api.github.com/users/killercup/orgs","repos_url":"https://api.github.com/users/killercup/repos","events_url":"https://api.github.com/users/killercup/events{/privacy}","received_events_url":"https://api.github.com/users/killercup/received_events","type":"User","site_admin":false},"labels":[{"id":729514129,"url":"https://api.github.com/repos/rust-lang-nursery/rustfix/labels/good%20first%20issue","name":"good first issue","color":"006b75","default":true},{"id":404338068,"url":"https://api.github.com/repos/rust-lang-nursery/rustfix/labels/help%20wanted","name":"help wanted","color":"128A0C","default":true}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-05-14T08:17:51Z","updated_at":"2018-05-14T08:17:51Z","closed_at":null,"author_association":"COLLABORATOR","body":"## What?\r\n\r\nThere are lints that give suggestions that only insert new snippets, but don't replace anything. An example I've found in https://github.com/rust-lang/rust/pull/50713 was the missing-comma-in-match lint.\r\n\r\n## Examples for \"insert only\"\r\n\r\nLet's look at this example code:\r\n\r\n```rust\r\nfn main() {\r\n    match &Some(3) {\r\n        &None => 1\r\n        &Some(2) => { 3 }\r\n        _ => 2\r\n    };\r\n}\r\n```\r\n\r\nRustc warns us that\r\n\r\n```\r\nerror: expected one of `,`, `.`, `?`, `}`, or an operator, found `=>`\r\n --> src/main.rs:4:18\r\n  |\r\n3 |         &None => 1\r\n  |                   - help: missing a comma here to end this `match` arm\r\n4 |         &Some(2) => { 3 }\r\n  |                  ^^ expected one of `,`, `.`, `?`, `}`, or an operator here\r\n```\r\n\r\nThe interesting part in the JSON diagnostics output is this:\r\n\r\n```json\r\n{\r\n  \"message\": \"expected one of `,`, `.`, `?`, `}`, or an operator, found `=>`\",\r\n  \"children\": [{\r\n    \"message\": \"missing a comma here to end this `match` arm\",\r\n    \"spans\": [{\r\n      \"byte_start\": 51,\r\n      \"byte_end\": 51,\r\n      \"suggested_replacement\": \",\"\r\n    }]\r\n  }]\r\n}\r\n```\r\n\r\nNote how the start and end byte index are the same here, which is the `\\n` in this case.\r\n\r\n<details>\r\n<summary>\r\nFull JSON output\r\n</summary>\r\n\r\n```json\r\n{\r\n  \"message\": \"expected one of `,`, `.`, `?`, `}`, or an operator, found `=>`\",\r\n  \"code\": null,\r\n  \"level\": \"error\",\r\n  \"spans\": [\r\n    {\r\n      \"file_name\": \"./tests/everything/missing-comma.rs\",\r\n      \"byte_start\": 69,\r\n      \"byte_end\": 71,\r\n      \"line_start\": 4,\r\n      \"line_end\": 4,\r\n      \"column_start\": 18,\r\n      \"column_end\": 20,\r\n      \"is_primary\": true,\r\n      \"text\": [\r\n        {\r\n          \"text\": \"        &Some(2) => { 3 }\",\r\n          \"highlight_start\": 18,\r\n          \"highlight_end\": 20\r\n        }\r\n      ],\r\n      \"label\": \"expected one of `,`, `.`, `?`, `}`, or an operator here\",\r\n      \"suggested_replacement\": null,\r\n      \"expansion\": null\r\n    }\r\n  ],\r\n  \"children\": [\r\n    {\r\n      \"message\": \"missing a comma here to end this `match` arm\",\r\n      \"code\": null,\r\n      \"level\": \"help\",\r\n      \"spans\": [\r\n        {\r\n          \"file_name\": \"./tests/everything/missing-comma.rs\",\r\n          \"byte_start\": 51,\r\n          \"byte_end\": 51,\r\n          \"line_start\": 3,\r\n          \"line_end\": 3,\r\n          \"column_start\": 19,\r\n          \"column_end\": 19,\r\n          \"is_primary\": true,\r\n          \"text\": [\r\n            {\r\n              \"text\": \"        &None => 1\",\r\n              \"highlight_start\": 19,\r\n              \"highlight_end\": 19\r\n            }\r\n          ],\r\n          \"label\": null,\r\n          \"suggested_replacement\": \",\",\r\n          \"suggestion_applicability\": \"Unspecified\",\r\n          \"expansion\": null\r\n        }\r\n      ],\r\n      \"children\": [],\r\n      \"rendered\": null\r\n    }\r\n  ],\r\n  \"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\"\r\n}\r\n{\r\n  \"message\": \"aborting due to previous error\",\r\n  \"code\": null,\r\n  \"level\": \"error\",\r\n  \"spans\": [],\r\n  \"children\": [],\r\n  \"rendered\": \"error: aborting due to previous error\\n\\n\"\r\n}\r\n```\r\n\r\n</details>\r\n\r\n## Detect \"insert only\"\r\n\r\nNow, 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:\r\n\r\n```rust\r\nfn main() {\r\n    let x = 42;\r\n}\r\n```\r\n\r\nrendered:\r\n\r\n```\r\nwarning: unused variable: `x`\r\n --> src/main.rs:2:9\r\n  |\r\n2 |     let x = 42;\r\n  |         ^ help: consider using `_x` instead\r\n  |\r\n  = note: #[warn(unused_variables)] on by default\r\n```\r\n\r\nor, in (reduced) JSON:\r\n\r\n```\r\n{\r\n  \"message\": \"unused variable: `x`\",\r\n  \"children\": [{\r\n      \"message\": \"consider using `_x` instead\",\r\n      \"spans\": [{\r\n        \"byte_start\": 20,\r\n        \"byte_end\": 21,\r\n        \"suggested_replacement\": \"_x\",\r\n      }]\r\n  }]\r\n}\r\n```\r\n\r\nSo, 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:\r\n\r\nhttps://github.com/rust-lang-nursery/rustfix/blob/ebca9b67a1af4acd3196d22a2fcfc51f1edea778/src/lib.rs#L199-L202\r\n\r\nIndeed, the original suggestion even makes rustfix bail out of fixing it:\r\n\r\nhttps://github.com/rust-lang-nursery/rustfix/blob/ebca9b67a1af4acd3196d22a2fcfc51f1edea778/src/replace.rs#L64-L69\r\n\r\n## How to fix this\r\n\r\n- Replace the `ensure!` with a `let insert_only = from > up_to_and_including;`\r\n- Special-case based on `insert_only` where we split a chunk into two where the inserted new chunk has a \"useful\" index\r\n- Ensure this works by adding tests that trigger this\r\n- Decide if we want to bail out when appending twice after the same index\r\n"}}
{"actor":{"display_login":"killercup"}}