-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Suggest .as_ref()
when appropriate for Option
and Result
#57158
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
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 | ||||
---|---|---|---|---|---|---|
|
@@ -956,6 +956,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { | |||||
diag.span_label(span, message); | ||||||
} | ||||||
} | ||||||
self.suggest_as_ref_where_appropriate(span, &exp_found, diag); | ||||||
} | ||||||
|
||||||
diag.note_expected_found(&"type", expected, found); | ||||||
|
@@ -972,6 +973,72 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { | |||||
self.note_error_origin(diag, &cause); | ||||||
} | ||||||
|
||||||
/// When encountering a case where `.as_ref()` on a `Result` or `Option` would be appropriate, | ||||||
/// suggest it. | ||||||
fn suggest_as_ref_where_appropriate( | ||||||
&self, | ||||||
span: Span, | ||||||
exp_found: &ty::error::ExpectedFound<Ty<'tcx>>, | ||||||
diag: &mut DiagnosticBuilder<'tcx>, | ||||||
) { | ||||||
match (&exp_found.expected.sty, &exp_found.found.sty) { | ||||||
(TyKind::Adt(exp_def, exp_substs), TyKind::Ref(_, found_ty, _)) => { | ||||||
if let TyKind::Adt(found_def, found_substs) = found_ty.sty { | ||||||
let path_str = format!("{:?}", exp_def); | ||||||
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. Relying on debug output for functionality seems like poor form? (On the grounds that it's meant for debugging convenience and should be subject to change rather than guaranteed stable, even if it happens to be probably-stable in this case.) |
||||||
if exp_def == &found_def { | ||||||
let opt_msg = "you can convert from `&Option<T>` to `Option<&T>` using \ | ||||||
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
I lean against the second person in diagnostic messages. (But that's my opinion; we haven't codifed that anywhere.) |
||||||
`.as_ref()`"; | ||||||
let result_msg = "you can convert from `&Result<T, E>` to \ | ||||||
`Result<&T, &E>` using `.as_ref()`"; | ||||||
let have_as_ref = &[ | ||||||
("std::option::Option", opt_msg), | ||||||
("core::option::Option", opt_msg), | ||||||
("std::result::Result", result_msg), | ||||||
("core::result::Result", result_msg), | ||||||
]; | ||||||
if let Some(msg) = have_as_ref.iter() | ||||||
.filter_map(|(path, msg)| if &path_str == path { | ||||||
Some(msg) | ||||||
} else { | ||||||
None | ||||||
}).next() | ||||||
{ | ||||||
let mut show_suggestion = true; | ||||||
for (exp_ty, found_ty) in exp_substs.types().zip(found_substs.types()) { | ||||||
match exp_ty.sty { | ||||||
TyKind::Ref(_, exp_ty, _) => { | ||||||
match (&exp_ty.sty, &found_ty.sty) { | ||||||
(_, TyKind::Param(_)) | | ||||||
(_, TyKind::Infer(_)) | | ||||||
(TyKind::Param(_), _) | | ||||||
(TyKind::Infer(_), _) => {} | ||||||
_ if ty::TyS::same_type(exp_ty, found_ty) => {} | ||||||
_ => show_suggestion = false, | ||||||
}; | ||||||
} | ||||||
TyKind::Param(_) | TyKind::Infer(_) => {} | ||||||
_ => show_suggestion = false, | ||||||
} | ||||||
} | ||||||
if let (Ok(snippet), true) = ( | ||||||
self.tcx.sess.source_map().span_to_snippet(span), | ||||||
show_suggestion, | ||||||
) { | ||||||
diag.span_suggestion_with_applicability( | ||||||
span, | ||||||
msg, | ||||||
format!("{}.as_ref()", snippet), | ||||||
Applicability::MachineApplicable, | ||||||
); | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
_ => {} | ||||||
} | ||||||
} | ||||||
|
||||||
pub fn report_and_explain_type_error( | ||||||
&self, | ||||||
trace: TypeTrace<'tcx>, | ||||||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
error[E0308]: mismatched types | ||
--> $DIR/as-ref.rs:6:27 | ||
| | ||
LL | opt.map(|arg| takes_ref(arg)); | ||
| - ^^^ expected &Foo, found struct `Foo` | ||
| | | ||
| help: consider using `as_ref` instead: `as_ref().` | ||
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.
In any case, the code that sets this was actually introduced in #51100, not here, so it doesn't affect the outcome of this review. |
||
| | ||
= note: expected type `&Foo` | ||
found type `Foo` | ||
|
||
error[E0308]: mismatched types | ||
--> $DIR/as-ref.rs:8:37 | ||
| | ||
LL | opt.and_then(|arg| Some(takes_ref(arg))); | ||
| - ^^^ expected &Foo, found struct `Foo` | ||
| | | ||
| help: consider using `as_ref` instead: `as_ref().` | ||
| | ||
= note: expected type `&Foo` | ||
found type `Foo` | ||
|
||
error[E0308]: mismatched types | ||
--> $DIR/as-ref.rs:11:27 | ||
| | ||
LL | opt.map(|arg| takes_ref(arg)); | ||
| - ^^^ expected &Foo, found struct `Foo` | ||
| | | ||
| help: consider using `as_ref` instead: `as_ref().` | ||
| | ||
= note: expected type `&Foo` | ||
found type `Foo` | ||
|
||
error[E0308]: mismatched types | ||
--> $DIR/as-ref.rs:13:35 | ||
| | ||
LL | opt.and_then(|arg| Ok(takes_ref(arg))); | ||
| - ^^^ expected &Foo, found struct `Foo` | ||
| | | ||
| help: consider using `as_ref` instead: `as_ref().` | ||
| | ||
= note: expected type `&Foo` | ||
found type `Foo` | ||
|
||
error[E0308]: mismatched types | ||
--> $DIR/as-ref.rs:16:27 | ||
| | ||
LL | let y: Option<&usize> = x; | ||
| ^ | ||
| | | ||
| expected enum `std::option::Option`, found reference | ||
| help: you can convert from `&Option<T>` to `Option<&T>` using `.as_ref()`: `x.as_ref()` | ||
| | ||
= note: expected type `std::option::Option<&usize>` | ||
found type `&std::option::Option<usize>` | ||
|
||
error[E0308]: mismatched types | ||
--> $DIR/as-ref.rs:19:35 | ||
| | ||
LL | let y: Result<&usize, &usize> = x; | ||
| ^ expected enum `std::result::Result`, found reference | ||
| | ||
= note: expected type `std::result::Result<&usize, &usize>` | ||
found type `&std::result::Result<usize, usize>` | ||
help: you can convert from `&Result<T, E>` to `Result<&T, &E>` using `.as_ref()` | ||
| | ||
LL | let y: Result<&usize, &usize> = x.as_ref(); | ||
| ^^^^^^^^^^ | ||
|
||
error[E0308]: mismatched types | ||
--> $DIR/as-ref.rs:23:34 | ||
| | ||
LL | let y: Result<&usize, usize> = x; | ||
| ^ expected enum `std::result::Result`, found reference | ||
| | ||
= note: expected type `std::result::Result<&usize, usize>` | ||
found type `&std::result::Result<usize, usize>` | ||
|
||
error: aborting due to 7 previous errors | ||
|
||
For more information about this error, try `rustc --explain E0308`. |
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.
I like
_where_appropriate
as part of a function name, but I don't think I've seen it used anywhere else, and I know we have a lot of these "conditionally set a diagnostic" functions ...