diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index c0a9f466e7b..dcd9a4adcbd 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -15,7 +15,6 @@ clippy_utils = { path = "../clippy_utils" } declare_clippy_lint = { path = "../declare_clippy_lint" } if_chain = "1.0" itertools = "0.10.1" -pulldown-cmark = { version = "0.9", default-features = false } quine-mc_cluskey = "0.2" regex-syntax = "0.7" serde = { version = "1.0", features = ["derive"] } diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index e29ab634c97..d8f2fbcea00 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -1,18 +1,16 @@ use clippy_utils::attrs::is_doc_hidden; use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_note, span_lint_and_then}; use clippy_utils::macros::{is_panic, root_macro_call_first_node}; -use clippy_utils::source::{first_line_of_span, snippet_with_applicability}; +use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; use clippy_utils::{is_entrypoint_fn, method_chain_args, return_ty}; use if_chain::if_chain; -use itertools::Itertools; use pulldown_cmark::Event::{ Code, End, FootnoteReference, HardBreak, Html, Rule, SoftBreak, Start, TaskListMarker, Text, }; use pulldown_cmark::Tag::{CodeBlock, Heading, Item, Link, Paragraph}; use pulldown_cmark::{BrokenLink, CodeBlockKind, CowStr, Options}; -use rustc_ast::ast::{Async, AttrKind, Attribute, Fn, FnRetTy, ItemKind}; -use rustc_ast::token::CommentKind; +use rustc_ast::ast::{Async, Attribute, Fn, FnRetTy, ItemKind}; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::sync::Lrc; use rustc_errors::emitter::EmitterWriter; @@ -26,6 +24,9 @@ use rustc_middle::lint::in_external_macro; use rustc_middle::ty; use rustc_parse::maybe_new_parser_from_source_str; use rustc_parse::parser::ForceCollect; +use rustc_resolve::rustdoc::{ + add_doc_fragment, attrs_to_doc_fragments, main_body_opts, source_span_for_markdown_range, DocFragment, +}; use rustc_session::parse::ParseSess; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::edition::Edition; @@ -450,53 +451,16 @@ fn lint_for_missing_headers( } } -/// Cleanup documentation decoration. -/// -/// We can't use `rustc_ast::attr::AttributeMethods::with_desugared_doc` or -/// `rustc_ast::parse::lexer::comments::strip_doc_comment_decoration` because we -/// need to keep track of -/// the spans but this function is inspired from the later. -#[expect(clippy::cast_possible_truncation)] -#[must_use] -pub fn strip_doc_comment_decoration(doc: &str, comment_kind: CommentKind, span: Span) -> (String, Vec<(usize, Span)>) { - // one-line comments lose their prefix - if comment_kind == CommentKind::Line { - let mut doc = doc.to_owned(); - doc.push('\n'); - let len = doc.len(); - // +3 skips the opening delimiter - return (doc, vec![(len, span.with_lo(span.lo() + BytePos(3)))]); - } +#[derive(Copy, Clone)] +struct Fragments<'a> { + doc: &'a str, + fragments: &'a [DocFragment], +} - let mut sizes = vec![]; - let mut contains_initial_stars = false; - for line in doc.lines() { - let offset = line.as_ptr() as usize - doc.as_ptr() as usize; - debug_assert_eq!(offset as u32 as usize, offset); - contains_initial_stars |= line.trim_start().starts_with('*'); - // +1 adds the newline, +3 skips the opening delimiter - sizes.push((line.len() + 1, span.with_lo(span.lo() + BytePos(3 + offset as u32)))); +impl Fragments<'_> { + fn span(self, cx: &LateContext<'_>, range: Range) -> Option { + source_span_for_markdown_range(cx.tcx, &self.doc, &range, &self.fragments) } - if !contains_initial_stars { - return (doc.to_string(), sizes); - } - // remove the initial '*'s if any - let mut no_stars = String::with_capacity(doc.len()); - for line in doc.lines() { - let mut chars = line.chars(); - for c in &mut chars { - if c.is_whitespace() { - no_stars.push(c); - } else { - no_stars.push(if c == '*' { ' ' } else { c }); - break; - } - } - no_stars.push_str(chars.as_str()); - no_stars.push('\n'); - } - - (no_stars, sizes) } #[derive(Copy, Clone, Default)] @@ -515,27 +479,12 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, attrs: &[ Some(("fake".into(), "fake".into())) } + let (fragments, _) = attrs_to_doc_fragments(attrs.iter().map(|attr| (attr, None)), true); let mut doc = String::new(); - let mut spans = vec![]; - - for attr in attrs { - if let AttrKind::DocComment(comment_kind, comment) = attr.kind { - let (comment, current_spans) = strip_doc_comment_decoration(comment.as_str(), comment_kind, attr.span); - spans.extend_from_slice(¤t_spans); - doc.push_str(&comment); - } else if attr.has_name(sym::doc) { - // ignore mix of sugared and non-sugared doc - // don't trigger the safety or errors check - return None; - } - } - - let mut current = 0; - for &mut (ref mut offset, _) in &mut spans { - let offset_copy = *offset; - *offset = current; - current += offset_copy; + for fragment in &fragments { + add_doc_fragment(&mut doc, fragment); } + doc.pop(); if doc.is_empty() { return Some(DocHeaders::default()); @@ -543,23 +492,19 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, attrs: &[ let mut cb = fake_broken_link_callback; - let parser = - pulldown_cmark::Parser::new_with_broken_link_callback(&doc, Options::empty(), Some(&mut cb)).into_offset_iter(); - // Iterate over all `Events` and combine consecutive events into one - let events = parser.coalesce(|previous, current| { - let previous_range = previous.1; - let current_range = current.1; + // disable smart punctuation to pick up ['link'] more easily + let opts = main_body_opts() - Options::ENABLE_SMART_PUNCTUATION; + let parser = pulldown_cmark::Parser::new_with_broken_link_callback(&doc, opts, Some(&mut cb)); - match (previous.0, current.0) { - (Text(previous), Text(current)) => { - let mut previous = previous.to_string(); - previous.push_str(¤t); - Ok((Text(previous.into()), previous_range)) - }, - (previous, current) => Err(((previous, previous_range), (current, current_range))), - } - }); - Some(check_doc(cx, valid_idents, events, &spans)) + Some(check_doc( + cx, + valid_idents, + parser.into_offset_iter(), + Fragments { + fragments: &fragments, + doc: &doc, + }, + )) } const RUST_CODE: &[&str] = &["rust", "no_run", "should_panic", "compile_fail"]; @@ -568,7 +513,7 @@ fn check_doc<'a, Events: Iterator, Range, valid_idents: &FxHashSet, events: Events, - spans: &[(usize, Span)], + fragments: Fragments<'_>, ) -> DocHeaders { // true if a safety header was found let mut headers = DocHeaders::default(); @@ -579,8 +524,8 @@ fn check_doc<'a, Events: Iterator, Range, Span)> = Vec::new(); - let mut paragraph_span = spans.get(0).expect("function isn't called if doc comment is empty").1; + let mut text_to_check: Vec<(CowStr<'_>, Range)> = Vec::new(); + let mut paragraph_range = 0..0; for (event, range) in events { match event { Start(CodeBlock(ref kind)) => { @@ -613,25 +558,28 @@ fn check_doc<'a, Events: Iterator, Range { if let End(Heading(_, _, _)) = event { in_heading = false; } - if ticks_unbalanced { + if ticks_unbalanced + && let Some(span) = fragments.span(cx, paragraph_range.clone()) + { span_lint_and_help( cx, DOC_MARKDOWN, - paragraph_span, + span, "backticks are unbalanced", None, "a backtick may be missing a pair", ); } else { - for (text, span) in text_to_check { - check_text(cx, valid_idents, &text, span); + for (text, range) in text_to_check { + if let Some(span) = fragments.span(cx, range) { + check_text(cx, valid_idents, &text, span); + } } } text_to_check = Vec::new(); @@ -640,8 +588,7 @@ fn check_doc<'a, Events: Iterator, Range (), // HTML is weird, just ignore it SoftBreak | HardBreak | TaskListMarker(_) | Code(_) | Rule => (), FootnoteReference(text) | Text(text) => { - let (begin, span) = get_current_span(spans, range.start); - paragraph_span = paragraph_span.with_hi(span.hi()); + paragraph_range.end = range.end; ticks_unbalanced |= text.contains('`') && !in_code; if Some(&text) == in_link.as_ref() || ticks_unbalanced { // Probably a link of the form `` @@ -658,19 +605,19 @@ fn check_doc<'a, Events: Iterator, Range, Range, - in_link: bool, - trimmed_text: &str, - span: Span, - range: &Range, - begin: usize, - text_len: usize, -) { - if in_link && trimmed_text.starts_with('\'') && trimmed_text.ends_with('\'') { - // fix the span to only point at the text within the link - let lo = span.lo() + BytePos::from_usize(range.start - begin); +fn check_link_quotes(cx: &LateContext<'_>, trimmed_text: &str, range: Range, fragments: Fragments<'_>) { + if trimmed_text.starts_with('\'') + && trimmed_text.ends_with('\'') + && let Some(span) = fragments.span(cx, range) + { span_lint( cx, DOC_LINK_WITH_QUOTES, - span.with_lo(lo).with_hi(lo + BytePos::from_usize(text_len)), + span, "possible intra-doc link using quotes instead of backticks", ); } } -fn get_current_span(spans: &[(usize, Span)], idx: usize) -> (usize, Span) { - let index = match spans.binary_search_by(|c| c.0.cmp(&idx)) { - Ok(o) => o, - Err(e) => e - 1, - }; - spans[index] -} - -fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) { +fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, range: Range, fragments: Fragments<'_>) { fn has_needless_main(code: String, edition: Edition) -> bool { rustc_driver::catch_fatal_errors(|| { rustc_span::create_session_globals_then(edition, || { @@ -774,12 +706,13 @@ fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) { .unwrap_or_default() } + let trailing_whitespace = text.len() - text.trim_end().len(); + // 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)) - .join() - .expect("thread::spawn failed") + if thread::spawn(move || has_needless_main(text, edition)).join().expect("thread::spawn failed") + && let Some(span) = fragments.span(cx, range.start..range.end - trailing_whitespace) { span_lint(cx, NEEDLESS_DOCTEST_MAIN, span, "needless `fn main` in doctest"); } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index ab71bfbdc58..20fdf26dc52 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -21,6 +21,7 @@ // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) +extern crate pulldown_cmark; extern crate rustc_arena; extern crate rustc_ast; extern crate rustc_ast_pretty; @@ -37,6 +38,7 @@ extern crate rustc_lexer; extern crate rustc_lint; extern crate rustc_middle; extern crate rustc_parse; +extern crate rustc_resolve; extern crate rustc_session; extern crate rustc_span; extern crate rustc_target; diff --git a/tests/compile-test.rs b/tests/compile-test.rs index e329a94ff6a..c1c3d4a868f 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -130,7 +130,9 @@ fn base_config(test_dir: &str) -> (Config, Args) { }; let mut config = Config { - mode: Mode::Yolo { rustfix: RustfixMode::Everything }, + mode: Mode::Yolo { + rustfix: RustfixMode::Everything, + }, stderr_filters: vec![(Match::PathBackslash, b"/")], stdout_filters: vec![], output_conflict_handling: if bless { diff --git a/tests/ui/doc/doc-fixable.fixed b/tests/ui/doc/doc-fixable.fixed index f7c2f14a482..47b56960a00 100644 --- a/tests/ui/doc/doc-fixable.fixed +++ b/tests/ui/doc/doc-fixable.fixed @@ -198,6 +198,16 @@ fn pulldown_cmark_crash() {} /// [plain text][path::to::item] fn intra_doc_link() {} +/// Ignore escaped\_underscores +/// +/// \\[ +/// \\prod\_{x\\in X} p\_x +/// \\] +fn issue_2581() {} + +/// Foo \[bar\] \[baz\] \[qux\]. `DocMarkdownLint` +fn lint_after_escaped_chars() {} + // issue #7033 - generic_const_exprs ICE struct S where [(); N.checked_next_power_of_two().unwrap()]: { diff --git a/tests/ui/doc/doc-fixable.rs b/tests/ui/doc/doc-fixable.rs index 51961e75b84..4d9a4eafa5f 100644 --- a/tests/ui/doc/doc-fixable.rs +++ b/tests/ui/doc/doc-fixable.rs @@ -198,6 +198,16 @@ fn pulldown_cmark_crash() {} /// [plain text][path::to::item] fn intra_doc_link() {} +/// Ignore escaped\_underscores +/// +/// \\[ +/// \\prod\_{x\\in X} p\_x +/// \\] +fn issue_2581() {} + +/// Foo \[bar\] \[baz\] \[qux\]. DocMarkdownLint +fn lint_after_escaped_chars() {} + // issue #7033 - generic_const_exprs ICE struct S where [(); N.checked_next_power_of_two().unwrap()]: { diff --git a/tests/ui/doc/doc-fixable.stderr b/tests/ui/doc/doc-fixable.stderr index a30ded81385..4c9ff41d918 100644 --- a/tests/ui/doc/doc-fixable.stderr +++ b/tests/ui/doc/doc-fixable.stderr @@ -308,5 +308,16 @@ help: try LL | /// An iterator over `mycrate::Collection`'s values. | ~~~~~~~~~~~~~~~~~~~~~ -error: aborting due to 28 previous errors +error: item in documentation is missing backticks + --> $DIR/doc-fixable.rs:208:34 + | +LL | /// Foo \[bar\] \[baz\] \[qux\]. DocMarkdownLint + | ^^^^^^^^^^^^^^^ + | +help: try + | +LL | /// Foo \[bar\] \[baz\] \[qux\]. `DocMarkdownLint` + | ~~~~~~~~~~~~~~~~~ + +error: aborting due to 29 previous errors diff --git a/tests/ui/doc/unbalanced_ticks.rs b/tests/ui/doc/unbalanced_ticks.rs index 4a1711f79a0..6f7bab72040 100644 --- a/tests/ui/doc/unbalanced_ticks.rs +++ b/tests/ui/doc/unbalanced_ticks.rs @@ -48,4 +48,4 @@ fn other_markdown() {} /// /// `lol` /// pub struct Struct; /// ``` -fn iss_7421() {} +fn issue_7421() {} diff --git a/tests/ui/doc/unbalanced_ticks.stderr b/tests/ui/doc/unbalanced_ticks.stderr index 92b6f8536c8..89ad8db3916 100644 --- a/tests/ui/doc/unbalanced_ticks.stderr +++ b/tests/ui/doc/unbalanced_ticks.stderr @@ -1,7 +1,8 @@ error: backticks are unbalanced - --> $DIR/unbalanced_ticks.rs:7:1 + --> $DIR/unbalanced_ticks.rs:7:5 | -LL | / /// This is a doc comment with `unbalanced_tick marks and several words that +LL | /// This is a doc comment with `unbalanced_tick marks and several words that + | _____^ LL | | LL | | /// should be `encompassed_by` tick marks because they `contain_underscores`. LL | | /// Because of the initial `unbalanced_tick` pair, the error message is @@ -13,10 +14,10 @@ LL | | /// very `confusing_and_misleading`. = help: to override `-D warnings` add `#[allow(clippy::doc_markdown)]` error: backticks are unbalanced - --> $DIR/unbalanced_ticks.rs:14:1 + --> $DIR/unbalanced_ticks.rs:14:5 | LL | /// This paragraph has `unbalanced_tick marks and should stop_linting. - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: a backtick may be missing a pair @@ -32,10 +33,10 @@ LL | /// This paragraph is fine and `should_be` linted normally. | ~~~~~~~~~~~ error: backticks are unbalanced - --> $DIR/unbalanced_ticks.rs:20:1 + --> $DIR/unbalanced_ticks.rs:20:5 | LL | /// Double unbalanced backtick from ``here to here` should lint. - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: a backtick may be missing a pair @@ -51,18 +52,18 @@ LL | /// ## `not_fine` | ~~~~~~~~~~ error: backticks are unbalanced - --> $DIR/unbalanced_ticks.rs:37:1 + --> $DIR/unbalanced_ticks.rs:37:5 | LL | /// ### `unbalanced - | ^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ | = help: a backtick may be missing a pair error: backticks are unbalanced - --> $DIR/unbalanced_ticks.rs:40:1 + --> $DIR/unbalanced_ticks.rs:40:5 | LL | /// - This `item has unbalanced tick marks - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: a backtick may be missing a pair diff --git a/tests/ui/doc_errors.rs b/tests/ui/doc_errors.rs index 86721f61d19..d661231c59e 100644 --- a/tests/ui/doc_errors.rs +++ b/tests/ui/doc_errors.rs @@ -85,6 +85,22 @@ impl Struct1 { async fn async_priv_method_missing_errors_header() -> Result<(), ()> { unimplemented!(); } + + /** + # Errors + A description of the errors goes here. + */ + fn block_comment() -> Result<(), ()> { + unimplemented!(); + } + + /** + * # Errors + * A description of the errors goes here. + */ + fn block_comment_leading_asterisks() -> Result<(), ()> { + unimplemented!(); + } } pub trait Trait1 { diff --git a/tests/ui/doc_errors.stderr b/tests/ui/doc_errors.stderr index 9cea864f1d8..28b2db2440b 100644 --- a/tests/ui/doc_errors.stderr +++ b/tests/ui/doc_errors.stderr @@ -38,7 +38,7 @@ LL | pub async fn async_pub_method_missing_errors_header() -> Result<(), ()> | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:92:5 + --> $DIR/doc_errors.rs:108:5 | LL | fn trait_method_missing_errors_header() -> Result<(), ()>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/needless_doc_main.rs b/tests/ui/needless_doc_main.rs index d31b9047eaa..fee05926ce4 100644 --- a/tests/ui/needless_doc_main.rs +++ b/tests/ui/needless_doc_main.rs @@ -10,7 +10,7 @@ /// unimplemented!(); /// } /// ``` -/// +/// /// With an explicit return type it should lint too /// ```edition2015 /// fn main() -> () { @@ -18,7 +18,7 @@ /// unimplemented!(); /// } /// ``` -/// +/// /// This should, too. /// ```rust /// fn main() { @@ -26,11 +26,12 @@ /// unimplemented!(); /// } /// ``` -/// +/// /// This one too. /// ```no_run -/// fn main() { +/// // the fn is not always the first line //~^ ERROR: needless `fn main` in doctest +/// fn main() { /// unimplemented!(); /// } /// ``` diff --git a/tests/ui/needless_doc_main.stderr b/tests/ui/needless_doc_main.stderr index 8dd93bb05dd..84256548671 100644 --- a/tests/ui/needless_doc_main.stderr +++ b/tests/ui/needless_doc_main.stderr @@ -1,29 +1,47 @@ error: needless `fn main` in doctest - --> $DIR/needless_doc_main.rs:7:4 + --> $DIR/needless_doc_main.rs:7:5 | -LL | /// fn main() { - | ^^^^^^^^^^^^ +LL | /// fn main() { + | _____^ +LL | | +LL | | +LL | | /// unimplemented!(); +LL | | /// } + | |_____^ | = note: `-D clippy::needless-doctest-main` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::needless_doctest_main)]` error: needless `fn main` in doctest - --> $DIR/needless_doc_main.rs:16:4 + --> $DIR/needless_doc_main.rs:16:5 | -LL | /// fn main() -> () { - | ^^^^^^^^^^^^^^^^^^ +LL | /// fn main() -> () { + | _____^ +LL | | +LL | | /// unimplemented!(); +LL | | /// } + | |_____^ error: needless `fn main` in doctest - --> $DIR/needless_doc_main.rs:24:4 + --> $DIR/needless_doc_main.rs:24:5 | -LL | /// fn main() { - | ^^^^^^^^^^^^ +LL | /// fn main() { + | _____^ +LL | | +LL | | /// unimplemented!(); +LL | | /// } + | |_____^ error: needless `fn main` in doctest - --> $DIR/needless_doc_main.rs:32:4 + --> $DIR/needless_doc_main.rs:32:5 | -LL | /// fn main() { - | ^^^^^^^^^^^^ +LL | /// // the fn is not always the first line + | _____^ +LL | | +LL | | /// fn main() { +LL | | /// unimplemented!(); +LL | | /// } + | |_____^ error: aborting due to 4 previous errors