Rollup merge of #78626 - fusion-engineering-forks:deprecated-trait-im…

…pl, r=estebank

Improve errors about #[deprecated] attribute

This change:

1. Turns `#[deprecated]` on a trait impl block into an error, which fixes #78625;
2. Changes these and other errors about `#[deprecated]` to use the span of the attribute instead of the item; and
3. Turns this error into a lint, to make sure it can be capped with `--cap-lints` and doesn't break any existing dependencies.

Can be reviewed per commit.

---
Example:
```rust
struct X;

#[deprecated = "a"]
impl Default for X {
    #[deprecated = "b"]
    fn default() -> Self {
        X
    }
}
```

Before:
```
error: This deprecation annotation is useless
 --> src/main.rs:6:5
  |
6 | /     fn default() -> Self {
7 | |         X
8 | |     }
  | |_____^
```

After:
```
error: this `#[deprecated]' annotation has no effect
 --> src/main.rs:3:1
  |
3 | #[deprecated = "a"]
  | ^^^^^^^^^^^^^^^^^^^ help: try removing the deprecation attribute
  |
  = note: `#[deny(useless_deprecated)]` on by default

error: this `#[deprecated]' annotation has no effect
 --> src/main.rs:5:5
  |
5 |     #[deprecated = "b"]
  |     ^^^^^^^^^^^^^^^^^^^ help: try removing the deprecation attribute
```

compiler/rustc_attr/src/builtin.rs

@@ -637,19 +637,15 @@ pub struct Deprecation {
 }
 
 /// Finds the deprecation attribute. `None` if none exists.
-pub fn find_deprecation(sess: &Session, attrs: &[Attribute], item_sp: Span) -> Option<Deprecation> {
- find_deprecation_generic(sess, attrs.iter(), item_sp)
+pub fn find_deprecation(sess: &Session, attrs: &[Attribute]) -> Option<(Deprecation, Span)> {
+ find_deprecation_generic(sess, attrs.iter())
 }
 
-fn find_deprecation_generic<'a, I>(
- sess: &Session,
- attrs_iter: I,
- item_sp: Span,
-) -> Option<Deprecation>
+fn find_deprecation_generic<'a, I>(sess: &Session, attrs_iter: I) -> Option<(Deprecation, Span)>
 where
  I: Iterator<Item = &'a Attribute>,
 {
- let mut depr: Option<Deprecation> = None;
+ let mut depr: Option<(Deprecation, Span)> = None;
  let diagnostic = &sess.parse_sess.span_diagnostic;
 
  'outer: for attr in attrs_iter {
@@ -658,8 +654,11 @@ where
  continue;
  }
 
- if depr.is_some() {
- struct_span_err!(diagnostic, item_sp, E0550, "multiple deprecated attributes").emit();
+ if let Some((_, span)) = &depr {
+ struct_span_err!(diagnostic, attr.span, E0550, "multiple deprecated attributes")
+ .span_label(attr.span, "repeated deprecation attribute")
+ .span_label(*span, "first deprecation attribute")
+ .emit();
  break;
  }
 
@@ -780,7 +779,7 @@ where
  sess.mark_attr_used(&attr);
 
  let is_since_rustc_version = sess.check_name(attr, sym::rustc_deprecated);
- depr = Some(Deprecation { since, note, suggestion, is_since_rustc_version });
+ depr = Some((Deprecation { since, note, suggestion, is_since_rustc_version }, attr.span));
  }
 
  depr

compiler/rustc_expand/src/base.rs

@@ -793,7 +793,7 @@ impl SyntaxExtension {
  allow_internal_unsafe: sess.contains_name(attrs, sym::allow_internal_unsafe),
  local_inner_macros,
  stability,
- deprecation: attr::find_deprecation(&sess, attrs, span),
+ deprecation: attr::find_deprecation(&sess, attrs).map(|(d, _)| d),
  helper_attrs,
  edition,
  is_builtin,

compiler/rustc_lint_defs/src/builtin.rs

@@ -2705,6 +2705,32 @@ declare_lint! {
  };
 }
 
+declare_lint! {
+ /// The `useless_deprecated` lint detects deprecation attributes with no effect.
+ ///
+ /// ### Example
+ ///
+ /// ```rust,compile_fail
+ /// struct X;
+ ///
+ /// #[deprecated = "message"]
+ /// impl Default for X {
+ /// fn default() -> Self {
+ /// X
+ /// }
+ /// }
+ /// ```
+ ///
+ /// {{produces}}
+ ///
+ /// ### Explanation
+ ///
+ /// Deprecation attributes have no effect on trait implementations.
+ pub USELESS_DEPRECATED,
+ Deny,
+ "detects deprecation attributes with no effect",
+}
+
 declare_tool_lint! {
  pub rustc::INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
  Deny,
@@ -2792,6 +2818,7 @@ declare_lint_pass! {
  INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
  UNINHABITED_STATIC,
  FUNCTION_ITEM_REFERENCES,
+ USELESS_DEPRECATED,
  ]
 }
 

