diff --git a/CHANGELOG.md b/CHANGELOG.md index abe975fa42b..2e9b755caa0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5545,6 +5545,7 @@ Released 2018-09-13 [`tabs_in_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#tabs_in_doc_comments [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment [`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr +[`test_attr_in_doctest`]: https://rust-lang.github.io/rust-clippy/master/index.html#test_attr_in_doctest [`tests_outside_test_module`]: https://rust-lang.github.io/rust-clippy/master/index.html#tests_outside_test_module [`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some [`to_string_in_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_display diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 5a109fcc2cc..b440e267efe 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -140,6 +140,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::doc::MISSING_SAFETY_DOC_INFO, crate::doc::NEEDLESS_DOCTEST_MAIN_INFO, crate::doc::SUSPICIOUS_DOC_COMMENTS_INFO, + crate::doc::TEST_ATTR_IN_DOCTEST_INFO, crate::doc::UNNECESSARY_SAFETY_DOC_INFO, crate::double_parens::DOUBLE_PARENS_INFO, crate::drop_forget_ref::DROP_NON_DROP_INFO, diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index 607af6ba905..f6c8f6269ab 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -199,6 +199,39 @@ declare_clippy_lint! { "presence of `fn main() {` in code examples" } +declare_clippy_lint! { + /// ### What it does + /// Checks for `#[test]` in doctests unless they are marked with + /// either `ignore`, `no_run` or `compile_fail`. + /// + /// ### Why is this bad? + /// Code in examples marked as `#[test]` will somewhat + /// surprisingly not be run by `cargo test`. If you really want + /// to show how to test stuff in an example, mark it `no_run` to + /// make the intent clear. + /// + /// ### Examples + /// ```no_run + /// /// An example of a doctest with a `main()` function + /// /// + /// /// # Examples + /// /// + /// /// ``` + /// /// #[test] + /// /// fn equality_works() { + /// /// assert_eq!(1_u8, 1); + /// /// } + /// /// ``` + /// fn test_attr_in_doctest() { + /// unimplemented!(); + /// } + /// ``` + #[clippy::version = "1.40.0"] + pub TEST_ATTR_IN_DOCTEST, + suspicious, + "presence of `#[test]` in code examples" +} + declare_clippy_lint! { /// ### What it does /// Detects the syntax `['foo']` in documentation comments (notice quotes instead of backticks) @@ -330,6 +363,7 @@ impl_lint_pass!(DocMarkdown => [ MISSING_ERRORS_DOC, MISSING_PANICS_DOC, NEEDLESS_DOCTEST_MAIN, + TEST_ATTR_IN_DOCTEST, UNNECESSARY_SAFETY_DOC, SUSPICIOUS_DOC_COMMENTS ]); @@ -516,6 +550,7 @@ fn check_doc<'a, Events: Iterator, Range, Range)> = Vec::new(); @@ -531,6 +566,8 @@ fn check_doc<'a, Events: Iterator, Range, Range { in_code = false; is_rust = false; + ignore = false; }, Start(Link(_, url, _)) => in_link = Some(url), End(Link(..)) => in_link = None, @@ -597,7 +635,7 @@ fn check_doc<'a, Events: Iterator, Range, text: &str, edition: Edition, range: Range, fragments: Fragments<'_>) { - fn has_needless_main(code: String, edition: Edition) -> bool { +fn get_test_spans(item: &Item, test_attr_spans: &mut Vec>) { + test_attr_spans.extend( + item.attrs + .iter() + .find(|attr| attr.has_name(sym::test)) + .map(|attr| attr.span.lo().to_usize()..item.ident.span.hi().to_usize()), + ); +} + +pub fn check( + cx: &LateContext<'_>, + text: &str, + edition: Edition, + range: Range, + fragments: Fragments<'_>, + ignore: bool, +) { + // return whether the code contains a needless `fn main` plus a vector of byte position ranges + // of all `#[test]` attributes in not ignored code examples + fn check_code_sample(code: String, edition: Edition, ignore: bool) -> (bool, Vec>) { rustc_driver::catch_fatal_errors(|| { rustc_span::create_session_globals_then(edition, || { + let mut test_attr_spans = vec![]; let filename = FileName::anon_source_code(&code); let fallback_bundle = @@ -35,17 +54,21 @@ pub fn check(cx: &LateContext<'_>, text: &str, edition: Edition, range: Range p, Err(errs) => { drop(errs); - return false; + return (false, test_attr_spans); }, }; let mut relevant_main_found = false; + let mut eligible = true; loop { match parser.parse_item(ForceCollect::No) { Ok(Some(item)) => match &item.kind { ItemKind::Fn(box Fn { sig, body: Some(block), .. }) if item.ident.name == sym::main => { + if !ignore { + get_test_spans(&item, &mut test_attr_spans); + } let is_async = matches!(sig.header.asyncness, Async::Yes { .. }); let returns_nothing = match &sig.decl.output { FnRetTy::Default(..) => true, @@ -58,27 +81,34 @@ pub fn check(cx: &LateContext<'_>, text: &str, edition: Edition, range: Range { + eligible = false; + if !ignore { + get_test_spans(&item, &mut test_attr_spans); } }, // Tests with one of these items are ignored ItemKind::Static(..) | ItemKind::Const(..) | ItemKind::ExternCrate(..) - | ItemKind::ForeignMod(..) - // Another function was found; this case is ignored - | ItemKind::Fn(..) => return false, + | ItemKind::ForeignMod(..) => { + eligible = false; + }, _ => {}, }, Ok(None) => break, Err(e) => { e.cancel(); - return false; + return (false, test_attr_spans); }, } } - relevant_main_found + (relevant_main_found & eligible, test_attr_spans) }) }) .ok() @@ -90,11 +120,16 @@ pub fn check(cx: &LateContext<'_>, text: &str, edition: Edition, range: Range $DIR/test_attr_in_doctest.rs:6:5 + | +LL | /// #[test] + | _____^ +LL | | /// fn should_be_linted() { + | |_______________________^ + | + = note: `-D clippy::test-attr-in-doctest` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::test_attr_in_doctest)]` + +error: unit tests in doctest are not executed + --> $DIR/test_attr_in_doctest.rs:16:5 + | +LL | /// #[test] + | _____^ +LL | | /// fn should_also_be_linted() { + | |____________________________^ + +error: unit tests in doctest are not executed + --> $DIR/test_attr_in_doctest.rs:22:5 + | +LL | /// #[test] + | _____^ +LL | | /// fn should_be_linted_too() { + | |___________________________^ + +error: aborting due to 3 previous errors +