fix(compliance): pull in spec test suite and fix issues (#40)

This commit includes a whole bunch of fixes, some of which are significant changes to the parser and some related functionality. But I consider all changes to be bugfixes because they were compliance failures.

Author: zkat

README.md

@@ -96,6 +96,32 @@ Error:
    ╰────
   help: Floating point numbers must be base 10, and have numbers after the decimal point.
 ```
+
+### Quirks
+
+#### Properties
+
+Multiple properties with the same name are allowed, and all duplicated
+**will be preserved**, meaning those documents will correctly round-trip.
+When using `node.get()`/`node["key"]` & company, the _last_ property with
+that name's value will be returned.
+
+#### Numbers
+
+KDL itself does not specify a particular representation for numbers and
+accepts just about anything valid, no matter how large and how small. This
+means a few things:
+
+* Numbers without a decimal point are interpreted as u64.
+* Numbers with a decimal point are interpreted as f64.
+* Floating point numbers that evaluate to f64::INFINITY or
+  f64::NEG_INFINITY or NaN will be represented as such in the values,
+  instead of the original numbers.
+* A similar restriction applies to overflowed u64 values.
+* The original _representation_ of these numbers will be preserved, unless
+  you `doc.fmt()`, in which case the original representation will be
+  thrown away and the actual value will be used when serializing.
+
 ### License
 
 The code in this repository is covered by [the Apache-2.0

src/document.rs

@@ -188,7 +188,12 @@ impl KdlDocument {
     /// Auto-formats this Document, making everything nice while preserving
     /// comments.
     pub fn fmt(&mut self) {
-        self.fmt_impl(0);
+        self.fmt_impl(0, false);
+    }
+
+    /// Formats the document and removes all comments from the document.
+    pub fn fmt_no_comments(&mut self) {
+        self.fmt_impl(0, true);
     }
 }
 
@@ -199,15 +204,20 @@ impl Display for KdlDocument {
 }
 
 impl KdlDocument {
-    pub(crate) fn fmt_impl(&mut self, indent: usize) {
+    pub(crate) fn fmt_impl(&mut self, indent: usize, no_comments: bool) {
         if let Some(s) = self.leading.as_mut() {
-            crate::fmt::fmt_leading(s, indent);
-        }
-        if let Some(s) = self.trailing.as_mut() {
-            crate::fmt::fmt_trailing(s);
+            crate::fmt::fmt_leading(s, indent, no_comments);
         }
+        let mut has_nodes = false;
         for node in &mut self.nodes {
-            node.fmt_impl(indent);
+            has_nodes = true;
+            node.fmt_impl(indent, no_comments);
+        }
+        if let Some(s) = self.trailing.as_mut() {
+            crate::fmt::fmt_trailing(s, no_comments);
+            if !has_nodes {
+                s.push('\n');
+            }
         }
     }
 

src/entry.rs

@@ -124,12 +124,12 @@ impl Display for KdlEntry {
         if let Some(leading) = &self.leading {
             write!(f, "{}", leading)?;
         }
-        if let Some(ty) = &self.ty {
-            write!(f, "({})", ty)?;
-        }
         if let Some(name) = &self.name {
             write!(f, "{}=", name)?;
         }
+        if let Some(ty) = &self.ty {
+            write!(f, "({})", ty)?;
+        }
         if let Some(repr) = &self.value_repr {
             write!(f, "{}", repr)?;
         } else {
@@ -165,7 +165,7 @@ impl FromStr for KdlEntry {
     type Err = KdlError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        parser::parse(s, parser::entry_with_node_space)
+        parser::parse(s, parser::entry_with_trailing)
     }
 }
 
@@ -217,7 +217,7 @@ mod test {
             }
         );
 
-        let entry: KdlEntry = " \\\n (\"m\\\"eh\")\"foo\"=0xDEADbeef\t\\\n".parse()?;
+        let entry: KdlEntry = " \\\n \"foo\"=(\"m\\\"eh\")0xDEADbeef\t\\\n".parse()?;
         assert_eq!(
             entry,
             KdlEntry {

src/fmt.rs

@@ -1,30 +1,34 @@
-pub(crate) fn fmt_leading(leading: &mut String, indent: usize) {
+pub(crate) fn fmt_leading(leading: &mut String, indent: usize, no_comments: bool) {
     if leading.is_empty() {
         return;
     }
-    let comments = crate::parser::parse(leading.trim(), crate::parser::leading_comments)
-        .expect("invalid leading text");
     let mut result = String::new();
-    for line in comments {
-        let trimmed = line.trim();
-        if !trimmed.is_empty() {
-            result.push_str(&format!("{:indent$}{}\n", "", trimmed, indent = indent));
+    if !no_comments {
+        let comments = crate::parser::parse(leading.trim(), crate::parser::leading_comments)
+            .expect("invalid leading text");
+        for line in comments {
+            let trimmed = line.trim();
+            if !trimmed.is_empty() {
+                result.push_str(&format!("{:indent$}{}\n", "", trimmed, indent = indent));
+            }
         }
     }
     result.push_str(&format!("{:indent$}", "", indent = indent));
     *leading = result;
 }
 
-pub(crate) fn fmt_trailing(decor: &mut String) {
+pub(crate) fn fmt_trailing(decor: &mut String, no_comments: bool) {
     if decor.is_empty() {
         return;
     }
     *decor = decor.trim().to_string();
     let mut result = String::new();
-    let comments = crate::parser::parse(decor, crate::parser::trailing_comments)
-        .expect("invalid trailing text");
-    for comment in comments {
-        result.push_str(comment);
+    if !no_comments {
+        let comments = crate::parser::parse(decor, crate::parser::trailing_comments)
+            .expect("invalid trailing text");
+        for comment in comments {
+            result.push_str(comment);
+        }
     }
     *decor = result;
 }

src/identifier.rs

@@ -4,7 +4,7 @@ use crate::{parser, KdlError};
 
 /// Represents a KDL
 /// [Identifier](https://github.com/kdl-org/kdl/blob/main/SPEC.md#identifier).
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct KdlIdentifier {
     pub(crate) value: String,
     pub(crate) repr: Option<String>,
@@ -199,9 +199,6 @@ mod test {
         let invalid = "\"x";
         assert!(invalid.parse::<KdlIdentifier>().is_err());
 
-        let invalid = "r#\"foo\"#";
-        assert!(invalid.parse::<KdlIdentifier>().is_err());
-
         Ok(())
     }
 

src/lib.rs

@@ -94,6 +94,32 @@
 //!    ╰────
 //!   help: Floating point numbers must be base 10, and have numbers after the decimal point.
 //! ```
+//!
+//! ## Quirks
+//!
+//! ### Properties
+//!
+//! Multiple properties with the same name are allowed, and all duplicated
+//! **will be preserved**, meaning those documents will correctly round-trip.
+//! When using `node.get()`/`node["key"]` & company, the _last_ property with
+//! that name's value will be returned.
+//!
+//! ### Numbers
+//!
+//! KDL itself does not specify a particular representation for numbers and
+//! accepts just about anything valid, no matter how large and how small. This
+//! means a few things:
+//!
+//! * Numbers without a decimal point are interpreted as [`u64`].
+//! * Numbers with a decimal point are interpreted as [`f64`].
+//! * Floating point numbers that evaluate to [`f64::INFINITY`] or
+//!   [`f64::NEG_INFINITY`] or NaN will be represented as such in the values,
+//!   instead of the original numbers.
+//! * A similar restriction applies to overflowed [`u64`] values.
+//! * The original _representation_ of these numbers will be preserved, unless
+//!   you [`KdlDocument::fmt`] in which case the original representation will be
+//!   thrown away and the actual value will be used when serializing.
+//!
 //! ## License
 //!
 //! The code in this repository is covered by [the Apache-2.0
@@ -102,6 +128,7 @@
 #![deny(missing_debug_implementations, nonstandard_style)]
 #![warn(missing_docs, unreachable_pub, rust_2018_idioms, unreachable_pub)]
 #![cfg_attr(test, deny(warnings))]
+#![doc(html_favicon_url = "https://kdl.dev/favicon.ico")]
 #![doc(html_logo_url = "https://kdl.dev/logo.svg")]
 
 pub use document::*;

src/node.rs

@@ -134,14 +134,15 @@ impl KdlNode {
     fn get_impl(&self, key: NodeKey) -> Option<&KdlEntry> {
         match key {
             NodeKey::Key(key) => {
+                let mut current = None;
                 for entry in &self.entries {
                     if entry.name.is_some()
                         && entry.name.as_ref().map(|i| i.value()) == Some(key.value())
                     {
-                        return Some(entry);
+                        current = Some(entry);
                     }
                 }
-                None
+                current
             }
             NodeKey::Index(idx) => {
                 let mut current_idx = 0;
@@ -170,14 +171,15 @@ impl KdlNode {
     fn get_mut_impl(&mut self, key: NodeKey) -> Option<&mut KdlEntry> {
         match key {
             NodeKey::Key(key) => {
+                let mut current = None;
                 for entry in &mut self.entries {
                     if entry.name.is_some()
                         && entry.name.as_ref().map(|i| i.value()) == Some(key.value())
                     {
-                        return Some(entry);
+                        current = Some(entry);
                     }
                 }
-                None
+                current
             }
             NodeKey::Index(idx) => {
                 let mut current_idx = 0;
@@ -340,7 +342,12 @@ impl KdlNode {
 
     /// Auto-formats this node and its contents.
     pub fn fmt(&mut self) {
-        self.fmt_impl(0);
+        self.fmt_impl(0, false);
+    }
+
+    /// Auto-formats this node and its contents, stripping comments.
+    pub fn fmt_no_comments(&mut self) {
+        self.fmt_impl(0, true);
     }
 }
 
@@ -421,12 +428,12 @@ impl Display for KdlNode {
 }
 
 impl KdlNode {
-    pub(crate) fn fmt_impl(&mut self, indent: usize) {
+    pub(crate) fn fmt_impl(&mut self, indent: usize, no_comments: bool) {
         if let Some(s) = self.leading.as_mut() {
-            crate::fmt::fmt_leading(s, indent);
+            crate::fmt::fmt_leading(s, indent, no_comments);
         }
         if let Some(s) = self.trailing.as_mut() {
-            crate::fmt::fmt_trailing(s);
+            crate::fmt::fmt_trailing(s, no_comments);
             if s.starts_with(';') {
                 s.remove(0);
             }
@@ -446,7 +453,7 @@ impl KdlNode {
             entry.fmt();
         }
         if let Some(children) = self.children.as_mut() {
-            children.fmt_impl(indent + 4);
+            children.fmt_impl(indent + 4, no_comments);
             if let Some(leading) = children.leading.as_mut() {
                 leading.push('\n');
             }
@@ -511,6 +518,11 @@ mod test {
         assert_eq!(node.name(), &"\"node\"".parse()?);
         assert_eq!(node.get(0), Some(&"0xDEADbeef".parse()?));
 
+        r#"
+        node "test" {
+            link "blah" anything="self"
+        }"#
+        .parse::<KdlNode>()?;
         Ok(())
     }
 
@@ -528,5 +540,9 @@ mod test {
 
         assert_eq!(node[0], false.into());
         assert_eq!(node["foo"], KdlValue::Null);
+
+        node.entries_mut().push(KdlEntry::new_prop("x", 1));
+        node.entries_mut().push(KdlEntry::new_prop("x", 2));
+        assert_eq!(&node["x"], &2.into())
     }
 }