do away with Trait and TraitName and check generic params for rustc_on_unimplemented

Author: mejrs

compiler/rustc_span/src/symbol.rs

@@ -398,7 +398,6 @@ symbols! {
         Wrapping,
         Yield,
         _DECLS,
-        _Self,
         __D,
         __H,
         __S,

compiler/rustc_trait_selection/messages.ftl

@@ -337,6 +337,8 @@ trait_selection_rustc_on_unimplemented_expected_one_predicate_in_not = expected
     .label = unexpected quantity of predicates here
 trait_selection_rustc_on_unimplemented_invalid_flag = invalid flag in `on`-clause
     .label = expected one of the `crate_local`, `direct` or `from_desugaring` flags, not `{$invalid_flag}`
+trait_selection_rustc_on_unimplemented_invalid_name = invalid name in `on`-clause
+    .label = expected one of `cause`, `from_desugaring`, `Self` or any generic parameter of the trait, not `{$invalid_name}`
 trait_selection_rustc_on_unimplemented_invalid_predicate = this predicate is invalid
     .label = expected one of `any`, `all` or `not` here, not `{$invalid_pred}`
 trait_selection_rustc_on_unimplemented_missing_value = this attribute must have a value

compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs

@@ -429,7 +429,19 @@ impl<'tcx> OnUnimplementedDirective {
                 .next()
                 .ok_or_else(|| tcx.dcx().emit_err(InvalidOnClause::Empty { span }))?;
 
-            match OnUnimplementedCondition::parse(cond) {
+            let generics: Vec<Symbol> = tcx
+                .generics_of(item_def_id)
+                .own_params
+                .iter()
+                .filter_map(|param| {
+                    if matches!(param.kind, GenericParamDefKind::Lifetime) {
+                        None
+                    } else {
+                        Some(param.name)
+                    }
+                })
+                .collect();
+            match OnUnimplementedCondition::parse(cond, &generics) {
                 Ok(condition) => Some(condition),
                 Err(e) => return Err(tcx.dcx().emit_err(e)),
             }

compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_condition.rs

@@ -26,9 +26,12 @@ impl OnUnimplementedCondition {
         })
     }
 
-    pub(crate) fn parse(input: &MetaItemInner) -> Result<Self, InvalidOnClause> {
+    pub(crate) fn parse(
+        input: &MetaItemInner,
+        generics: &[Symbol],
+    ) -> Result<Self, InvalidOnClause> {
         let span = input.span();
-        let pred = Predicate::parse(input)?;
+        let pred = Predicate::parse(input, generics)?;
         Ok(OnUnimplementedCondition { span, pred })
     }
 }
