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

Hint on missing tuple parens in pattern #63962

Closed
cramertj opened this issue Aug 27, 2019 · 9 comments · Fixed by #64666
Closed

Hint on missing tuple parens in pattern #63962

cramertj opened this issue Aug 27, 2019 · 9 comments · Fixed by #64666
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cramertj
Copy link
Member

cramertj commented Aug 27, 2019

It'd be nice to smartly hint about missing tuple parentheses in patterns, especially in positions where parentheses nesting is required.

Example of incorrect code:

fn main() {
    match Some((1, 2)) {
        Some(x, y) => {}
        None => {}
    }
}

The error:

error[E0023]: this pattern has 2 fields, but the corresponding tuple variant has 1 field
 --> src/main.rs:3:9
  |
3 |         Some(x, y) => {}
  |         ^^^^^^^^^^ expected 1 field, found 2

This error is confusing, especially since Some is a "tuple pattern" which is confusable with its tuple field. An error that suggests missing tuple parentheses would be helpful.

This issue has been assigned to @sam09 via this comment.

@cramertj cramertj added the A-diagnostics Area: Messages for errors, warnings, and lints label Aug 27, 2019
@jonas-schievink jonas-schievink added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 27, 2019
@estebank estebank added the A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. label Aug 27, 2019
@Centril
Copy link
Contributor

Centril commented Aug 27, 2019

I think the right approach here would be to check if the scrutinee type (expected type in this context) has a single field of a tuple type with as many fields as the tuple struct pattern has.

Place to change:

let subpats_ending = if subpats.len() == 1 { "" } else { "s" };
let fields_ending = if variant.fields.len() == 1 { "" } else { "s" };
struct_span_err!(tcx.sess, pat.span, E0023,
"this pattern has {} field{}, but the corresponding {} has {} field{}",
subpats.len(), subpats_ending, res.descr(),
variant.fields.len(), fields_ending)
.span_label(pat.span, format!("expected {} field{}, found {}",
variant.fields.len(), fields_ending, subpats.len()))
.emit();
on_error();
return tcx.types.err;

If you tackle this, first please also refactor the snippet highlighted above into a diagnostics-only method and then do your changes in there.

@Centril Centril added E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Aug 27, 2019
@sam09
Copy link
Contributor

sam09 commented Aug 31, 2019

Can I pick up this one? Doesn't look like anybody working on this.

@zackmdavis
Copy link
Member

@sam09 Please do.

@rustbot assign @sam09

@rustbot rustbot self-assigned this Aug 31, 2019
@sam09
Copy link
Contributor

sam09 commented Sep 1, 2019

I think the right approach here would be to check if the scrutinee type (expected type in this context) has a single field of a tuple type with as many fields as the tuple struct pattern has.

variant.fields.len() gives me the number of fields of expected type, which should be 1 here. I have a condition, varaints.fields.len()==1. However I can't see how many fields varaints.field[0] has, when trying to compare to the fields the tuple struct pattern has (subpats.len() in this case)

Maybe I misunderstood it, or am missing something here

@sam09
Copy link
Contributor

sam09 commented Sep 7, 2019

This #64161 seems to have changed some things here. Is this issue still relevant?

@Centril
Copy link
Contributor

Centril commented Sep 7, 2019

cc @estebank

Ostensibly it is still relevant as we could emit a specific suggestion in this case.

@estebank
Copy link
Contributor

estebank commented Sep 7, 2019

Yes, that PR only adds a bit of context that should generally be useful, but this is about adding a structured suggestion for this case in particular. This is more focused and more useful for this case in particular :)

Looking forward to it!

@sam09
Copy link
Contributor

sam09 commented Sep 8, 2019

Thanks @Centril @estebank. I have one more question though,

fn e0023(&self, pat_span: Span, res: Res, subpats: &'tcx [P<Pat>], fields: &[ty::FieldDef]) {
let subpats_ending = if subpats.len() == 1 { "" } else { "s" };
let fields_ending = if fields.len() == 1 { "" } else { "s" };
let res_span = self.tcx.def_span(res.def_id());
struct_span_err!(
self.tcx.sess,
pat_span,
E0023,
"this pattern has {} field{}, but the corresponding {} has {} field{}",
subpats.len(),
subpats_ending,
res.descr(),
fields.len(),
fields_ending,
)
.span_label(pat_span, format!(
"expected {} field{}, found {}",
fields.len(),
fields_ending,
subpats.len(),
))
.span_label(res_span, format!("{} defined here", res.descr()))
.emit();
}

This fields parameter in the above function gives the number of fields of expected type, while subpats gives me the number of fields in the tuple pattern.

The condition to show a missing parenthesis message should be something like:

 if fields.len() == 1 && subpats.len() == fields[0].len() {
            parentheses_missing = true;
 } 

But fields[0] is not a vector and I cannot seem to figure out how to find the number of elements in the expected type

@Centril
Copy link
Contributor

Centril commented Sep 8, 2019

But fields[0] is not a vector and I cannot seem to figure out how to find the number of elements in the expected type

You'll need to look at expected: Ty<'tcx> to see that it resolves to an TyKind::Adt(adt_def, substs) and then you'll need to match up the variant with the right one in the AdtDef. Once you have the right variant, you'll need to check that it has one field and extract it. You'll need to check that the single field is also a tuple and match the length of that tuple up with subpats.len(). You'll also need to handle the possibility that the enum is generic, e.g. as in the case of Option<T> and use substs to account for this.

Centril added a commit to Centril/rust that referenced this issue Sep 21, 2019
Fixes rust-lang#63962. Hint about missing tuple parentheses in patterns
bors added a commit that referenced this issue Sep 22, 2019
Rollup of 9 pull requests

Successful merges:

 - #63907 (Add explanation to type mismatch involving type params and assoc types)
 - #64615 (rustbuild: Turn down compression on exe installers)
 - #64617 (rustbuild: Turn down compression on msi installers)
 - #64618 (rustbuild: Improve output of `dist` step)
 - #64619 (Fixes #63962. Hint about missing tuple parentheses in patterns)
 - #64634 (Update to LLVM 9.0.0)
 - #64635 (Allow using fn pointers in const fn with unleash miri)
 - #64660 (unify errors for tuple/struct variants)
 - #64664 (fully remove AstBuilder)

Failed merges:

r? @ghost
@bors bors closed this as completed in a2a57bc Sep 22, 2019
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 A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants