Incorporates pending feedback from lazy reader PRs (#642)

Feedback from PRs:
* #609
* #614
* #616
* #619
* #620
* #627
* #628
* #638
* #639

Author: zslayton

src/lazy/reader.rs

@@ -17,11 +17,11 @@ use crate::{IonError, IonResult};
 /// which may contain either a scalar value or a lazy container that may itself be traversed.
 ///
 /// The values that the reader yields ([`LazyValue`],
-/// [`LazyBinarySequence`](crate::lazy::sequence::LazyBinarySequence), and
-/// [`LazyBinaryStruct`](crate::lazy::struct::LazyStruct)) are
-/// immutable references to the data stream, and remain valid until [`LazyApplicationReader::next`] is called
-/// again to advance the reader to the next top level value. This means that these references can
-/// be stored, read, and re-read as long as the reader remains on the same top-level value.
+/// [`LazyList`](crate::lazy::sequence::LazyList), [`LazySExp`](crate::lazy::sequence::LazySExp),
+/// and [`LazyStruct`](crate::lazy::struct::LazyStruct)) are immutable references to the data
+/// stream, and remain valid until [`LazyApplicationReader::next`] is called again to advance the
+/// reader to the next top level value. This means that these references can be stored, read, and
+/// re-read as long as the reader remains on the same top-level value.
 /// ```
 ///# use ion_rs::IonResult;
 ///# fn main() -> IonResult<()> {

src/lazy/str_ref.rs

@@ -5,22 +5,24 @@ use std::borrow::Cow;
 use std::fmt::{Display, Formatter};
 use std::ops::Deref;
 
+/// A reference to an immutable in-memory representation of an Ion string. To get an owned [`Str`]
+/// instead, see [`StrRef::to_owned`].
 #[derive(Clone, PartialEq, Debug)]
 pub struct StrRef<'data> {
     text: Cow<'data, str>,
 }
 
 impl<'data> StrRef<'data> {
     pub fn to_owned(&self) -> Str {
-        Str::from(self.as_ref())
+        Str::from(self.text.as_ref())
     }
 
     pub fn into_owned(self) -> Str {
         Str::from(self)
     }
 
     pub fn text(&self) -> &str {
-        self.as_ref()
+        self.text.as_ref()
     }
 }
 

src/lazy/text/buffer.rs

@@ -113,8 +113,6 @@ pub(crate) struct TextBufferView<'a> {
     offset: usize,
 }
 
