Skip to content

Commit

Permalink
Auto merge of #6458 - ebroto:6022_parse_doctest, r=Manishearth
Browse files Browse the repository at this point in the history 
needless_doctest_main: handle correctly parse errors

Before this change, finding an error when parsing a doctest would make Clippy exit without emitting an error. Now we properly catch a fatal error and ignore it.

Also, if a doctest specifies an edition in the info line, it will be used when parsing it.

changelog: needless_doctest_main: handle correctly parse errors

Fixes #6022
  • Loading branch information
@bors
bors committed Dec 18, 2020
2 parents 5c00931 + bb68ec6 commit 88323e8
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 62 deletions.
139 changes: 79 additions & 60 deletions clippy_lints/src/doc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use rustc_middle::ty;
use rustc_parse::maybe_new_parser_from_source_str;
use rustc_session::parse::ParseSess;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::edition::Edition;
use rustc_span::source_map::{BytePos, FilePathMapping, MultiSpan, SourceMap, Span};
use rustc_span::{sym, FileName, Pos};
use std::io;
Expand Down Expand Up @@ -377,7 +378,7 @@ fn check_attrs<'a>(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs
check_doc(cx, valid_idents, events, &spans)
}

const RUST_CODE: &[&str] = &["rust", "no_run", "should_panic", "compile_fail", "edition2018"];
const RUST_CODE: &[&str] = &["rust", "no_run", "should_panic", "compile_fail"];

fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize>)>>(
cx: &LateContext<'_>,
Expand All @@ -400,13 +401,24 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
let mut in_link = None;
let mut in_heading = false;
let mut is_rust = false;
let mut edition = None;
for (event, range) in events {
match event {
Start(CodeBlock(ref kind)) => {
in_code = true;
if let CodeBlockKind::Fenced(lang) = kind {
is_rust =
lang.is_empty() || !lang.contains("ignore") && lang.split(',').any(|i| RUST_CODE.contains(&i));
for item in lang.split(',') {
if item == "ignore" {
is_rust = false;
break;
}
if let Some(stripped) = item.strip_prefix("edition") {
is_rust = true;
edition = stripped.parse::<Edition>().ok();
} else if item.is_empty() || RUST_CODE.contains(&item) {
is_rust = true;
}
}
}
},
End(CodeBlock(_)) => {
Expand Down Expand Up @@ -436,7 +448,8 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
let (begin, span) = spans[index];
if in_code {
if is_rust {
check_code(cx, &text, span);
let edition = edition.unwrap_or_else(|| cx.tcx.sess.edition());
check_code(cx, &text, edition, span);
}
} else {
// Adjust for the beginning of the current `Event`
Expand All @@ -450,67 +463,73 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
headers
}

fn check_code(cx: &LateContext<'_>, text: &str, span: Span) {
fn has_needless_main(code: &str) -> bool {
let filename = FileName::anon_source_code(code);

let sm = Lrc::new(SourceMap::new(FilePathMapping::empty()));
let emitter = EmitterWriter::new(box io::sink(), None, false, false, false, None, false);
let handler = Handler::with_emitter(false, None, box emitter);
let sess = ParseSess::with_span_handler(handler, sm);

let mut parser = match maybe_new_parser_from_source_str(&sess, filename, code.into()) {
Ok(p) => p,
Err(errs) => {
for mut err in errs {
err.cancel();
}
return false;
},
};

let mut relevant_main_found = false;
loop {
match parser.parse_item() {
Ok(Some(item)) => match &item.kind {
// Tests with one of these items are ignored
ItemKind::Static(..)
| ItemKind::Const(..)
| ItemKind::ExternCrate(..)
| ItemKind::ForeignMod(..) => return false,
// We found a main function ...
ItemKind::Fn(_, sig, _, Some(block)) if item.ident.name == sym::main => {
let is_async = matches!(sig.header.asyncness, Async::Yes { .. });
let returns_nothing = match &sig.decl.output {
FnRetTy::Default(..) => true,
FnRetTy::Ty(ty) if ty.kind.is_unit() => true,
_ => false,
};

if returns_nothing && !is_async && !block.stmts.is_empty() {
// This main function should be linted, but only if there are no other functions
relevant_main_found = true;
} else {
// This main function should not be linted, we're done
return false;
fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) {
fn has_needless_main(code: &str, edition: Edition) -> bool {
rustc_driver::catch_fatal_errors(|| {
rustc_span::with_session_globals(edition, || {
let filename = FileName::anon_source_code(code);

let sm = Lrc::new(SourceMap::new(FilePathMapping::empty()));
let emitter = EmitterWriter::new(box io::sink(), None, false, false, false, None, false);
let handler = Handler::with_emitter(false, None, box emitter);
let sess = ParseSess::with_span_handler(handler, sm);

let mut parser = match maybe_new_parser_from_source_str(&sess, filename, code.into()) {
Ok(p) => p,
Err(errs) => {
for mut err in errs {
err.cancel();
}
return false;
},
// Another function was found; this case is ignored too
ItemKind::Fn(..) => return false,
_ => {},
},
Ok(None) => break,
Err(mut e) => {
e.cancel();
return false;
},
}
}
};

let mut relevant_main_found = false;
loop {
match parser.parse_item() {
Ok(Some(item)) => match &item.kind {
// Tests with one of these items are ignored
ItemKind::Static(..)
| ItemKind::Const(..)
| ItemKind::ExternCrate(..)
| ItemKind::ForeignMod(..) => return false,
// We found a main function ...
ItemKind::Fn(_, sig, _, Some(block)) if item.ident.name == sym::main => {
let is_async = matches!(sig.header.asyncness, Async::Yes { .. });
let returns_nothing = match &sig.decl.output {
FnRetTy::Default(..) => true,
FnRetTy::Ty(ty) if ty.kind.is_unit() => true,
_ => false,
};

if returns_nothing && !is_async && !block.stmts.is_empty() {
// This main function should be linted, but only if there are no other functions
relevant_main_found = true;
} else {
// This main function should not be linted, we're done
return false;
}
},
// Another function was found; this case is ignored too
ItemKind::Fn(..) => return false,
_ => {},
},
Ok(None) => break,
Err(mut e) => {
e.cancel();
return false;
},
}
}

relevant_main_found
relevant_main_found
})
})
.ok()
.unwrap_or_default()
}

if has_needless_main(text) {
if has_needless_main(text, edition) {
span_lint(cx, NEEDLESS_DOCTEST_MAIN, span, "needless `fn main` in doctest");
}
}
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ extern crate rustc_ast;
extern crate rustc_ast_pretty;
extern crate rustc_attr;
extern crate rustc_data_structures;
extern crate rustc_driver;
extern crate rustc_errors;
extern crate rustc_hir;
extern crate rustc_hir_pretty;
Expand Down
10 changes: 8 additions & 2 deletions tests/ui/needless_doc_main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
/// ```
///
/// With an explicit return type it should lint too
/// ```
/// ```edition2015
/// fn main() -> () {
/// unimplemented!();
/// }
Expand Down Expand Up @@ -39,7 +39,7 @@ fn bad_doctests() {}
/// ```
///
/// This shouldn't lint either, because main is async:
/// ```
/// ```edition2018
/// async fn main() {
/// assert_eq!(42, ANSWER);
/// }
Expand Down Expand Up @@ -128,6 +128,12 @@ fn bad_doctests() {}
/// ```
fn no_false_positives() {}

/// Yields a parse error when interpreted as rust code:
/// ```
/// r#"hi"
/// ```
fn issue_6022() {}

fn main() {
bad_doctests();
no_false_positives();
Expand Down

0 comments on commit 88323e8

Please sign in to comment.