Simplify broken link detection without state machine usage

Author: maxclaus

clippy_lints/src/doc/broken_link.rs

@@ -23,79 +23,53 @@ enum BrokenLinkReason {
     MultipleLines,
 }
 
-#[derive(Debug)]
-enum State {
-    ProcessingLinkText,
-    ProcessedLinkText,
-    ProcessingLinkUrl(UrlState),
-}
-
-#[derive(Debug)]
-enum UrlState {
-    Empty,
-    FilledEntireSingleLine,
-    FilledBrokenMultipleLines,
-}
-
 fn warn_if_broken_link(cx: &LateContext<'_>, bl: &PullDownBrokenLink<'_>, doc: &String, fragments: &Vec<DocFragment>) {
     if let Some(span) = source_span_for_markdown_range(cx.tcx, doc, &bl.span, fragments) {
-        // `PullDownBrokenLink` provided by pulldown_cmark always
-        // start with `[` which makes pulldown_cmark consider this a link tag.
-        let mut state = State::ProcessingLinkText;
+        let mut len = 0;
 
-        // Whether it was detected a line break within the link tag url part.
-        let mut reading_link_url_new_line = false;
+        // grab raw link data
+        let (_, raw_link) = doc.split_at(bl.span.start);
 
-        // Skip the first char because we already know it is a `[` char.
-        for (abs_pos, c) in doc.char_indices().skip(bl.span.start + 1) {
-            match &state {
-                State::ProcessingLinkText => {
-                    if c == ']' {
-                        state = State::ProcessedLinkText;
-                    }
-                },
-                State::ProcessedLinkText => {
-                    if c == '(' {
-                        state = State::ProcessingLinkUrl(UrlState::Empty);
-                    } else {
-                        // not a real link, just skip it without reporting a broken link for it.
-                        break;
-                    }
-                },
-                State::ProcessingLinkUrl(url_state) => {
-                    if c == '\n' {
-                        reading_link_url_new_line = true;
-                        continue;
-                    }
+        // strip off link text part
+        let raw_link = match raw_link.split_once(']') {
+            None => return,
+            Some((prefix, suffix)) => {
+                len += prefix.len() + 1;
+                suffix
+            },
+        };
 
-                    if c == ')' {
-                        // record full broken link tag
-                        if let UrlState::FilledBrokenMultipleLines = url_state {
-                            let offset = abs_pos - bl.span.start;
-                            report_broken_link(cx, span, offset, BrokenLinkReason::MultipleLines);
-                        }
-                        break;
-                    }
+        let raw_link = match raw_link.split_once('(') {
+            None => return,
+            Some((prefix, suffix)) => {
+                if !prefix.is_empty() {
+                    // there is text between ']' and '(' chars, so it is not a valid link
+                    return;
+                }
+                len += prefix.len() + 1;
+                suffix
+            },
+        };
 
-                    if !c.is_whitespace() {
-                        if reading_link_url_new_line {
-                            // It was reading a link url which was entirely in a single line, but a new char
-                            // was found in this new line which turned the url into a broken state.
-                            state = State::ProcessingLinkUrl(UrlState::FilledBrokenMultipleLines);
-                            continue;
-                        }
+        for c in raw_link.chars() {
+            if c == ')' {
+                // it is a valid link
+                return;
+            }
 
-                        state = State::ProcessingLinkUrl(UrlState::FilledEntireSingleLine);
-                    }
-                },
-            };
+            if c == '\n' {
+                // detected break line within the url part
+                report_broken_link(cx, span, len, BrokenLinkReason::MultipleLines);
+                break;
+            }
+            len += 1;
         }
     }
 }
 
 fn report_broken_link(cx: &LateContext<'_>, frag_span: Span, offset: usize, reason: BrokenLinkReason) {
     let start = frag_span.lo();
-    let end = start + BytePos::from_usize(offset + 5);
+    let end = start + BytePos::from_usize(offset);
 
     let span = Span::new(start, end, frag_span.ctxt(), frag_span.parent());
 

tests/ui/doc_broken_link.rs

@@ -4,6 +4,18 @@ fn main() {}
 
 pub struct FakeType {}
 
+/// This might be considered a link false positive
+/// and should be ignored by this lint rule:
+/// Example of referencing some code with brackets [FakeType].
+pub fn doc_ignore_link_false_positive_1() {}
+
+/// This might be considered a link false positive
+/// and should be ignored by this lint rule:
+/// [`FakeType`]. Continue text after brackets,
+/// then (something in
+/// parenthesis).
+pub fn doc_ignore_link_false_positive_2() {}
+
 /// Test valid link, whole link single line.
 /// [doc valid link](https://test.fake/doc_valid_link)
 pub fn doc_valid_link() {}
@@ -58,15 +70,3 @@ pub fn doc_invalid_link_broken_url_host_part() {}
 //~^^ ERROR: possible broken doc link: broken across multiple lines
 /// line of comment.
 pub fn doc_multiple_invalid_link_broken_url() {}
-
-/// This might be considered a link false positive
-/// and should be ignored by this lint rule:
-/// Example of referencing some code with brackets [FakeType].
-pub fn doc_ignore_link_false_positive_1() {}
-
-/// This might be considered a link false positive
-/// and should be ignored by this lint rule:
-/// [`FakeType`]. Continue text after brackets,
-/// then (something in
-/// parenthesis).
-pub fn doc_ignore_link_false_positive_2() {}

tests/ui/doc_broken_link.stderr

@@ -1,37 +1,29 @@
 error: possible broken doc link: broken across multiple lines
-  --> tests/ui/doc_broken_link.rs:41:5
+  --> tests/ui/doc_broken_link.rs:53:5
    |
-LL |   /// [doc invalid link broken url scheme part part](https://
-   |  _____^
-LL | | /// test.fake/doc_invalid_link_broken_url_scheme_part)
-   | |______________________________________________________^
+LL | /// [doc invalid link broken url scheme part part](https://
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::doc-broken-link` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::doc_broken_link)]`
 
 error: possible broken doc link: broken across multiple lines
-  --> tests/ui/doc_broken_link.rs:47:5
+  --> tests/ui/doc_broken_link.rs:59:5
    |
-LL |   /// [doc invalid link broken url host part](https://test
-   |  _____^
-LL | | /// .fake/doc_invalid_link_broken_url_host_part)
-   | |________________________________________________^
+LL | /// [doc invalid link broken url host part](https://test
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: possible broken doc link: broken across multiple lines
-  --> tests/ui/doc_broken_link.rs:53:16
+  --> tests/ui/doc_broken_link.rs:65:16
    |
-LL |   /// There is a [fist link - invalid](https://test
-   |  ________________^
-LL | | /// .fake) then it continues
-   | |__________^
+LL | /// There is a [fist link - invalid](https://test
+   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: possible broken doc link: broken across multiple lines
-  --> tests/ui/doc_broken_link.rs:56:80
+  --> tests/ui/doc_broken_link.rs:68:80
    |
-LL |   /// with a [second link - valid](https://test.fake/doc_valid_link) and another [third link - invalid](https://test
-   |  ________________________________________________________________________________^
-LL | | /// .fake). It ends with another
-   | |__________^
+LL | /// with a [second link - valid](https://test.fake/doc_valid_link) and another [third link - invalid](https://test
+   |                                                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 4 previous errors