feat: Diagnose trait errors 🎉

crates/hir-def/src/attrs.rs

@@ -263,6 +263,12 @@ fn match_attr_flags(attr_flags: &mut AttrFlags, attr: ast::Meta) -> ControlFlow<
                         }
                         _ => {}
                     },
+                    "diagnostic" => match &*second_segment {
+                        "do_not_recommend" => {
+                            attr_flags.insert(AttrFlags::DIAGNOSTIC_DO_NOT_RECOMMEND)
+                        }
+                        _ => {}
+                    },
                     _ => {}
                 },
             }
@@ -335,6 +341,8 @@ bitflags::bitflags! {
         const MACRO_STYLE_PARENTHESES = 1 << 48;
 
         const PREFER_UNDERSCORE_IMPORT = 1 << 49;
+
+        const DIAGNOSTIC_DO_NOT_RECOMMEND = 1 << 50;
     }
 }
 

crates/hir-def/src/expr_store/lower/format_args.rs

@@ -30,12 +30,14 @@ impl<'db> ExprCollector<'db> {
     ) -> ExprId {
         let mut args = FormatArgumentsCollector::default();
         f.args().for_each(|arg| {
+            let expr = arg.expr();
             args.add(FormatArgument {
                 kind: match arg.arg_name() {
                     Some(name) => FormatArgumentKind::Named(Name::new_root(name.name().text())),
                     None => FormatArgumentKind::Normal,
                 },
-                expr: self.collect_expr_opt(arg.expr()),
+                syntax: expr.as_ref().map(AstPtr::new),
+                expr: self.collect_expr_opt(expr),
             });
         });
         let template = f.template();
@@ -53,8 +55,10 @@ impl<'db> ExprCollector<'db> {
             Some(((s, is_direct_literal), template)) => {
                 let call_ctx = SyntaxContext::root(self.def_map.edition());
                 let hygiene = self.hygiene_id_for(s.syntax().text_range());
+                let template_ptr = AstPtr::new(&template);
                 let fmt = format_args::parse(
                     &s,
+                    template_ptr,
                     fmt_snippet,
                     args,
                     is_direct_literal,
@@ -65,10 +69,7 @@ impl<'db> ExprCollector<'db> {
                                 .template_map
                                 .get_or_insert_with(Default::default)
                                 .implicit_capture_to_source
-                                .insert(
-                                    expr_id,
-                                    self.expander.in_file((AstPtr::new(&template), range)),
-                                );
+                                .insert(expr_id, self.expander.in_file((template_ptr, range)));
                         }
                         if !hygiene.is_root() {
                             self.store.ident_hygiene.insert(expr_id.into(), hygiene);
@@ -340,7 +341,8 @@ impl<'db> ExprCollector<'db> {
                         expr: args_ident_expr,
                         name: Name::new_tuple_field(arg_index),
                     });
-                    self.make_argument(arg, ty)
+                    let arg_ptr = arguments.get(arg_index).and_then(|it| it.syntax);
+                    self.make_argument(arg_ptr, arg, ty)
                 })
                 .collect();
             let args =
@@ -557,7 +559,8 @@ impl<'db> ExprCollector<'db> {
                         rawness: Rawness::Ref,
                         mutability: Mutability::Shared,
                     });
-                    self.make_argument(arg, ty)
+                    let arg_ptr = arguments.get(arg_index).and_then(|it| it.syntax);
+                    self.make_argument(arg_ptr, arg, ty)
                 })
                 .collect();
             let array =
@@ -649,7 +652,8 @@ impl<'db> ExprCollector<'db> {
                         rawness: Rawness::Ref,
                         mutability: Mutability::Shared,
                     });
-                    self.make_argument(ref_arg, ty)
+                    let arg_ptr = arguments.get(arg_index).and_then(|it| it.syntax);
+                    self.make_argument(arg_ptr, ref_arg, ty)
                 })
                 .collect();
             let args =
@@ -716,7 +720,8 @@ impl<'db> ExprCollector<'db> {
                         expr: args_ident_expr,
                         name: Name::new_tuple_field(arg_index),
                     });
-                    self.make_argument(arg, ty)
+                    let arg_ptr = arguments.get(arg_index).and_then(|it| it.syntax);
+                    self.make_argument(arg_ptr, arg, ty)
                 })
                 .collect();
             let array =
