Skip to content

Commit

Permalink
Auto merge of #80300 - LeSeulArtichaut:80275-doc-inline, r=Manishearth
Browse files Browse the repository at this point in the history 
Emit errors/warns on some wrong uses of rustdoc attributes

This PR adds a few diagnostics:
- error if conflicting `#[doc(inline)]`/`#[doc(no_inline)]` are found
- introduce the `invalid_doc_attributes` lint (warn-by-default) which triggers:
  - if a crate-level attribute is used on a non-`crate` item
  - if `#[doc(inline)]`/`#[doc(no_inline)]` is used on a non-`use` item

The code could probably be improved but I wanted to get feedback first. Also, some of those changes could be considered breaking changes, so I don't know what the procedure would be? ~~And finally, for the warnings, they are currently hard warnings, maybe it would be better to introduce a lint?~~ (EDIT: introduced the `invalid_doc_attributes` lint)

Closes #80275.
r? `@jyn514`
  • Loading branch information
@bors
bors committed May 11, 2021
2 parents 455c5e0 + 804ab9f commit fe62c6e
Show file tree
Hide file tree
Showing 10 changed files with 367 additions and 94 deletions.
166 changes: 147 additions & 19 deletions compiler/rustc_passes/src/check_attr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_middle::hir::map::Map;
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::TyCtxt;

use rustc_ast::{Attribute, Lit, LitKind, NestedMetaItem};
use rustc_ast::{AttrStyle, Attribute, Lit, LitKind, NestedMetaItem};
use rustc_errors::{pluralize, struct_span_err, Applicability};
use rustc_hir as hir;
use rustc_hir::def_id::LocalDefId;
Expand All @@ -22,7 +22,7 @@ use rustc_session::lint::builtin::{
};
use rustc_session::parse::feature_err;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::{Span, DUMMY_SP};
use rustc_span::{MultiSpan, Span, DUMMY_SP};

