Create new rustdoc lint to check for code blocks tags

This commit is contained in:
Guillaume Gomez 2020-04-21 23:49:06 +02:00
parent 2dc5b602ee
commit c687d0490e
8 changed files with 217 additions and 26 deletions

View File

@ -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
);

View File

@ -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,

View File

@ -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))

View File

@ -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;

View File

@ -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(

View File

@ -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);
}
}

View File

@ -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());

View File

@ -5,9 +5,11 @@ use rustc_errors::ErrorReported;
use rustc_feature::UnstableFeatures;
use rustc_hir as hir;
use rustc_hir::intravisit;
use rustc_hir::{HirId, CRATE_HIR_ID};
use rustc_interface::interface;
use rustc_middle::hir::map::Map;
use rustc_session::{self, config, DiagnosticOutput, Session};
use rustc_middle::ty::TyCtxt;
use rustc_session::{self, config, lint, DiagnosticOutput, Session};
use rustc_span::edition::Edition;
use rustc_span::source_map::SourceMap;
use rustc_span::symbol::sym;
@ -25,6 +27,7 @@ use tempfile::Builder as TempFileBuilder;
use crate::clean::Attributes;
use crate::config::Options;
use crate::html::markdown::{self, ErrorCodes, Ignore, LangString};
use crate::passes::span_of_attrs;
#[derive(Clone, Default)]
pub struct TestOptions {
@ -40,6 +43,45 @@ pub struct TestOptions {
pub fn run(options: Options) -> i32 {
let input = config::Input::File(options.input.clone());
let warnings_lint_name = lint::builtin::WARNINGS.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.
let mut whitelisted_lints =
vec![warnings_lint_name.to_owned(), invalid_codeblock_attribute_name.to_owned()];
whitelisted_lints.extend(options.lint_opts.iter().map(|(lint, _)| lint).cloned());
let lints = || {
lint::builtin::HardwiredLints::get_lints()
.into_iter()
.chain(rustc_lint::SoftLints::get_lints().into_iter())
};
let lint_opts = lints()
.filter_map(|lint| {
if lint.name == warnings_lint_name || lint.name == invalid_codeblock_attribute_name {
None
} else {
Some((lint.name_lower(), lint::Allow))
}
})
.chain(options.lint_opts.clone().into_iter())
.collect::<Vec<_>>();
let lint_caps = lints()
.filter_map(|lint| {
// We don't want to whitelist *all* lints so let's
// ignore those ones.
if whitelisted_lints.iter().any(|l| lint.name == l) {
None
} else {
Some((lint::LintId::of(lint), lint::Allow))
}
})
.collect();
let crate_types = if options.proc_macro_crate {
vec![config::CrateType::ProcMacro]
} else {
@ -50,10 +92,11 @@ pub fn run(options: Options) -> i32 {
maybe_sysroot: options.maybe_sysroot.clone(),
search_paths: options.libs.clone(),
crate_types,
lint_opts: if !options.display_warnings { lint_opts } else { vec![] },
lint_cap: Some(options.lint_cap.clone().unwrap_or_else(|| lint::Forbid)),
cg: options.codegen_options.clone(),
externs: options.externs.clone(),
unstable_features: UnstableFeatures::from_environment(),
lint_cap: Some(rustc_session::lint::Level::Allow),
actually_rustdoc: true,
debugging_opts: config::DebuggingOptions { ..config::basic_debugging_options() },
edition: options.edition,
@ -75,7 +118,7 @@ pub fn run(options: Options) -> i32 {
diagnostic_output: DiagnosticOutput::Default,
stderr: None,
crate_name: options.crate_name.clone(),
lint_caps: Default::default(),
lint_caps,
register_lints: None,
override_queries: None,
registry: rustc_driver::diagnostics_registry(),
@ -105,6 +148,7 @@ pub fn run(options: Options) -> i32 {
global_ctxt.enter(|tcx| {
let krate = tcx.hir().krate();
let mut hir_collector = HirCollector {
sess: compiler.session(),
collector: &mut collector,
@ -112,10 +156,17 @@ pub fn run(options: Options) -> i32 {
codes: ErrorCodes::from(
compiler.session().opts.unstable_features.is_nightly_build(),
),
tcx,
};
hir_collector.visit_testable("".to_string(), &krate.item.attrs, |this| {
intravisit::walk_crate(this, krate);
});
hir_collector.visit_testable(
"".to_string(),
&krate.item.attrs,
CRATE_HIR_ID,
krate.item.span,
|this| {
intravisit::walk_crate(this, krate);
},
);
});
compiler.session().abort_if_errors();
@ -881,18 +932,21 @@ impl Tester for Collector {
}
}
struct HirCollector<'a, 'hir> {
struct HirCollector<'a, 'hir, 'tcx> {
sess: &'a Session,
collector: &'a mut Collector,
map: Map<'hir>,
codes: ErrorCodes,
tcx: TyCtxt<'tcx>,
}
impl<'a, 'hir> HirCollector<'a, 'hir> {
impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
fn visit_testable<F: FnOnce(&mut Self)>(
&mut self,
name: String,
attrs: &[ast::Attribute],
hir_id: HirId,
sp: Span,
nested: F,
) {
let mut attrs = Attributes::from_ast(self.sess.diagnostic(), attrs);
@ -918,6 +972,11 @@ impl<'a, 'hir> HirCollector<'a, 'hir> {
self.collector,
self.codes,
self.collector.enable_per_target_ignores,
Some(&crate::html::markdown::ExtraInfo::new(
&self.tcx,
hir_id,
span_of_attrs(&attrs).unwrap_or(sp),
)),
);
}
@ -929,7 +988,7 @@ impl<'a, 'hir> HirCollector<'a, 'hir> {
}
}
impl<'a, 'hir> intravisit::Visitor<'hir> for HirCollector<'a, 'hir> {
impl<'a, 'hir, 'tcx> intravisit::Visitor<'hir> for HirCollector<'a, 'hir, 'tcx> {
type Map = Map<'hir>;
fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
@ -943,25 +1002,25 @@ impl<'a, 'hir> intravisit::Visitor<'hir> for HirCollector<'a, 'hir> {
item.ident.to_string()
};
self.visit_testable(name, &item.attrs, |this| {
self.visit_testable(name, &item.attrs, item.hir_id, item.span, |this| {
intravisit::walk_item(this, item);
});
}
fn visit_trait_item(&mut self, item: &'hir hir::TraitItem) {
self.visit_testable(item.ident.to_string(), &item.attrs, |this| {
self.visit_testable(item.ident.to_string(), &item.attrs, item.hir_id, item.span, |this| {
intravisit::walk_trait_item(this, item);
});
}
fn visit_impl_item(&mut self, item: &'hir hir::ImplItem) {
self.visit_testable(item.ident.to_string(), &item.attrs, |this| {
self.visit_testable(item.ident.to_string(), &item.attrs, item.hir_id, item.span, |this| {
intravisit::walk_impl_item(this, item);
});
}
fn visit_foreign_item(&mut self, item: &'hir hir::ForeignItem) {
self.visit_testable(item.ident.to_string(), &item.attrs, |this| {
self.visit_testable(item.ident.to_string(), &item.attrs, item.hir_id, item.span, |this| {
intravisit::walk_foreign_item(this, item);
});
}
@ -972,19 +1031,25 @@ impl<'a, 'hir> intravisit::Visitor<'hir> for HirCollector<'a, 'hir> {
g: &'hir hir::Generics,
item_id: hir::HirId,
) {
self.visit_testable(v.ident.to_string(), &v.attrs, |this| {
self.visit_testable(v.ident.to_string(), &v.attrs, v.id, v.span, |this| {
intravisit::walk_variant(this, v, g, item_id);
});
}
fn visit_struct_field(&mut self, f: &'hir hir::StructField) {
self.visit_testable(f.ident.to_string(), &f.attrs, |this| {
self.visit_testable(f.ident.to_string(), &f.attrs, f.hir_id, f.span, |this| {
intravisit::walk_struct_field(this, f);
});
}
fn visit_macro_def(&mut self, macro_def: &'hir hir::MacroDef) {
self.visit_testable(macro_def.ident.to_string(), &macro_def.attrs, |_| ());
self.visit_testable(
macro_def.ident.to_string(),
&macro_def.attrs,
macro_def.hir_id,
macro_def.span,
|_| (),
);
}
}