WiP: comrak sourcepos (#80)

eipw-lint/src/lib.rs

@@ -1002,6 +1002,12 @@ fn process<'a>(
         } else {
             data.sourcepos.start.line += preamble_lines;
         }
+
+        if data.sourcepos.end.line == 0 {
+            data.sourcepos.end.line = data.sourcepos.start.line;
+        } else {
+            data.sourcepos.end.line += preamble_lines;
+        }
     }
 
     Ok(Some(InnerContext {

eipw-lint/src/lints.rs

@@ -8,11 +8,12 @@ mod known_lints;
 pub mod markdown;
 pub mod preamble;
 
-use eipw_snippets::{Level, Message};
+use eipw_snippets::{Level, Message, Snippet};
 
-use comrak::nodes::AstNode;
+use comrak::nodes::{Ast, AstNode, LineColumn};
 
 use crate::reporters::{self, Reporter};
+use crate::{LevelExt, SnippetExt};
 
 use educe::Educe;
 
@@ -23,7 +24,6 @@ pub use self::known_lints::DefaultLint;
 use snafu::Snafu;
 
 use std::cell::RefCell;
-use std::cmp::max;
 use std::collections::{HashMap, HashSet};
 use std::fmt::Debug;
 use std::ops::Deref;
@@ -90,29 +90,69 @@ where
         &self.inner.preamble
     }
 
-    /// XXX: comrak doesn't include a source field with its `AstNode`, so use
-    ///      this instead. Don't expose it publicly since it's really hacky.
-    ///      Yes, lines start at one.
-    pub(crate) fn line(&self, mut line: usize) -> &'a str {
-        assert_ne!(line, 0);
-        line -= 1;
-        self.inner.source.split('\n').nth(line).unwrap()
+    pub fn line_index(&self, line: usize) -> usize {
+        let src = self.inner.source;
+        let (idx, _) = src
+            .bytes()
+            .enumerate()
+            .filter(|(_, chr)| *chr == b'\n')
+            .take(line - 1)
+            .last()
+            .expect("could not find ast line in source");
+        assert_eq!(src.as_bytes().get(idx), Some(&b'\n'));
+        idx + 1
     }
 
-    /// XXX: comrak doesn't include a source field with its `AstNode`, so use
-    ///      this instead. Don't expose it publicly since it's really hacky.
-    pub(crate) fn source_for_text(&self, line: usize, text: &str) -> String {
-        assert_ne!(line, 0);
+    fn line_column_index(&self, line_column: LineColumn) -> usize {
+        let line_index = self.line_index(line_column.line);
+        line_index + line_column.column - 1
+    }
+
+    pub fn ast_source(&self, ast: &Ast) -> &'a str {
+        let start = self.line_column_index(ast.sourcepos.start);
+        let end = self.line_column_index(ast.sourcepos.end);
+        &self.inner.source[start..=end]
+    }
 
-        let newlines = max(1, text.chars().filter(|c| *c == '\n').count());
+    pub fn ast_lines(&self, ast: &Ast) -> &'a str {
+        let line_start_index = self.line_index(ast.sourcepos.start.line);
+        let line_end_index = self.line_index(ast.sourcepos.end.line);
+        let line_end_index = self.inner.source[line_end_index..]
+            .find('\n')
+            .map(|idx| idx + line_end_index)
+            .unwrap_or_else(|| self.inner.source.len());
 
-        self.inner
-            .source
-            .split('\n')
-            .skip(line - 1)
-            .take(newlines)
-            .collect::<Vec<_>>()
-            .join("\n")
+        &self.inner.source[line_start_index..line_end_index]
+    }
+
+    pub fn ast_snippet<'l, L: Into<Option<Level>>, O: Into<Option<&'l str>>>(
+        &self,
+        ast: &Ast,
+        level: L,
+        label: O,
+    ) -> Snippet<'l>
+    where
+        'a: 'l,
+    {
+        let line_start_index = self.line_index(ast.sourcepos.start.line);
+        let level = level.into().unwrap_or(self.annotation_level());
+
+        let start_index = self.line_column_index(ast.sourcepos.start) - line_start_index;
+        let end_index = self.line_column_index(ast.sourcepos.end) - line_start_index;
+
+        let source = self.ast_lines(ast);
+        let annotation = level.span_utf8(source, start_index, end_index + 1);
+
+        let annotation = match label.into() {
+            None => annotation,
+            Some(label) => annotation.label(label.as_ref()),
+        };
+
+        Snippet::source(source)
+            .fold(true)
+            .line_start(ast.sourcepos.start.line)
+            .origin_opt(self.origin())
+            .annotation(annotation)
     }
 
     pub fn body_source(&self) -> &'a str {
@@ -192,3 +232,190 @@ impl Lint for Box<dyn Lint> {
         lint.lint(slug, ctx)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use comrak::{
+        arena_tree::{Node, NodeEdge},
+        nodes::NodeValue,
+    };
+
+    use super::*;
+
+    fn get_context_ast_source(source: &str, pred: impl FnMut(&Ast) -> bool) -> String {
+        let arena = comrak::Arena::new();
+        let context = Context {
+            annotation_level: Level::Error,
+            eips: &Default::default(),
+            reporter: &crate::reporters::Null,
+            inner: crate::process(&crate::reporters::Null, &arena, Some("eip-1234.md"), source)
+                .unwrap()
+                .unwrap(),
+        };
+
+        let link = context
+            .body()
+            .traverse()
+            .filter_map(|x| match x {
+                NodeEdge::Start(Node { data, .. }) => Some(data.borrow().clone()),
+                _ => None,
+            })
+            .filter(pred)
+            .next()
+            .unwrap();
+
+        context.ast_source(&link).to_owned()
+    }
+
+    #[test]
+    #[ignore] // https://github.com/kivikakk/comrak/issues/478
+    fn context_ast_source_autolink_email() {
+        let source = r#"
+---
+eip: 1234
+---
+
+[email protected] hello world
+"#
+        .trim();
+
+        let actual = get_context_ast_source(source, |d| matches!(d.value, NodeValue::Link(_)));
+        assert_eq!(actual, "[email protected]");
+    }
+
+    #[test]
+    #[ignore] // https://github.com/kivikakk/comrak/issues/478
+    fn context_ast_source_link_start() {
+        let source = r#"
+---
+eip: 1234
+---
+
+<https://example.com> hello world
+"#
+        .trim();
+
+        let actual = get_context_ast_source(source, |d| matches!(d.value, NodeValue::Link(_)));
+        assert_eq!(actual, "<https://example.com>");
+    }
+
+    #[test]
+    fn context_ast_source_inline_link_start() {
+        let source = r#"
+---
+eip: 1234
+---
+
+[hello](https://example.com) hello world
+"#
+        .trim();
+
+        let actual = get_context_ast_source(source, |d| matches!(d.value, NodeValue::Link(_)));
+        assert_eq!(actual, "[hello](https://example.com)");
+    }
+
+    #[test]
+    fn context_ast_source_emphasis_unicode() {
+        let source = r#"
+---
+eip: 1234
+---
+
+*áemphá* hello world
+"#
+        .trim();
+
+        let actual = get_context_ast_source(source, |d| matches!(d.value, NodeValue::Emph));
+        assert_eq!(actual, "*áemphá*");
+    }
+
+    #[test]
+    fn context_ast_source_emphasis_start() {
+        let source = r#"
+---
+eip: 1234
+---
+
+*emphasis* hello world
+"#
+        .trim();
+
+        let actual = get_context_ast_source(source, |d| matches!(d.value, NodeValue::Emph));
+        assert_eq!(actual, "*emphasis*");
+    }
+
+    #[test]
+    #[ignore] // https://github.com/kivikakk/comrak/issues/478
+    fn context_ast_source_link_mid() {
+        let source = r#"
+---
+eip: 1234
+---
+
+hello <https://example.com> world
+"#
+        .trim();
+
+        let actual = get_context_ast_source(source, |d| matches!(d.value, NodeValue::Link(_)));
+        assert_eq!(actual, "<https://example.com>");
+    }
+
+    #[test]
+    fn context_ast_source_inline_link_mid() {
+        let source = r#"
+---
+eip: 1234
+---
+
+hello [hello](https://example.com) world
+"#
+        .trim();
+
+        let actual = get_context_ast_source(source, |d| matches!(d.value, NodeValue::Link(_)));
+        assert_eq!(actual, "[hello](https://example.com)");
+    }
+
+    #[test]
+    fn context_ast_source_emphasis_mid() {
+        let source = r#"
+---
+eip: 1234
+---
+
+hello *emphasis* world
+"#
+        .trim();
+
+        let actual = get_context_ast_source(source, |d| matches!(d.value, NodeValue::Emph));
+        assert_eq!(actual, "*emphasis*");
+    }
+
+    #[test]
+    fn context_ast_source_code_block() {
+        let source = r#"
+---
+eip: 1234
+---
+
+hello
+
+```
+this is a
+multiline
+code block
+```
+
+world
+"#
+        .trim();
+
+        let expected = r#"```
+this is a
+multiline
+code block
+```"#;
+
+        let actual = get_context_ast_source(source, |d| matches!(d.value, NodeValue::CodeBlock(_)));
+        assert_eq!(actual, expected);
+    }
+}

eipw-lint/src/lints/markdown/html_comments.rs

@@ -4,14 +4,11 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-use eipw_snippets::{Level, Snippet};
+use eipw_snippets::Level;
 
 use comrak::nodes::NodeValue;
 
-use crate::{
-    lints::{Context, Error, Lint},
-    SnippetExt,
-};
+use crate::lints::{Context, Error, Lint};
 
 use scraper::node::Node as HtmlNode;
 use scraper::Html;
@@ -59,12 +56,7 @@ where
                     continue;
                 }
 
-                slices.push(
-                    Snippet::source(ctx.line(data.sourcepos.start.line))
-                        .line_start(data.sourcepos.start.line)
-                        .fold(false)
-                        .origin_opt(ctx.origin()),
-                );
+                slices.push(ctx.ast_snippet(&data, annotation_type, None));
             }
         }
 

eipw-lint/src/lints/markdown/json_schema.rs

@@ -88,28 +88,16 @@ impl<'a, 'b, 'c> tree::Visitor for Visitor<'a, 'b, 'c> {
             Ok(v) => v,
             Err(e) => {
                 let label = format!("code block of type `{}` does not contain valid JSON", info);
-                let source = self
-                    .ctx
-                    .source_for_text(ast.sourcepos.start.line, &node.literal);
                 let slice_label = e.to_string();
                 self.ctx.report(
                     self.ctx
                         .annotation_level()
                         .title(&label)
                         .id(self.slug)
                         .snippet(
-                            Snippet::source(&source)
-                                .origin_opt(self.ctx.origin())
-                                // TODO: The serde_json error actually has line/column
-                                //       information. Use it.
-                                .line_start(ast.sourcepos.start.line)
-                                .fold(false)
-                                .annotation(
-                                    self.ctx
-                                        .annotation_level()
-                                        .span(0..source.len())
-                                        .label(&slice_label),
-                                ),
+                            // TODO: The serde_json error actually has line/column
+                            //       information. Use it.
+                            self.ctx.ast_snippet(ast, None, slice_label.as_str()),
                         ),
                 )?;
                 return Ok(Next::SkipChildren);
@@ -125,9 +113,7 @@ impl<'a, 'b, 'c> tree::Visitor for Visitor<'a, 'b, 'c> {
             .into_iter()
             .map(|d| d.error_description().to_string())
             .collect();
-        let source = self
-            .ctx
-            .source_for_text(ast.sourcepos.start.line, &node.literal);
+        let source = self.ctx.ast_lines(ast);
         let annotations = labels
             .iter()
             .map(|l| self.ctx.annotation_level().span(0..source.len()).label(l));
@@ -142,7 +128,7 @@ impl<'a, 'b, 'c> tree::Visitor for Visitor<'a, 'b, 'c> {
                 .title(&label)
                 .id(self.slug)
                 .snippet(
-                    Snippet::source(&source)
+                    Snippet::source(source)
                         .fold(false)
                         .line_start(ast.sourcepos.start.line)
                         .origin_opt(self.ctx.origin())