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

update to new Rust error format and/or move to json solely #61

Closed
oli-obk opened this issue Sep 13, 2016 · 4 comments · Fixed by #62
Closed

update to new Rust error format and/or move to json solely #61

oli-obk opened this issue Sep 13, 2016 · 4 comments · Fixed by #62

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Sep 13, 2016

We have 17 days until the old error format won't even be in stable anymore.

I'd be happy with moving to json entirely, it still has some quirks, but we could circumvent them for now by showing the json field that contains the full message in the popups.

@alygin
Copy link
Collaborator

alygin commented Sep 13, 2016

I agree that it's time to switch to JSON as the default option.

At the same time, I personally would prefer to keep the possibility of switching to the standard format even though the new version is not parser-friendly anymore. I like the human readable output. Maybe it's just a matter of habit, but there might be people who feels the same.

I would break the issue into the following subtasks (from the highest priority to the lowest):

  1. Make JSON the default output format.
  2. Fix the known problems in the JSON parser.
  3. Implement a parser for the new format.
  4. Keep the old format (the one that we have now) parser too for those who doesn't switch to the new Rust versions quickly. Switching between the new and the old formats must be automatic.

What do you think?

I could certainly do 1 and 2 and then give a try to 3 and 4. But I know nothing about the quirks of the JSON parser because I haven't been using it much. @oli-obk, could you, please, amplify on this? Maybe give an example?

@oli-obk
Copy link
Contributor Author

oli-obk commented Sep 15, 2016

I don't have any examples off hand, but I often need to read the json in the build window to grok what the error popup is telling me. We don't display all information.

@alygin
Copy link
Collaborator

alygin commented Sep 15, 2016

Last time when I tweaked things in the JSON parser and tested it on various examples, I saw a lot of duplication in JSON messages. Some nodes convey the same information but use different wording. For instance, here's the JSON for the simple "mismatched types" error:

    {
        "message": "mismatched types",
        "code": {
            "code": "E0308",
            "explanation": "\nThis error occurs when the compiler was unable ..."
        },
        "level": "error",
        "spans": [{
                "file_name": "src/main.rs",
                "byte_start": 110,
                "byte_end": 115,
                "line_start": 11,
                "line_end": 11,
                "column_start": 9,
                "column_end": 14,
                "is_primary": true,
                "text": [{
                        "text": "    b = 43i32;",
                        "highlight_start": 9,
                        "highlight_end": 14
                }],
                "label": "expected u32, found i32",
                "suggested_replacement": null,
                "expansion": null
        }],
        "children": [{
                "message": "expected type `u32`",
                "code": null,
                "level": "note",
                "spans": [],
                "children": [],
                "rendered": null
            }, {
                "message": "   found type `i32`",
                "code": null,
                    "level": "note",
                "spans": [],
                "children": [],
                "rendered": null
        }],
        "rendered": null
    }

Such error is now displayed this way:

screen shot 2016-09-15 at 10 40 50

It will be better if we add the message from the span to the main message (it will become "mismatched types: expected u32, found i32") so sometimes there will be no need to expand the message. But we already have this information in the Notes. So, actually, no useful information is lost, it just is not duplicated as it is in the JSON.

All other examples I encountered were pretty similar to this one. If you come across some error message with a useful information completely removed in the lint, please, post it here.

@alygin
Copy link
Collaborator

alygin commented Sep 15, 2016

Errors in macros are tricky ones. They repeat the definition of the macro several times. I'm not sure if all this stuff is really needed to be displayed.

Below is an example produced by println!("{}");. The only suggestion I have is to add the text of the span to the main error message as a separate line. So the main error lint in this case will be "invalid reference to argument 0 (no arguments given)" and contain an additional line

