Skip to content

Commit

Permalink
Rollup merge of rust-lang#78626 - fusion-engineering-forks:deprecated…
Browse files Browse the repository at this point in the history
…-trait-impl, r=estebank

Improve errors about #[deprecated] attribute

This change:

1. Turns `#[deprecated]` on a trait impl block into an error, which fixes rust-lang#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
```
  • Loading branch information
Dylan-DPC authored Nov 3, 2020
2 parents da3091b + 9c647d1 commit d159121
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 38 deletions.
21 changes: 10 additions & 11 deletions compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
27 changes: 27 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -2792,6 +2818,7 @@ declare_lint_pass! {
INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
UNINHABITED_STATIC,
FUNCTION_ITEM_REFERENCES,
USELESS_DEPRECATED,
]
}

Expand Down
33 changes: 22 additions & 11 deletions compiler/rustc_passes/src/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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,
}
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand Down
13 changes: 11 additions & 2 deletions src/test/ui/deprecation/deprecation-sanity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() { }
18 changes: 14 additions & 4 deletions src/test/ui/deprecation/deprecation-sanity.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
6 changes: 3 additions & 3 deletions src/test/ui/stability-attribute/stability-attribute-sanity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() { }
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down

0 comments on commit d159121

Please sign in to comment.