Skip to content
This repository was archived by the owner on Nov 24, 2023. It is now read-only.

Don't eat laundry #1

Closed
killercup opened this issue Jul 7, 2016 · 4 comments
Closed

Don't eat laundry #1

killercup opened this issue Jul 7, 2016 · 4 comments

Comments

@killercup
Copy link
Member

Currently, it replaces the wrong bytes in my input file examples. I haven't investigated if this is an error in the JSON input (it blindly trusts byte_start/byte_end) or some mistake on my end.

@killercup
Copy link
Member Author

I'm now using the line number to edit the files (but totally ignore the columns). This works for now but we should replace it with something nice later on.

@mcarton
Copy link
Member

mcarton commented Jul 9, 2016

Not all suggestions can be applied trivially (e.g. clippy's "You should use Default instead of that fn new() you just wrote"-lint will replace your new-method with an impl block -- which is obvisouly invalid syntax.)

I just fixed that lint locally¹. The way it works is that it “replaces” a 0-width span with the suggestion before the code. The json of that looks like:

{"message":"you should consider deriving a `Default` implementation for `Foo`","code":null,"level":"error","spans":[{"file_name":"tests/compile-fail/new_without_default.rs","byte_start":162,"byte_end":189,"line_start":10,"line_end":10,"column_start":5,"column_end":32,"is_primary":true,"text":[{"text":"    pub fn new() -> Foo { Foo }","highlight_start":5,"highlight_end":32}],"label":null,"suggested_replacement":null,"expansion":null}],"children":[{"message":"lint level defined here","code":null,"level":"note","spans":[{"file_name":"tests/compile-fail/new_without_default.rs","byte_start":100,"byte_end":126,"line_start":5,"line_end":5,"column_start":30,"column_end":56,"is_primary":true,"text":[{"text":"#![deny(new_without_default, new_without_default_derive)]","highlight_start":30,"highlight_end":56}],"label":null,"suggested_replacement":null,"expansion":null}],"children":[],"rendered":null},{"message":"try this","code":null,"level":"help","spans":[{"file_name":"tests/compile-fail/new_without_default.rs","byte_start":162,"byte_end":162,"line_start":10,"line_end":10,"column_start":5,"column_end":5,"is_primary":true,"text":[{"text":"    pub fn new() -> Foo { Foo }","highlight_start":5,"highlight_end":5}],"label":null,"suggested_replacement":"#[derive(Default)]\n    ","expansion":null}],"children":[],"rendered":"    #[derive(Default)]\n    pub fn new() -> Foo { Foo }"},{"message":"for further information visit https://github.com/Manishearth/rust-clippy/wiki#new_without_default_derive","code":null,"level":"help","spans":[],"children":[],"rendered":null}],"rendered":null}

with the ugly hack:

{
    "file_name": "tests/compile-fail/new_without_default.rs",
    "byte_start": 162,
    "byte_end": 162,
    "line_start": 10,
    "line_end": 10,
    "column_start": 5, 
    "column_end": 5, 
    "is_primary": true,
    "text": [
        {
            "text": "    pub fn new() -> Foo { Foo }",
            "highlight_start": 5,
            "highlight_end": 5
        }
    ],
    "label": null,
    "suggested_replacement": "#[derive(Default)]\n    ",
    "expansion": null
}

Also when/if https://github.com/Manishearth/rust-clippy/pull/1060 is merged, some suggestions will have several children (ie. several pieces of code to replace at once):

{"message":"the loop variable `i` is only used to index `vec`.","code":null,"level":"error","spans":[{"file_name":"tests/compile-fail/for_loop.rs","byte_start":3015,"byte_end":3226,"line_start":99,"line_end":105,"column_start":5,"column_end":6,"is_primary":true,"text":[{"text":"    for i in 0..vec.len() {","highlight_start":5,"highlight_end":28},{"text":"        //~^ ERROR `i` is only used to index `vec`","highlight_start":1,"highlight_end":51},{"text":"        //~| HELP consider","highlight_start":1,"highlight_end":27},{"text":"        //~| HELP consider","highlight_start":1,"highlight_end":27},{"text":"        //~| SUGGESTION for <item> in &vec {","highlight_start":1,"highlight_end":45},{"text":"        println!(\"{}\", vec[i]);","highlight_start":1,"highlight_end":32},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":null,"expansion":null}],"children":[{"message":"lint level defined here","code":null,"level":"note","spans":[{"file_name":"tests/compile-fail/for_loop.rs","byte_start":2639,"byte_end":2658,"line_start":90,"line_end":90,"column_start":8,"column_end":27,"is_primary":true,"text":[{"text":"#[deny(needless_range_loop, explicit_iter_loop, iter_next_loop, reverse_range_loop, explicit_counter_loop)]","highlight_start":8,"highlight_end":27}],"label":null,"suggested_replacement":null,"expansion":null}],"children":[],"rendered":null},{"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 {"},{"message":"for further information visit https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop","code":null,"level":"help","spans":[],"children":[],"rendered":null}],"rendered":null}

1: btw, firefox tells me “obvisouly” is not a word 😄

@killercup
Copy link
Member Author

Thanks for the heads up! I think this should "just work", but I probably need to fix the handling of column indices (it currently replaces whole lines).


btw, firefox tells me “obvisouly” is not a word 😄

It's obvisouly msitaken. Maybe there is an udptae for it?

killercup added a commit that referenced this issue Jul 9, 2016
killercup added a commit that referenced this issue Jul 9, 2016
@killercup killercup modified the milestone: Rustfix Servo Jul 12, 2016
@killercup
Copy link
Member Author

Things happend, lints got fixed, #57 is tracking this in another way… let's close this

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants