From 1a140dcc1c933ae84365bd1153372a7430f6f647 Mon Sep 17 00:00:00 2001 From: Eduardo Broto Date: Sun, 16 Aug 2020 00:25:54 +0200 Subject: [PATCH] Improve needless_doctest_main by using the parser --- clippy_lints/src/doc.rs | 75 ++++++++++++++++++++++++++++--- tests/ui/needless_doc_main.rs | 68 ++++++++++++++++++++++++++-- tests/ui/needless_doc_main.stderr | 12 +++-- 3 files changed, 142 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 6ce36fd2360..9555459e240 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -1,16 +1,22 @@ use crate::utils::{implements_trait, is_entrypoint_fn, is_type_diagnostic_item, return_ty, span_lint}; use if_chain::if_chain; use itertools::Itertools; -use rustc_ast::ast::{AttrKind, Attribute}; +use rustc_ast::ast::{Async, AttrKind, Attribute, FnRetTy, ItemKind}; use rustc_ast::token::CommentKind; use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::sync::Lrc; +use rustc_errors::emitter::EmitterWriter; +use rustc_errors::Handler; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::lint::in_external_macro; 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::source_map::{BytePos, MultiSpan, Span}; -use rustc_span::Pos; +use rustc_span::source_map::{BytePos, FilePathMapping, MultiSpan, SourceMap, Span}; +use rustc_span::{FileName, Pos}; +use std::io; use std::ops::Range; use url::Url; @@ -431,10 +437,67 @@ fn check_doc<'a, Events: Iterator, Range, text: &str, span: Span) { - if text.contains("fn main() {") && !LEAVE_MAIN_PATTERNS.iter().any(|p| text.contains(p)) { + 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; + } + }, + // 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 + } + + if has_needless_main(text) { span_lint(cx, NEEDLESS_DOCTEST_MAIN, span, "needless `fn main` in doctest"); } } diff --git a/tests/ui/needless_doc_main.rs b/tests/ui/needless_doc_main.rs index 682d7b3c4ce..883683e08a2 100644 --- a/tests/ui/needless_doc_main.rs +++ b/tests/ui/needless_doc_main.rs @@ -9,8 +9,14 @@ /// } /// ``` /// -/// This should, too. +/// With an explicit return type it should lint too +/// ``` +/// fn main() -> () { +/// unimplemented!(); +/// } +/// ``` /// +/// This should, too. /// ```rust /// fn main() { /// unimplemented!(); @@ -18,7 +24,6 @@ /// ``` /// /// This one too. -/// /// ```no_run /// fn main() { /// unimplemented!(); @@ -33,6 +38,20 @@ fn bad_doctests() {} /// fn main(){} /// ``` /// +/// This shouldn't lint either, because main is async: +/// ``` +/// async fn main() { +/// assert_eq!(42, ANSWER); +/// } +/// ``` +/// +/// Same here, because the return type is not the unit type: +/// ``` +/// fn main() -> Result<()> { +/// Ok(()) +/// } +/// ``` +/// /// This shouldn't lint either, because there's a `static`: /// ``` /// static ANSWER: i32 = 42; @@ -42,6 +61,15 @@ fn bad_doctests() {} /// } /// ``` /// +/// This shouldn't lint either, because there's a `const`: +/// ``` +/// fn main() { +/// assert_eq!(42, ANSWER); +/// } +/// +/// const ANSWER: i32 = 42; +/// ``` +/// /// Neither should this lint because of `extern crate`: /// ``` /// #![feature(test)] @@ -51,8 +79,41 @@ fn bad_doctests() {} /// } /// ``` /// -/// We should not lint ignored examples: +/// Neither should this lint because it has an extern block: +/// ``` +/// extern {} +/// fn main() { +/// unimplemented!(); +/// } +/// ``` +/// +/// This should not lint because there is another function defined: +/// ``` +/// fn fun() {} +/// +/// fn main() { +/// unimplemented!(); +/// } +/// ``` /// +/// We should not lint inside raw strings ... +/// ``` +/// let string = r#" +/// fn main() { +/// unimplemented!(); +/// } +/// "#; +/// ``` +/// +/// ... or comments +/// ``` +/// // fn main() { +/// // let _inception = 42; +/// // } +/// let _inception = 42; +/// ``` +/// +/// We should not lint ignored examples: /// ```rust,ignore /// fn main() { /// unimplemented!(); @@ -60,7 +121,6 @@ fn bad_doctests() {} /// ``` /// /// Or even non-rust examples: -/// /// ```text /// fn main() { /// is what starts the program diff --git a/tests/ui/needless_doc_main.stderr b/tests/ui/needless_doc_main.stderr index 65d40ee6832..05c7f9d33a7 100644 --- a/tests/ui/needless_doc_main.stderr +++ b/tests/ui/needless_doc_main.stderr @@ -7,16 +7,22 @@ LL | /// fn main() { = note: `-D clippy::needless-doctest-main` implied by `-D warnings` error: needless `fn main` in doctest - --> $DIR/needless_doc_main.rs:15:4 + --> $DIR/needless_doc_main.rs:14:4 + | +LL | /// fn main() -> () { + | ^^^^^^^^^^^^^^^^^^ + +error: needless `fn main` in doctest + --> $DIR/needless_doc_main.rs:21:4 | LL | /// fn main() { | ^^^^^^^^^^^^ error: needless `fn main` in doctest - --> $DIR/needless_doc_main.rs:23:4 + --> $DIR/needless_doc_main.rs:28:4 | LL | /// fn main() { | ^^^^^^^^^^^^ -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors -- 2.44.0