Make lint type_alias_bounds's removal sugg maybe-incorrect if the RHS contains shorthand assoc tys

Author: fmease

Diff for: compiler/rustc_lint/src/builtin.rs

@@ -32,11 +32,10 @@ use crate::{
         BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc,
         BuiltinMutablesTransmutes, BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns,
         BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, BuiltinTypeAliasBounds,
-        BuiltinTypeAliasParamBoundsSuggestion, BuiltinUngatedAsyncFnTrackCaller,
-        BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub,
-        BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment,
-        BuiltinUnusedDocCommentSub, BuiltinWhileTrue, InvalidAsmLabel,
-        TypeAliasBoundsQualifyAssocTysSugg,
+        BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit,
+        BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, BuiltinUnsafe,
+        BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
+        BuiltinWhileTrue, InvalidAsmLabel,
     },
     EarlyContext, EarlyLintPass, LateContext, LateLintPass, Level, LintContext,
 };
@@ -1406,9 +1405,23 @@ declare_lint_pass!(
     TypeAliasBounds => [TYPE_ALIAS_BOUNDS]
 );
 
+impl TypeAliasBounds {
+    pub(crate) fn affects_object_lifetime_defaults(pred: &hir::WherePredicate<'_>) -> bool {
+        // Bounds of the form `T: 'a` with `T` type param affect object lifetime defaults.
+        if let hir::WherePredicate::BoundPredicate(pred) = pred
+            && pred.bounds.iter().any(|bound| matches!(bound, hir::GenericBound::Outlives(_)))
+            && pred.bound_generic_params.is_empty() // indeed, even if absent from the RHS
+            && pred.bounded_ty.as_generic_param().is_some()
+        {
+            return true;
+        }
+        false
+    }
+}
+
 impl<'tcx> LateLintPass<'tcx> for TypeAliasBounds {
     fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) {
-        let hir::ItemKind::TyAlias(hir_ty, generics) = &item.kind else { return };
+        let hir::ItemKind::TyAlias(hir_ty, generics) = item.kind else { return };
 
         // There must not be a where clause.
         if generics.predicates.is_empty() {
@@ -1437,7 +1450,6 @@ impl<'tcx> LateLintPass<'tcx> for TypeAliasBounds {
         let mut where_spans = Vec::new();
         let mut inline_spans = Vec::new();
         let mut inline_sugg = Vec::new();
-        let mut affects_object_lifetime_defaults = false;
 
         for p in generics.predicates {
             let span = p.span();
@@ -1449,66 +1461,59 @@ impl<'tcx> LateLintPass<'tcx> for TypeAliasBounds {
                 }
                 inline_sugg.push((span, String::new()));
             }
-
-            // FIXME(fmease): Move this into a "diagnostic decorator" for increased laziness
-            // Bounds of the form `T: 'a` where `T` is a type param of
-            // the type alias affect object lifetime defaults.
-            if !affects_object_lifetime_defaults
-                && let hir::WherePredicate::BoundPredicate(pred) = p
-                && pred.bounds.iter().any(|bound| matches!(bound, hir::GenericBound::Outlives(_)))
-                && pred.bound_generic_params.is_empty()
-                && let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = pred.bounded_ty.kind
-                && let Res::Def(DefKind::TyParam, _) = path.res
-            {
-                affects_object_lifetime_defaults = true;
-            }
         }
 
-        // FIXME(fmease): Add a disclaimer (in the form of a multi-span note) that the removal of
-        //                type-param-outlives-bounds affects OLDs and explicit object lifetime
-        //                bounds might be required [...].
-        // FIXME(fmease): The applicability should also depend on the outcome of the HIR walker
-        //                inside of `TypeAliasBoundsQualifyAssocTysSugg`: Whether it found a
-        //                shorthand projection or not.
-        let applicability = if affects_object_lifetime_defaults {
-            Applicability::MaybeIncorrect
-        } else {
-            Applicability::MachineApplicable
-        };
-
-        let mut qualify_assoc_tys_sugg = Some(TypeAliasBoundsQualifyAssocTysSugg { ty: hir_ty });
-        let enable_feat_help = cx.tcx.sess.is_nightly_build().then_some(());
+        let mut ty = Some(hir_ty);
+        let enable_feat_help = cx.tcx.sess.is_nightly_build();
 
         if let [.., label_sp] = *where_spans {
             cx.emit_span_lint(
                 TYPE_ALIAS_BOUNDS,
                 where_spans,
-                BuiltinTypeAliasBounds::WhereClause {
+                BuiltinTypeAliasBounds {
+                    in_where_clause: true,
                     label: label_sp,
                     enable_feat_help,
-                    suggestion: (generics.where_clause_span, applicability),
-                    qualify_assoc_tys_sugg: qualify_assoc_tys_sugg.take(),
+                    suggestions: vec![(generics.where_clause_span, String::new())],
+                    preds: generics.predicates,
+                    ty: ty.take(),
                 },
             );
         }
         if let [.., label_sp] = *inline_spans {
             cx.emit_span_lint(
                 TYPE_ALIAS_BOUNDS,
                 inline_spans,
-                BuiltinTypeAliasBounds::ParamBounds {
+                BuiltinTypeAliasBounds {
+                    in_where_clause: false,
                     label: label_sp,
                     enable_feat_help,
-                    suggestion: BuiltinTypeAliasParamBoundsSuggestion {
-                        suggestions: inline_sugg,
-                        applicability,
-                    },
-                    qualify_assoc_tys_sugg,
+                    suggestions: inline_sugg,
+                    preds: generics.predicates,
+                    ty,
                 },
             );
         }
     }
 }
 
+pub(crate) struct ShorthandAssocTyCollector {
+    pub(crate) qselves: Vec<Span>,
+}
+
+impl hir::intravisit::Visitor<'_> for ShorthandAssocTyCollector {
+    fn visit_qpath(&mut self, qpath: &hir::QPath<'_>, id: hir::HirId, _: Span) {
+        // Look for "type-parameter shorthand-associated-types". I.e., paths of the
+        // form `T::Assoc` with `T` type param. These are reliant on trait bounds.
+        if let hir::QPath::TypeRelative(qself, _) = qpath
+            && qself.as_generic_param().is_some()
+        {
+            self.qselves.push(qself.span);
+        }
+        hir::intravisit::walk_qpath(self, qpath, id)
+    }
+}
+
 declare_lint! {
     /// The `trivial_bounds` lint detects trait bounds that don't depend on
     /// any type parameters.

Diff for: compiler/rustc_lint/src/lints.rs

@@ -2,8 +2,10 @@
 #![allow(rustc::untranslatable_diagnostic)]
 use std::num::NonZero;
 
-use crate::errors::RequestedLevel;
+use crate::builtin::{InitError, ShorthandAssocTyCollector, TypeAliasBounds};
+use crate::errors::{OverruledAttributeSub, RequestedLevel};
 use crate::fluent_generated as fluent;
+use crate::LateContext;
 use rustc_errors::{
     codes::*, Applicability, Diag, DiagArgValue, DiagMessage, DiagStyledString,
     ElidedLifetimeInPathSubdiag, EmissionGuarantee, LintDiagnostic, MultiSpan, SubdiagMessageOp,
@@ -22,8 +24,6 @@ use rustc_span::{
     Span, Symbol,
 };
 
-use crate::{builtin::InitError, errors::OverruledAttributeSub, LateContext};
-
 // array_into_iter.rs
 #[derive(LintDiagnostic)]
 #[diag(lint_shadowed_into_iter)]
@@ -268,97 +268,72 @@ pub struct MacroExprFragment2024 {
     pub suggestion: Span,
 }
 
-#[derive(LintDiagnostic)]
-pub enum BuiltinTypeAliasBounds<'a, 'hir> {
-    #[diag(lint_builtin_type_alias_bounds_where_clause)]
-    #[note(lint_builtin_type_alias_bounds_limitation_note)]
-    WhereClause {
-        #[label(lint_builtin_type_alias_bounds_label)]
-        label: Span,
-        #[help(lint_builtin_type_alias_bounds_enable_feat_help)]
-        enable_feat_help: Option<()>,
-        #[suggestion(code = "")]
-        suggestion: (Span, Applicability),
-        #[subdiagnostic]
-        qualify_assoc_tys_sugg: Option<TypeAliasBoundsQualifyAssocTysSugg<'a, 'hir>>,
-    },
-    #[diag(lint_builtin_type_alias_bounds_param_bounds)]
-    #[note(lint_builtin_type_alias_bounds_limitation_note)]
-    ParamBounds {
-        #[label(lint_builtin_type_alias_bounds_label)]
-        label: Span,
-        #[help(lint_builtin_type_alias_bounds_enable_feat_help)]
-        enable_feat_help: Option<()>,
-        #[subdiagnostic]
-        suggestion: BuiltinTypeAliasParamBoundsSuggestion,
-        #[subdiagnostic]
-        qualify_assoc_tys_sugg: Option<TypeAliasBoundsQualifyAssocTysSugg<'a, 'hir>>,
-    },
-}
-
-pub struct BuiltinTypeAliasParamBoundsSuggestion {
+pub struct BuiltinTypeAliasBounds<'a, 'hir> {
+    pub in_where_clause: bool,
+    pub label: Span,
+    pub enable_feat_help: bool,
     pub suggestions: Vec<(Span, String)>,
-    pub applicability: Applicability,
+    pub preds: &'hir [hir::WherePredicate<'hir>],
+    pub ty: Option<&'a hir::Ty<'hir>>,
 }
 
-impl Subdiagnostic for BuiltinTypeAliasParamBoundsSuggestion {
-    fn add_to_diag_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
-        self,
-        diag: &mut Diag<'_, G>,
-        _f: &F,
-    ) {
-        diag.arg("count", self.suggestions.len());
-        diag.multipart_suggestion(fluent::lint_suggestion, self.suggestions, self.applicability);
-    }
-}
-
-pub struct TypeAliasBoundsQualifyAssocTysSugg<'a, 'hir> {
-    pub ty: &'a hir::Ty<'hir>,
-}
+impl<'a> LintDiagnostic<'a, ()> for BuiltinTypeAliasBounds<'_, '_> {
+    fn decorate_lint<'b>(self, diag: &'b mut Diag<'a, ()>) {
+        diag.primary_message(if self.in_where_clause {
+            fluent::lint_builtin_type_alias_bounds_where_clause
+        } else {
+            fluent::lint_builtin_type_alias_bounds_param_bounds
+        });
+        diag.span_label(self.label, fluent::lint_builtin_type_alias_bounds_label);
+        diag.note(fluent::lint_builtin_type_alias_bounds_limitation_note);
+        if self.enable_feat_help {
+            diag.help(fluent::lint_builtin_type_alias_bounds_enable_feat_help);
+        }
 
-impl<'a, 'hir> Subdiagnostic for TypeAliasBoundsQualifyAssocTysSugg<'a, 'hir> {
-    fn add_to_diag_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
-        self,
-        diag: &mut Diag<'_, G>,
-        _f: &F,
-    ) {
         // We perform the walk in here instead of in `<TypeAliasBounds as LateLintPass>` to
         // avoid doing throwaway work in case the lint ends up getting suppressed.
-
-        use hir::intravisit::Visitor;
-        struct ProbeShorthandAssocTys<'a, 'b, G: EmissionGuarantee> {
-            diag: &'a mut Diag<'b, G>,
+        let mut collector = ShorthandAssocTyCollector { qselves: Vec::new() };
+        if let Some(ty) = self.ty {
+            hir::intravisit::Visitor::visit_ty(&mut collector, ty);
         }
-        impl<'a, 'b, G: EmissionGuarantee> Visitor<'_> for ProbeShorthandAssocTys<'a, 'b, G> {
-            fn visit_qpath(&mut self, qpath: &hir::QPath<'_>, id: hir::HirId, _: Span) {
-                // Look for "type-parameter shorthand-associated-types". I.e., paths of the
-                // form `T::Assoc` with `T` type param. These are reliant on trait bounds.
-                // Suggest fully qualifying them via `<T as /* Trait */>::Assoc`.
-                //
-                // Instead of attempting to figure out the necessary trait ref, just use a
-                // placeholder. Since we don't record type-dependent resolutions for non-body
-                // items like type aliases, we can't simply deduce the corresp. trait from
-                // the HIR path alone without rerunning parts of HIR ty lowering here
-                // (namely `probe_single_ty_param_bound_for_assoc_ty`) which is infeasible.
-                //
-                // (We could employ some simple heuristics but that's likely not worth it).
-                if let hir::QPath::TypeRelative(qself, _) = qpath
-                    && let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = qself.kind
-                    && let hir::def::Res::Def(hir::def::DefKind::TyParam, _) = path.res
-                {
-                    self.diag.multipart_suggestion(
-                        fluent::lint_builtin_type_alias_bounds_qualify_assoc_tys_sugg,
-                        vec![
-                            (qself.span.shrink_to_lo(), "<".into()),
-                            (qself.span.shrink_to_hi(), " as /* Trait */>".into()),
-                        ],
-                        Applicability::HasPlaceholders,
-                    );
-                }
-                hir::intravisit::walk_qpath(self, qpath, id)
-            }
+
+        let affect_object_lifetime_defaults = self
+            .preds
+            .iter()
+            .filter(|pred| pred.in_where_clause() == self.in_where_clause)
+            .any(|pred| TypeAliasBounds::affects_object_lifetime_defaults(pred));
+
+        // If there are any shorthand assoc tys, then the bounds can't be removed automatically.
+        // The user first needs to fully qualify the assoc tys.
+        let applicability = if !collector.qselves.is_empty() || affect_object_lifetime_defaults {
+            Applicability::MaybeIncorrect
+        } else {
+            Applicability::MachineApplicable
+        };
+
+        diag.arg("count", self.suggestions.len());
+        diag.multipart_suggestion(fluent::lint_suggestion, self.suggestions, applicability);
+
+        // Suggest fully qualifying paths of the form `T::Assoc` with `T` type param via
+        // `<T as /* Trait */>::Assoc` to remove their reliance on any type param bounds.
+        //
+        // Instead of attempting to figure out the necessary trait ref, just use a
+        // placeholder. Since we don't record type-dependent resolutions for non-body
+        // items like type aliases, we can't simply deduce the corresp. trait from
+        // the HIR path alone without rerunning parts of HIR ty lowering here
+        // (namely `probe_single_ty_param_bound_for_assoc_ty`) which is infeasible.
+        //
+        // (We could employ some simple heuristics but that's likely not worth it).
+        for qself in collector.qselves {
+            diag.multipart_suggestion(
+                fluent::lint_builtin_type_alias_bounds_qualify_assoc_tys_sugg,
+                vec![
+                    (qself.shrink_to_lo(), "<".into()),
+                    (qself.shrink_to_hi(), " as /* Trait */>".into()),
+                ],
+                Applicability::HasPlaceholders,
+            );
         }
-        ProbeShorthandAssocTys { diag }.visit_ty(self.ty);
     }
 }