Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check for unbalanced tick pairs in doc-markdown lint #7357

Merged
merged 1 commit into from
Jun 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 50 additions & 14 deletions clippy_lints/src/doc.rs
Original file line number Diff line number Diff line change
@@ -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::{is_entrypoint_fn, is_expn_of, match_panic_def_id, method_chain_args, return_ty};
use if_chain::if_chain;
Expand Down Expand Up @@ -37,7 +38,8 @@ declare_clippy_lint! {
/// consider that.
///
/// **Known problems:** Lots of bad docs won’t 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
/// inside a link text would trip the parser. Therfore, documenting link with
Expand Down Expand Up @@ -469,11 +471,11 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
spans: &[(usize, Span)],
) -> DocHeaders {
// true if a safety header was found
use pulldown_cmark::CodeBlockKind;
use pulldown_cmark::Event::{
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 {
safety: false,
Expand All @@ -485,6 +487,9 @@ 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 edition = None;
let mut ticks_unbalanced = false;
ebobrow marked this conversation as resolved.
Show resolved Hide resolved
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 {
match event {
Start(CodeBlock(ref kind)) => {
Expand All @@ -510,13 +515,42 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
},
Start(Link(_, url, _)) => in_link = Some(url),
End(Link(..)) => in_link = None,
Start(Heading(_)) => in_heading = true,
End(Heading(_)) => in_heading = false,
Start(Heading(_) | Paragraph | Item) => {
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
Html(_html) => (), // HTML is weird, just ignore it
SoftBreak | HardBreak | TaskListMarker(_) | Code(_) | Rule => (),
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>`
// Which are represented as a link to "http://example.com" with
// text "http://example.com" by pulldown-cmark
Expand All @@ -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.errors |= in_heading && text.trim() == "Errors";
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 is_rust {
let edition = edition.unwrap_or_else(|| cx.tcx.sess.edition());
Expand All @@ -538,15 +567,22 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
} else {
// Adjust for the beginning of the current `Event`
let span = span.with_lo(span.lo() + BytePos::from_usize(range.start - begin));

check_text(cx, valid_idents, &text, span);
text_to_check.push((text, span));
}
},
}
}
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 has_needless_main(code: &str, edition: Edition) -> bool {
rustc_driver::catch_fatal_errors(|| {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/if_let_mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
mutex_lock_called: bool,
found_mutex: Option<&'tcx Expr<'tcx>>,
Expand Down
File renamed without changes.
File renamed without changes.
36 changes: 36 additions & 0 deletions tests/ui/doc/unbalanced_ticks.rs
Original file line number Diff line number Diff line change
@@ -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() {}
64 changes: 64 additions & 0 deletions tests/ui/doc/unbalanced_ticks.stderr
Original file line number Diff line number Diff line change
@@ -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