From ee93d22d3cecd6941a2659f5e551f7ac32b8f826 Mon Sep 17 00:00:00 2001
From: Georg Semmler <github@weiznich.de>
Date: Sat, 4 May 2024 14:26:24 +0200
Subject: [PATCH] Actually use the `#[do_not_recommend]` attribute if present

This change tweaks the error message generation to actually use the
`#[do_not_recommend]` attribute if present by just skipping the marked
trait impl in favour of the parent impl. It also adds a compile test for
this behaviour. Without this change the test would output the following
error:

```
error[E0277]: the trait bound `&str: Expression` is not satisfied
  --> /home/weiznich/Documents/rust/rust/tests/ui/diagnostic_namespace/do_not_recommend.rs:53:15
   |
LL |     SelectInt.check("bar");
   |               ^^^^^ the trait `Expression` is not implemented for `&str`, which is required by `&str: AsExpression<Integer>`
   |
   = help: the following other types implement trait `Expression`:
             Bound<T>
             SelectInt
note: required for `&str` to implement `AsExpression<Integer>`
  --> /home/weiznich/Documents/rust/rust/tests/ui/diagnostic_namespace/do_not_recommend.rs:26:13
   |
LL | impl<T, ST> AsExpression<ST> for T
   |             ^^^^^^^^^^^^^^^^     ^
LL | where
LL |     T: Expression<SqlType = ST>,
   |        ------------------------ unsatisfied trait bound introduced here
```

Note how that mentions `&str: Expression` before and now mentions `&str:
AsExpression<Integer>` instead which is much more helpful for users.

Open points for further changes before stabilization:

* We likely want to move the attribute to the `#[diagnostic]` namespace
to relax the guarantees given?
* How does it interact with the new trait solver?
---
 .../error_reporting/type_err_ctxt_ext.rs      | 33 +++++++----
 .../diagnostic_namespace/do_not_recommend.rs  | 55 +++++++++++++++++++
 .../do_not_recommend.stderr                   | 13 +++++
 3 files changed, 89 insertions(+), 12 deletions(-)
 create mode 100644 tests/ui/diagnostic_namespace/do_not_recommend.rs
 create mode 100644 tests/ui/diagnostic_namespace/do_not_recommend.stderr

diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
index e50cb2af4b857..852e0837d3624 100644
--- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
+++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
@@ -463,6 +463,19 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                                 ),
                                 root_obligation,
                             )
+                        } else if matches!(
+                            obligation.cause.code(),
+                            ObligationCauseCode::ImplDerivedObligation(c) if tcx.has_attr(c.impl_or_alias_def_id, sym::do_not_recommend)
+                        ) {
+                            let (code, base) = obligation.cause.code().peel_derives_with_predicate();
+                            if let Some(base) = base {
+                                let code = code.clone();
+                                obligation.cause.map_code(|_| code);
+                                // FIXME: somehow we need to overwrite obligation.predicate here
+                                (base, &obligation)
+                            } else {
+                                (trait_predicate, &obligation)
+                            }
                         } else {
                             (trait_predicate, &obligation)
                         };
@@ -2473,18 +2486,14 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                             | hir::ExprKind::Path(hir::QPath::Resolved(None, path)),
                         ..
                     }) = expr_finder.result
-                        && let [
-                            ..,
-                            trait_path_segment @ hir::PathSegment {
-                                res: Res::Def(DefKind::Trait, trait_id),
-                                ..
-                            },
-                            hir::PathSegment {
-                                ident: assoc_item_name,
-                                res: Res::Def(_, item_id),
-                                ..
-                            },
-                        ] = path.segments
+                        && let [.., trait_path_segment @ hir::PathSegment {
+                            res: Res::Def(DefKind::Trait, trait_id),
+                            ..
+                        }, hir::PathSegment {
+                            ident: assoc_item_name,
+                            res: Res::Def(_, item_id),
+                            ..
+                        }] = path.segments
                         && data.trait_ref.def_id == *trait_id
                         && self.tcx.trait_of_item(*item_id) == Some(*trait_id)
                         && let None = self.tainted_by_errors()
diff --git a/tests/ui/diagnostic_namespace/do_not_recommend.rs b/tests/ui/diagnostic_namespace/do_not_recommend.rs
new file mode 100644
index 0000000000000..8b7ab81011f60
--- /dev/null
+++ b/tests/ui/diagnostic_namespace/do_not_recommend.rs
@@ -0,0 +1,55 @@
+#![feature(do_not_recommend)]
+
+pub trait Expression {
+    type SqlType;
+}
+
+pub trait AsExpression<ST> {
+    type Expression: Expression<SqlType = ST>;
+}
+
+pub struct Text;
+pub struct Integer;
+
+pub struct Bound<T>(T);
+pub struct SelectInt;
+
+impl Expression for SelectInt {
+    type SqlType = Integer;
+}
+
+impl<T> Expression for Bound<T> {
+    type SqlType = T;
+}
+
+#[do_not_recommend]
+impl<T, ST> AsExpression<ST> for T
+where
+    T: Expression<SqlType = ST>,
+{
+    type Expression = T;
+}
+
+impl AsExpression<Integer> for i32 {
+    type Expression = Bound<Integer>;
+}
+
+impl AsExpression<Text> for &'_ str {
+    type Expression = Bound<Text>;
+}
+
+trait Foo: Expression + Sized {
+    fn check<T>(&self, _: T) -> <T as AsExpression<<Self as Expression>::SqlType>>::Expression
+    where
+        T: AsExpression<Self::SqlType>,
+    {
+        todo!()
+    }
+}
+
+impl<T> Foo for T where T: Expression {}
+
+fn main() {
+    SelectInt.check("bar");
+    //~^ERROR the trait bound `&str: AsExpression<Integer>` is not satisfied
+}
diff --git a/tests/ui/diagnostic_namespace/do_not_recommend.stderr b/tests/ui/diagnostic_namespace/do_not_recommend.stderr
new file mode 100644
index 0000000000000..e85f15f2f7a1f
--- /dev/null
+++ b/tests/ui/diagnostic_namespace/do_not_recommend.stderr
@@ -0,0 +1,13 @@
+error[E0277]: the trait bound `&str: AsExpression<Integer>` is not satisfied
+  --> $DIR/do_not_recommend.rs:53:15
+   |
+LL |     SelectInt.check("bar");
+   |               ^^^^^ the trait `Expression` is not implemented for `&str`
+   |
+   = help: the following other types implement trait `AsExpression<ST>`:
+             <&str as AsExpression<Text>>
+             <i32 as AsExpression<Integer>>
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0277`.