Rollup merge of #151530 - reddevilmidzy:e0106, r=fee1-dead

Fix invalid `mut T` suggestion for `&mut T` in missing lifetime error

close: rust-lang/rust#150077

When suggesting to return an owned value instead of a borrowed one, the diagnostic was only removing `&` instead of `&mut `, resulting in invalid syntax like `mut T`. This PR fixes the span calculation to properly cover the entire `&mut ` prefix.

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -4060,25 +4060,32 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
                             "instead, you are more likely to want"
                         };
                         let mut owned_sugg = lt.kind == MissingLifetimeKind::Ampersand;
+                        let mut sugg_is_str_to_string = false;
                         let mut sugg = vec![(lt.span, String::new())];
                         if let Some((kind, _span)) = self.diag_metadata.current_function
                             && let FnKind::Fn(_, _, ast::Fn { sig, .. }) = kind
-                            && let ast::FnRetTy::Ty(ty) = &sig.decl.output
                         {
                             let mut lt_finder =
                                 LifetimeFinder { lifetime: lt.span, found: None, seen: vec![] };
-                            lt_finder.visit_ty(&ty);
+                            for param in &sig.decl.inputs {
-
+                                lt_finder.visit_ty(&param.ty);
-                            if let [Ty { span, kind: TyKind::Ref(_, mut_ty), .. }] =
+                            }
-                                &lt_finder.seen[..]
+                            if let ast::FnRetTy::Ty(ret_ty) = &sig.decl.output {
-                            {
+                                lt_finder.visit_ty(ret_ty);
-                                // We might have a situation like
+                                let mut ret_lt_finder =
-                                // fn g(mut x: impl Iterator<Item = &'_ ()>) -> Option<&'_ ()>
+                                    LifetimeFinder { lifetime: lt.span, found: None, seen: vec![] };
-                                // but `lt.span` only points at `'_`, so to suggest `-> Option<()>`
+                                ret_lt_finder.visit_ty(ret_ty);
-                                // we need to find a more accurate span to end up with
+                                if let [Ty { span, kind: TyKind::Ref(_, mut_ty), .. }] =
-                                // fn g<'a>(mut x: impl Iterator<Item = &'_ ()>) -> Option<()>
+                                    &ret_lt_finder.seen[..]
-                                sugg = vec![(span.with_hi(mut_ty.ty.span.lo()), String::new())];
+                                {
-                                owned_sugg = true;
+                                    // We might have a situation like
+                                    // fn g(mut x: impl Iterator<Item = &'_ ()>) -> Option<&'_ ()>
+                                    // but `lt.span` only points at `'_`, so to suggest `-> Option<()>`
+                                    // we need to find a more accurate span to end up with
+                                    // fn g<'a>(mut x: impl Iterator<Item = &'_ ()>) -> Option<()>
+                                    sugg = vec![(span.with_hi(mut_ty.ty.span.lo()), String::new())];
+                                    owned_sugg = true;
+                                }
                             }
                             if let Some(ty) = lt_finder.found {
                                 if let TyKind::Path(None, path) = &ty.kind {
@@ -4098,6 +4105,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
                                                         lt.span.with_hi(ty.span.hi()),
                                                         "String".to_string(),
                                                     )];
+                                                    sugg_is_str_to_string = true;
                                                 }
                                                 Some(Res::PrimTy(..)) => {}
                                                 Some(Res::Def(
@@ -4124,6 +4132,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
                                                         lt.span.with_hi(ty.span.hi()),
                                                         "String".to_string(),
                                                     )];
+                                                    sugg_is_str_to_string = true;
                                                 }
                                                 Res::PrimTy(..) => {}
                                                 Res::Def(
@@ -4158,6 +4167,12 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
                             }
                         }
                         if owned_sugg {
+                            if let Some(span) =
+                                self.find_ref_prefix_span_for_owned_suggestion(lt.span)
+                                && !sugg_is_str_to_string
+                            {
+                                sugg = vec![(span, String::new())];
+                            }
                             err.multipart_suggestion_verbose(
                                 format!("{pre} to return an owned value"),
                                 sugg,
@@ -4184,6 +4199,23 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
             }
         }
     }
