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

Improve error message and snippet for "did you mean x" #36798

Merged
merged 1 commit into from
Oct 4, 2016

Conversation

gavinb
Copy link
Contributor

@gavinb gavinb commented Sep 28, 2016

Based on the standalone example https://is.gd/8STXMd posted by @nikomatsakis and using the third formatting option mentioned in #36164 and agreed by @jonathandturner.

Note however this does not address the question of how to handle an empty or unknown suggestion. @nikomatsakis any suggestions on how best to address that part?

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@sophiajt
Copy link
Contributor

For the default cause, I'd just use an "unknown field" label

@nikomatsakis
Copy link
Contributor

@gavinb nice! Can you change the test to be a UI-style test?

I agree with @jonathandturner, something generic like "unknown field" would be a good fallback label. Ideally, we'd have a test for that case too.

@gavinb
Copy link
Contributor Author

gavinb commented Sep 29, 2016

@nikomatsakis I've added the "unknown field" handling, converted the test to UI style test and added a test specifically for the unknown field case.

@sophiajt
Copy link
Contributor

Looks like adding the label means also updating a set of tests that don't expect it yet. Here's what I see from Travis:


failures:
    [compile-fail] compile-fail/attempted-access-non-fatal.rs
    [compile-fail] compile-fail/cast-rfc0401.rs
    [compile-fail] compile-fail/derived-errors/issue-30580.rs
    [compile-fail] compile-fail/issue-11004.rs
    [compile-fail] compile-fail/issue-14721.rs
    [compile-fail] compile-fail/issue-19244-2.rs
    [compile-fail] compile-fail/issue-23253.rs
    [compile-fail] compile-fail/issue-24363.rs
    [compile-fail] compile-fail/issue-24365.rs
    [compile-fail] compile-fail/issue-31011.rs
    [compile-fail] compile-fail/no-type-for-node-ice.rs
    [compile-fail] compile-fail/struct-fields-typo.rs
    [compile-fail] compile-fail/struct-pat-derived-error.rs
    [compile-fail] compile-fail/union/union-suggest-field.rs
    [compile-fail] compile-fail/unsafe-fn-autoderef.rs

@gavinb
Copy link
Contributor Author

gavinb commented Oct 1, 2016

Ok, I'll run the complete test suite and fix up the above. (Didn't before as I was running on batteries!)

Will update PR once full build and test suite pass.

@gavinb gavinb force-pushed the fix/36164 branch 2 times, most recently from 27f3c70 to 338bef8 Compare October 1, 2016 20:21
- Fixes rust-lang#36164
- Part of rust-lang#35233
- handles unknown fields
- uses UI-style tests
- update all related tests (cfail, ui, incremental)
@gavinb
Copy link
Contributor Author

gavinb commented Oct 2, 2016

@jonathandturner I went through and updated all the tests so that make check passes everything now. Once Travis has finished, this is good to go.

@GuillaumeGomez
Copy link
Member

Thanks!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Oct 2, 2016

📌 Commit 99aae9b has been approved by GuillaumeGomez

Manishearth added a commit to Manishearth/rust that referenced this pull request Oct 4, 2016
Improve error message and snippet for "did you mean `x`"

- Fixes rust-lang#36164
- Part of rust-lang#35233

Based on the standalone example https://is.gd/8STXMd posted by @nikomatsakis and using the third formatting option mentioned in rust-lang#36164 and agreed by @jonathandturner.

Note however this does not address the question of [how to handle an empty or unknown suggestion](rust-lang#36164 (comment)). @nikomatsakis any suggestions on how best to address that part?
bors added a commit that referenced this pull request Oct 4, 2016
Rollup of 12 pull requests

- Successful merges: #36798, #36878, #36902, #36903, #36908, #36916, #36917, #36921, #36928, #36938, #36941, #36951
- Failed merges:
@bors bors merged commit 99aae9b into rust-lang:master Oct 4, 2016
@gavinb gavinb deleted the fix/36164 branch October 5, 2016 11:30
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.

6 participants