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

show rendered cargo error in detail for clippy errors #2559

Merged
merged 1 commit into from
Jun 8, 2019

Conversation

nerdrew
Copy link
Contributor

@nerdrew nerdrew commented Jun 5, 2019

Sample clippy output:

{
    "reason": "compiler-message",
    "package_id": "snow_white 0.1.0 (path+file:///Users/lazarus/dev/grim/snow_white)",
    "target": {
        "kind": [
            "lib"
        ],
        "crate_types": [
            "lib"
        ],
        "name": "snow_white",
        "src_path": "/Users/lazarus/dev/grim/snow_white/src/lib.rs",
        "edition": "2018",
        "doctest": true
    },
    "message": {
        "rendered": "warning: You are using an explicit closure for copying elements\n   --> snow_white/src/save.rs:289:21\n    |\n289 |                     pending_seqs.seqs.iter().map(|seq| *seq).collect::<Vec<Seq>>()\n    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `copied` method: `pending_seqs.seqs.iter().copied()`\n    |\n    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_clone\n\n",
        "children": [
            {
                "children": [

                ],
                "code": null,
                "level": "help",
                "message": "for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_clone",
                "rendered": null,
                "spans": [

                ]
            },
            {
                "children": [

                ],
                "code": null,
                "level": "help",
                "message": "Consider calling the dedicated `copied` method",
                "rendered": null,
                "spans": [
                    {
                        "byte_end": 11764,
                        "byte_start": 11724,
                        "column_end": 61,
                        "column_start": 21,
                        "expansion": null,
                        "file_name": "snow_white/src/save.rs",
                        "is_primary": true,
                        "label": null,
                        "line_end": 289,
                        "line_start": 289,
                        "suggested_replacement": "pending_seqs.seqs.iter().copied()",
                        "suggestion_applicability": "MachineApplicable",
                        "text": [
                            {
                                "highlight_end": 61,
                                "highlight_start": 21,
                                "text": "                    pending_seqs.seqs.iter().map(|seq| *seq).collect::<Vec<Seq>>()"
                            }
                        ]
                    }
                ]
            }
        ],
        "code": {
            "code": "clippy::map_clone",
            "explanation": null
        },
        "level": "warning",
        "message": "You are using an explicit closure for copying elements",
        "spans": [
            {
                "byte_end": 11764,
                "byte_start": 11724,
                "column_end": 61,
                "column_start": 21,
                "expansion": null,
                "file_name": "snow_white/src/save.rs",
                "is_primary": true,
                "label": null,
                "line_end": 289,
                "line_start": 289,
                "suggested_replacement": null,
                "suggestion_applicability": null,
                "text": [
                    {
                        "highlight_end": 61,
                        "highlight_start": 21,
                        "text": "                    pending_seqs.seqs.iter().map(|seq| *seq).collect::<Vec<Seq>>()"
                    }
                ]
            }
        ]
    }
}

The current detail is:

You are using an explicit closure for copying elements

The new detail is:

warning: You are using an explicit closure for copying elements
   --> snow_white/src/save.rs:278:27
    |
278 |         let all_log_ids = logs.keys().map(|log_id| *log_id).collect::<Vec<LogId>>();
    |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Consider calling the dedicated `copied` method: `logs.keys().copied()`
    |
    = note: #[warn(clippy::map_clone)] on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_clone

@RyanSquared
Copy link
Member

The reason we don't do this as-is (and have had issues with Rust with this in the past) is that ALE does not support multiple lines given there's only one line that can be displayed when hovering over a message. If there's a way to separate :ALEDetail from the message ALE shows otherwise, I'd be fine with this.

@nerdrew
Copy link
Contributor Author

nerdrew commented Jun 5, 2019

I'm not entirely sure what you are asking... is "hovering" the same as having the cursor over it in non-gui vim?

Screen shots:
"hovering" (assuming hovering is having the cursor over the error):
Screen Shot 2019-06-04 at 22 32 15

Detail view:
Screen Shot 2019-06-04 at 22 32 20

@RyanSquared
Copy link
Member

Right, I totally glossed over the fact that it's only set for the detail and not for the short view. LGTM then. And yeah hovering basically implying that the cursor is above text in the same way hovering would work in a graphical client.

@nerdrew
Copy link
Contributor Author

nerdrew commented Jun 7, 2019

Thoughts on merging this?

@w0rp w0rp merged commit d9931b9 into dense-analysis:master Jun 8, 2019
@w0rp
Copy link
Member

w0rp commented Jun 8, 2019

Cheers! 🍻

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

Successfully merging this pull request may close these issues.

3 participants