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

JSON errors with suggestions are incomplete #30701

Closed
nrc opened this issue Jan 4, 2016 · 17 comments
Closed

JSON errors with suggestions are incomplete #30701

nrc opened this issue Jan 4, 2016 · 17 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@nrc
Copy link
Member

nrc commented Jan 4, 2016

The suggestion code snippet does not include the suggested change, it has the original code.

To fix this, look for the FIXME for this issue in src/libsyntax/errors/json.rs. We should extend DiagnosticSpan with another field: override_snippet and use this in the suggestion case.

@nrc nrc added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-diagnostics Area: Messages for errors, warnings, and lints E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Jan 4, 2016
@AnthonyBroadCrawford
Copy link

@nrc mind if I run with this one? Additionally, if I have any questions is it okay to ping you?

@nrc
Copy link
Member Author

nrc commented Jan 6, 2016

@AnthonyBroadCrawford that would be great and please feel free to ping me. Note that JSON errors haven't landed yet, you'll need this to land first: #30711 (or you could pull that branch and work off that).

@AnthonyBroadCrawford
Copy link

@nrc sounds good. What's the ETA on #30711 if you don't mind me asking?

@nrc
Copy link
Member Author

nrc commented Jan 6, 2016

@AnthonyBroadCrawford I wish I knew! As soon as it gets a review and passes tests :-). Hopefully, by the start of next week.

@AnthonyBroadCrawford
Copy link

@nrc Awesome. For now I'll just merge that branch into my fork/branch to work from there. I can rebase if any changes come about via the review.

Thanks again!

@nrc
Copy link
Member Author

nrc commented Jan 15, 2016

@AnthonyBroadCrawford #30711 has finally landed, sorry that took so long. Did you manage to get started? Do you need any pointers?

@AnthonyBroadCrawford
Copy link

Not yet. I'll be on it tonight. Looking forward to digging into @nrc. Are you in the IRC channel usually? If so, I'll message you there.

@AnthonyBroadCrawford
Copy link

Also, I'll take any pointers or advice you have. This would be my first commit to this project and I'm just starting to get the lay of the land reading the code-base et al.

@nrc
Copy link
Member Author

nrc commented Jan 19, 2016

@AnthonyBroadCrawford yes, I tend to be in #rust-internals and #rustc during working hours in NZ. I'll also see mentions if you ping me when I'm not online.

This is where you should start: https://dxr.mozilla.org/rust/source/src/libsyntax/errors/json.rs#187

This is how it is done for the normal error emitter: https://dxr.mozilla.org/rust/source/src/libsyntax/errors/emitter.rs#221-265. In particular, you probably want something like complete as the text for the JSON.

The way JSON errors work is we make a bunch of structs and then serialise them to JSON. The struct which represents a span is DiagnosticSpan. At the moment, we don't include the actual code in there, just enough detail to find the code in the source text. With suggestions, we want to suggest some code to replace that span. So we should add a field for that (replacement_text: Option<String> or something). You could add that to either Diagnostic or DiagnosticSpan, probably Diagnostic is better.

@AnthonyBroadCrawford
Copy link

@nrc Thanks, this is helpful. What's the best way to produce the errors as json. Are those details available in the previous pr that this issue depends on? (I'm going to go look there and if I find what I'm looking for I'll update this thread)

@nrc
Copy link
Member Author

nrc commented Jan 22, 2016

run rustc with the flags -Zunstable-options --error-format=json

@AnthonyBroadCrawford
Copy link

Keeping you in the loop @nrc as I should have a PR for you to pee at tomorrow.

@mitaa
Copy link
Contributor

mitaa commented Jan 29, 2016

@AnthonyBroadCrawford @nrc Note that the layout of RenderSpan::Suggestion has changed with #30411

@AnthonyBroadCrawford
Copy link

thanks @mitaa

@vegai
Copy link
Contributor

vegai commented Mar 3, 2016

Is anyone working at this? I could take a shot.

@nrc
Copy link
Member Author

nrc commented Mar 14, 2016

ping @AnthonyBroadCrawford re the above comment. Are you still looking at this?

@sanxiyn
Copy link
Member

sanxiyn commented May 23, 2016

This was fixed as part of porting compiletest to JSON in #33020, and FIXME is no more.

@sanxiyn sanxiyn closed this as completed May 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
None yet
Development

No branches or pull requests

5 participants