Skip to content

Commit 683b265

Browse files
clubby789clubby789
committed
Refactor suggest_copied_cloned_or_as_ref 
1 parent 601a34d commit 683b265

File tree

7 files changed

+93
-72
lines changed

7 files changed

+93
-72
lines changed

Diff for: ‎compiler/rustc_hir_typeck/messages.ftl

+4
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ hir_typeck_note_edition_guide = for more on editions, read https://doc.rust-lang
7777
7878
hir_typeck_op_trait_generic_params = `{$method_name}` must not have any generic parameters
7979
80+
hir_typeck_option_result_asref = use `{$def_path}::as_ref` to convert `{$expected_ty}` to `{$expr_ty}`
81+
hir_typeck_option_result_cloned = use `{$def_path}::cloned` to clone the value inside the `{$def_path}`
82+
hir_typeck_option_result_copied = use `{$def_path}::copied` to copy the value inside the `{$def_path}`
83+
8084
hir_typeck_return_stmt_outside_of_fn_body =
8185
{$statement_kind} statement outside of function body
8286
.encl_body_label = the {$statement_kind} is part of this body...

Diff for: ‎compiler/rustc_hir_typeck/src/errors.rs

+39
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,45 @@ impl HelpUseLatestEdition {
252252
}
253253
}
254254

255+
#[derive(Subdiagnostic)]
256+
pub enum OptionResultRefMismatch<'tcx> {
257+
#[suggestion(
258+
hir_typeck_option_result_copied,
259+
code = ".copied()",
260+
style = "verbose",
261+
applicability = "machine-applicable"
262+
)]
263+
Copied {
264+
#[primary_span]
265+
span: Span,
266+
def_path: String,
267+
},
268+
#[suggestion(
269+
hir_typeck_option_result_cloned,
270+
code = ".cloned()",
271+
style = "verbose",
272+
applicability = "machine-applicable"
273+
)]
274+
Cloned {
275+
#[primary_span]
276+
span: Span,
277+
def_path: String,
278+
},
279+
#[suggestion(
280+
hir_typeck_option_result_asref,
281+
code = ".as_ref()",
282+
style = "verbose",
283+
applicability = "machine-applicable"
284+
)]
285+
AsRef {
286+
#[primary_span]
287+
span: Span,
288+
def_path: String,
289+
expected_ty: Ty<'tcx>,
290+
expr_ty: Ty<'tcx>,
291+
},
292+
}
293+
255294
#[derive(Diagnostic)]
256295
#[diag(hir_typeck_const_select_must_be_const)]
257296
#[help]

Diff for: ‎compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

+46-68
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
use super::FnCtxt;
22

