Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve errors about #[deprecated] attribute #78626

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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