compiler/rustc_passes/src/stability.rs

@@ -15,7 +15,7 @@ use rustc_middle::middle::privacy::AccessLevels;
 use rustc_middle::middle::stability::{DeprecationEntry, Index};
 use rustc_middle::ty::{self, query::Providers, TyCtxt};
 use rustc_session::lint;
-use rustc_session::lint::builtin::INEFFECTIVE_UNSTABLE_TRAIT_IMPL;
+use rustc_session::lint::builtin::{INEFFECTIVE_UNSTABLE_TRAIT_IMPL, USELESS_DEPRECATED};
 use rustc_session::parse::feature_err;
 use rustc_session::Session;
 use rustc_span::symbol::{sym, Symbol};
@@ -31,6 +31,8 @@ enum AnnotationKind {
  Required,
  // Annotation is useless, reject it
  Prohibited,
+ // Deprecation annotation is useless, reject it. (Stability attribute is still required.)
+ DeprecationProhibited,
  // Annotation itself is useless, but it can be propagated to children
  Container,
 }
@@ -83,14 +85,22 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
  did_error = self.forbid_staged_api_attrs(hir_id, attrs, inherit_deprecation.clone());
  }
 
- let depr =
- if did_error { None } else { attr::find_deprecation(&self.tcx.sess, attrs, item_sp) };
+ let depr = if did_error { None } else { attr::find_deprecation(&self.tcx.sess, attrs) };
  let mut is_deprecated = false;
