mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #11872 - llogiq:test-attr-in-doctest, r=xFrednet

Browse Source 
add lint against unit tests in doctests

During RustLab, Alice Ryhl brought to my attention that the Andoid team stumbled over the fact that if one attempts to write a unit test within a doctest, it will be summarily ignored. So this lint should help people wondering why their tests won't run.

---

changelog: New lint: [`test_attr_in_doctest`]
[#11872](https://github.com/rust-lang/rust-clippy/pull/11872)
This commit is contained in:
bors 2023-11-30 10:24:16 +00:00
commit 665fd5219a
6 changed files with 172 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

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

1
clippy_lints/src/declared_lints.rs
View File

@ -140,6 +140,7 @@
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,

40
clippy_lints/src/doc/mod.rs
View File

@ -199,6 +199,39 @@
"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)
@ -329,6 +362,7 @@ pub fn new(valid_idents: &[String], check_private_items: bool) -> Self {
MISSING_ERRORS_DOC,
MISSING_PANICS_DOC,
NEEDLESS_DOCTEST_MAIN,
TEST_ATTR_IN_DOCTEST,
UNNECESSARY_SAFETY_DOC,
SUSPICIOUS_DOC_COMMENTS
]);
@ -515,6 +549,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
let mut in_heading = false;
let mut is_rust = false;
let mut no_test = false;
let mut ignore = false;
let mut edition = None;
let mut ticks_unbalanced = false;
let mut text_to_check: Vec<(CowStr<'_>, Range<usize>)> = Vec::new();
@ -530,6 +565,8 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
break;
} else if item == "no_test" {
no_test = true;
} else if item == "no_run" || item == "compile_fail" {
ignore = true;
}
if let Some(stripped) = item.strip_prefix("edition") {
is_rust = true;
@ -543,6 +580,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
End(CodeBlock(_)) => {
in_code = false;
is_rust = false;
ignore = false;
},
Start(Link(_, url, _)) => in_link = Some(url),
End(Link(..)) => in_link = None,
@ -596,7 +634,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
if in_code {
if is_rust && !no_test {
let edition = edition.unwrap_or_else(|| cx.tcx.sess.edition());
needless_doctest_main::check(cx, &text, edition, range.clone(), fragments);
needless_doctest_main::check(cx, &text, edition, range.clone(), fragments, ignore);
}
} else {
if in_link.is_some() {

67
clippy_lints/src/doc/needless_doctest_main.rs
View File

@ -1,9 +1,9 @@
use std::ops::Range;
use std::{io, thread};
use crate::doc::NEEDLESS_DOCTEST_MAIN;
use crate::doc::{NEEDLESS_DOCTEST_MAIN, TEST_ATTR_IN_DOCTEST};
use clippy_utils::diagnostics::span_lint;
use rustc_ast::{Async, Fn, FnRetTy, ItemKind};
use rustc_ast::{Async, Fn, FnRetTy, Item, ItemKind};
use rustc_data_structures::sync::Lrc;
use rustc_errors::emitter::EmitterWriter;
use rustc_errors::Handler;
@ -13,14 +13,33 @@
use rustc_session::parse::ParseSess;
use rustc_span::edition::Edition;
use rustc_span::source_map::{FilePathMapping, SourceMap};
use rustc_span::{sym, FileName};
use rustc_span::{sym, FileName, Pos};
use super::Fragments;
pub fn check(cx: &LateContext<'_>, text: &str, edition: Edition, range: Range<usize>, fragments: Fragments<'_>) {
fn has_needless_main(code: String, edition: Edition) -> bool {
fn get_test_spans(item: &Item, test_attr_spans: &mut Vec<Range<usize>>) {
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<usize>,
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<Range<usize>>) {
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 @@ fn has_needless_main(code: String, edition: Edition) -> bool {
Ok(p) => 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 @@ fn has_needless_main(code: String, edition: Edition) -> bool {
relevant_main_found = true;
} else {
// This main function should not be linted, we're done
return false;
eligible = false;
}
},
// Another function was found; this case is ignored for needless_doctest_main
ItemKind::Fn(box Fn { .. }) => {
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 @@ fn has_needless_main(code: String, edition: Edition) -> bool {
// Because of the global session, we need to create a new session in a different thread with
// the edition we need.
let text = text.to_owned();
if thread::spawn(move || has_needless_main(text, edition))
let (has_main, test_attr_spans) = thread::spawn(move || check_code_sample(text, edition, ignore))
.join()
.expect("thread::spawn failed")
&& let Some(span) = fragments.span(cx, range.start..range.end - trailing_whitespace)
{
.expect("thread::spawn failed");
if has_main && let Some(span) = fragments.span(cx, range.start..range.end - trailing_whitespace) {
span_lint(cx, NEEDLESS_DOCTEST_MAIN, span, "needless `fn main` in doctest");
}
for span in test_attr_spans {
let span = (range.start + span.start)..(range.start + span.end);
if let Some(span) = fragments.span(cx, span) {
span_lint(cx, TEST_ATTR_IN_DOCTEST, span, "unit tests in doctest are not executed");
}
}
}

51
tests/ui/test_attr_in_doctest.rs Normal file
View File

@ -0,0 +1,51 @@
/// This is a test for `#[test]` in doctests
///
/// # Examples
///
/// ```
/// #[test]
/// fn should_be_linted() {
/// assert_eq!(1, 1);
/// }
/// ```
///
/// Make sure we catch multiple tests in one example,
/// and show that we really parse the attr:
///
/// ```
/// #[test]
/// fn should_also_be_linted() {
/// #[cfg(test)]
/// assert!(true);
/// }
///
/// #[test]
/// fn should_be_linted_too() {
/// assert_eq!("#[test]", "
/// #[test]
/// ");
/// }
/// ```
///
/// We don't catch examples that aren't run:
///
/// ```ignore
/// #[test]
/// fn ignored() { todo!() }
/// ```
///
/// ```no_run
/// #[test]
/// fn ignored() { todo!() }
/// ```
///
/// ```compile_fail
/// #[test]
/// fn ignored() { Err(()) }
/// ```
///
/// ```txt
/// #[test]
/// fn not_even_rust() { panic!("Ouch") }
/// ```
fn test_attr_in_doctests() {}

29
tests/ui/test_attr_in_doctest.stderr Normal file
View File

@ -0,0 +1,29 @@
error: unit tests in doctest are not executed
--> $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