pub(crate) fn target_from_impl_item<'tcx>(
tcx: TyCtxt<'tcx>,
Expand Down Expand Up @@ -67,6 +67,7 @@ impl CheckAttrVisitor<'tcx> {
item: Option<ItemLike<'_>>,
) {
let mut is_valid = true;
let mut specified_inline = None;
let attrs = self.tcx.hir().attrs(hir_id);
for attr in attrs {
is_valid &= match attr.name_or_empty() {
Expand All @@ -77,7 +78,7 @@ impl CheckAttrVisitor<'tcx> {
sym::track_caller => {
self.check_track_caller(hir_id, &attr.span, attrs, span, target)
}
sym::doc => self.check_doc_attrs(attr, hir_id, target),
sym::doc => self.check_doc_attrs(attr, hir_id, target, &mut specified_inline),
sym::no_link => self.check_no_link(hir_id, &attr, span, target),
sym::export_name => self.check_export_name(hir_id, &attr, span, target),
sym::rustc_args_required_const => {
Expand Down Expand Up @@ -564,7 +565,71 @@ impl CheckAttrVisitor<'tcx> {
true
}

fn check_attr_crate_level(
/// Checks `#[doc(inline)]`/`#[doc(no_inline)]` attributes. Returns `true` if valid.
///
/// A doc inlining attribute is invalid if it is applied to a non-`use` item, or
/// if there are conflicting attributes for one item.
///
/// `specified_inline` is used to keep track of whether we have
/// already seen an inlining attribute for this item.
/// If so, `specified_inline` holds the value and the span of
/// the first `inline`/`no_inline` attribute.
fn check_doc_inline(
&self,
attr: &Attribute,
meta: &NestedMetaItem,
hir_id: HirId,
target: Target,
specified_inline: &mut Option<(bool, Span)>,
) -> bool {
if target == Target::Use {
let do_inline = meta.name_or_empty() == sym::inline;
if let Some((prev_inline, prev_span)) = *specified_inline {
if do_inline != prev_inline {
let mut spans = MultiSpan::from_spans(vec![prev_span, meta.span()]);
spans.push_span_label(prev_span, String::from("this attribute..."));
spans.push_span_label(
meta.span(),
String::from("...conflicts with this attribute"),
);
self.tcx
.sess
.struct_span_err(spans, "conflicting doc inlining attributes")
.help("remove one of the conflicting attributes")
.emit();
return false;
}
true
} else {
*specified_inline = Some((do_inline, meta.span()));
true
}
} else {
self.tcx.struct_span_lint_hir(
INVALID_DOC_ATTRIBUTES,
hir_id,
meta.span(),
|lint| {
let mut err = lint.build(
"this attribute can only be applied to a `use` item",
);
err.span_label(meta.span(), "only applicable on `use` items");
if attr.style == AttrStyle::Outer {
err.span_label(
self.tcx.hir().span(hir_id),
"not a `use` item",
);
}
err.note("read https://doc.rust-lang.org/nightly/rustdoc/the-doc-attribute.html#docno_inlinedocinline for more information")
.emit();
},
);
false
}
}

/// Checks that an attribute is *not* used at the crate level. Returns `true` if valid.
fn check_attr_not_crate_level(
&self,
meta: &NestedMetaItem,
hir_id: HirId,
Expand All @@ -586,40 +651,103 @@ impl CheckAttrVisitor<'tcx> {
true
}

fn check_doc_attrs(&self, attr: &Attribute, hir_id: HirId, target: Target) -> bool {
/// Checks that an attribute is used at the crate level. Returns `true` if valid.
fn check_attr_crate_level(
&self,
attr: &Attribute,
meta: &NestedMetaItem,
hir_id: HirId,
) -> bool {
if hir_id != CRATE_HIR_ID {
self.tcx.struct_span_lint_hir(
INVALID_DOC_ATTRIBUTES,
hir_id,
meta.span(),
|lint| {
let mut err = lint.build(
"this attribute can only be applied at the crate level",
);
if attr.style == AttrStyle::Outer && self.tcx.hir().get_parent_item(hir_id) == CRATE_HIR_ID {
if let Ok(mut src) =
self.tcx.sess.source_map().span_to_snippet(attr.span)
{
src.insert(1, '!');
err.span_suggestion_verbose(
attr.span,
"to apply to the crate, use an inner attribute",
src,
Applicability::MaybeIncorrect,
);
} else {
err.span_help(
attr.span,
"to apply to the crate, use an inner attribute",
);
}
}
err.note("read https://doc.rust-lang.org/nightly/rustdoc/the-doc-attribute.html#at-the-crate-level for more information")
.emit();
},
);
return false;
}
true
}

/// Runs various checks on `#[doc]` attributes. Returns `true` if valid.
///
/// `specified_inline` should be initialized to `None` and kept for the scope
/// of one item. Read the documentation of [`check_doc_inline`] for more information.
///
/// [`check_doc_inline`]: Self::check_doc_inline
fn check_doc_attrs(
&self,
attr: &Attribute,
hir_id: HirId,
target: Target,
specified_inline: &mut Option<(bool, Span)>,
) -> bool {
let mut is_valid = true;

if let Some(list) = attr.meta().and_then(|mi| mi.meta_item_list().map(|l| l.to_vec())) {
for meta in list {
if let Some(i_meta) = meta.meta_item() {
match i_meta.name_or_empty() {
sym::alias
if !self.check_attr_crate_level(&meta, hir_id, "alias")
if !self.check_attr_not_crate_level(&meta, hir_id, "alias")
|| !self.check_doc_alias(&meta, hir_id, target) =>
{
is_valid = false
}

sym::keyword
if !self.check_attr_crate_level(&meta, hir_id, "keyword")
if !self.check_attr_not_crate_level(&meta, hir_id, "keyword")
|| !self.check_doc_keyword(&meta, hir_id) =>
{
is_valid = false
}

sym::test if CRATE_HIR_ID != hir_id => {
self.tcx.struct_span_lint_hir(
INVALID_DOC_ATTRIBUTES,
sym::html_favicon_url
| sym::html_logo_url
| sym::html_playground_url
| sym::issue_tracker_base_url
| sym::html_root_url
| sym::html_no_source
| sym::test
if !self.check_attr_crate_level(&attr, &meta, hir_id) =>
{
is_valid = false;
}

sym::inline | sym::no_inline
if !self.check_doc_inline(
&attr,
&meta,
hir_id,
meta.span(),
|lint| {
lint.build(
"`#![doc(test(...)]` is only allowed \
as a crate-level attribute",
)
.emit();
},
);
target,
specified_inline,
) =>
{
is_valid = false;
}

Expand Down
1 change: 0 additions & 1 deletion library/std/src/os/emscripten/raw.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ pub type mode_t = u32;
#[stable(feature = "pthread_t", since = "1.8.0")]
pub type pthread_t = c_ulong;

#[doc(inline)]
#[stable(feature = "raw_ext", since = "1.1.0")]
pub type blkcnt_t = u64;
#[stable(feature = "raw_ext", since = "1.1.0")]
Expand Down
11 changes: 0 additions & 11 deletions src/test/rustdoc-ui/doc-attr2.rs
View file

This file was deleted.

26 changes: 0 additions & 26 deletions src/test/rustdoc-ui/doc-attr2.stderr
View file

This file was deleted.

32 changes: 32 additions & 0 deletions src/test/rustdoc-ui/invalid-doc-attr.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#![crate_type = "lib"]
#![deny(warnings)]

#[doc(test(no_crate_inject))]
//~^ ERROR can only be applied at the crate level
//~| WARN is being phased out
//~| HELP to apply to the crate, use an inner attribute
//~| SUGGESTION #![doc(test(no_crate_inject))]
#[doc(inline)]
//~^ ERROR can only be applied to a `use` item
//~| WARN is being phased out
pub fn foo() {}

pub mod bar {
#![doc(test(no_crate_inject))]
//~^ ERROR can only be applied at the crate level
//~| WARN is being phased out

#[doc(test(no_crate_inject))]
//~^ ERROR can only be applied at the crate level
//~| WARN is being phased out
#[doc(inline)]
//~^ ERROR can only be applied to a `use` item
//~| WARN is being phased out
pub fn baz() {}
}

#[doc(inline)]
#[doc(no_inline)]
//~^^ ERROR conflicting doc inlining attributes
//~| HELP remove one of the conflicting attributes
pub use bar::baz;
78 changes: 78 additions & 0 deletions src/test/rustdoc-ui/invalid-doc-attr.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
error: this attribute can only be applied at the crate level
--> $DIR/invalid-doc-attr.rs:4:7
|
LL | #[doc(test(no_crate_inject))]
| ^^^^^^^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/invalid-doc-attr.rs:2:9
|
LL | #![deny(warnings)]
| ^^^^^^^^
= note: `#[deny(invalid_doc_attributes)]` implied by `#[deny(warnings)]`
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #82730 <https://github.com/rust-lang/rust/issues/82730>
= note: read https://doc.rust-lang.org/nightly/rustdoc/the-doc-attribute.html#at-the-crate-level for more information
help: to apply to the crate, use an inner attribute
|
LL | #![doc(test(no_crate_inject))]
|

error: this attribute can only be applied to a `use` item
--> $DIR/invalid-doc-attr.rs:9:7
|
LL | #[doc(inline)]
| ^^^^^^ only applicable on `use` items
...
LL | pub fn foo() {}
| ------------ not a `use` item
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #82730 <https://github.com/rust-lang/rust/issues/82730>
= note: read https://doc.rust-lang.org/nightly/rustdoc/the-doc-attribute.html#docno_inlinedocinline for more information

error: this attribute can only be applied at the crate level
--> $DIR/invalid-doc-attr.rs:15:12
|
LL | #![doc(test(no_crate_inject))]
| ^^^^^^^^^^^^^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #82730 <https://github.com/rust-lang/rust/issues/82730>
= note: read https://doc.rust-lang.org/nightly/rustdoc/the-doc-attribute.html#at-the-crate-level for more information

error: conflicting doc inlining attributes
--> $DIR/invalid-doc-attr.rs:28:7
|
LL | #[doc(inline)]
| ^^^^^^ this attribute...
LL | #[doc(no_inline)]
| ^^^^^^^^^ ...conflicts with this attribute
|
= help: remove one of the conflicting attributes

error: this attribute can only be applied at the crate level
--> $DIR/invalid-doc-attr.rs:19:11
|
LL | #[doc(test(no_crate_inject))]
| ^^^^^^^^^^^^^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #82730 <https://github.com/rust-lang/rust/issues/82730>
= note: read https://doc.rust-lang.org/nightly/rustdoc/the-doc-attribute.html#at-the-crate-level for more information

error: this attribute can only be applied to a `use` item
--> $DIR/invalid-doc-attr.rs:22:11
|
LL | #[doc(inline)]
| ^^^^^^ only applicable on `use` items
...
LL | pub fn baz() {}
| ------------ not a `use` item
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #82730 <https://github.com/rust-lang/rust/issues/82730>
= note: read https://doc.rust-lang.org/nightly/rustdoc/the-doc-attribute.html#docno_inlinedocinline for more information

error: aborting due to 6 previous errors

11 changes: 0 additions & 11 deletions src/test/ui/attributes/doc-attr2.rs
View file

This file was deleted.

Loading

0 comments on commit fe62c6e

Please sign in to comment.