Skip to content

Commit

Permalink
Rollup merge of #118872 - GuillaumeGomez:codeblock-attr-lint, r=notri…
Browse files Browse the repository at this point in the history 
…ddle

Add rustX check to codeblock attributes lint

We discovered this issue [here](#118802 (comment)).

I assume that the issue will be present in other places outside of the compiler so it's worth adding a check for it.

First commit is just a small cleanup about variables creation which was a bit strange (at least more than necessary).

r? ```@notriddle```
  • Loading branch information
@workingjubilee
workingjubilee authored Dec 13, 2023
2 parents a33f1a3 + 58327c1 commit 0430782
Show file tree
Hide file tree
Showing 5 changed files with 168 additions and 70 deletions.
99 changes: 56 additions & 43 deletions src/librustdoc/html/markdown.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
//! ```

use rustc_data_structures::fx::FxHashMap;
use rustc_errors::{DiagnosticMessage, SubdiagnosticMessage};
use rustc_errors::{DiagnosticBuilder, DiagnosticMessage};
use rustc_hir::def_id::DefId;
use rustc_middle::ty::TyCtxt;
pub(crate) use rustc_resolve::rustdoc::main_body_opts;
Expand Down Expand Up @@ -234,10 +234,6 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {

fn next(&mut self) -> Option<Self::Item> {
let event = self.inner.next();
let compile_fail;
let should_panic;
let ignore;
let edition;
let Some(Event::Start(Tag::CodeBlock(kind))) = event else {
return event;
};
Expand All @@ -253,49 +249,44 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {
}
}

let parse_result = match kind {
CodeBlockKind::Fenced(ref lang) => {
let parse_result = LangString::parse_without_check(
lang,
self.check_error_codes,
false,
self.custom_code_classes_in_docs,
);
if !parse_result.rust {
let added_classes = parse_result.added_classes;
let lang_string = if let Some(lang) = parse_result.unknown.first() {
format!("language-{}", lang)
} else {
String::new()
};
let whitespace = if added_classes.is_empty() { "" } else { " " };
return Some(Event::Html(
format!(
"<div class=\"example-wrap\">\
let LangString { added_classes, compile_fail, should_panic, ignore, edition, .. } =
match kind {
CodeBlockKind::Fenced(ref lang) => {
let parse_result = LangString::parse_without_check(
lang,
self.check_error_codes,
false,
self.custom_code_classes_in_docs,
);
if !parse_result.rust {
let added_classes = parse_result.added_classes;
let lang_string = if let Some(lang) = parse_result.unknown.first() {
format!("language-{}", lang)
} else {
String::new()
};
let whitespace = if added_classes.is_empty() { "" } else { " " };
return Some(Event::Html(
format!(
"<div class=\"example-wrap\">\
<pre class=\"{lang_string}{whitespace}{added_classes}\">\
<code>{text}</code>\
</pre>\
</div>",
added_classes = added_classes.join(" "),
text = Escape(&original_text),
)
.into(),
));
added_classes = added_classes.join(" "),
text = Escape(&original_text),
)
.into(),
));
}
parse_result
}
parse_result
}
CodeBlockKind::Indented => Default::default(),
};
CodeBlockKind::Indented => Default::default(),
};

let added_classes = parse_result.added_classes;
let lines = original_text.lines().filter_map(|l| map_line(l).for_html());
let text = lines.intersperse("\n".into()).collect::<String>();

compile_fail = parse_result.compile_fail;
should_panic = parse_result.should_panic;
ignore = parse_result.ignore;
edition = parse_result.edition;

let explicit_edition = edition.is_some();
let edition = edition.unwrap_or(self.edition);

Expand Down Expand Up @@ -852,15 +843,17 @@ impl<'tcx> ExtraInfo<'tcx> {
fn error_invalid_codeblock_attr_with_help(
&self,
msg: impl Into<DiagnosticMessage>,
help: impl Into<SubdiagnosticMessage>,
f: impl for<'a, 'b> FnOnce(
&'b mut DiagnosticBuilder<'a, ()>,
) -> &'b mut DiagnosticBuilder<'a, ()>,
) {
if let Some(def_id) = self.def_id.as_local() {
self.tcx.struct_span_lint_hir(
crate::lint::INVALID_CODEBLOCK_ATTRIBUTES,
self.tcx.local_def_id_to_hir_id(def_id),
self.sp,
msg,
|lint| lint.help(help),
f,
);
}
}
Expand Down Expand Up @@ -1293,6 +1286,21 @@ impl LangString {
LangStringToken::LangToken(x) if x.starts_with("edition") => {
data.edition = x[7..].parse::<Edition>().ok();
}
LangStringToken::LangToken(x)
if x.starts_with("rust") && x[4..].parse::<Edition>().is_ok() =>
{
if let Some(extra) = extra {
extra.error_invalid_codeblock_attr_with_help(
format!("unknown attribute `{x}`"),
|lint| {
lint.help(format!(
"there is an attribute with a similar name: `edition{}`",
&x[4..],
))
},
);
}
}
LangStringToken::LangToken(x)
if allow_error_code_check && x.starts_with('E') && x.len() == 5 =>
{
Expand Down Expand Up @@ -1337,8 +1345,13 @@ impl LangString {
} {
if let Some(extra) = extra {
extra.error_invalid_codeblock_attr_with_help(
format!("unknown attribute `{x}`. Did you mean `{flag}`?"),
help,
format!("unknown attribute `{x}`"),
|lint| {
lint.help(format!(
"there is an attribute with a similar name: `{flag}`"
))
.help(help)
},
);
}
}
Expand Down
36 changes: 24 additions & 12 deletions tests/rustdoc-ui/doctest/check-attr-test.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: unknown attribute `compile-fail`. Did you mean `compile_fail`?
error: unknown attribute `compile-fail`
--> $DIR/check-attr-test.rs:5:1
|
5 | / /// foo
Expand All @@ -8,14 +8,15 @@ error: unknown attribute `compile-fail`. Did you mean `compile_fail`?
9 | | /// ```
| |_______^
|
= help: there is an attribute with a similar name: `compile_fail`
= help: the code block will either not be tested if not marked as a rust one or won't fail if it compiles successfully
note: the lint level is defined here
--> $DIR/check-attr-test.rs:3:9
|
3 | #![deny(rustdoc::invalid_codeblock_attributes)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: unknown attribute `compilefail`. Did you mean `compile_fail`?
error: unknown attribute `compilefail`
--> $DIR/check-attr-test.rs:5:1
|
5 | / /// foo
Expand All @@ -25,9 +26,10 @@ error: unknown attribute `compilefail`. Did you mean `compile_fail`?
9 | | /// ```
| |_______^
|
= help: there is an attribute with a similar name: `compile_fail`
= help: the code block will either not be tested if not marked as a rust one or won't fail if it compiles successfully

error: unknown attribute `comPile_fail`. Did you mean `compile_fail`?
error: unknown attribute `comPile_fail`
--> $DIR/check-attr-test.rs:5:1
|
5 | / /// foo
Expand All @@ -37,9 +39,10 @@ error: unknown attribute `comPile_fail`. Did you mean `compile_fail`?
9 | | /// ```
| |_______^
|
= help: there is an attribute with a similar name: `compile_fail`
= help: the code block will either not be tested if not marked as a rust one or won't fail if it compiles successfully

error: unknown attribute `should-panic`. Did you mean `should_panic`?
error: unknown attribute `should-panic`
--> $DIR/check-attr-test.rs:12:1
|
12 | / /// bar
Expand All @@ -49,9 +52,10 @@ error: unknown attribute `should-panic`. Did you mean `should_panic`?
16 | | /// ```
| |_______^
|
= help: there is an attribute with a similar name: `should_panic`
= help: the code block will either not be tested if not marked as a rust one or won't fail if it doesn't panic when running

error: unknown attribute `shouldpanic`. Did you mean `should_panic`?
error: unknown attribute `shouldpanic`
--> $DIR/check-attr-test.rs:12:1
|
12 | / /// bar
Expand All @@ -61,9 +65,10 @@ error: unknown attribute `shouldpanic`. Did you mean `should_panic`?
16 | | /// ```
| |_______^
|
= help: there is an attribute with a similar name: `should_panic`
= help: the code block will either not be tested if not marked as a rust one or won't fail if it doesn't panic when running

error: unknown attribute `shOuld_panic`. Did you mean `should_panic`?
error: unknown attribute `shOuld_panic`
--> $DIR/check-attr-test.rs:12:1
|
12 | / /// bar
Expand All @@ -73,9 +78,10 @@ error: unknown attribute `shOuld_panic`. Did you mean `should_panic`?
16 | | /// ```
| |_______^
|
= help: there is an attribute with a similar name: `should_panic`
= help: the code block will either not be tested if not marked as a rust one or won't fail if it doesn't panic when running

error: unknown attribute `no-run`. Did you mean `no_run`?
error: unknown attribute `no-run`
--> $DIR/check-attr-test.rs:19:1
|
19 | / /// foobar
Expand All @@ -85,9 +91,10 @@ error: unknown attribute `no-run`. Did you mean `no_run`?
23 | | /// ```
| |_______^
|
= help: there is an attribute with a similar name: `no_run`
= help: the code block will either not be tested if not marked as a rust one or will be run (which you might not want)

error: unknown attribute `norun`. Did you mean `no_run`?
error: unknown attribute `norun`
--> $DIR/check-attr-test.rs:19:1
|
19 | / /// foobar
Expand All @@ -97,9 +104,10 @@ error: unknown attribute `norun`. Did you mean `no_run`?
23 | | /// ```
| |_______^
|
= help: there is an attribute with a similar name: `no_run`
= help: the code block will either not be tested if not marked as a rust one or will be run (which you might not want)

error: unknown attribute `nO_run`. Did you mean `no_run`?
error: unknown attribute `nO_run`
--> $DIR/check-attr-test.rs:19:1
|
19 | / /// foobar
Expand All @@ -109,9 +117,10 @@ error: unknown attribute `nO_run`. Did you mean `no_run`?
23 | | /// ```
| |_______^
|
= help: there is an attribute with a similar name: `no_run`
= help: the code block will either not be tested if not marked as a rust one or will be run (which you might not want)

error: unknown attribute `test-harness`. Did you mean `test_harness`?
error: unknown attribute `test-harness`
--> $DIR/check-attr-test.rs:26:1
|
26 | / /// b
Expand All @@ -121,9 +130,10 @@ error: unknown attribute `test-harness`. Did you mean `test_harness`?
30 | | /// ```
| |_______^
|
= help: there is an attribute with a similar name: `test_harness`
= help: the code block will either not be tested if not marked as a rust one or the code will be wrapped inside a main function

error: unknown attribute `testharness`. Did you mean `test_harness`?
error: unknown attribute `testharness`
--> $DIR/check-attr-test.rs:26:1
|
26 | / /// b
Expand All @@ -133,9 +143,10 @@ error: unknown attribute `testharness`. Did you mean `test_harness`?
30 | | /// ```
| |_______^
|
= help: there is an attribute with a similar name: `test_harness`
= help: the code block will either not be tested if not marked as a rust one or the code will be wrapped inside a main function

error: unknown attribute `tesT_harness`. Did you mean `test_harness`?
error: unknown attribute `tesT_harness`
--> $DIR/check-attr-test.rs:26:1
|
26 | / /// b
Expand All @@ -145,6 +156,7 @@ error: unknown attribute `tesT_harness`. Did you mean `test_harness`?
30 | | /// ```
| |_______^
|
= help: there is an attribute with a similar name: `test_harness`
= help: the code block will either not be tested if not marked as a rust one or the code will be wrapped inside a main function

error: aborting due to 12 previous errors
Expand Down
17 changes: 17 additions & 0 deletions tests/rustdoc-ui/lints/check-attr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,20 @@ pub fn foobar() {}
/// boo
/// ```
pub fn b() {}

/// b
//~^ ERROR
///
/// ```rust2018
/// boo
/// ```
pub fn c() {}

/// b
//~^ ERROR
//~| ERROR
///
/// ```rust2018 shouldpanic
/// boo
/// ```
pub fn d() {}
Loading

0 comments on commit 0430782

Please sign in to comment.