Rollup merge of #71408 - GuillaumeGomez:check-code-blocks-tags, r=kinnison

Check code blocks tags

Fixes #71347.

Explanations here: I realized recently that it was a common issue to confuse/misspell tags on code blocks. This is actually quite a big issue since it generally ends up in a code blocks being ignored since it's not being considered as a rust one. With this new warning, users will at least be notified about it.

PS: some improvements can be done on the error rendering but considering how big the PR already is, I think it's better to do it afterwards.

r? @ollie27

cc @rust-lang/rustdoc

Author: Dylan-DPC

src/librustc_lint/lib.rs

@@ -61,7 +61,8 @@ use rustc_middle::ty::query::Providers;
 use rustc_middle::ty::TyCtxt;
 use rustc_session::lint::builtin::{
     BARE_TRAIT_OBJECTS, ELIDED_LIFETIMES_IN_PATHS, EXPLICIT_OUTLIVES_REQUIREMENTS,
-    INTRA_DOC_LINK_RESOLUTION_FAILURE, MISSING_DOC_CODE_EXAMPLES, PRIVATE_DOC_TESTS,
+    INTRA_DOC_LINK_RESOLUTION_FAILURE, INVALID_CODEBLOCK_ATTRIBUTE, MISSING_DOC_CODE_EXAMPLES,
+    PRIVATE_DOC_TESTS,
 };
 use rustc_span::Span;
 
@@ -299,6 +300,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
     add_lint_group!(
         "rustdoc",
         INTRA_DOC_LINK_RESOLUTION_FAILURE,
+        INVALID_CODEBLOCK_ATTRIBUTE,
         MISSING_DOC_CODE_EXAMPLES,
         PRIVATE_DOC_TESTS
     );

src/librustc_session/lint/builtin.rs

@@ -386,6 +386,12 @@ declare_lint! {
     "failures in resolving intra-doc link targets"
 }
 
