Define Warning type and start annotating possible parse errors (#131)

Cargo.toml

@@ -12,6 +12,7 @@ rust-version = "1.74.0"
 [dependencies]
 bytecount = "0.6.7"
 memchr = "2.6.4"
+thiserror = "1.0.63"
 
 [dev-dependencies]
 codspeed-criterion-compat = "2.3.3"

src/attributes/attrlist.rs

@@ -1,6 +1,11 @@
 use std::{ops::Deref, slice::Iter};
 
-use crate::{attributes::ElementAttribute, span::MatchedItem, HasSpan, Span};
+use crate::{
+    attributes::ElementAttribute,
+    span::MatchedItem,
+    warnings::{MatchAndWarnings, Warning, WarningType},
+    HasSpan, Span,
+};
 
 /// The source text that’s used to define attributes for an element is referred
 /// to as an attrlist. An attrlist is always enclosed in a pair of square
@@ -20,22 +25,28 @@ impl<'src> Attrlist<'src> {
     /// the opening or closing square brackets for the attrlist. This is because
     /// the rules for closing brackets differ when parsing inline, macro, and
     /// block elements.
-    pub(crate) fn parse(source: Span<'src>) -> Option<MatchedItem<'src, Self>> {
+    pub(crate) fn parse(source: Span<'src>) -> MatchAndWarnings<'src, MatchedItem<'src, Self>> {
         let mut after = source;
         let mut attributes: Vec<ElementAttribute> = vec![];
         let mut parse_shorthand_items = true;
+        let mut warnings: Vec<Warning<'src>> = vec![];
 
         if source.starts_with('[') && source.ends_with(']') {
             todo!("Parse block anchor syntax (issue #122)");
         }
 
         loop {
-            let maybe_attr = if parse_shorthand_items {
+            let mut maybe_attr_and_warnings = if parse_shorthand_items {
                 ElementAttribute::parse_with_shorthand(after)
             } else {
                 ElementAttribute::parse(after)
             };
 
+            if !maybe_attr_and_warnings.warnings.is_empty() {
+                warnings.append(&mut maybe_attr_and_warnings.warnings);
+            }
+
+            let maybe_attr = maybe_attr_and_warnings.item;
             let Some(attr) = maybe_attr else {
                 break;
             };
@@ -50,6 +61,14 @@ impl<'src> Attrlist<'src> {
             match after.take_prefix(",") {
                 Some(comma) => {
                     after = comma.after.take_whitespace().after;
+                    if after.starts_with(",") {
+                        warnings.push(Warning {
+                            source: comma.item,
+                            warning: WarningType::EmptyAttributeValue,
+                        });
+                        after = after.discard(1);
+                        continue;
+                    }
                 }
                 None => {
                     break;
@@ -58,13 +77,21 @@ impl<'src> Attrlist<'src> {
         }
 
         if !after.is_empty() {
-            return None;
+            warnings.push(Warning {
+                source: after,
+                warning: WarningType::MissingCommaAfterQuotedAttributeValue,
+            });
+
+            after = after.discard_all();
         }
 
-        Some(MatchedItem {
-            item: Self { attributes, source },
-            after,
-        })
+        MatchAndWarnings {
+            item: MatchedItem {
+                item: Self { attributes, source },
+                after,
+            },
+            warnings,
+        }
     }
 
     /// Returns an iterator over the attributes contained within

src/attributes/element_attribute.rs

@@ -1,4 +1,8 @@
-use crate::{span::MatchedItem, HasSpan, Span};
+use crate::{
+    span::MatchedItem,
+    warnings::{MatchAndWarnings, Warning, WarningType},
+    HasSpan, Span,
+};
 
 /// This struct represents a single element attribute.
 ///
@@ -16,25 +20,32 @@ pub struct ElementAttribute<'src> {
 }
 
 impl<'src> ElementAttribute<'src> {
-    pub(crate) fn parse(source: Span<'src>) -> Option<MatchedItem<'src, Self>> {
+    pub(crate) fn parse(
+        source: Span<'src>,
+    ) -> MatchAndWarnings<'src, Option<MatchedItem<'src, Self>>> {
         Self::parse_internal(source, false)
     }
 
-    pub(crate) fn parse_with_shorthand(source: Span<'src>) -> Option<MatchedItem<'src, Self>> {
+    pub(crate) fn parse_with_shorthand(
+        source: Span<'src>,
+    ) -> MatchAndWarnings<'src, Option<MatchedItem<'src, Self>>> {
         Self::parse_internal(source, true)
     }
 
     fn parse_internal(
         source: Span<'src>,
         parse_shorthand: bool,
-    ) -> Option<MatchedItem<'src, Self>> {
+    ) -> MatchAndWarnings<'src, Option<MatchedItem<'src, Self>>> {
+        let mut warnings: Vec<Warning<'src>> = vec![];
+
         let (name, after): (Option<Span>, Span) = match source.take_attr_name() {
             Some(name) => {
                 let space = name.after.take_whitespace();
                 match space.after.take_prefix("=") {
                     Some(equals) => {
                         let space = equals.after.take_whitespace();
                         if space.after.is_empty() || space.after.starts_with(',') {
+                            // TO DO: Is this a warning? Possible spec ambiguity.
                             (None, source)
                         } else {
                             (Some(name.item), space.after)
@@ -50,33 +61,47 @@ impl<'src> ElementAttribute<'src> {
             Some('\'') | Some('"') => match after.take_quoted_string() {
                 Some(v) => v,
                 None => {
-                    return None;
+                    warnings.push(Warning {
+                        source: after,
+                        warning: WarningType::AttributeValueMissingTerminatingQuote,
+                    });
+
+                    return MatchAndWarnings {
+                        item: None,
+                        warnings,
+                    };
                 }
             },
             _ => after.take_while(|c| c != ','),
         };
 
         if value.item.is_empty() {
-            return None;
+            return MatchAndWarnings {
+                item: None,
+                warnings,
+            };
         }
 
         let source = source.trim_remainder(value.after);
 
         let shorthand_items = if name.is_none() && parse_shorthand {
-            parse_shorthand_items(source)
+            parse_shorthand_items(source, &mut warnings)
         } else {
             vec![]
         };
 
-        Some(MatchedItem {
-            item: Self {
-                name,
-                shorthand_items,
-                value: value.item,
-                source,
-            },
-            after: value.after,
-        })
+        MatchAndWarnings {
+            item: Some(MatchedItem {
+                item: Self {
+                    name,
+                    shorthand_items,
+                    value: value.item,
+                    source,
+                },
+                after: value.after,
+            }),
+            warnings,
+        }
     }
 
     /// Return a [`Span`] describing the attribute name.
@@ -143,8 +168,10 @@ impl<'src> HasSpan<'src> for ElementAttribute<'src> {
     }
 }
 
-fn parse_shorthand_items<'src>(span: Span<'src>) -> Vec<Span<'src>> {
-    let mut span = span;
+fn parse_shorthand_items<'src>(
+    mut span: Span<'src>,
+    warnings: &mut Vec<Warning<'src>>,
+) -> Vec<Span<'src>> {
     let mut shorthand_items: Vec<Span<'src>> = vec![];
 
     // Look for block style selector.
@@ -159,14 +186,22 @@ fn parse_shorthand_items<'src>(span: Span<'src>) -> Vec<Span<'src>> {
         match after_delimiter.position(is_shorthand_delimiter) {
             None => {
                 if after_delimiter.is_empty() {
-                    todo!("Flag warning for empty shorthand item (issue #120)");
+                    warnings.push(Warning {
+                        source: span,
+                        warning: WarningType::EmptyShorthandItem,
+                    });
+                    span = after_delimiter;
                 } else {
                     shorthand_items.push(span);
                     span = span.discard_all();
                 }
             }
             Some(0) => {
-                todo!("Flag warning for duplicate shorthand delimiter (issue #121)");
+                warnings.push(Warning {
+                    source: span.trim_remainder(after_delimiter),
+                    warning: WarningType::EmptyShorthandItem,
+                });
+                span = after_delimiter;
             }
             Some(index) => {
                 let mi: MatchedItem<Span> = span.into_parse_result(index + 1);

src/blocks/block.rs

@@ -4,6 +4,7 @@ use crate::{
     blocks::{ContentModel, IsBlock, MacroBlock, SectionBlock, SimpleBlock},
     span::MatchedItem,
     strings::CowStr,
+    warnings::{MatchAndWarnings, Warning},
     HasSpan, Span,
 };
 
@@ -40,38 +41,65 @@ impl<'src> Block<'src> {
     /// Parse a block of any type and return a `Block` that describes it.
     ///
     /// Consumes any blank lines before and after the block.
-    pub(crate) fn parse(source: Span<'src>) -> Option<MatchedItem<'src, Self>> {
+    pub(crate) fn parse(
+        source: Span<'src>,
+    ) -> MatchAndWarnings<'src, Option<MatchedItem<'src, Self>>> {
+        let mut warnings: Vec<Warning<'src>> = vec![];
         let source = source.discard_empty_lines();
 
         // Try to discern the block type by scanning the first line.
         let line = source.take_normalized_line();
         if line.item.contains("::") {
-            if let Some(macro_block) = MacroBlock::parse(source) {
-                return Some(MatchedItem {
-                    item: Self::Macro(macro_block.item),
-                    after: macro_block.after,
-                });
+            let mut macro_block_maw = MacroBlock::parse(source);
+
+            if let Some(macro_block) = macro_block_maw.item {
+                // Only propagate warnings from macro block parsing if we think this
+                // *is* a macro block. Otherwise, there would likely be too many false
+                // positives.
+                if !macro_block_maw.warnings.is_empty() {
+                    warnings.append(&mut macro_block_maw.warnings);
+                }
+
+                return MatchAndWarnings {
+                    item: Some(MatchedItem {
+                        item: Self::Macro(macro_block.item),
+                        after: macro_block.after,
+                    }),
+                    warnings,
+                };
             }
 
             // A line containing `::` might be some other kind of block, so we
             // don't automatically error out on a parse failure.
-        } else if line.item.starts_with('=') {
-            if let Some(section_block) = SectionBlock::parse(source) {
-                return Some(MatchedItem {
-                    item: Self::Section(section_block.item),
-                    after: section_block.after,
-                });
+        }
+
+        if line.item.starts_with('=') {
+            if let Some(mut maw_section_block) = SectionBlock::parse(source) {
+                if !maw_section_block.warnings.is_empty() {
+                    warnings.append(&mut maw_section_block.warnings);
+                }
+
+                return MatchAndWarnings {
+                    item: Some(MatchedItem {
+                        item: Self::Section(maw_section_block.item.item),
+                        after: maw_section_block.item.after,
+                    }),
+                    warnings,
+                };
             }
 
             // A line starting with `=` might be some other kind of block, so we
             // don't automatically error out on a parse failure.
         }
 
         // If no other block kind matches, we can always use SimpleBlock.
-        SimpleBlock::parse(source).map(|mi| MatchedItem {
-            item: Self::Simple(mi.item),
-            after: mi.after,
-        })
+        MatchAndWarnings {
+            item: SimpleBlock::parse(source).map(|mi| MatchedItem {
+                item: Self::Simple(mi.item),
+                after: mi.after,
+            }),
+            warnings,
+        }
     }
 }