- if let Some(depr) = &depr {
+ if let Some((depr, span)) = &depr {
  is_deprecated = true;
 
- if kind == AnnotationKind::Prohibited {
- self.tcx.sess.span_err(item_sp, "This deprecation annotation is useless");
+ if kind == AnnotationKind::Prohibited || kind == AnnotationKind::DeprecationProhibited {
+ self.tcx.struct_span_lint_hir(USELESS_DEPRECATED, hir_id, *span, |lint| {
+ lint.build("this `#[deprecated]` annotation has no effect")
+ .span_suggestion_short(
+ *span,
+ "remove the unnecessary deprecation attribute",
+ String::new(),
+ rustc_errors::Applicability::MachineApplicable,
+ )
+ .emit()
+ });
  }
 
  // `Deprecation` is just two pointers, no need to intern it
@@ -114,7 +124,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
  }
  } else {
  self.recurse_with_stability_attrs(
- depr.map(|d| DeprecationEntry::local(d, hir_id)),
+ depr.map(|(d, _)| DeprecationEntry::local(d, hir_id)),
  None,
  None,
  visit_children,
@@ -139,11 +149,11 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
  }
  }
 
- if depr.as_ref().map_or(false, |d| d.is_since_rustc_version) {
+ if let Some((rustc_attr::Deprecation { is_since_rustc_version: true, .. }, span)) = &depr {
  if stab.is_none() {
  struct_span_err!(
  self.tcx.sess,
- item_sp,
+ *span,
  E0549,
  "rustc_deprecated attribute must be paired with \
  either stable or unstable attribute"
@@ -166,7 +176,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
  // Check if deprecated_since < stable_since. If it is,
  // this is *almost surely* an accident.
  if let (&Some(dep_since), &attr::Stable { since: stab_since }) =
- (&depr.as_ref().and_then(|d| d.since), &stab.level)
+ (&depr.as_ref().and_then(|(d, _)| d.since), &stab.level)
  {
  // Explicit version of iter::order::lt to handle parse errors properly
  for (dep_v, stab_v) in
@@ -212,7 +222,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
  }
 
  self.recurse_with_stability_attrs(
- depr.map(|d| DeprecationEntry::local(d, hir_id)),
+ depr.map(|(d, _)| DeprecationEntry::local(d, hir_id)),
  stab,
  const_stab,
  visit_children,
@@ -322,6 +332,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
  }
  hir::ItemKind::Impl { of_trait: Some(_), .. } => {
  self.in_trait_impl = true;
+ kind = AnnotationKind::DeprecationProhibited;
  }
  hir::ItemKind::Struct(ref sd, _) => {
  if let Some(ctor_hir_id) = sd.ctor_hir_id() {

src/test/ui/deprecation/deprecation-sanity.rs

@@ -24,10 +24,19 @@ mod bogus_attribute_types_1 {
 }
 
 #[deprecated(since = "a", note = "b")]
-#[deprecated(since = "a", note = "b")]
-fn multiple1() { } //~ ERROR multiple deprecated attributes
+#[deprecated(since = "a", note = "b")] //~ ERROR multiple deprecated attributes
+fn multiple1() { }
 
 #[deprecated(since = "a", since = "b", note = "c")] //~ ERROR multiple 'since' items
 fn f1() { }
 
+struct X;
+
+#[deprecated = "hello"] //~ ERROR this `#[deprecated]` annotation has no effect
+impl Default for X {
+ fn default() -> Self {
+ X
+ }
+}
+
 fn main() { }

src/test/ui/deprecation/deprecation-sanity.stderr

@@ -41,18 +41,28 @@ LL | #[deprecated("test")]
  | ^^^^^^
 
 error[E0550]: multiple deprecated attributes
- --> $DIR/deprecation-sanity.rs:28:1
+ --> $DIR/deprecation-sanity.rs:27:1
  |
-LL | fn multiple1() { }
- | ^^^^^^^^^^^^^^^^^^
+LL | #[deprecated(since = "a", note = "b")]
+ | -------------------------------------- first deprecation attribute
+LL | #[deprecated(since = "a", note = "b")]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ repeated deprecation attribute
 
 error[E0538]: multiple 'since' items
  --> $DIR/deprecation-sanity.rs:30:27
  |
 LL | #[deprecated(since = "a", since = "b", note = "c")]
  | ^^^^^^^^^^^
 
-error: aborting due to 9 previous errors
+error: this `#[deprecated]` annotation has no effect
+ --> $DIR/deprecation-sanity.rs:35:1
+ |
+LL | #[deprecated = "hello"]
+ | ^^^^^^^^^^^^^^^^^^^^^^^ help: remove the unnecessary deprecation attribute
+ |
+ = note: `#[deny(useless_deprecated)]` on by default
+
+error: aborting due to 10 previous errors
 
 Some errors have detailed explanations: E0538, E0541, E0550, E0551, E0565.
 For more information about an error, try `rustc --explain E0538`.

src/test/ui/stability-attribute/stability-attribute-sanity.rs

@@ -59,14 +59,14 @@ fn multiple3() { }
 
 #[stable(feature = "a", since = "b")]
 #[rustc_deprecated(since = "b", reason = "text")]
-#[rustc_deprecated(since = "b", reason = "text")]
+#[rustc_deprecated(since = "b", reason = "text")] //~ ERROR multiple deprecated attributes
 #[rustc_const_unstable(feature = "c", issue = "none")]
 #[rustc_const_unstable(feature = "d", issue = "none")] //~ ERROR multiple stability levels
-pub const fn multiple4() { } //~ ERROR multiple deprecated attributes
+pub const fn multiple4() { }
 //~^ ERROR Invalid stability or deprecation version found
 
 #[rustc_deprecated(since = "a", reason = "text")]
 fn deprecated_without_unstable_or_stable() { }
-//~^ ERROR rustc_deprecated attribute must be paired with either stable or unstable attribute
+//~^^ ERROR rustc_deprecated attribute must be paired with either stable or unstable attribute
 
 fn main() { }

src/test/ui/stability-attribute/stability-attribute-sanity.stderr

@@ -83,10 +83,12 @@ LL | #[stable(feature = "a", since = "b")]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error[E0550]: multiple deprecated attributes
- --> $DIR/stability-attribute-sanity.rs:65:1
+ --> $DIR/stability-attribute-sanity.rs:62:1
  |
-LL | pub const fn multiple4() { }
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL | #[rustc_deprecated(since = "b", reason = "text")]
+ | ------------------------------------------------- first deprecation attribute
+LL | #[rustc_deprecated(since = "b", reason = "text")]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ repeated deprecation attribute
 
 error[E0544]: multiple stability levels
  --> $DIR/stability-attribute-sanity.rs:64:1
@@ -101,10 +103,10 @@ LL | pub const fn multiple4() { }
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error[E0549]: rustc_deprecated attribute must be paired with either stable or unstable attribute
- --> $DIR/stability-attribute-sanity.rs:69:1
+ --> $DIR/stability-attribute-sanity.rs:68:1
  |
-LL | fn deprecated_without_unstable_or_stable() { }
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL | #[rustc_deprecated(since = "a", reason = "text")]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 18 previous errors