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

add lint against unit tests in doctests

Browse Source
This commit is contained in:
Andre Bogus 2023-11-26 06:58:25 +01:00
parent 3664d6328d
commit 0ba9bf9f9a
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)
@ -330,6 +363,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
]);
@ -516,6 +550,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();
@ -531,6 +566,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;
@ -544,6 +581,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,
@ -597,7 +635,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