From b484ef812571479c670f573240b78d75d3599890 Mon Sep 17 00:00:00 2001
From: Jean-Christophe Amiel <jeanchristophe.amiel@orange.com>
Date: Mon, 1 Jul 2024 16:52:20 +0200
Subject: [PATCH] Some cosmetic changes on Reader.

---
 packages/hurl/src/jsonpath/parser/parse.rs    |  2 +-
 .../hurl/src/jsonpath/parser/primitives.rs    |  2 +-
 packages/hurl_core/src/parser/cookiepath.rs   |  6 +-
 packages/hurl_core/src/parser/expr.rs         |  2 +-
 packages/hurl_core/src/parser/filename.rs     |  2 +-
 .../hurl_core/src/parser/filename_password.rs |  2 +-
 packages/hurl_core/src/parser/json.rs         | 14 ++--
 packages/hurl_core/src/parser/key_string.rs   |  2 +-
 packages/hurl_core/src/parser/number.rs       |  2 +-
 packages/hurl_core/src/parser/option.rs       |  4 +-
 packages/hurl_core/src/parser/primitives.rs   |  2 +-
 packages/hurl_core/src/parser/string.rs       |  6 +-
 packages/hurl_core/src/reader.rs              | 72 +++++++++++--------
 13 files changed, 67 insertions(+), 51 deletions(-)

diff --git a/packages/hurl/src/jsonpath/parser/parse.rs b/packages/hurl/src/jsonpath/parser/parse.rs
index 5c4f2b170..c70c3f5dd 100644
--- a/packages/hurl/src/jsonpath/parser/parse.rs
+++ b/packages/hurl/src/jsonpath/parser/parse.rs
@@ -163,7 +163,7 @@ fn selector_object_key(reader: &mut Reader) -> Result<Selector, ParseError> {
     };
     _ = reader.read();
 
-    let s = reader.read_while(|c| c.is_alphanumeric() || *c == '_' || *c == '-');
+    let s = reader.read_while(|c| c.is_alphanumeric() || c == '_' || c == '-');
     if s.is_empty() {
         let kind = ParseErrorKind::Expecting("empty value".to_string());
         let error = ParseError::new(reader.cursor().pos, false, kind);
diff --git a/packages/hurl/src/jsonpath/parser/primitives.rs b/packages/hurl/src/jsonpath/parser/primitives.rs
index 97d135d78..5d46e7540 100644
--- a/packages/hurl/src/jsonpath/parser/primitives.rs
+++ b/packages/hurl/src/jsonpath/parser/primitives.rs
@@ -136,7 +136,7 @@ pub fn key_name(reader: &mut Reader) -> Result<String, ParseError> {
             return Err(error);
         }
     };
-    let s = reader.read_while(|c| c.is_alphanumeric() || *c == '_');
+    let s = reader.read_while(|c| c.is_alphanumeric() || c == '_');
     whitespace(reader);
     Ok(format!("{first_char}{s}"))
 }
