Skip to content

Find more invalid doc attributes #83098

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Mar 15, 2021
Merged

Find more invalid doc attributes #83098

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9613a88
Refactor `check_doc_attrs` body
camelid Mar 13, 2021
7189c05
Lint non-meta doc attributes
camelid Mar 13, 2021
7e972a3
Report error for each invalid nested attribute
camelid Mar 11, 2021
fe64970
Add another test case
camelid Mar 12, 2021
5134047
Add hyphen to "crate level"
camelid Mar 13, 2021
13884dc
Update `rustdoc-ui` versions of the `doc-attr` test
camelid Mar 14, 2021
13076f9
Tweak diagnostics
camelid Mar 14, 2021
8f40e11
Use pretty-printer instead of `span_to_snippet`
camelid Mar 14, 2021
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
102 changes: 60 additions & 42 deletions compiler/rustc_passes/src/check_attr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ impl CheckAttrVisitor<'tcx> {
.struct_span_err(
meta.span(),
&format!(
"`#![doc({} = \"...\")]` isn't allowed as a crate level attribute",
"`#![doc({} = \"...\")]` isn't allowed as a crate-level attribute",
attr_name,
),
)
Expand All @@ -531,60 +531,67 @@ impl CheckAttrVisitor<'tcx> {
}

fn check_doc_attrs(&self, attr: &Attribute, hir_id: HirId, target: Target) -> bool {
if let Some(mi) = attr.meta() {
if let Some(list) = mi.meta_item_list() {
for meta in list {
if meta.has_name(sym::alias) {
if !self.check_attr_crate_level(meta, hir_id, "alias")
|| !self.check_doc_alias(meta, hir_id, target)
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")
|| !self.check_doc_alias(&meta, hir_id, target) =>
{
return false;
is_valid = false
}
} else if meta.has_name(sym::keyword) {
if !self.check_attr_crate_level(meta, hir_id, "keyword")
|| !self.check_doc_keyword(meta, hir_id)

sym::keyword
if !self.check_attr_crate_level(&meta, hir_id, "keyword")
|| !self.check_doc_keyword(&meta, hir_id) =>
{
return false;
is_valid = false
}
} else if meta.has_name(sym::test) {
if CRATE_HIR_ID != hir_id {

sym::test if CRATE_HIR_ID != hir_id => {
self.tcx.struct_span_lint_hir(
INVALID_DOC_ATTRIBUTES,
hir_id,
meta.span(),
|lint| {
lint.build(
"`#![doc(test(...)]` is only allowed as a crate level attribute"
"`#![doc(test(...)]` is only allowed \
as a crate-level attribute",
)
.emit();
},
);
return false;
is_valid = false;
}
} else if let Some(i_meta) = meta.meta_item() {
if ![
sym::cfg,
sym::hidden,
sym::html_favicon_url,
sym::html_logo_url,
sym::html_no_source,
sym::html_playground_url,
sym::html_root_url,
sym::include,
sym::inline,
sym::issue_tracker_base_url,
sym::masked,
sym::no_default_passes, // deprecated
sym::no_inline,
sym::passes, // deprecated
sym::plugins, // removed, but rustdoc warns about it itself
sym::primitive,
sym::spotlight,
sym::test,
]
.iter()
.any(|m| i_meta.has_name(*m))
{

// no_default_passes: deprecated
// passes: deprecated
// plugins: removed, but rustdoc warns about it itself
sym::alias
| sym::cfg
| sym::hidden
| sym::html_favicon_url
| sym::html_logo_url
| sym::html_no_source
| sym::html_playground_url
| sym::html_root_url
| sym::include
| sym::inline
| sym::issue_tracker_base_url
| sym::keyword
| sym::masked
| sym::no_default_passes
| sym::no_inline
| sym::passes
| sym::plugins
| sym::primitive
| sym::spotlight
| sym::test => {}

_ => {
self.tcx.struct_span_lint_hir(
INVALID_DOC_ATTRIBUTES,
hir_id,
Expand All @@ -597,13 +604,24 @@ impl CheckAttrVisitor<'tcx> {
.emit();
},
);
return false;
is_valid = false;
}
}
} else {
self.tcx.struct_span_lint_hir(
INVALID_DOC_ATTRIBUTES,
hir_id,
meta.span(),
|lint| {
lint.build(&format!("unknown `doc` attribute")).emit();
},
);
is_valid = false;
}
}
}
true

is_valid
}

/// Checks if `#[cold]` is applied to a non-function. Returns `true` if valid.
Expand Down
2 changes: 1 addition & 1 deletion src/test/rustdoc-ui/doc-alias-crate-level.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: '\'' character isn't allowed in `#[doc(alias = "...")]`
LL | #[doc(alias = "shouldn't work!")]
| ^^^^^^^^^^^^^^^^^

error: `#![doc(alias = "...")]` isn't allowed as a crate level attribute
error: `#![doc(alias = "...")]` isn't allowed as a crate-level attribute
--> $DIR/doc-alias-crate-level.rs:1:8
|
LL | #![doc(alias = "crate-level-not-working")]
Expand Down
15 changes: 15 additions & 0 deletions src/test/rustdoc-ui/doc-attr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,18 @@
//~^ ERROR unknown `doc` attribute
//~^^ WARN
pub fn foo() {}

#[doc(123)]
//~^ ERROR unknown `doc` attribute
//~| WARN
#[doc("hello", "bar")]
//~^ ERROR unknown `doc` attribute
//~| WARN
//~| ERROR unknown `doc` attribute
//~| WARN
#[doc(foo::bar, crate::bar::baz = "bye")]
//~^ ERROR unknown `doc` attribute
//~| WARN
//~| ERROR unknown `doc` attribute
//~| WARN
fn bar() {}
47 changes: 46 additions & 1 deletion src/test/rustdoc-ui/doc-attr.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,51 @@ LL | #![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>

error: unknown `doc` attribute
--> $DIR/doc-attr.rs:12:7
|
LL | #[doc(123)]
| ^^^
|
= 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>

error: unknown `doc` attribute
--> $DIR/doc-attr.rs:15:7
|
LL | #[doc("hello", "bar")]
| ^^^^^^^
|
= 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>

error: unknown `doc` attribute
--> $DIR/doc-attr.rs:15:16
|
LL | #[doc("hello", "bar")]
| ^^^^^
|
= 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>

error: unknown `doc` attribute ``
--> $DIR/doc-attr.rs:20:7
|
LL | #[doc(foo::bar, crate::bar::baz = "bye")]
| ^^^^^^^^
|
= 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>

error: unknown `doc` attribute ``
--> $DIR/doc-attr.rs:20:17
|
LL | #[doc(foo::bar, crate::bar::baz = "bye")]
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= 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>

error: unknown `doc` attribute `as_ptr`
--> $DIR/doc-attr.rs:3:8
|
Expand All @@ -22,5 +67,5 @@ LL | #![doc(as_ptr)]
= 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>

error: aborting due to 2 previous errors
error: aborting due to 7 previous errors

4 changes: 2 additions & 2 deletions src/test/rustdoc-ui/doc-attr2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: `#![doc(test(...)]` is only allowed as a crate level attribute
error: `#![doc(test(...)]` is only allowed as a crate-level attribute
--> $DIR/doc-attr2.rs:4:7
|
LL | #[doc(test(no_crate_inject))]
Expand All @@ -13,7 +13,7 @@ LL | #![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>

error: `#![doc(test(...)]` is only allowed as a crate level attribute
error: `#![doc(test(...)]` is only allowed as a crate-level attribute
--> $DIR/doc-attr2.rs:9:12
|
LL | #![doc(test(no_crate_inject))]
Expand Down
15 changes: 15 additions & 0 deletions src/test/ui/attributes/doc-attr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,18 @@
//~^ ERROR unknown `doc` attribute
//~^^ WARN
pub fn foo() {}

#[doc(123)]
//~^ ERROR unknown `doc` attribute
//~| WARN
#[doc("hello", "bar")]
//~^ ERROR unknown `doc` attribute
//~| WARN
//~| ERROR unknown `doc` attribute
//~| WARN
#[doc(foo::bar, crate::bar::baz = "bye")]
//~^ ERROR unknown `doc` attribute
//~| WARN
//~| ERROR unknown `doc` attribute
//~| WARN
fn bar() {}
47 changes: 46 additions & 1 deletion src/test/ui/attributes/doc-attr.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,51 @@ LL | #![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>

error: unknown `doc` attribute
--> $DIR/doc-attr.rs:12:7
|
LL | #[doc(123)]
| ^^^
|
= 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>

error: unknown `doc` attribute
--> $DIR/doc-attr.rs:15:7
|
LL | #[doc("hello", "bar")]
| ^^^^^^^
|
= 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>

error: unknown `doc` attribute
--> $DIR/doc-attr.rs:15:16
|
LL | #[doc("hello", "bar")]
| ^^^^^
|
= 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>

error: unknown `doc` attribute ``
--> $DIR/doc-attr.rs:20:7
|
LL | #[doc(foo::bar, crate::bar::baz = "bye")]
| ^^^^^^^^
|
= 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>

error: unknown `doc` attribute ``
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I display the Path? There doesn't seem to be a Display impl.

Note that this is not a regression; this test is just exposing the previous behavior.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of name_or_empty, could use name_value_literal_span and then get the snippet for that Span? Or is path_to_string accessible here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think rustc_ast_pretty is accessible here, though I could add it as a dependency. I'm trying out the span_to_snippet approach now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both span_to_snippet and path_to_string worked. I'm leaning towards path_to_string because I think it will handle foo :: bar:: baz gracefully as foo::bar::baz, but which one would you prefer I use? You can see what the code looks like with span_to_snippet vs path_to_string here: 8f40e11

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd normally want to lean against introducing more dependencies between our crates, but depending on rust_ast_pretty when we already depend on rust_ast is probably fine. Let's stick with path_to_string.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like rustc_passes already depends transitively on rustc_ast_pretty anyway.

--> $DIR/doc-attr.rs:20:17
|
LL | #[doc(foo::bar, crate::bar::baz = "bye")]
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= 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>

error: unknown `doc` attribute `as_ptr`
--> $DIR/doc-attr.rs:3:8
|
Expand All @@ -22,5 +67,5 @@ LL | #![doc(as_ptr)]
= 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>

error: aborting due to 2 previous errors
error: aborting due to 7 previous errors

4 changes: 2 additions & 2 deletions src/test/ui/attributes/doc-attr2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: `#![doc(test(...)]` is only allowed as a crate level attribute
error: `#![doc(test(...)]` is only allowed as a crate-level attribute
--> $DIR/doc-attr2.rs:4:7
|
LL | #[doc(test(no_crate_inject))]
Expand All @@ -13,7 +13,7 @@ LL | #![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>

error: `#![doc(test(...)]` is only allowed as a crate level attribute
error: `#![doc(test(...)]` is only allowed as a crate-level attribute
--> $DIR/doc-attr2.rs:9:12
|
LL | #![doc(test(no_crate_inject))]
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/rustdoc/doc-alias-crate-level.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: '\'' character isn't allowed in `#[doc(alias = "...")]`
LL | #[doc(alias = "shouldn't work!")]
| ^^^^^^^^^^^^^^^^^

error: `#![doc(alias = "...")]` isn't allowed as a crate level attribute
error: `#![doc(alias = "...")]` isn't allowed as a crate-level attribute
--> $DIR/doc-alias-crate-level.rs:5:8
|
LL | #![doc(alias = "not working!")]
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/rustdoc/doc_keyword.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ error: `#[doc(keyword = "...")]` can only be used on modules
LL | #[doc(keyword = "hall")]
| ^^^^^^^^^^^^^^^^

error: `#![doc(keyword = "...")]` isn't allowed as a crate level attribute
error: `#![doc(keyword = "...")]` isn't allowed as a crate-level attribute
--> $DIR/doc_keyword.rs:4:8
|
LL | #![doc(keyword = "hello")]
Expand Down