3-
use crate::errors::{
4-
AddReturnTypeSuggestion, ExpectedReturnTypeLabel, SuggestBoxing, SuggestConvertViaMethod,
5-
};
3+
use crate::errors;
64
use crate::fluent_generated as fluent;
75
use crate::method::probe::{IsSuggestion, Mode, ProbeScope};
86
use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX};
@@ -434,7 +432,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
434432
// FIXME: This could/should be extended to suggest `as_mut` and `as_deref_mut`,
435433
// but those checks need to be a bit more delicate and the benefit is diminishing.
436434
if self.can_eq(self.param_env, found_ty_inner, peeled) && error_tys_equate_as_ref {
437-
err.subdiagnostic(SuggestConvertViaMethod {
435+
err.subdiagnostic(errors::SuggestConvertViaMethod {
438436
span: expr.span.shrink_to_hi(),
439437
sugg: ".as_ref()",
440438
expected,
@@ -447,7 +445,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
447445
&& self.can_eq(self.param_env, deref_ty, peeled)
448446
&& error_tys_equate_as_ref
449447
{
450-
err.subdiagnostic(SuggestConvertViaMethod {
448+
err.subdiagnostic(errors::SuggestConvertViaMethod {
451449
span: expr.span.shrink_to_hi(),
452450
sugg: ".as_deref()",
453451
expected,
@@ -521,17 +519,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
521519
if self.can_coerce(Ty::new_box(self.tcx, found), expected) {
522520
let suggest_boxing = match found.kind() {
523521
ty::Tuple(tuple) if tuple.is_empty() => {
524-
SuggestBoxing::Unit { start: span.shrink_to_lo(), end: span }
522+
errors::SuggestBoxing::Unit { start: span.shrink_to_lo(), end: span }
525523
}
526524
ty::Generator(def_id, ..)
527525
if matches!(
528526
self.tcx.generator_kind(def_id),
529527
Some(GeneratorKind::Async(AsyncGeneratorKind::Closure))
530528
) =>
531529
{
532-
SuggestBoxing::AsyncBody
530+
errors::SuggestBoxing::AsyncBody
533531
}
534-
_ => SuggestBoxing::Other { start: span.shrink_to_lo(), end: span.shrink_to_hi() },
532+
_ => errors::SuggestBoxing::Other {
533+
start: span.shrink_to_lo(),
534+
end: span.shrink_to_hi(),
535+
},
535536
};
536537
err.subdiagnostic(suggest_boxing);
537538

@@ -756,23 +757,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
756757
match &fn_decl.output {
757758
&hir::FnRetTy::DefaultReturn(span) if expected.is_unit() && !can_suggest => {
758759
// `fn main()` must return `()`, do not suggest changing return type
759-
err.subdiagnostic(ExpectedReturnTypeLabel::Unit { span });
760+
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Unit { span });
760761
return true;
761762
}
762763
&hir::FnRetTy::DefaultReturn(span) if expected.is_unit() => {
763764
if let Some(found) = found.make_suggestable(self.tcx, false) {
764-
err.subdiagnostic(AddReturnTypeSuggestion::Add { span, found: found.to_string() });
765+
err.subdiagnostic(errors::AddReturnTypeSuggestion::Add { span, found: found.to_string() });
765766
return true;
766767
} else if let ty::Closure(_, args) = found.kind()
767768
// FIXME(compiler-errors): Get better at printing binders...
768769
&& let closure = args.as_closure()
769770
&& closure.sig().is_suggestable(self.tcx, false)
770771
{
771-
err.subdiagnostic(AddReturnTypeSuggestion::Add { span, found: closure.print_as_impl_trait().to_string() });
772+
err.subdiagnostic(errors::AddReturnTypeSuggestion::Add { span, found: closure.print_as_impl_trait().to_string() });
772773
return true;
773774
} else {
774775
// FIXME: if `found` could be `impl Iterator` we should suggest that.
775-
err.subdiagnostic(AddReturnTypeSuggestion::MissingHere { span });
776+
err.subdiagnostic(errors::AddReturnTypeSuggestion::MissingHere { span });
776777
return true
777778
}
778779
}
@@ -794,10 +795,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
794795
debug!(?found);
795796
if found.is_suggestable(self.tcx, false) {
796797
if term.span.is_empty() {
797-
err.subdiagnostic(AddReturnTypeSuggestion::Add { span, found: found.to_string() });
798+
err.subdiagnostic(errors::AddReturnTypeSuggestion::Add { span, found: found.to_string() });
798799
return true;
799800
} else {
800-
err.subdiagnostic(ExpectedReturnTypeLabel::Other { span, expected });
801+
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Other { span, expected });
801802
}
802803
}
803804
}
@@ -813,7 +814,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
813814
let ty = self.normalize(span, ty);
814815
let ty = self.tcx.erase_late_bound_regions(ty);
815816
if self.can_coerce(expected, ty) {
816-
err.subdiagnostic(ExpectedReturnTypeLabel::Other { span, expected });
817+
err.subdiagnostic(errors::ExpectedReturnTypeLabel::Other { span, expected });
817818
self.try_suggest_return_impl_trait(err, expected, ty, fn_id);
818819
return true;
819820
}
@@ -1103,65 +1104,42 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
11031104
return false;
11041105
}
11051106

1106-
let mut suggest_copied_cloned_or_as_ref = || {
1107+
if Some(adt_def.did()) == self.tcx.get_diagnostic_item(sym::Result)
1108+
&& self.can_eq(self.param_env, args.type_at(1), expected_args.type_at(1))
1109+
|| Some(adt_def.did()) == self.tcx.get_diagnostic_item(sym::Option)
1110+
{
11071111
let expr_inner_ty = args.type_at(0);
11081112
let expected_inner_ty = expected_args.type_at(0);
11091113
if let &ty::Ref(_, ty, hir::Mutability::Not) = expr_inner_ty.kind()
1110-
&& self.can_eq(self.param_env, ty, expected_inner_ty)
1111-
{
1112-
let def_path = self.tcx.def_path_str(adt_def.did());
1113-
if self.type_is_copy_modulo_regions(self.param_env, ty) {
1114-
diag.span_suggestion_verbose(
1115-
expr.span.shrink_to_hi(),
1116-
format!(
1117-
"use `{def_path}::copied` to copy the value inside the `{def_path}`"
1118-
),
1119-
".copied()",
1120-
Applicability::MachineApplicable,
1121-
);
1122-
return true;
1123-
} else if let Some(expected_ty_expr) = expected_ty_expr {
1124-
diag.span_suggestion_verbose(
1125-
expected_ty_expr.span.shrink_to_hi(),
1126-
format!(
1127-
"use `{def_path}::as_ref()` to convert `{expected_ty}` to `{expr_ty}`"
1128-
),
1129-
".as_ref()",
1130-
Applicability::MachineApplicable,
1131-
);
1132-
return true;
1133-
} else if let Some(clone_did) = self.tcx.lang_items().clone_trait()
1134-
&& rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions(
1135-
self,
1136-
self.param_env,
1137-
ty,
1138-
clone_did,
1139-
)
1114+
&& self.can_eq(self.param_env, ty, expected_inner_ty)
11401115
{
1141-
diag.span_suggestion_verbose(
1142-
expr.span.shrink_to_hi(),
1143-
format!(
1144-
"use `{def_path}::cloned` to clone the value inside the `{def_path}`"
1145-
),
1146-
".cloned()",
1147-
Applicability::MachineApplicable,
1148-
);
1116+
let def_path = self.tcx.def_path_str(adt_def.did());
1117+
let span = expr.span.shrink_to_hi();
1118+
let subdiag = if self.type_is_copy_modulo_regions(self.param_env, ty) {
1119+
errors::OptionResultRefMismatch::Copied {
1120+
span, def_path
1121+
}
1122+
} else if let Some(expected_ty_expr) = expected_ty_expr {
1123+
errors::OptionResultRefMismatch::AsRef {
1124+
span: expected_ty_expr.span.shrink_to_hi(), expected_ty, expr_ty, def_path
1125+
}
1126+
} else if let Some(clone_did) = self.tcx.lang_items().clone_trait()
1127+
&& rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions(
1128+
self,
1129+
self.param_env,
1130+
ty,
1131+
clone_did,
1132+
)
1133+
{
1134+
errors::OptionResultRefMismatch::Cloned {
1135+
span, def_path
1136+
}
1137+
} else {
1138+
return false;
1139+
};
1140+
diag.subdiagnostic(subdiag);
11491141
return true;
11501142
}
1151-
}
1152-
false
1153-
};
1154-
1155-
if let Some(result_did) = self.tcx.get_diagnostic_item(sym::Result)
1156-
&& adt_def.did() == result_did
1157-
// Check that the error types are equal
1158-
&& self.can_eq(self.param_env, args.type_at(1), expected_args.type_at(1))
1159-
{
1160-
return suggest_copied_cloned_or_as_ref();
1161-
} else if let Some(option_did) = self.tcx.get_diagnostic_item(sym::Option)
1162-
&& adt_def.did() == option_did
1163-
{
1164-
return suggest_copied_cloned_or_as_ref();
11651143
}
11661144

11671145
false

Diff for: ‎tests/ui/associated-types/dont-suggest-cyclic-constraint.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ LL | debug_assert_eq!(iter.next(), Some(value));
66
|
77
= note: expected enum `Option<<I as Iterator>::Item>`
88
found enum `Option<&<I as Iterator>::Item>`
9-
help: use `Option::as_ref()` to convert `Option<<I as Iterator>::Item>` to `Option<&<I as Iterator>::Item>`
9+
help: use `Option::as_ref` to convert `Option<<I as Iterator>::Item>` to `Option<&<I as Iterator>::Item>`
1010
|
1111
LL | debug_assert_eq!(iter.next().as_ref(), Some(value));
1212
| +++++++++

Diff for: ‎tests/ui/suggestions/copied-and-cloned.fixed

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,6 @@ fn main() {
2626
let y = Some(&s);
2727
println!("{}", x.as_ref() == y);
2828
//~^ ERROR mismatched types
29-
//~| HELP use `Option::as_ref()` to convert `Option<String>` to `Option<&String>`
29+
//~| HELP use `Option::as_ref` to convert `Option<String>` to `Option<&String>`
3030

3131
}

Diff for: ‎tests/ui/suggestions/copied-and-cloned.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,6 @@ fn main() {
2626
let y = Some(&s);
2727
println!("{}", x == y);
2828
//~^ ERROR mismatched types
29-
//~| HELP use `Option::as_ref()` to convert `Option<String>` to `Option<&String>`
29+
//~| HELP use `Option::as_ref` to convert `Option<String>` to `Option<&String>`
3030

3131
}

Diff for: ‎tests/ui/suggestions/copied-and-cloned.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ LL | println!("{}", x == y);
8686
|
8787
= note: expected enum `Option<String>`
8888
found enum `Option<&String>`
89-
help: use `Option::as_ref()` to convert `Option<String>` to `Option<&String>`
89+
help: use `Option::as_ref` to convert `Option<String>` to `Option<&String>`
9090
|
9191
LL | println!("{}", x.as_ref() == y);
9292
| +++++++++

0 commit comments

Comments
 (0)