Auto merge of #6458 - ebroto:6022_parse_doctest, r=Manishearth

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

Author: bors

clippy_lints/src/doc.rs

@@ -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;
@@ -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<'_>,
@@ -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(_)) => {
@@ -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`
@@ -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");
     }
 }

clippy_lints/src/lib.rs

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

tests/ui/needless_doc_main.rs

@@ -10,7 +10,7 @@
 /// ```
 ///
 /// With an explicit return type it should lint too
-/// ```
+/// ```edition2015
 /// fn main() -> () {
 ///     unimplemented!();
 /// }
@@ -39,7 +39,7 @@ fn bad_doctests() {}
 /// ```
 ///
 /// This shouldn't lint either, because main is async:
-/// ```
+/// ```edition2018
 /// async fn main() {
 ///     assert_eq!(42, ANSWER);
 /// }
@@ -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();