-pub(crate) type ParseResult<'a, T> = IonResult<(T, TextBufferView<'a>)>;
-
 impl<'data> TextBufferView<'data> {
     /// Constructs a new `TextBufferView` that wraps `data`, setting the view's `offset` to zero.
     #[inline]
@@ -432,10 +430,10 @@ impl<'data> TextBufferView<'data> {
         alt((
             // For `null` and `bool`, we use `read_` instead of `match_` because there's no additional
             // parsing to be done.
-            map(match_and_length(Self::read_null), |(ion_type, length)| {
+            map(match_and_length(Self::match_null), |(ion_type, length)| {
                 EncodedTextValue::new(MatchedValue::Null(ion_type), self.offset(), length)
             }),
-            map(match_and_length(Self::read_bool), |(value, length)| {
+            map(match_and_length(Self::match_bool), |(value, length)| {
                 EncodedTextValue::new(MatchedValue::Bool(value), self.offset(), length)
             }),
             // For `int` and the other types, we use `match` and store the partially-processed input in the
@@ -662,37 +660,27 @@ impl<'data> TextBufferView<'data> {
         Ok((remaining, matched))
     }
 
-    /// Matches a boolean value.
-    pub fn match_bool(self) -> IonMatchResult<'data> {
-        recognize(Self::read_bool)(self)
-    }
-
     /// Matches and returns a boolean value.
-    pub fn read_bool(self) -> IonParseResult<'data, bool> {
+    pub fn match_bool(self) -> IonParseResult<'data, bool> {
         terminated(
             alt((value(true, tag("true")), value(false, tag("false")))),
             Self::peek_stop_character,
         )(self)
     }
 
-    /// Matches any type of null. (`null`, `null.null`, `null.int`, etc)
-    pub fn match_null(self) -> IonMatchResult<'data> {
-        recognize(Self::read_null)(self)
-    }
-
-    /// Matches and returns a null value.
-    pub fn read_null(self) -> IonParseResult<'data, IonType> {
+    /// Matches and returns any type of null. (`null`, `null.null`, `null.int`, etc)
+    pub fn match_null(self) -> IonParseResult<'data, IonType> {
         delimited(
             complete_tag("null"),
-            opt(preceded(complete_char('.'), Self::read_ion_type)),
+            opt(preceded(complete_char('.'), Self::match_ion_type)),
             Self::peek_stop_character,
         )
         .map(|explicit_ion_type| explicit_ion_type.unwrap_or(IonType::Null))
         .parse(self)
     }
 
     /// Matches and returns an Ion type.
-    fn read_ion_type(self) -> IonParseResult<'data, IonType> {
+    fn match_ion_type(self) -> IonParseResult<'data, IonType> {
         alt((
             value(IonType::Null, complete_tag("null")),
             value(IonType::Bool, complete_tag("bool")),
@@ -1929,6 +1917,7 @@ mod tests {
 
         mismatch_ivm("ion_1_0");
         mismatch_ivm("$ion__1_0");
+        mismatch_ivm("$ion_1_0_0");
         mismatch_ivm("$$ion_1_0");
         mismatch_ivm("$ion_FF_FF");
     }
@@ -2199,8 +2188,6 @@ mod tests {
             MatchTest::new(input).expect_mismatch(match_length(TextBufferView::match_symbol));
         }
 
-        // These inputs have leading/trailing whitespace to make them more readable, but the string
-        // matcher doesn't accept whitespace. We'll trim each one before testing it.
         let good_inputs = &[
             "'hello'",
             "'😀😀😀'",
@@ -2264,7 +2251,8 @@ mod tests {
             MatchTest::new(input).expect_mismatch(match_length(TextBufferView::match_decimal));
         }
         let good_inputs = &[
-            "5.", "-5.", "5.0", "-5.0", "5.0d0", "-5.0d0", "5.0D0", "-5.0D0", "5.0d+1", "-5.0d-1",
+            "5.", "-5.", "5.0", "-5.0", "5d0", "5.d0", "5.0d0", "-5.0d0", "5.0D0", "-5.0D0",
+            "5.0d+1", "-5.0d-1",
         ];
         for input in good_inputs {
             match_decimal(input);
@@ -2295,6 +2283,7 @@ mod tests {
             "(a b)",
             "(a++)",
             "(++a)",
+            "(a+=b)",
             "(())",
             "((()))",
             "(1 (2 (3 4) 5) 6)",

src/lazy/text/encoded_value.rs

@@ -63,6 +63,9 @@ pub(crate) struct EncodedTextValue {
     // recognized during matching and partial information like subfield offsets can be stored here.
     matched_value: MatchedValue,
 
+    // If this value is a struct field value, this will be populated with an enum indicating
+    // the syntax of the associated field name. If the field name is later read, the decoder
+    // can avoid re-parsing the input from scratch.
     field_name_syntax: Option<MatchedFieldName>,
 }
 

src/lazy/text/matched.rs

@@ -364,7 +364,7 @@ impl MatchedString {
         let body = matched_input.slice(3, matched_input.len() - 6);
         // There are no escaped characters, so we can just validate the string in-place.
         let mut sanitized = Vec::with_capacity(matched_input.len());
-        decode_text_containing_escapes(
+        replace_escapes_with_byte_values(
             body,
             &mut sanitized,
             // Normalize newlines
@@ -396,7 +396,7 @@ impl MatchedString {
         )(remaining)
         {
             remaining = remaining_after_match;
-            decode_text_containing_escapes(
+            replace_escapes_with_byte_values(
                 segment_body,
                 &mut sanitized,
                 // Normalize newlines
@@ -430,7 +430,7 @@ impl MatchedString {
         // There are escaped characters. We need to build a new version of our string
         // that replaces the escaped characters with their corresponding bytes.
         let mut sanitized = Vec::with_capacity(matched_input.len());
-        decode_text_containing_escapes(
+        replace_escapes_with_byte_values(
             body,
             &mut sanitized,
             // Do not normalize newlines
@@ -443,7 +443,7 @@ impl MatchedString {
     }
 }
 
-fn decode_text_containing_escapes(
+fn replace_escapes_with_byte_values(
     matched_input: TextBufferView,
     sanitized: &mut Vec<u8>,
     // If the text being escaped is in a long string or a clob, then unescaped \r\n and \r get
@@ -513,6 +513,9 @@ fn normalize_newline<'data>(
     }
 }
 
+/// Matches an escape sequence at the beginning of `input` and pushes its corresponding
+/// byte values onto the end of `sanitized`. Returns the remaining input following the escape
+/// sequence.
 fn decode_escape_into_bytes<'data>(
     input: TextBufferView<'data>,
     sanitized: &mut Vec<u8>,
@@ -799,7 +802,7 @@ impl MatchedSymbol {
         // that replaces the escaped characters with their corresponding bytes.
         let mut sanitized = Vec::with_capacity(matched_input.len());
 
-        decode_text_containing_escapes(body, &mut sanitized, false, true)?;
+        replace_escapes_with_byte_values(body, &mut sanitized, false, true)?;
         let text = String::from_utf8(sanitized).unwrap();
         Ok(RawSymbolTokenRef::Text(text.into()))
     }
@@ -1055,7 +1058,9 @@ impl MatchedBlob {
 
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub enum MatchedClob {
+    // Indicates that the content of the clob was written using short-form string syntax.
     Short,
+    // Indicates that the content of the clob was written using long-form string syntax.
     Long,
 }
 
@@ -1093,7 +1098,7 @@ impl MatchedClob {
         // There are escaped characters. We need to build a new version of our string
         // that replaces the escaped characters with their corresponding bytes.
         let mut sanitized = Vec::with_capacity(body.len());
-        decode_text_containing_escapes(
+        replace_escapes_with_byte_values(
             body,
             &mut sanitized,
             // Do not normalize newlines
@@ -1123,7 +1128,7 @@ impl MatchedClob {
         )(remaining)
         {
             remaining = remaining_after_match;
-            decode_text_containing_escapes(
+            replace_escapes_with_byte_values(
                 segment_body,
                 &mut sanitized,
                 // Normalize newlines

src/result/decoding_error.rs

@@ -7,6 +7,10 @@ use thiserror::Error;
 #[error("{description}")]
 pub struct DecodingError {
     description: Cow<'static, str>,
+    // This is optional because sometimes data is found to be malformed or invalid but the original
+    // data source is not available. For example, consider a deserializer reading a symbol table
+    // from an `Element`. If the `symbols` field is missing, it needs to raise a decoding error, but
+    // no source position is available. Whenever possible, usages should specify the position.
     position: Option<Position>,
 }