print ! ( concat ! ( $ fmt , \"\\n\" ) , $ ( $ arg ) * ) ) ;

It will help when the message itself doesn't say enough to figure out the error cause.

{
    "message": "invalid reference to argument `0` (no arguments given)",
    "code": null,
    "level": "error",
    "spans": [
        {
            "file_name": "",
            "byte_start": 3956,
            "byte_end": 3981,
            "line_start": 3,
            "line_end": 3,
            "column_start": 11,
            "column_end": 36,
            "is_primary": true,
            "text": [
                {
                    "text": "print ! ( concat ! ( $ fmt , \"\\n\" ) , $ ( $ arg ) * ) ) ;",
                    "highlight_start": 11,
                    "highlight_end": 36
                }
            ],
            "label": null,
            "suggested_replacement": null,
            "expansion": {
                "span": {
                    "file_name": "",
                    "byte_start": 3956,
                    "byte_end": 3981,
                    "line_start": 3,
                    "line_end": 3,
                    "column_start": 11,
                    "column_end": 36,
                    "is_primary": false,
                    "text": [
                        {
                            "text": "print ! ( concat ! ( $ fmt , \"\\n\" ) , $ ( $ arg ) * ) ) ;",
                            "highlight_start": 11,
                            "highlight_end": 36
                        }
                    ],
                    "label": null,
                    "suggested_replacement": null,
                    "expansion": {
                        "span": {
                            "file_name": "",
                            "byte_start": 3801,
                            "byte_end": 3832,
                            "line_start": 2,
                            "line_end": 2,
                            "column_start": 27,
                            "column_end": 58,
                            "is_primary": false,
                            "text": [
                                {
                                    "text": "$ crate :: io :: _print ( format_args ! ( $ ( $ arg ) * ) ) ) ;",
                                    "highlight_start": 27,
                                    "highlight_end": 58
                                }
                            ],
                            "label": null,
                            "suggested_replacement": null,
                            "expansion": {
                                "span": {
                                    "file_name": "",
                                    "byte_start": 3946,
                                    "byte_end": 3999,
                                    "line_start": 3,
                                    "line_end": 3,
                                    "column_start": 1,
                                    "column_end": 54,
                                    "is_primary": false,
                                    "text": [
                                        {
                                            "text": "print ! ( concat ! ( $ fmt , \"\\n\" ) , $ ( $ arg ) * ) ) ;",
                                            "highlight_start": 1,
                                            "highlight_end": 54
                                        }
                                    ],
                                    "label": null,
                                    "suggested_replacement": null,
                                    "expansion": {
                                        "span": {
                                            "file_name": "src/main.rs",
                                            "byte_start": 102,
                                            "byte_end": 128,
                                            "line_start": 11,
                                            "line_end": 11,
                                            "column_start": 5,
                                            "column_end": 31,
                                            "is_primary": false,
                                            "text": [
                                                {
                                                    "text": "    println!(\"{}\");",
                                                    "highlight_start": 5,
                                                    "highlight_end": 31
                                                }
                                            ],
                                            "label": null,
                                            "suggested_replacement": null,
                                            "expansion": null
                                        },
                                        "macro_decl_name": "println!",
                                        "def_site_span": {
                                            "file_name": "",
                                            "byte_start": 3839,
                                            "byte_end": 4003,
                                            "line_start": 1,
                                            "line_end": 3,
                                            "column_start": 1,
                                            "column_end": 58,
                                            "is_primary": false,
                                            "text": [
                                                {
                                                    "text": "( $ fmt : expr ) => ( print ! ( concat ! ( $ fmt , \"\\n\" ) ) ) ; (",
                                                    "highlight_start": 1,
                                                    "highlight_end": 66
                                                },
                                                {
                                                    "text": "$ fmt : expr , $ ( $ arg : tt ) * ) => (",
                                                    "highlight_start": 1,
                                                    "highlight_end": 41
                                                },
                                                {
                                                    "text": "print ! ( concat ! ( $ fmt , \"\\n\" ) , $ ( $ arg ) * ) ) ;",
                                                    "highlight_start": 1,
                                                    "highlight_end": 58
                                                }
                                            ],
                                            "label": null,
                                            "suggested_replacement": null,
                                            "expansion": null
                                        }
                                    }
                                },
                                "macro_decl_name": "print!",
                                "def_site_span": {
                                    "file_name": "",
                                    "byte_start": 3747,
                                    "byte_end": 3838,
                                    "line_start": 1,
                                    "line_end": 2,
                                    "column_start": 1,
                                    "column_end": 64,
                                    "is_primary": false,
                                    "text": [
                                        {
                                            "text": "( $ ( $ arg : tt ) * ) => (",
                                            "highlight_start": 1,
                                            "highlight_end": 28
                                        },
                                        {
                                            "text": "$ crate :: io :: _print ( format_args ! ( $ ( $ arg ) * ) ) );",
                                            "highlight_start": 1,
                                            "highlight_end": 64
                                        }
                                    ],
                                    "label": null,
                                    "suggested_replacement": null,
                                    "expansion": null
                                }
                            }
                        },
                        "macro_decl_name": "format_args!",
                        "def_site_span": null
                    }
                },
                "macro_decl_name": "concat!",
                "def_site_span": null
            }
        }
    ],
    "children": [],
    "rendered": null
}

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 a pull request may close this issue.

2 participants