+
+    fn find_ref_prefix_span_for_owned_suggestion(&self, lifetime: Span) -> Option<Span> {
+        let mut finder = RefPrefixSpanFinder { lifetime, span: None };
+        if let Some(item) = self.diag_metadata.current_item {
+            finder.visit_item(item);
+        } else if let Some((kind, _span)) = self.diag_metadata.current_function
+            && let FnKind::Fn(_, _, ast::Fn { sig, .. }) = kind
+        {
+            for param in &sig.decl.inputs {
+                finder.visit_ty(&param.ty);
+            }
+            if let ast::FnRetTy::Ty(ret_ty) = &sig.decl.output {
+                finder.visit_ty(ret_ty);
+            }
+        }
+        finder.span
+    }
 }
 
 fn mk_where_bound_predicate(
@@ -4285,6 +4317,26 @@ impl<'ast> Visitor<'ast> for LifetimeFinder<'ast> {
     }
 }
 
+struct RefPrefixSpanFinder {
+    lifetime: Span,
+    span: Option<Span>,
+}
+
+impl<'ast> Visitor<'ast> for RefPrefixSpanFinder {
+    fn visit_ty(&mut self, t: &'ast Ty) {
+        if self.span.is_some() {
+            return;
+        }
+        if let TyKind::Ref(_, mut_ty) | TyKind::PinnedRef(_, mut_ty) = &t.kind
+            && t.span.lo() == self.lifetime.lo()
+        {
+            self.span = Some(t.span.with_hi(mut_ty.ty.span.lo()));
+            return;
+        }
+        walk_ty(self, t);
+    }
+}
+
 /// Shadowing involving a label is only a warning for historical reasons.
 //FIXME: make this a proper lint.
 pub(super) fn signal_label_shadowing(sess: &Session, orig: Span, shadower: Ident) {

tests/ui/lifetimes/mut-ref-owned-suggestion.rs

@@ -0,0 +1,29 @@
+//! Regression test for <https://github.com/rust-lang/rust/issues/150077>
+//! Tests that `&mut T` suggests `T`, not `mut T`, `&mut str` suggests `String`, not `str`,
+//! when recommending an owned value.
+fn with_fn(_f: impl Fn() -> &mut ()) {}
+//~^ ERROR: missing lifetime specifier
+
+fn with_ref_mut_str(_f: impl Fn() -> &mut str) {}
+//~^ ERROR: missing lifetime specifier
+
+fn with_fn_has_return(_f: impl Fn() -> &mut ()) -> i32 {
+    //~^ ERROR: missing lifetime specifier
+    2
+}
+
+fn with_dyn(_f: Box<dyn Fn() -> &mut i32>) {}
+//~^ ERROR: missing lifetime specifier
+
+fn trait_bound<F: Fn() -> &mut i32>(_f: F) {}
+//~^ ERROR: missing lifetime specifier
+
+fn nested_result(_f: impl Fn() -> Result<&mut i32, ()>) {}
+//~^ ERROR: missing lifetime specifier
+
+struct Holder<F: Fn() -> &mut i32> {
+    //~^ ERROR: missing lifetime specifier
+    f: F,
+}
+
+fn main() {}

tests/ui/lifetimes/mut-ref-owned-suggestion.stderr

@@ -0,0 +1,137 @@
+error[E0106]: missing lifetime specifier
+  --> $DIR/mut-ref-owned-suggestion.rs:4:29
+   |
+LL | fn with_fn(_f: impl Fn() -> &mut ()) {}
+   |                             ^ expected named lifetime parameter
+   |
+   = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
+help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
+   |
+LL | fn with_fn(_f: impl Fn() -> &'static mut ()) {}
+   |                              +++++++
+help: instead, you are more likely to want to return an owned value
+   |
+LL - fn with_fn(_f: impl Fn() -> &mut ()) {}
+LL + fn with_fn(_f: impl Fn() -> ()) {}
+   |
+
+error[E0106]: missing lifetime specifier
+  --> $DIR/mut-ref-owned-suggestion.rs:7:38
+   |
+LL | fn with_ref_mut_str(_f: impl Fn() -> &mut str) {}
+   |                                      ^ expected named lifetime parameter
+   |
+   = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
+help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
+   |
+LL | fn with_ref_mut_str(_f: impl Fn() -> &'static mut str) {}
+   |                                       +++++++
+help: instead, you are more likely to want to return an owned value
+   |
+LL - fn with_ref_mut_str(_f: impl Fn() -> &mut str) {}
+LL + fn with_ref_mut_str(_f: impl Fn() -> String) {}
+   |
+
+error[E0106]: missing lifetime specifier
+  --> $DIR/mut-ref-owned-suggestion.rs:10:40
+   |
+LL | fn with_fn_has_return(_f: impl Fn() -> &mut ()) -> i32 {
+   |                                        ^ expected named lifetime parameter
+   |
+   = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
+   = note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
+help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
+   |
+LL | fn with_fn_has_return(_f: impl Fn() -> &'static mut ()) -> i32 {
+   |                                         +++++++
+help: consider making the bound lifetime-generic with a new `'a` lifetime
+   |
+LL - fn with_fn_has_return(_f: impl Fn() -> &mut ()) -> i32 {
+LL + fn with_fn_has_return(_f: impl for<'a> Fn() -> &'a ()) -> i32 {
+   |
+help: consider introducing a named lifetime parameter
+   |
+LL - fn with_fn_has_return(_f: impl Fn() -> &mut ()) -> i32 {
+LL + fn with_fn_has_return<'a>(_f: impl Fn() -> &'a ()) -> i32 {
+   |
+help: alternatively, you might want to return an owned value
+   |
+LL - fn with_fn_has_return(_f: impl Fn() -> &mut ()) -> i32 {
+LL + fn with_fn_has_return(_f: impl Fn() -> ()) -> i32 {
+   |
+
+error[E0106]: missing lifetime specifier
+  --> $DIR/mut-ref-owned-suggestion.rs:15:33
+   |
+LL | fn with_dyn(_f: Box<dyn Fn() -> &mut i32>) {}
+   |                                 ^ expected named lifetime parameter
+   |
+   = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
+help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
+   |
+LL | fn with_dyn(_f: Box<dyn Fn() -> &'static mut i32>) {}
+   |                                  +++++++
+help: instead, you are more likely to want to return an owned value
+   |
+LL - fn with_dyn(_f: Box<dyn Fn() -> &mut i32>) {}
+LL + fn with_dyn(_f: Box<dyn Fn() -> i32>) {}
+   |
+
+error[E0106]: missing lifetime specifier
+  --> $DIR/mut-ref-owned-suggestion.rs:18:27
+   |
+LL | fn trait_bound<F: Fn() -> &mut i32>(_f: F) {}
+   |                           ^ expected named lifetime parameter
+   |
+   = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
+help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
+   |
+LL | fn trait_bound<F: Fn() -> &'static mut i32>(_f: F) {}
+   |                            +++++++
+help: instead, you are more likely to want to change the argument to be borrowed...
+   |
+LL | fn trait_bound<F: Fn() -> &mut i32>(_f: &F) {}
+   |                                         +
+help: ...or alternatively, you might want to return an owned value
+   |
+LL - fn trait_bound<F: Fn() -> &mut i32>(_f: F) {}
+LL + fn trait_bound<F: Fn() -> i32>(_f: F) {}
+   |
+
+error[E0106]: missing lifetime specifier
+  --> $DIR/mut-ref-owned-suggestion.rs:21:42
+   |
+LL | fn nested_result(_f: impl Fn() -> Result<&mut i32, ()>) {}
+   |                                          ^ expected named lifetime parameter
+   |
+   = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
+help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
+   |
+LL | fn nested_result(_f: impl Fn() -> Result<&'static mut i32, ()>) {}
+   |                                           +++++++
+help: instead, you are more likely to want to return an owned value
+   |
+LL - fn nested_result(_f: impl Fn() -> Result<&mut i32, ()>) {}
+LL + fn nested_result(_f: impl Fn() -> Result<i32, ()>) {}
+   |
+
+error[E0106]: missing lifetime specifier
+  --> $DIR/mut-ref-owned-suggestion.rs:24:26
+   |
+LL | struct Holder<F: Fn() -> &mut i32> {
+   |                          ^ expected named lifetime parameter
+   |
+   = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
+help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
+   |
+LL | struct Holder<F: Fn() -> &'static mut i32> {
+   |                           +++++++
+help: instead, you are more likely to want to return an owned value
+   |
+LL - struct Holder<F: Fn() -> &mut i32> {
+LL + struct Holder<F: Fn() -> i32> {
+   |
+
+error: aborting due to 7 previous errors
+
+For more information about this error, try `rustc --explain E0106`.