diff --git a/packages/hurl_core/src/parser/cookiepath.rs b/packages/hurl_core/src/parser/cookiepath.rs
index 95d1bd55f..c07304563 100644
--- a/packages/hurl_core/src/parser/cookiepath.rs
+++ b/packages/hurl_core/src/parser/cookiepath.rs
@@ -28,7 +28,7 @@ pub fn cookiepath(reader: &mut Reader) -> ParseResult<CookiePath> {
 
     // We create a specialized reader for the templated, error and created structures are
     // relative tho the main reader.
-    let s = reader.read_while(|c| *c != '[');
+    let s = reader.read_while(|c| c != '[');
     let mut template_reader = Reader::with_pos(s.as_str(), start);
     let name = unquoted_template(&mut template_reader)?;
     let attribute = optional(cookiepath_attribute, reader)?;
@@ -50,7 +50,7 @@ fn cookiepath_attribute(reader: &mut Reader) -> ParseResult<CookieAttribute> {
 
 fn cookiepath_attribute_name(reader: &mut Reader) -> ParseResult<CookieAttributeName> {
     let start = reader.cursor().pos;
-    let s = reader.read_while(|c| c.is_alphabetic() || *c == '-');
+    let s = reader.read_while(|c| c.is_alphabetic() || c == '-');
     match s.to_lowercase().as_str() {
         "value" => Ok(CookieAttributeName::Value(s)),
         "expires" => Ok(CookieAttributeName::Expires(s)),
@@ -180,7 +180,7 @@ mod tests {
 
         // Check that errors are well reported with a buffer that have already read data.
         let mut reader = Reader::new("xxxx{{cookie[Domain]");
-        _ = reader.read_while(|&c| c == 'x');
+        _ = reader.read_while(|c| c == 'x');
 
         let error = cookiepath(&mut reader).err().unwrap();
         assert_eq!(
diff --git a/packages/hurl_core/src/parser/expr.rs b/packages/hurl_core/src/parser/expr.rs
index 570b5bedf..feacb8419 100644
--- a/packages/hurl_core/src/parser/expr.rs
+++ b/packages/hurl_core/src/parser/expr.rs
@@ -52,7 +52,7 @@ pub fn parse2(reader: &mut Reader) -> ParseResult<Expr> {
 
 fn variable_name(reader: &mut Reader) -> ParseResult<Variable> {
     let start = reader.cursor();
-    let name = reader.read_while(|c| c.is_alphanumeric() || *c == '_' || *c == '-');
+    let name = reader.read_while(|c| c.is_alphanumeric() || c == '_' || c == '-');
     if name.is_empty() {
         return Err(ParseError::new(
             start.pos,
diff --git a/packages/hurl_core/src/parser/filename.rs b/packages/hurl_core/src/parser/filename.rs
index 024057711..30a4e743b 100644
--- a/packages/hurl_core/src/parser/filename.rs
+++ b/packages/hurl_core/src/parser/filename.rs
@@ -45,7 +45,7 @@ pub fn parse(reader: &mut Reader) -> ParseResult<Template> {
                     if value.is_empty() {
                         break;
                     }
-                    let encoded = reader.peek_back(start.offset);
+                    let encoded = reader.read_from(start.offset);
                     let element = TemplateElement::String { value, encoded };
                     elements.push(element);
                 } else {
diff --git a/packages/hurl_core/src/parser/filename_password.rs b/packages/hurl_core/src/parser/filename_password.rs
index 4b6f81f13..fca20792b 100644
--- a/packages/hurl_core/src/parser/filename_password.rs
+++ b/packages/hurl_core/src/parser/filename_password.rs
@@ -46,7 +46,7 @@ pub fn parse(reader: &mut Reader) -> ParseResult<Template> {
                     if value.is_empty() {
                         break;
                     }
-                    let encoded = reader.peek_back(start.offset);
+                    let encoded = reader.read_from(start.offset);
                     let element = TemplateElement::String { value, encoded };
                     elements.push(element);
                 } else {
diff --git a/packages/hurl_core/src/parser/json.rs b/packages/hurl_core/src/parser/json.rs
index 69bd804bf..6328ec474 100644
--- a/packages/hurl_core/src/parser/json.rs
+++ b/packages/hurl_core/src/parser/json.rs
@@ -110,7 +110,7 @@ fn string_template(reader: &mut Reader) -> ParseResult<Template> {
 fn any_char(reader: &mut Reader) -> ParseResult<(char, String, Pos)> {
     let start = reader.cursor();
     match escape_char(reader) {
-        Ok(c) => Ok((c, reader.peek_back(start.offset), start.pos)),
+        Ok(c) => Ok((c, reader.read_from(start.offset), start.pos)),
         Err(e) => {
             if e.recoverable {
                 reader.seek(start);
@@ -128,7 +128,7 @@ fn any_char(reader: &mut Reader) -> ParseResult<(char, String, Pos)> {
                             };
                             Err(ParseError::new(start.pos, true, kind))
                         } else {
-                            Ok((c, reader.peek_back(start.offset), start.pos))
+                            Ok((c, reader.read_from(start.offset), start.pos))
                         }
                     }
                 }
@@ -299,7 +299,7 @@ fn list_value(reader: &mut Reader) -> ParseResult<JsonValue> {
             literal(",", reader)?;
             // If there is one more comma, e.g. [1, 2,], it's better to report to the user because
             // this occurrence is common.
-            if reader.peek_ignoring_whitespace() == Some(']') {
+            if reader.peek_if(|c| !is_whitespace(c)) == Some(']') {
                 let kind = ParseErrorKind::Json(JsonErrorVariant::TrailingComma);
                 return Err(ParseError::new(save.pos, false, kind));
             }
@@ -346,7 +346,7 @@ pub fn object_value(reader: &mut Reader) -> ParseResult<JsonValue> {
             literal(",", reader)?;
             // If there is one more comma, e.g. {"a": "b",}, it's better to report to the user
             // because this occurrence is common.
-            if reader.peek_ignoring_whitespace() == Some('}') {
+            if reader.peek_if(|c| !is_whitespace(c)) == Some('}') {
                 let kind = ParseErrorKind::Json(JsonErrorVariant::TrailingComma);
                 return Err(ParseError::new(save.pos, false, kind));
             }
@@ -396,8 +396,12 @@ fn object_element(reader: &mut Reader) -> ParseResult<JsonObjectElement> {
     })
 }
 
+fn is_whitespace(c: char) -> bool {
+    c == ' ' || c == '\t' || c == '\n' || c == '\r'
+}
+
 fn whitespace(reader: &mut Reader) -> String {
-    reader.read_while(|c| *c == ' ' || *c == '\t' || *c == '\n' || *c == '\r')
+    reader.read_while(is_whitespace)
 }
 
 #[cfg(test)]
diff --git a/packages/hurl_core/src/parser/key_string.rs b/packages/hurl_core/src/parser/key_string.rs
index 48b8b2162..64fbbcef7 100644
--- a/packages/hurl_core/src/parser/key_string.rs
+++ b/packages/hurl_core/src/parser/key_string.rs
@@ -38,7 +38,7 @@ pub fn parse(reader: &mut Reader) -> ParseResult<Template> {
                     if value.is_empty() {
                         break;
                     }
-                    let encoded = reader.peek_back(start.offset);
+                    let encoded = reader.read_from(start.offset);
                     let element = TemplateElement::String { value, encoded };
                     elements.push(element);
                 } else {
diff --git a/packages/hurl_core/src/parser/number.rs b/packages/hurl_core/src/parser/number.rs
index 2cac4f5e7..b8dfa2322 100644
--- a/packages/hurl_core/src/parser/number.rs
+++ b/packages/hurl_core/src/parser/number.rs
@@ -102,7 +102,7 @@ pub fn number(reader: &mut Reader) -> ParseResult<Number> {
         }
         match format!("{sign}{integer_digits}.{decimal_digits}").parse() {
             Ok(value) => {
-                let encoded = reader.peek_back(start.offset);
+                let encoded = reader.read_from(start.offset);
                 Ok(Number::Float(Float { value, encoded }))
             }
             Err(_) => {
diff --git a/packages/hurl_core/src/parser/option.rs b/packages/hurl_core/src/parser/option.rs
index 07f270996..8b3ae8f6c 100644
--- a/packages/hurl_core/src/parser/option.rs
+++ b/packages/hurl_core/src/parser/option.rs
@@ -34,7 +34,7 @@ pub fn parse(reader: &mut Reader) -> ParseResult<EntryOption> {
     // enter the parsing of the option name and to have better error description (ex: 'max-redirs'
     // vs 'max_redirs').
     let option =
-        reader.read_while(|c| c.is_ascii_alphanumeric() || *c == '-' || *c == '.' || *c == '_');
+        reader.read_while(|c| c.is_ascii_alphanumeric() || c == '-' || c == '.' || c == '_');
     let space1 = zero_or_more_spaces(reader)?;
     try_literal(":", reader)?;
     let space2 = zero_or_more_spaces(reader)?;
@@ -371,7 +371,7 @@ fn variable_definition(reader: &mut Reader) -> ParseResult<VariableDefinition> {
 
 fn variable_name(reader: &mut Reader) -> ParseResult<String> {
     let start = reader.cursor();
-    let name = reader.read_while(|c| c.is_alphanumeric() || *c == '_' || *c == '-');
+    let name = reader.read_while(|c| c.is_alphanumeric() || c == '_' || c == '-');
     if name.is_empty() {
         let kind = ParseErrorKind::Expecting {
             value: "variable name".to_string(),
diff --git a/packages/hurl_core/src/parser/primitives.rs b/packages/hurl_core/src/parser/primitives.rs
index 01e96032f..4c1c45e83 100644
--- a/packages/hurl_core/src/parser/primitives.rs
+++ b/packages/hurl_core/src/parser/primitives.rs
@@ -261,7 +261,7 @@ pub fn hex(reader: &mut Reader) -> ParseResult<Hex> {
             ParseErrorKind::OddNumberOfHexDigits,
         ));
     }
-    let encoded = reader.peek_back(start.offset);
+    let encoded = reader.read_from(start.offset);
     let space1 = zero_or_more_spaces(reader)?;
     literal(";", reader)?;
 
diff --git a/packages/hurl_core/src/parser/string.rs b/packages/hurl_core/src/parser/string.rs
index 98da2cac6..50db08b8d 100644
--- a/packages/hurl_core/src/parser/string.rs
+++ b/packages/hurl_core/src/parser/string.rs
@@ -75,7 +75,7 @@ pub fn unquoted_template(reader: &mut Reader) -> ParseResult<Template> {
 // (decoding escape sequence)
 pub fn quoted_oneline_string(reader: &mut Reader) -> ParseResult<String> {
     literal("\"", reader)?;
-    let s = reader.read_while(|c| *c != '"' && *c != '\n');
+    let s = reader.read_while(|c| c != '"' && c != '\n');
     literal("\"", reader)?;
     Ok(s)
 }
@@ -156,7 +156,7 @@ pub fn backtick_template(reader: &mut Reader) -> ParseResult<Template> {
 fn any_char(except: Vec<char>, reader: &mut Reader) -> ParseResult<(char, String)> {
     let start = reader.cursor();
     match escape_char(reader) {
-        Ok(c) => Ok((c, reader.peek_back(start.offset))),
+        Ok(c) => Ok((c, reader.read_from(start.offset))),
         Err(e) => {
             if e.recoverable {
                 reader.seek(start);
@@ -176,7 +176,7 @@ fn any_char(except: Vec<char>, reader: &mut Reader) -> ParseResult<(char, String
                             };
                             Err(ParseError::new(start.pos, true, kind))
                         } else {
-                            Ok((c, reader.peek_back(start.offset)))
+                            Ok((c, reader.read_from(start.offset)))
                         }
                     }
                 }
diff --git a/packages/hurl_core/src/reader.rs b/packages/hurl_core/src/reader.rs
index c65b7a6e4..c44a88563 100644
--- a/packages/hurl_core/src/reader.rs
+++ b/packages/hurl_core/src/reader.rs
@@ -45,7 +45,7 @@
 /// ```
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Reader {
-    buffer: Vec<char>,
+    buf: Vec<char>,
     cursor: Cursor,
 }
 
@@ -81,7 +81,7 @@ impl Reader {
     /// Creates a new reader, position of the index is at the first char.
     pub fn new(s: &str) -> Self {
         Reader {
-            buffer: s.chars().collect(),
+            buf: s.chars().collect(),
             cursor: Cursor {
                 offset: 0,
                 pos: Pos { line: 1, column: 1 },
@@ -95,7 +95,7 @@ impl Reader {
     /// Note: the `buffer` offset is still initialized to 0.
     pub fn with_pos(s: &str, pos: Pos) -> Self {
         Reader {
-            buffer: s.chars().collect(),
+            buf: s.chars().collect(),
             cursor: Cursor { offset: 0, pos },
         }
     }
@@ -112,12 +112,12 @@ impl Reader {
 
     /// Returns true if the reader has read all the buffer, false otherwise.
     pub fn is_eof(&self) -> bool {
-        self.cursor.offset == self.buffer.len()
+        self.cursor.offset == self.buf.len()
     }
 
     /// Returns the next char from the buffer advancing the internal state.
     pub fn read(&mut self) -> Option<char> {
-        match self.buffer.get(self.cursor.offset) {
+        match self.buf.get(self.cursor.offset) {
             None => None,
             Some(c) => {
                 self.cursor.offset += 1;
@@ -147,14 +147,15 @@ impl Reader {
     }
 
     /// Returns chars from the buffer while `predicate` is true, advancing the internal state.
-    pub fn read_while(&mut self, predicate: fn(&char) -> bool) -> String {
+    pub fn read_while(&mut self, predicate: fn(char) -> bool) -> String {
         let mut s = String::new();
         loop {
             match self.peek() {
                 None => return s,
                 Some(c) => {
-                    if predicate(&c) {
-                        s.push(self.read().unwrap());
+                    if predicate(c) {
+                        _ = self.read();
+                        s.push(c);
                     } else {
                         return s;
                     }
@@ -163,39 +164,38 @@ impl Reader {
         }
     }
 
-    /// Returns the next char from the buffer without advancing the internal state.
-    pub fn peek(&self) -> Option<char> {
-        self.buffer.get(self.cursor.offset).copied()
+    /// Reads a string from a `start` position to the current position (excluded).
+    ///
+    /// This method doesn't modify the read index since we're reading "backwards" to the current
+    /// read index.
+    pub fn read_from(&self, start: usize) -> String {
+        let end = self.cursor.offset;
+        self.buf[start..end].iter().collect()
     }
 
-    /// Returns the next char ignoring whitespace without advancing the internal state.
-    pub fn peek_ignoring_whitespace(&self) -> Option<char> {
+    /// Peeks the next char from the buffer without advancing the internal state.
+    pub fn peek(&self) -> Option<char> {
+        self.buf.get(self.cursor.offset).copied()
+    }
+
+    /// Peeks the next char that meet a `predicate`.
+    pub fn peek_if(&self, predicate: fn(char) -> bool) -> Option<char> {
         let mut i = self.cursor.offset;
         loop {
-            if let Some(c) = self.buffer.get(i).copied() {
-                if c != ' ' && c != '\t' && c != '\n' && c != '\r' {
-                    return Some(c);
-                }
-            } else {
-                return None;
+            let &c = self.buf.get(i)?;
+            if predicate(c) {
+                return Some(c);
             }
             i += 1;
         }
     }
 
-    /// Reads a string of `count` char without advancing the internal state.
+    /// Peeks a string of `count` char without advancing the internal state.
     /// This methods can return less than `count` chars if there is not enough chars in the buffer.
     pub fn peek_n(&self, count: usize) -> String {
         let start = self.cursor.offset;
-        let end = (start + count).min(self.buffer.len());
-        self.buffer[start..end].iter().collect()
-    }
-
-    /// Reads a string backward from a `start` position to the current position (excluded), without
-    /// resetting the internal state.
-    pub fn peek_back(&self, start: usize) -> String {
-        let end = self.cursor.offset;
-        self.buffer[start..end].iter().collect()
+        let end = (start + count).min(self.buf.len());
+        self.buf[start..end].iter().collect()
     }
 }
 
@@ -233,7 +233,7 @@ mod tests {
         assert_eq!(reader.read(), Some('d'));
         assert_eq!(reader.read(), Some('e'));
         assert_eq!(reader.peek(), Some('f'));
-        assert_eq!(reader.peek_back(3), "de");
+        assert_eq!(reader.read_from(3), "de");
     }
 
     #[test]
@@ -280,4 +280,16 @@ mod tests {
             }
         );
     }
+
+    #[test]
+    fn peek_ignoring_whitespace() {
+        fn is_whitespace(c: char) -> bool {
+            c == ' ' || c == '\t'
+        }
+        let reader = Reader::new("\t\t\tabc");
+        assert_eq!(reader.peek_if(|c| !is_whitespace(c)), Some('a'));
+
+        let reader = Reader::new("foo");
+        assert_eq!(reader.peek_if(|c| !is_whitespace(c)), Some('f'));
+    }
 }