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.

Sign up for GitHub

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

needless_doctest_main: handle correctly parse errors #6458

Merged
merged 3 commits into from
Dec 18, 2020
Merged

needless_doctest_main: handle correctly parse errors #6458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39bcf8e
Handle fatal errors when parsing doctests
ebroto Dec 15, 2020
41b5ebe
needless_doctest_main: add edition support
ebroto Dec 15, 2020
bb68ec6
Apply suggestion from PR review
ebroto Dec 17, 2020
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
139 changes: 79 additions & 60 deletions clippy_lints/src/doc.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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