Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -917,6 +917,30 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
"`count` is defined on `{iterator_trait}`, which `{rcvr_ty}` does not implement"
));
}
} else if matches!(item_ident.name.as_str(), "cloned" | "copied")
&& let ty::Adt(adt_def, args) = rcvr_ty.kind()
&& tcx.is_diagnostic_item(sym::Option, adt_def.did())
&& let inner_ty = args.type_at(0)
// Skip refs (`Option<&T>.into_iter().cloned()` is valid, let the default branch
// handle it), and params/infer where we can't statically rule out a reference.
&& !matches!(inner_ty.kind(), ty::Ref(..) | ty::Param(_) | ty::Infer(_))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you motivate this (and add a test that the correct error is emitted when the inner type is one of these)?

{
// The default branch below would suggest `.into_iter()`, but that still
// fails: `Option<T>` yields `T` by value, not `&T`, so `.cloned()`/`.copied()`
// can't resolve. Give a targeted diagnostic instead.
err.span_label(span, format!("this method is only available on `Option<&_>`"));
if let SelfSource::MethodCall(rcvr_expr) = source
&& !span.in_external_macro(tcx.sess.source_map())
{
let call_expr = self.tcx.hir_expect_expr(self.tcx.parent_hir_id(rcvr_expr.hir_id));
err.span_suggestion(
rcvr_expr.span.shrink_to_hi().to(call_expr.span.shrink_to_hi()),
format!("consider removing the `.{}()` call", item_ident.name),
"",
Applicability::MaybeIncorrect,
);
}
return Err(());
} else if self.impl_into_iterator_should_be_iterator(rcvr_ty, span, unsatisfied_predicates)
{
err.span_label(span, format!("`{rcvr_ty}` is not an iterator"));
Expand Down
27 changes: 27 additions & 0 deletions tests/ui/methods/option-cloned-copied-non-ref-excluded.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Tests that the guard for `.cloned()`/`.copied()` on `Option<T>` correctly
// excludes inner types where the targeted diagnostic would be wrong:
//
// - `Option<&T>`: `Option<&T>` has inherent `cloned()`/`copied()` methods,
// so these compile successfully and the guard must not fire.
// - `Option<T>` (generic param): falls through to the standard "not an
// iterator / call .into_iter() first" diagnostic.
//
// See https://github.com/rust-lang/rust/issues/151147

// Reference inner type: these should compile without error.
pub fn cloned_on_ref(x: Option<&i32>) -> Option<i32> {
x.cloned()
}

pub fn copied_on_ref(x: Option<&i32>) -> Option<i32> {
x.copied()
}

// Generic param inner type: falls through to the standard diagnostic.
pub fn cloned_on_param<T: Clone>(x: Option<T>) {
x.cloned();
//~^ ERROR no method named `cloned` found for enum `Option<T>` in the current scope
//~| HELP call `.into_iter()` first
}

fn main() {}
14 changes: 14 additions & 0 deletions tests/ui/methods/option-cloned-copied-non-ref-excluded.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0599]: no method named `cloned` found for enum `Option<T>` in the current scope
--> $DIR/option-cloned-copied-non-ref-excluded.rs:22:7
|
LL | x.cloned();
| ^^^^^^ `Option<T>` is not an iterator
|
help: call `.into_iter()` first
|
LL | x.into_iter().cloned();
| ++++++++++++

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0599`.
26 changes: 26 additions & 0 deletions tests/ui/methods/option-cloned-copied-non-ref.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Tests that calling `.cloned()` or `.copied()` on `Option<T>` where `T` is an
// owned (non-reference) type gives a targeted diagnostic instead of the
// misleading "not an iterator / call .into_iter() first" suggestion.
// See https://github.com/rust-lang/rust/issues/151147

//@ run-rustfix

pub fn cloned_on_owned(x: Option<i32>) -> i32 {
x.unwrap()
//~^ ERROR no method named `cloned` found for enum `Option<i32>` in the current scope
//~| HELP consider removing the `.cloned()` call
}

pub fn copied_on_owned(x: Option<i32>) -> i32 {
x.unwrap()
//~^ ERROR no method named `copied` found for enum `Option<i32>` in the current scope
//~| HELP consider removing the `.copied()` call
}

pub fn cloned_on_string(x: Option<String>) -> String {
x.unwrap()
//~^ ERROR no method named `cloned` found for enum `Option<String>` in the current scope
//~| HELP consider removing the `.cloned()` call
}

fn main() {}
26 changes: 26 additions & 0 deletions tests/ui/methods/option-cloned-copied-non-ref.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Tests that calling `.cloned()` or `.copied()` on `Option<T>` where `T` is an
// owned (non-reference) type gives a targeted diagnostic instead of the
// misleading "not an iterator / call .into_iter() first" suggestion.
// See https://github.com/rust-lang/rust/issues/151147

//@ run-rustfix

pub fn cloned_on_owned(x: Option<i32>) -> i32 {
x.cloned().unwrap()
//~^ ERROR no method named `cloned` found for enum `Option<i32>` in the current scope
//~| HELP consider removing the `.cloned()` call
}

pub fn copied_on_owned(x: Option<i32>) -> i32 {
x.copied().unwrap()
//~^ ERROR no method named `copied` found for enum `Option<i32>` in the current scope
//~| HELP consider removing the `.copied()` call
}

pub fn cloned_on_string(x: Option<String>) -> String {
x.cloned().unwrap()
//~^ ERROR no method named `cloned` found for enum `Option<String>` in the current scope
//~| HELP consider removing the `.cloned()` call
}

fn main() {}
30 changes: 30 additions & 0 deletions tests/ui/methods/option-cloned-copied-non-ref.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
error[E0599]: no method named `cloned` found for enum `Option<i32>` in the current scope
--> $DIR/option-cloned-copied-non-ref.rs:9:7
|
LL | x.cloned().unwrap()
| -^^^^^^--
| ||
| |this method is only available on `Option<&_>`
| help: consider removing the `.cloned()` call

error[E0599]: no method named `copied` found for enum `Option<i32>` in the current scope
--> $DIR/option-cloned-copied-non-ref.rs:15:7
|
LL | x.copied().unwrap()
| -^^^^^^--
| ||
| |this method is only available on `Option<&_>`
| help: consider removing the `.copied()` call

error[E0599]: no method named `cloned` found for enum `Option<String>` in the current scope
--> $DIR/option-cloned-copied-non-ref.rs:21:7
|
LL | x.cloned().unwrap()
| -^^^^^^--
| ||
| |this method is only available on `Option<&_>`
| help: consider removing the `.cloned()` call

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0599`.
Loading