Auto merge of #7357 - ebobrow:unbalanced-tick, r=xFrednet,flip1995

check for unbalanced tick pairs in doc-markdown lint

fixes #6753

changelog: check for unbalanced tick pairs in doc-markdown lint
This commit is contained in:
bors 2021-06-21 17:15:12 +00:00
commit 48fa1dc8bb
6 changed files with 151 additions and 15 deletions

View file

@ -1,4 +1,5 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_note}; use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_note};
use clippy_utils::source::first_line_of_span;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
use clippy_utils::{is_entrypoint_fn, is_expn_of, match_panic_def_id, method_chain_args, return_ty}; use clippy_utils::{is_entrypoint_fn, is_expn_of, match_panic_def_id, method_chain_args, return_ty};
use if_chain::if_chain; use if_chain::if_chain;
@ -37,7 +38,8 @@ declare_clippy_lint! {
/// consider that. /// consider that.
/// ///
/// **Known problems:** Lots of bad docs wont be fixed, what the lint checks /// **Known problems:** Lots of bad docs wont be fixed, what the lint checks
/// for is limited, and there are still false positives. /// for is limited, and there are still false positives. HTML elements and their
/// content are not linted.
/// ///
/// In addition, when writing documentation comments, including `[]` brackets /// In addition, when writing documentation comments, including `[]` brackets
/// inside a link text would trip the parser. Therfore, documenting link with /// inside a link text would trip the parser. Therfore, documenting link with
@ -469,11 +471,11 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
spans: &[(usize, Span)], spans: &[(usize, Span)],
) -> DocHeaders { ) -> DocHeaders {
// true if a safety header was found // true if a safety header was found
use pulldown_cmark::CodeBlockKind;
use pulldown_cmark::Event::{ use pulldown_cmark::Event::{
Code, End, FootnoteReference, HardBreak, Html, Rule, SoftBreak, Start, TaskListMarker, Text, Code, End, FootnoteReference, HardBreak, Html, Rule, SoftBreak, Start, TaskListMarker, Text,
}; };
use pulldown_cmark::Tag::{CodeBlock, Heading, Link}; use pulldown_cmark::Tag::{CodeBlock, Heading, Item, Link, Paragraph};
use pulldown_cmark::{CodeBlockKind, CowStr};
let mut headers = DocHeaders { let mut headers = DocHeaders {
safety: false, safety: false,
@ -485,6 +487,9 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
let mut in_heading = false; let mut in_heading = false;
let mut is_rust = false; let mut is_rust = false;
let mut edition = None; let mut edition = None;
let mut ticks_unbalanced = false;
let mut text_to_check: Vec<(CowStr<'_>, Span)> = Vec::new();
let mut paragraph_span = spans.get(0).expect("function isn't called if doc comment is empty").1;
for (event, range) in events { for (event, range) in events {
match event { match event {
Start(CodeBlock(ref kind)) => { Start(CodeBlock(ref kind)) => {
@ -510,13 +515,42 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
}, },
Start(Link(_, url, _)) => in_link = Some(url), Start(Link(_, url, _)) => in_link = Some(url),
End(Link(..)) => in_link = None, End(Link(..)) => in_link = None,
Start(Heading(_)) => in_heading = true, Start(Heading(_) | Paragraph | Item) => {
End(Heading(_)) => in_heading = false, if let Start(Heading(_)) = event {
in_heading = true;
}
ticks_unbalanced = false;
let (_, span) = get_current_span(spans, range.start);
paragraph_span = first_line_of_span(cx, span);
},
End(Heading(_) | Paragraph | Item) => {
if let End(Heading(_)) = event {
in_heading = false;
}
if ticks_unbalanced {
span_lint_and_help(
cx,
DOC_MARKDOWN,
paragraph_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);
}
}
text_to_check = Vec::new();
},
Start(_tag) | End(_tag) => (), // We don't care about other tags Start(_tag) | End(_tag) => (), // We don't care about other tags
Html(_html) => (), // HTML is weird, just ignore it Html(_html) => (), // HTML is weird, just ignore it
SoftBreak | HardBreak | TaskListMarker(_) | Code(_) | Rule => (), SoftBreak | HardBreak | TaskListMarker(_) | Code(_) | Rule => (),
FootnoteReference(text) | Text(text) => { FootnoteReference(text) | Text(text) => {
if Some(&text) == in_link.as_ref() { let (begin, span) = get_current_span(spans, range.start);
paragraph_span = paragraph_span.with_hi(span.hi());
ticks_unbalanced |= text.contains('`');
if Some(&text) == in_link.as_ref() || ticks_unbalanced {
// Probably a link of the form `<http://example.com>` // Probably a link of the form `<http://example.com>`
// Which are represented as a link to "http://example.com" with // Which are represented as a link to "http://example.com" with
// text "http://example.com" by pulldown-cmark // text "http://example.com" by pulldown-cmark
@ -525,11 +559,6 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
headers.safety |= in_heading && text.trim() == "Safety"; headers.safety |= in_heading && text.trim() == "Safety";
headers.errors |= in_heading && text.trim() == "Errors"; headers.errors |= in_heading && text.trim() == "Errors";
headers.panics |= in_heading && text.trim() == "Panics"; headers.panics |= in_heading && text.trim() == "Panics";
let index = match spans.binary_search_by(|c| c.0.cmp(&range.start)) {
Ok(o) => o,
Err(e) => e - 1,
};
let (begin, span) = spans[index];
if in_code { if in_code {
if is_rust { if is_rust {
let edition = edition.unwrap_or_else(|| cx.tcx.sess.edition()); let edition = edition.unwrap_or_else(|| cx.tcx.sess.edition());
@ -538,8 +567,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
} else { } else {
// Adjust for the beginning of the current `Event` // Adjust for the beginning of the current `Event`
let span = span.with_lo(span.lo() + BytePos::from_usize(range.start - begin)); let span = span.with_lo(span.lo() + BytePos::from_usize(range.start - begin));
text_to_check.push((text, span));
check_text(cx, valid_idents, &text, span);
} }
}, },
} }
@ -547,6 +575,14 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
headers headers
} }
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, span: Span) {
fn has_needless_main(code: &str, edition: Edition) -> bool { fn has_needless_main(code: &str, edition: Edition) -> bool {
rustc_driver::catch_fatal_errors(|| { rustc_driver::catch_fatal_errors(|| {

View file

@ -83,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
} }
} }
/// Checks if `Mutex::lock` is called in the `if let _ = expr. /// Checks if `Mutex::lock` is called in the `if let` expr.
pub struct OppVisitor<'a, 'tcx> { pub struct OppVisitor<'a, 'tcx> {
mutex_lock_called: bool, mutex_lock_called: bool,
found_mutex: Option<&'tcx Expr<'tcx>>, found_mutex: Option<&'tcx Expr<'tcx>>,

View file

@ -0,0 +1,36 @@
//! This file tests for the `DOC_MARKDOWN` lint, specifically cases
//! where ticks are unbalanced (see issue #6753).
#![allow(dead_code)]
#![warn(clippy::doc_markdown)]
/// This is a doc comment with `unbalanced_tick marks and several words that
/// should be `encompassed_by` tick marks because they `contain_underscores`.
/// Because of the initial `unbalanced_tick` pair, the error message is
/// very `confusing_and_misleading`.
fn main() {}
/// This paragraph has `unbalanced_tick marks and should stop_linting.
///
/// This paragraph is fine and should_be linted normally.
///
/// Double unbalanced backtick from ``here to here` should lint.
///
/// Double balanced back ticks ``start end`` is fine.
fn multiple_paragraphs() {}
/// ```
/// // Unbalanced tick mark in code block shouldn't warn:
/// `
/// ```
fn in_code_block() {}
/// # `Fine`
///
/// ## not_fine
///
/// ### `unbalanced
///
/// - This `item has unbalanced tick marks
/// - This item needs backticks_here
fn other_markdown() {}

View file

@ -0,0 +1,64 @@
error: backticks are unbalanced
--> $DIR/unbalanced_ticks.rs:7:1
|
LL | / /// This is a doc comment with `unbalanced_tick marks and several words that
LL | | /// should be `encompassed_by` tick marks because they `contain_underscores`.
LL | | /// Because of the initial `unbalanced_tick` pair, the error message is
LL | | /// very `confusing_and_misleading`.
| |____________________________________^
|
= note: `-D clippy::doc-markdown` implied by `-D warnings`
= help: a backtick may be missing a pair
error: backticks are unbalanced
--> $DIR/unbalanced_ticks.rs:13:1
|
LL | /// This paragraph has `unbalanced_tick marks and should stop_linting.
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: a backtick may be missing a pair
error: you should put `should_be` between ticks in the documentation
--> $DIR/unbalanced_ticks.rs:15:32
|
LL | /// This paragraph is fine and should_be linted normally.
| ^^^^^^^^^
error: backticks are unbalanced
--> $DIR/unbalanced_ticks.rs:17:1
|
LL | /// Double unbalanced backtick from ``here to here` should lint.
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: a backtick may be missing a pair
error: you should put `not_fine` between ticks in the documentation
--> $DIR/unbalanced_ticks.rs:30:8
|
LL | /// ## not_fine
| ^^^^^^^^
error: backticks are unbalanced
--> $DIR/unbalanced_ticks.rs:32:1
|
LL | /// ### `unbalanced
| ^^^^^^^^^^^^^^^^^^^
|
= help: a backtick may be missing a pair
error: backticks are unbalanced
--> $DIR/unbalanced_ticks.rs:34:1
|
LL | /// - This `item has unbalanced tick marks
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: a backtick may be missing a pair
error: you should put `backticks_here` between ticks in the documentation
--> $DIR/unbalanced_ticks.rs:35:23
|
LL | /// - This item needs backticks_here
| ^^^^^^^^^^^^^^
error: aborting due to 8 previous errors