+declare_lint! {
+    pub INVALID_CODEBLOCK_ATTRIBUTE,
+    Warn,
+    "codeblock attribute looks a lot like a known one"
+}
+
 declare_lint! {
     pub MISSING_CRATE_LEVEL_DOCS,
     Allow,
@@ -553,6 +559,7 @@ declare_lint_pass! {
         UNSTABLE_NAME_COLLISIONS,
         IRREFUTABLE_LET_PATTERNS,
         INTRA_DOC_LINK_RESOLUTION_FAILURE,
+        INVALID_CODEBLOCK_ATTRIBUTE,
         MISSING_CRATE_LEVEL_DOCS,
         MISSING_DOC_CODE_EXAMPLES,
         PRIVATE_DOC_TESTS,

src/librustdoc/core.rs

@@ -253,6 +253,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
     let missing_doc_example = rustc_lint::builtin::MISSING_DOC_CODE_EXAMPLES.name;
     let private_doc_tests = rustc_lint::builtin::PRIVATE_DOC_TESTS.name;
     let no_crate_level_docs = rustc_lint::builtin::MISSING_CRATE_LEVEL_DOCS.name;
+    let invalid_codeblock_attribute_name = rustc_lint::builtin::INVALID_CODEBLOCK_ATTRIBUTE.name;
 
     // In addition to those specific lints, we also need to whitelist those given through
     // command line, otherwise they'll get ignored and we don't want that.
@@ -263,6 +264,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
         missing_doc_example.to_owned(),
         private_doc_tests.to_owned(),
         no_crate_level_docs.to_owned(),
+        invalid_codeblock_attribute_name.to_owned(),
     ];
 
     whitelisted_lints.extend(lint_opts.iter().map(|(lint, _)| lint).cloned());
@@ -275,7 +277,10 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
 
     let lint_opts = lints()
         .filter_map(|lint| {
-            if lint.name == warnings_lint_name || lint.name == intra_link_resolution_failure_name {
+            if lint.name == warnings_lint_name
+                || lint.name == intra_link_resolution_failure_name
+                || lint.name == invalid_codeblock_attribute_name
+            {
                 None
             } else {
                 Some((lint.name_lower(), lint::Allow))

src/librustdoc/html/markdown.rs

@@ -20,7 +20,12 @@
 #![allow(non_camel_case_types)]
 
 use rustc_data_structures::fx::FxHashMap;
+use rustc_hir::def_id::DefId;
+use rustc_hir::HirId;
+use rustc_middle::ty::TyCtxt;
+use rustc_session::lint;
 use rustc_span::edition::Edition;
+use rustc_span::Span;
 use std::borrow::Cow;
 use std::cell::RefCell;
 use std::collections::VecDeque;
@@ -192,7 +197,7 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {
         if let Some(Event::Start(Tag::CodeBlock(kind))) = event {
             let parse_result = match kind {
                 CodeBlockKind::Fenced(ref lang) => {
-                    LangString::parse(&lang, self.check_error_codes, false)
+                    LangString::parse_without_check(&lang, self.check_error_codes, false)
                 }
                 CodeBlockKind::Indented => LangString::all_false(),
             };
@@ -560,6 +565,7 @@ pub fn find_testable_code<T: test::Tester>(
     tests: &mut T,
     error_codes: ErrorCodes,
     enable_per_target_ignores: bool,
+    extra_info: Option<&ExtraInfo<'_, '_>>,
 ) {
     let mut parser = Parser::new(doc).into_offset_iter();
     let mut prev_offset = 0;
@@ -573,7 +579,12 @@ pub fn find_testable_code<T: test::Tester>(
                         if lang.is_empty() {
                             LangString::all_false()
                         } else {
-                            LangString::parse(lang, error_codes, enable_per_target_ignores)
+                            LangString::parse(
+                                lang,
+                                error_codes,
+                                enable_per_target_ignores,
+                                extra_info,
+                            )
                         }
                     }
                     CodeBlockKind::Indented => LangString::all_false(),
@@ -615,6 +626,49 @@ pub fn find_testable_code<T: test::Tester>(
     }
 }
 
+pub struct ExtraInfo<'a, 'b> {
+    hir_id: Option<HirId>,
+    item_did: Option<DefId>,
+    sp: Span,
+    tcx: &'a TyCtxt<'b>,
+}
+
+impl<'a, 'b> ExtraInfo<'a, 'b> {
+    pub fn new(tcx: &'a TyCtxt<'b>, hir_id: HirId, sp: Span) -> ExtraInfo<'a, 'b> {
+        ExtraInfo { hir_id: Some(hir_id), item_did: None, sp, tcx }
+    }
+
+    pub fn new_did(tcx: &'a TyCtxt<'b>, did: DefId, sp: Span) -> ExtraInfo<'a, 'b> {
+        ExtraInfo { hir_id: None, item_did: Some(did), sp, tcx }
+    }
+
+    fn error_invalid_codeblock_attr(&self, msg: &str, help: &str) {
+        let hir_id = match (self.hir_id, self.item_did) {
+            (Some(h), _) => h,
+            (None, Some(item_did)) => {
+                match self.tcx.hir().as_local_hir_id(item_did) {
+                    Some(hir_id) => hir_id,
+                    None => {
+                        // If non-local, no need to check anything.
+                        return;
+                    }
+                }
+            }
+            (None, None) => return,
+        };
+        self.tcx.struct_span_lint_hir(
+            lint::builtin::INVALID_CODEBLOCK_ATTRIBUTE,
+            hir_id,
+            self.sp,
+            |lint| {
+                let mut diag = lint.build(msg);
+                diag.help(help);
+                diag.emit();
+            },
+        );
+    }
+}
+
 #[derive(Eq, PartialEq, Clone, Debug)]
 pub struct LangString {
     original: String,
@@ -652,10 +706,19 @@ impl LangString {
         }
     }
 
+    fn parse_without_check(
+        string: &str,
+        allow_error_code_check: ErrorCodes,
+        enable_per_target_ignores: bool,
+    ) -> LangString {
+        Self::parse(string, allow_error_code_check, enable_per_target_ignores, None)
+    }
+
     fn parse(
         string: &str,
         allow_error_code_check: ErrorCodes,
         enable_per_target_ignores: bool,
+        extra: Option<&ExtraInfo<'_, '_>>,
     ) -> LangString {
         let allow_error_code_check = allow_error_code_check.as_bool();
         let mut seen_rust_tags = false;
@@ -715,6 +778,53 @@ impl LangString {
                         seen_other_tags = true;
                     }
                 }
+                x if extra.is_some() => {
+                    let s = x.to_lowercase();
+                    match if s == "compile-fail" || s == "compile_fail" || s == "compilefail" {
+                        Some((
+                            "compile_fail",
+                            "the code block will either not be tested if not marked as a rust one \
+                             or won't fail if it compiles successfully",
+                        ))
+                    } else if s == "should-panic" || s == "should_panic" || s == "shouldpanic" {
+                        Some((
+                            "should_panic",
+                            "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",
+                        ))
+                    } else if s == "no-run" || s == "no_run" || s == "norun" {
+                        Some((
+                            "no_run",
+                            "the code block will either not be tested if not marked as a rust one \
+                             or will be run (which you might not want)",
+                        ))
+                    } else if s == "allow-fail" || s == "allow_fail" || s == "allowfail" {
+                        Some((
+                            "allow_fail",
+                            "the code block will either not be tested if not marked as a rust one \
+                             or will be run (which you might not want)",
+                        ))
+                    } else if s == "test-harness" || s == "test_harness" || s == "testharness" {
+                        Some((
+                            "test_harness",
+                            "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",
+                        ))
+                    } else {
+                        None
+                    } {
+                        Some((flag, help)) => {
+                            if let Some(ref extra) = extra {
+                                extra.error_invalid_codeblock_attr(
+                                    &format!("unknown attribute `{}`. Did you mean `{}`?", x, flag),
+                                    help,
+                                );
+                            }
+                        }
+                        None => {}
+                    }
+                    seen_other_tags = true;
+                }
                 _ => seen_other_tags = true,
             }
         }
@@ -934,7 +1044,7 @@ crate struct RustCodeBlock {
 
 /// Returns a range of bytes for each code block in the markdown that is tagged as `rust` or
 /// untagged (and assumed to be rust).
-crate fn rust_code_blocks(md: &str) -> Vec<RustCodeBlock> {
+crate fn rust_code_blocks(md: &str, extra_info: &ExtraInfo<'_, '_>) -> Vec<RustCodeBlock> {
     let mut code_blocks = vec![];
 
     if md.is_empty() {
@@ -951,7 +1061,7 @@ crate fn rust_code_blocks(md: &str) -> Vec<RustCodeBlock> {
                     let lang_string = if syntax.is_empty() {
                         LangString::all_false()
                     } else {
-                        LangString::parse(&*syntax, ErrorCodes::Yes, false)
+                        LangString::parse(&*syntax, ErrorCodes::Yes, false, Some(extra_info))
                     };
                     if !lang_string.rust {
                         continue;

src/librustdoc/html/markdown/tests.rs

@@ -64,7 +64,7 @@ fn test_lang_string_parse() {
         edition: Option<Edition>,
     ) {
         assert_eq!(
-            LangString::parse(s, ErrorCodes::Yes, true),
+            LangString::parse(s, ErrorCodes::Yes, true, None),
             LangString {
                 should_panic,
                 no_run,

src/librustdoc/markdown.rs

@@ -153,7 +153,7 @@ pub fn test(mut options: Options, diag: &rustc_errors::Handler) -> i32 {
     collector.set_position(DUMMY_SP);
     let codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build());
 
-    find_testable_code(&input_str, &mut collector, codes, options.enable_per_target_ignores);
+    find_testable_code(&input_str, &mut collector, codes, options.enable_per_target_ignores, None);
 
     options.test_args.insert(0, "rustdoctest".to_string());
     testing::test_main(

src/librustdoc/passes/check_code_block_syntax.rs

@@ -10,7 +10,7 @@ use crate::clean;
 use crate::core::DocContext;
 use crate::fold::DocFolder;
 use crate::html::markdown::{self, RustCodeBlock};
-use crate::passes::Pass;
+use crate::passes::{span_of_attrs, Pass};
 
 pub const CHECK_CODE_BLOCK_SYNTAX: Pass = Pass {
     name: "check-code-block-syntax",
@@ -114,7 +114,9 @@ impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> {
 impl<'a, 'tcx> DocFolder for SyntaxChecker<'a, 'tcx> {
     fn fold_item(&mut self, item: clean::Item) -> Option<clean::Item> {
         if let Some(dox) = &item.attrs.collapsed_doc_value() {
-            for code_block in markdown::rust_code_blocks(&dox) {
+            let sp = span_of_attrs(&item.attrs).unwrap_or(item.source.span());
+            let extra = crate::html::markdown::ExtraInfo::new_did(&self.cx.tcx, item.def_id, sp);
+            for code_block in markdown::rust_code_blocks(&dox, &extra) {
                 self.check_rust_syntax(&item, &dox, code_block);
             }
         }

src/librustdoc/passes/mod.rs

@@ -338,7 +338,7 @@ pub fn look_for_tests<'tcx>(
 
     let mut tests = Tests { found_tests: 0 };
 
-    find_testable_code(&dox, &mut tests, ErrorCodes::No, false);
+    find_testable_code(&dox, &mut tests, ErrorCodes::No, false, None);
 
     if check_missing_code && tests.found_tests == 0 {
         let sp = span_of_attrs(&item.attrs).unwrap_or(item.source.span());