@@ -976,11 +981,16 @@ impl<'db> ExprCollector<'db> {
     /// ```text
     ///     <core::fmt::Argument>::new_…(arg)
     /// ```
-    fn make_argument(&mut self, arg: ExprId, ty: ArgumentType) -> ExprId {
+    fn make_argument(
+        &mut self,
+        arg_ptr: Option<AstPtr<ast::Expr>>,
+        arg: ExprId,
+        ty: ArgumentType,
+    ) -> ExprId {
         use ArgumentType::*;
         use FormatTrait::*;
 
-        let new_fn = self.ty_rel_lang_path_desugared_expr(
+        let new_fn = self.ty_rel_lang_path(
             self.lang_items().FormatArgument,
             match ty {
                 Format(Display) => sym::new_display,
@@ -995,6 +1005,22 @@ impl<'db> ExprCollector<'db> {
                 Usize => sym::from_usize,
             },
         );
+        let new_fn = match new_fn {
+            Some(new_fn) => {
+                let new_fn = self.store.exprs.alloc(Expr::Path(new_fn));
+                if let Some(arg_ptr) = arg_ptr {
+                    // Trait errors (the argument does not implement the expected fmt trait) will show
+                    // on this path, so to not end up with synthetic syntax we insert this mapping. We
+                    // don't want to insert the other way's mapping in order to not override the source
+                    // for the argument.
+                    self.store
+                        .expr_map_back
+                        .insert(new_fn, self.expander.in_file(arg_ptr.wrap_left()));
+                }
+                new_fn
+            }
+            None => self.missing_expr(),
+        };
         self.alloc_expr_desugared(Expr::Call { callee: new_fn, args: Box::new([arg]) })
     }
 

crates/hir-def/src/hir/format_args.rs

@@ -7,7 +7,7 @@ use rustc_parse_format as parse;
 use span::SyntaxContext;
 use stdx::TupleExt;
 use syntax::{
-    TextRange,
+    AstPtr, TextRange,
     ast::{self, IsString},
 };
 
@@ -146,6 +146,7 @@ pub enum FormatCount {
 pub struct FormatArgument {
     pub kind: FormatArgumentKind,
     pub expr: ExprId,
+    pub syntax: Option<AstPtr<ast::Expr>>,
 }
 
 #[derive(Clone, PartialEq, Eq, Debug)]
@@ -171,6 +172,7 @@ use PositionUsedAs::*;
 #[allow(clippy::unnecessary_lazy_evaluations)]
 pub(crate) fn parse(
     s: &ast::String,
+    string_ptr: AstPtr<ast::Expr>,
     fmt_snippet: Option<String>,
     mut args: FormatArgumentsCollector,
     is_direct_literal: bool,
@@ -273,6 +275,11 @@ pub(crate) fn parse(
                             // FIXME: This is problematic, we might want to synthesize a dummy
                             // expression proper and/or desugar these.
                             expr: synth(name, span),
+                            // FIXME: This will lead us to show failed trait bounds (e.g. `T: Display`)
+                            // on the whole template string for implicit arguments instead of just their name.
+                            // Fixing this is hard since we don't have an `AstPtr` for the arg, and it's
+                            // only part of an expression.
+                            syntax: Some(string_ptr),
                         }))
                     }
                 }

crates/hir-expand/src/files.rs

@@ -128,6 +128,16 @@ impl ErasedAstId {
     }
 }
 
+impl<FileKind, N: AstNode> InFileWrapper<FileKind, AstPtr<N>> {
+    #[inline]
+    pub fn upcast<M: AstNode>(self) -> InFileWrapper<FileKind, AstPtr<M>>
+    where
+        N: Into<M>,
+    {
+        self.map(|it| it.upcast())
+    }
+}
+
 impl<FileKind, T> InFileWrapper<FileKind, T> {
     pub fn new(file_id: FileKind, value: T) -> Self {
         Self { file_id, value }

crates/hir-ty/src/autoderef.rs

@@ -40,7 +40,7 @@ pub fn autoderef<'db>(
     let interner = DbInterner::new_with(db, env.krate);
     let infcx = interner.infer_ctxt().build(TypingMode::PostAnalysis);
     let (ty, _) = infcx.instantiate_canonical(Span::Dummy, &ty);
-    let autoderef = Autoderef::new(&infcx, env.param_env, ty);
+    let autoderef = Autoderef::new(&infcx, env.param_env, ty, Span::Dummy);
     let mut v = Vec::new();
     for (ty, _steps) in autoderef {
         // `ty` may contain unresolved inference variables. Since there's no chance they would be
@@ -155,6 +155,7 @@ pub(crate) struct GeneralAutoderef<'db, Ctx, Steps = Vec<(Ty<'db>, AutoderefKind
     // Configurations:
     include_raw_pointers: bool,
     use_receiver_trait: bool,
+    span: Span,
 }
 
 pub(crate) type Autoderef<'a, 'db, Steps = Vec<(Ty<'db>, AutoderefKind)>> =
@@ -200,7 +201,7 @@ where
                 // autoderef expect this type to have been structurally normalized.
                 if let TyKind::Alias(..) = ty.kind() {
                     let (normalized_ty, obligations) =
-                        structurally_normalize_ty(self.infcx(), self.param_env(), ty)?;
+                        structurally_normalize_ty(self.infcx(), self.param_env(), ty, self.span)?;
                     self.state.obligations.extend(obligations);
                     (AutoderefKind::Builtin, normalized_ty)
                 } else {
@@ -232,8 +233,9 @@ impl<'a, 'db> Autoderef<'a, 'db> {
         infcx: &'a InferCtxt<'db>,
         param_env: ParamEnv<'db>,
         base_ty: Ty<'db>,
+        span: Span,
     ) -> Self {
-        Self::new_impl(DefaultAutoderefCtx { infcx, param_env }, base_ty)
+        Self::new_impl(DefaultAutoderefCtx { infcx, param_env }, base_ty, span)
     }
 }
 
@@ -242,8 +244,9 @@ impl<'a, 'b, 'db> InferenceContextAutoderef<'a, 'b, 'db> {
     pub(crate) fn new_from_inference_context(
         ctx: &'a mut InferenceContext<'b, 'db>,
         base_ty: Ty<'db>,
+        span: Span,
     ) -> Self {
-        Self::new_impl(InferenceContextAutoderefCtx(ctx), base_ty)
+        Self::new_impl(InferenceContextAutoderefCtx(ctx), base_ty, span)
     }
 
     #[inline]
@@ -258,8 +261,9 @@ impl<'a, 'db> Autoderef<'a, 'db, usize> {
         infcx: &'a InferCtxt<'db>,
         param_env: ParamEnv<'db>,
         base_ty: Ty<'db>,
+        span: Span,
     ) -> Self {
-        Self::new_impl(DefaultAutoderefCtx { infcx, param_env }, base_ty)
+        Self::new_impl(DefaultAutoderefCtx { infcx, param_env }, base_ty, span)
     }
 }
 
@@ -269,7 +273,7 @@ where
     Steps: TrackAutoderefSteps<'db>,
 {
     #[inline]
-    fn new_impl(ctx: Ctx, base_ty: Ty<'db>) -> Self {
+    fn new_impl(ctx: Ctx, base_ty: Ty<'db>, span: Span) -> Self {
         GeneralAutoderef {
             state: AutoderefSnapshot {
                 steps: Steps::default(),
@@ -282,6 +286,7 @@ where
             traits: None,
             include_raw_pointers: false,
             use_receiver_trait: false,
+            span,
         }
     }
 
@@ -338,7 +343,7 @@ where
 
         let trait_ref = TraitRef::new(interner, trait_.into(), [ty]);
         let obligation =
-            Obligation::new(interner, ObligationCause::new(), self.param_env(), trait_ref);
+            Obligation::new(interner, ObligationCause::new(self.span), self.param_env(), trait_ref);
         // We detect whether the self type implements `Deref` before trying to
         // structurally normalize. We use `predicate_may_hold_opaque_types_jank`
         // to support not-yet-defined opaque types. It will succeed for `impl Deref`
@@ -352,6 +357,7 @@ where
             self.infcx(),
             self.param_env(),
             Ty::new_projection(interner, trait_target.into(), [ty]),
+            self.span,
         )?;
         debug!("overloaded_deref_ty({:?}) = ({:?}, {:?})", ty, normalized_ty, obligations);
         self.state.obligations.extend(obligations);
@@ -403,9 +409,11 @@ fn structurally_normalize_ty<'db>(
     infcx: &InferCtxt<'db>,
     param_env: ParamEnv<'db>,
     ty: Ty<'db>,
+    span: Span,
 ) -> Option<(Ty<'db>, PredicateObligations<'db>)> {
     let mut ocx = ObligationCtxt::new(infcx);
-    let Ok(normalized_ty) = ocx.structurally_normalize_ty(&ObligationCause::misc(), param_env, ty)
+    let Ok(normalized_ty) =
+        ocx.structurally_normalize_ty(&ObligationCause::new(span), param_env, ty)
     else {
         // We shouldn't have errors here in the old solver, except for
         // evaluate/fulfill mismatches, but that's not a reason for an ICE.

crates/hir-ty/src/diagnostics/expr.rs

@@ -177,7 +177,7 @@ impl<'db> ExprValidator<'db> {
         }
         // Check that the number of arguments matches the number of parameters.
 
-        if self.infer.expr_type_mismatches().next().is_some() {
+        if self.infer.exprs_have_type_mismatches() {
             // FIXME: Due to shortcomings in the current type system implementation, only emit
             // this diagnostic if there are no type mismatches in the containing function.
         } else if let Expr::MethodCall { receiver, .. } = expr {
@@ -331,7 +331,7 @@ impl<'db> ExprValidator<'db> {
             let &Statement::Let { pat, initializer, else_branch: None, .. } = stmt else {
                 continue;
             };
-            if self.infer.type_mismatch_for_pat(pat).is_some() {
+            if self.infer.pat_has_type_mismatch(pat) {
                 continue;
             }
             let Some(initializer) = initializer else { continue };
@@ -628,13 +628,13 @@ pub fn record_pattern_missing_fields(
 
 fn types_of_subpatterns_do_match(pat: PatId, body: &Body, infer: &InferenceResult) -> bool {
     fn walk(pat: PatId, body: &Body, infer: &InferenceResult, has_type_mismatches: &mut bool) {
-        match infer.type_mismatch_for_pat(pat) {
-            Some(_) => *has_type_mismatches = true,
-            None if *has_type_mismatches => (),
-            None => {
+        match infer.pat_has_type_mismatch(pat) {
+            true => *has_type_mismatches = true,
+            false if *has_type_mismatches => (),
+            false => {
                 let pat = &body[pat];
                 if let Pat::ConstBlock(expr) | Pat::Lit(expr) = *pat {
-                    *has_type_mismatches |= infer.type_mismatch_for_expr(expr).is_some();
+                    *has_type_mismatches |= infer.expr_has_type_mismatch(expr);
                     if *has_type_mismatches {
                         return;
                     }

crates/hir-ty/src/display.rs

@@ -59,8 +59,8 @@ use crate::{
     next_solver::{
         AliasTy, Allocation, Clause, ClauseKind, Const, ConstKind, DbInterner,
         ExistentialPredicate, FnSig, GenericArg, GenericArgKind, GenericArgs, ParamEnv, PolyFnSig,
-        Region, SolverDefId, StoredEarlyBinder, StoredTy, Term, TermKind, TraitRef, Ty, TyKind,
-        TypingMode, ValTree,
+        Region, SolverDefId, StoredEarlyBinder, StoredTy, Term, TermKind, TraitPredicate, TraitRef,
+        Ty, TyKind, TypingMode, ValTree,
         abi::Safety,
         infer::{DbInternerInferExt, traits::ObligationCause},
     },
@@ -771,7 +771,7 @@ fn render_const_scalar<'db>(
 ) -> Result {
     let param_env = ParamEnv::empty();
     let infcx = f.interner.infer_ctxt().build(TypingMode::PostAnalysis);
-    let ty = infcx.at(&ObligationCause::new(), param_env).deeply_normalize(ty).unwrap_or(ty);
+    let ty = infcx.at(&ObligationCause::dummy(), param_env).deeply_normalize(ty).unwrap_or(ty);
     render_const_scalar_inner(f, b, memory_map, ty, param_env)
 }
 
@@ -1056,7 +1056,7 @@ fn render_const_scalar_from_valtree<'db>(
 ) -> Result {
     let param_env = ParamEnv::empty();
     let infcx = f.interner.infer_ctxt().build(TypingMode::PostAnalysis);
-    let ty = infcx.at(&ObligationCause::new(), param_env).deeply_normalize(ty).unwrap_or(ty);
+    let ty = infcx.at(&ObligationCause::dummy(), param_env).deeply_normalize(ty).unwrap_or(ty);
     render_const_scalar_from_valtree_inner(f, ty, valtree, param_env)
 }
 
@@ -2269,6 +2269,23 @@ impl<'db> HirDisplay<'db> for TraitRef<'db> {
     }
 }
 
+impl<'db> HirDisplay<'db> for TraitPredicate<'db> {
+    fn hir_fmt(&self, f: &mut HirFormatter<'_, 'db>) -> Result {
+        self.self_ty().hir_fmt(f)?;
+        f.write_str(": ")?;
+        match self.polarity {
+            rustc_type_ir::PredicatePolarity::Positive => {}
+            rustc_type_ir::PredicatePolarity::Negative => f.write_char('!')?,
+        }
+        let trait_ = self.def_id().0;
+        f.start_location_link(trait_.into());
+        write!(f, "{}", TraitSignature::of(f.db, trait_).name.display(f.db, f.edition()))?;
+        f.end_location_link();
+        let substs = &self.trait_ref.args[1..];
+        hir_fmt_generic_args(f, substs, None, None)
+    }
+}
+
 impl<'db> HirDisplay<'db> for Region<'db> {
     fn hir_fmt(&self, f: &mut HirFormatter<'_, 'db>) -> Result {
         match self.kind() {