-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
Suggest struct pattern when destructuring Range with .. syntax #149952
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -440,6 +440,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { | |||||||||
|
|
||||||||||
| self.detect_missing_binding_available_from_pattern(&mut err, path, following_seg); | ||||||||||
| self.suggest_at_operator_in_slice_pat_with_range(&mut err, path); | ||||||||||
| self.suggest_range_struct_destructuring(&mut err, path, source); | ||||||||||
| self.suggest_swapping_misplaced_self_ty_and_trait(&mut err, source, res, base_error.span); | ||||||||||
|
|
||||||||||
| if let Some((span, label)) = base_error.span_label { | ||||||||||
|
|
@@ -1383,6 +1384,41 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { | |||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| fn suggest_range_struct_destructuring( | ||||||||||
| &self, | ||||||||||
| err: &mut Diag<'_>, | ||||||||||
| path: &[Segment], | ||||||||||
| source: PathSource<'_, '_, '_>, | ||||||||||
| ) { | ||||||||||
| // We accept Expr here because range bounds (start..end) are parsed as expressions | ||||||||||
| if !matches!(source, PathSource::Pat | PathSource::TupleStruct(..) | PathSource::Expr(..)) { | ||||||||||
| return; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if let Some(pat) = self.diag_metadata.current_pat | ||||||||||
| && let ast::PatKind::Range(Some(start_expr), Some(end_expr), _) = &pat.kind | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be very nice to also handle the open ranges, |
||||||||||
| && let (ast::ExprKind::Path(None, start_path), ast::ExprKind::Path(None, end_path)) = | ||||||||||
| (&start_expr.kind, &end_expr.kind) | ||||||||||
|
Comment on lines
+1400
to
+1401
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that you're disqualifying cases like |
||||||||||
| && path.len() == 1 | ||||||||||
| { | ||||||||||
| let ident = path[0].ident; | ||||||||||
|
|
||||||||||
| if (start_path.segments.len() == 1 && start_path.segments[0].ident == ident) | ||||||||||
| || (end_path.segments.len() == 1 && end_path.segments[0].ident == ident) | ||||||||||
|
Comment on lines
+1402
to
+1407
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use slice pattern |
||||||||||
| { | ||||||||||
| let start_name = start_path.segments[0].ident; | ||||||||||
| let end_name = end_path.segments[0].ident; | ||||||||||
|
Comment on lines
+1409
to
+1410
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is incorrect since you're using an Concretely, it means that you're suggesting |
||||||||||
|
|
||||||||||
| err.span_suggestion_verbose( | ||||||||||
| pat.span, | ||||||||||
| "if you meant to destructure a `Range`, use a struct pattern", | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
or
Suggested change
where |
||||||||||
| format!("std::ops::Range {{ start: {}, end: {} }}", start_name, end_name), | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be helpful if this suggested
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regarding new_range: I'm sticking with
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please add a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of unconditionally suggesting a qualified path, IINM you should be able to take advantage of path trimming (that's a machinery that shortens paths according to what's in scope / imported).
Suggested change
where I hope this works in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (low priority) in the special case of |
||||||||||
| Applicability::MaybeIncorrect, | ||||||||||
| ); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| fn suggest_swapping_misplaced_self_ty_and_trait( | ||||||||||
| &mut self, | ||||||||||
| err: &mut Diag<'_>, | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| use std::ops::Range; | ||
|
|
||
| fn test_basic_range(r: Range<u32>) { | ||
| let start..end = r; | ||
| //~^ ERROR cannot find value `start` in this scope | ||
| //~| ERROR cannot find value `end` in this scope | ||
| } | ||
|
|
||
| fn test_different_names(r: Range<u32>) { | ||
| let min..max = r; | ||
| //~^ ERROR cannot find value `min` in this scope | ||
| //~| ERROR cannot find value `max` in this scope | ||
| } | ||
|
|
||
| fn main() {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| error[E0425]: cannot find value `start` in this scope | ||
| --> $DIR/suggest-range-struct-destructuring.rs:4:9 | ||
| | | ||
| LL | let start..end = r; | ||
| | ^^^^^ not found in this scope | ||
| | | ||
| help: if you meant to destructure a `Range`, use a struct pattern | ||
| | | ||
| LL - let start..end = r; | ||
| LL + let std::ops::Range { start: start, end: end } = r; | ||
| | | ||
|
|
||
| error[E0425]: cannot find value `end` in this scope | ||
| --> $DIR/suggest-range-struct-destructuring.rs:4:16 | ||
| | | ||
| LL | let start..end = r; | ||
| | ^^^ not found in this scope | ||
| | | ||
| help: if you meant to destructure a `Range`, use a struct pattern | ||
| | | ||
| LL - let start..end = r; | ||
| LL + let std::ops::Range { start: start, end: end } = r; | ||
| | | ||
|
|
||
| error[E0425]: cannot find value `min` in this scope | ||
| --> $DIR/suggest-range-struct-destructuring.rs:10:9 | ||
| | | ||
| LL | let min..max = r; | ||
| | ^^^ | ||
| ... | ||
| LL | fn main() {} | ||
| | --------- similarly named function `main` defined here | ||
| | | ||
| help: if you meant to destructure a `Range`, use a struct pattern | ||
| | | ||
| LL - let min..max = r; | ||
| LL + let std::ops::Range { start: min, end: max } = r; | ||
| | | ||
| help: a function with a similar name exists | ||
| | | ||
| LL | let main..max = r; | ||
| | + | ||
| help: consider importing this function | ||
| | | ||
| LL + use std::cmp::min; | ||
| | | ||
|
|
||
| error[E0425]: cannot find value `max` in this scope | ||
| --> $DIR/suggest-range-struct-destructuring.rs:10:14 | ||
| | | ||
| LL | let min..max = r; | ||
| | ^^^ not found in this scope | ||
| | | ||
| help: if you meant to destructure a `Range`, use a struct pattern | ||
| | | ||
| LL - let min..max = r; | ||
| LL + let std::ops::Range { start: min, end: max } = r; | ||
| | | ||
| help: consider importing this function | ||
| | | ||
| LL + use std::cmp::max; | ||
| | | ||
|
|
||
| error: aborting due to 4 previous errors | ||
|
|
||
| For more information about this error, try `rustc --explain E0425`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As alluded to in another review comment of mine, you can't ignore the
RangeEndof thePatKind::Range, it's crucial for suggesting the right struct!