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

Check code blocks tags #71408

Merged
merged 3 commits into from
Apr 23, 2020
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
4 changes: 3 additions & 1 deletion src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
);
Expand Down
7 changes: 7 additions & 0 deletions src/librustc_session/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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());
Expand All @@ -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))
Expand Down
118 changes: 114 additions & 4 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
};
Expand Down Expand Up @@ -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;
Expand All @@ -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(),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -715,6 +778,53 @@ impl LangString {
seen_other_tags = true;
}
}
x if extra.is_some() => {
let s = x.to_lowercase();
kinnison marked this conversation as resolved.
Show resolved Hide resolved
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 \
kinnison marked this conversation as resolved.
Show resolved Hide resolved
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,
}
}
Expand Down Expand Up @@ -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() {
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/markdown/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 4 additions & 2 deletions src/librustdoc/passes/check_code_block_syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Loading