@@ -52,7 +55,7 @@ enum Predicate {
 }
 
 impl Predicate {
-    fn parse(input: &MetaItemInner) -> Result<Self, InvalidOnClause> {
+    fn parse(input: &MetaItemInner, generics: &[Symbol]) -> Result<Self, InvalidOnClause> {
         let meta_item = match input {
             MetaItemInner::MetaItem(meta_item) => meta_item,
             MetaItemInner::Lit(lit) => {
@@ -69,10 +72,10 @@ impl Predicate {
 
         match meta_item.kind {
             MetaItemKind::List(ref mis) => match predicate.name {
-                sym::any => Ok(Predicate::Any(Predicate::parse_sequence(mis)?)),
-                sym::all => Ok(Predicate::All(Predicate::parse_sequence(mis)?)),
+                sym::any => Ok(Predicate::Any(Predicate::parse_sequence(mis, generics)?)),
+                sym::all => Ok(Predicate::All(Predicate::parse_sequence(mis, generics)?)),
                 sym::not => match &**mis {
-                    [one] => Ok(Predicate::Not(Box::new(Predicate::parse(one)?))),
+                    [one] => Ok(Predicate::Not(Box::new(Predicate::parse(one, generics)?))),
                     [first, .., last] => Err(InvalidOnClause::ExpectedOnePredInNot {
                         span: first.span().to(last.span()),
                     }),
@@ -83,7 +86,7 @@ impl Predicate {
                 }
             },
             MetaItemKind::NameValue(MetaItemLit { symbol, .. }) => {
-                let name = Name::parse(predicate);
+                let name = Name::parse(predicate, generics)?;
                 let value = FilterFormatString::parse(symbol);
                 let kv = NameValue { name, value };
                 Ok(Predicate::Match(kv))
@@ -95,8 +98,11 @@ impl Predicate {
         }
     }
 
-    fn parse_sequence(sequence: &[MetaItemInner]) -> Result<Vec<Self>, InvalidOnClause> {
-        sequence.iter().map(Predicate::parse).collect()
+    fn parse_sequence(
+        sequence: &[MetaItemInner],
+        generics: &[Symbol],
+    ) -> Result<Vec<Self>, InvalidOnClause> {
+        sequence.iter().map(|item| Predicate::parse(item, generics)).collect()
     }
 
     fn eval(&self, eval: &mut impl FnMut(FlagOrNv<'_>) -> bool) -> bool {
@@ -156,14 +162,13 @@ enum Name {
 }
 
 impl Name {
-    fn parse(Ident { name, .. }: Ident) -> Self {
+    fn parse(Ident { name, span }: Ident, generics: &[Symbol]) -> Result<Self, InvalidOnClause> {
         match name {
-            sym::_Self | kw::SelfUpper => Name::SelfUpper,
-            sym::from_desugaring => Name::FromDesugaring,
-            sym::cause => Name::Cause,
-            // FIXME(mejrs) Perhaps we should start checking that
-            // this actually is a valid generic parameter?
-            generic => Name::GenericArg(generic),
+            kw::SelfUpper => Ok(Name::SelfUpper),
+            sym::from_desugaring => Ok(Name::FromDesugaring),
+            sym::cause => Ok(Name::Cause),
+            generic if generics.contains(&generic) => Ok(Name::GenericArg(generic)),
+            invalid_name => Err(InvalidOnClause::InvalidName { invalid_name, span }),
         }
     }
 }

compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented_format.rs

@@ -2,8 +2,8 @@ use std::fmt;
 use std::ops::Range;
 
 use errors::*;
-use rustc_middle::ty::TyCtxt;
 use rustc_middle::ty::print::TraitRefPrintSugared;
+use rustc_middle::ty::{GenericParamDefKind, TyCtxt};
 use rustc_parse_format::{
     Alignment, Argument, Count, FormatSpec, ParseError, ParseMode, Parser, Piece as RpfPiece,
     Position,
@@ -232,48 +232,16 @@ fn parse_arg<'tcx>(
 ) -> FormatArg {
     let (Ctx::RustcOnUnimplemented { tcx, trait_def_id }
     | Ctx::DiagnosticOnUnimplemented { tcx, trait_def_id }) = ctx;
-    let trait_name = tcx.item_ident(*trait_def_id);
-    let generics = tcx.generics_of(trait_def_id);
+
     let span = slice_span(input_span, arg.position_span.clone());
 
     match arg.position {
         // Something like "hello {name}"
         Position::ArgumentNamed(name) => match (ctx, Symbol::intern(name)) {
-            // accepted, but deprecated
-            (Ctx::RustcOnUnimplemented { .. }, sym::_Self) => {
-                warnings
-                    .push(FormatWarning::FutureIncompat { span, help: String::from("use {Self}") });
-                FormatArg::SelfUpper
-            }
-            (
-                Ctx::RustcOnUnimplemented { .. },
-                sym::from_desugaring
-                | sym::crate_local
-                | sym::direct
-                | sym::cause
-                | sym::float
-                | sym::integer_
-                | sym::integral,
-            ) => {
-                warnings.push(FormatWarning::FutureIncompat {
-                    span,
-                    help: String::from("don't use this in a format string"),
-                });
-                FormatArg::AsIs(String::new())
-            }
-
             // Only `#[rustc_on_unimplemented]` can use these
             (Ctx::RustcOnUnimplemented { .. }, sym::ItemContext) => FormatArg::ItemContext,
             (Ctx::RustcOnUnimplemented { .. }, sym::This) => FormatArg::This,
             (Ctx::RustcOnUnimplemented { .. }, sym::Trait) => FormatArg::Trait,
-            // `{ThisTraitsName}`. Some attrs in std use this, but I'd like to change it to the more general `{This}`
-            // because that'll be simpler to parse and extend in the future
-            (Ctx::RustcOnUnimplemented { .. }, name) if name == trait_name.name => {
-                warnings
-                    .push(FormatWarning::FutureIncompat { span, help: String::from("use {This}") });
-                FormatArg::This
-            }
-
             // Any attribute can use these
             (
                 Ctx::RustcOnUnimplemented { .. } | Ctx::DiagnosticOnUnimplemented { .. },
@@ -282,7 +250,10 @@ fn parse_arg<'tcx>(
             (
                 Ctx::RustcOnUnimplemented { .. } | Ctx::DiagnosticOnUnimplemented { .. },
                 generic_param,
-            ) if generics.own_params.iter().any(|param| param.name == generic_param) => {
+            ) if tcx.generics_of(trait_def_id).own_params.iter().any(|param| {
+                !matches!(param.kind, GenericParamDefKind::Lifetime) && param.name == generic_param
+            }) =>
+            {
                 FormatArg::GenericParam { generic_param }
             }
 
@@ -375,39 +346,4 @@ pub mod errors {
     #[diag(trait_selection_missing_options_for_on_unimplemented_attr)]
     #[help]
     pub struct MissingOptionsForOnUnimplementedAttr;
-
-    #[derive(LintDiagnostic)]
-    #[diag(trait_selection_ignored_diagnostic_option)]
-    pub struct IgnoredDiagnosticOption {
-        pub option_name: &'static str,
-        #[label]
-        pub span: Span,
-        #[label(trait_selection_other_label)]
-        pub prev_span: Span,
-    }
-
-    impl IgnoredDiagnosticOption {
-        pub fn maybe_emit_warning<'tcx>(
-            tcx: TyCtxt<'tcx>,
-            item_def_id: DefId,
-            new: Option<Span>,
-            old: Option<Span>,
-            option_name: &'static str,
-        ) {
-            if let (Some(new_item), Some(old_item)) = (new, old) {
-                if let Some(item_def_id) = item_def_id.as_local() {
-                    tcx.emit_node_span_lint(
-                        UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                        tcx.local_def_id_to_hir_id(item_def_id),
-                        new_item,
-                        IgnoredDiagnosticOption {
-                            span: new_item,
-                            prev_span: old_item,
-                            option_name,
-                        },
-                    );
-                }
-            }
-        }
-    }
 }

compiler/rustc_trait_selection/src/errors.rs

@@ -72,6 +72,13 @@ pub enum InvalidOnClause {
         span: Span,
         invalid_flag: Symbol,
     },
+    #[diag(trait_selection_rustc_on_unimplemented_invalid_name, code = E0232)]
+    InvalidName {
+        #[primary_span]
+        #[label]
+        span: Span,
+        invalid_name: Symbol,
+    },
 }
 
 #[derive(Diagnostic)]

src/tools/clippy/tests/ui/duplicated_attributes.rs

@@ -21,7 +21,7 @@ fn foo() {}
 fn bar() {}
 
 // No warning:
-#[rustc_on_unimplemented(on(_Self = "&str", label = "`a"), on(_Self = "alloc::string::String", label = "a"))]
+#[rustc_on_unimplemented(on(Self = "&str", label = "`a"), on(Self = "alloc::string::String", label = "a"))]
 trait Abc {}
 
 #[proc_macro_attr::duplicated_attr()] // Should not warn!

tests/ui/diagnostic_namespace/on_unimplemented/do_not_accept_options_of_the_internal_rustc_attribute.rs

@@ -3,7 +3,7 @@
 //@ reference: attributes.diagnostic.on_unimplemented.syntax
 //@ reference: attributes.diagnostic.on_unimplemented.invalid-formats
 #[diagnostic::on_unimplemented(
-    on(_Self = "&str"),
+    on(Self = "&str"),
     //~^WARN malformed `on_unimplemented` attribute
     //~|WARN malformed `on_unimplemented` attribute
     message = "trait has `{Self}` and `{T}` as params",
@@ -41,7 +41,7 @@ impl Bar for i32 {}
     //~|WARN there is no parameter `integral` on trait `Baz`
     //~|WARN there is no parameter `integer` on trait `Baz`
     //~|WARN there is no parameter `integer` on trait `Baz`
-    label = "{float}{_Self}{crate_local}{Trait}{ItemContext}"
+    label = "{float}{_Self}{crate_local}{Trait}{ItemContext}{This}"
     //~^WARN there is no parameter `float` on trait `Baz`
     //~|WARN there is no parameter `float` on trait `Baz`
     //~|WARN there is no parameter `_Self` on trait `Baz`
@@ -52,6 +52,8 @@ impl Bar for i32 {}
     //~|WARN there is no parameter `Trait` on trait `Baz`
     //~|WARN there is no parameter `ItemContext` on trait `Baz`
     //~|WARN there is no parameter `ItemContext` on trait `Baz`
+    //~|WARN there is no parameter `This` on trait `Baz`
+    //~|WARN there is no parameter `This` on trait `Baz`
 )]
 trait Baz {}