From c6347604eed5005587c1b3577d6102f6d3e1f0a6 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 10 Feb 2025 16:05:55 +0100 Subject: [PATCH] Replace accept_if function with next_if, eat_char and eat_not_char methods This makes the call sites slightly clearer and takes advantage of Peekable::next_if instead of manually reimplementing it. --- src/sudoers/ast.rs | 22 ++++++++++---------- src/sudoers/basic_parser.rs | 41 ++++++++++++++----------------------- src/sudoers/char_stream.rs | 22 ++++++++++++++------ 3 files changed, 42 insertions(+), 43 deletions(-) diff --git a/src/sudoers/ast.rs b/src/sudoers/ast.rs index 93d55e8fe..bc639c772 100644 --- a/src/sudoers/ast.rs +++ b/src/sudoers/ast.rs @@ -185,7 +185,7 @@ impl Sudo { /// ``` impl Parse for Identifier { fn parse(stream: &mut CharStream) -> Parsed { - if accept_if(|c| c == '#', stream).is_some() { + if stream.eat_char('#') { let Digits(guid) = expect_nonterminal(stream)?; make(Identifier::ID(guid)) } else { @@ -261,15 +261,15 @@ impl Parse for Meta { /// ``` impl Parse for UserSpecifier { fn parse(stream: &mut CharStream) -> Parsed { - let userspec = if accept_if(|c| c == '%', stream).is_some() { - let ctor = if accept_if(|c| c == ':', stream).is_some() { + let userspec = if stream.eat_char('%') { + let ctor = if stream.eat_char(':') { UserSpecifier::NonunixGroup } else { UserSpecifier::Group }; // in this case we must fail 'hard', since input has been consumed ctor(expect_nonterminal(stream)?) - } else if accept_if(|c| c == '+', stream).is_some() { + } else if stream.eat_char('+') { // TODO Netgroups unrecoverable!(stream, "netgroups are not supported yet"); } else { @@ -489,7 +489,7 @@ impl Parse for Sudo { // but accept: // "user, User_Alias machine = command"; this does the same fn parse(stream: &mut CharStream) -> Parsed { - if accept_if(|c| c == '@', stream).is_some() { + if stream.eat_char('@') { return parse_include(stream); } @@ -512,7 +512,7 @@ impl Parse for Sudo { // the failed "try_nonterminal::" will have consumed the '#' // the most ignominious part of sudoers: having to parse bits of comments parse_include(stream).or_else(|_| { - while accept_if(|c| c != '\n', stream).is_some() {} + while stream.eat_not_char('\n') {} make(Sudo::LineComment) }) }; @@ -541,7 +541,7 @@ impl Parse for Sudo { /// Parse the include/include dir part that comes after the '#' or '@' prefix symbol fn parse_include(stream: &mut CharStream) -> Parsed { fn get_path(stream: &mut CharStream, key_pos: (usize, usize)) -> Parsed<(String, Span)> { - let path = if accept_if(|c| c == '"', stream).is_some() { + let path = if stream.eat_char('"') { let QuotedInclude(path) = expect_nonterminal(stream)?; expect_syntax('"', stream)?; path @@ -643,8 +643,8 @@ impl Parse for defaults::SettingsModifier { let id_pos = stream.get_pos(); // Parse multiple entries enclosed in quotes (for list-like Defaults-settings) - let parse_vars = |stream: &mut _| -> Parsed> { - if accept_if(|c| c == '"', stream).is_some() { + let parse_vars = |stream: &mut CharStream| -> Parsed> { + if stream.eat_char('"') { let mut result = Vec::new(); while let Some(EnvVar(name)) = try_nonterminal(stream)? { result.push(name); @@ -681,8 +681,8 @@ impl Parse for defaults::SettingsModifier { }; // Parse a text parameter - let text_item = |stream: &mut _| { - if accept_if(|c| c == '"', stream).is_some() { + let text_item = |stream: &mut CharStream| { + if stream.eat_char('"') { let QuotedText(text) = expect_nonterminal(stream)?; expect_syntax('"', stream)?; make(text) diff --git a/src/sudoers/basic_parser.rs b/src/sudoers/basic_parser.rs index 23ccd3257..a26c02b80 100644 --- a/src/sudoers/basic_parser.rs +++ b/src/sudoers/basic_parser.rs @@ -85,21 +85,6 @@ pub trait Parse { Self: Sized; } -/// Primitive function: accepts one character that satisfies `predicate`. This is used in the majority -/// of all the other `Parse` implementations instead of interfacing with the iterator directly -/// (this can facilitate an easy switch to a different method of stream representation in the future). -/// Unlike `Parse` implementations this *does not* consume trailing whitespace. -/// This function is modelled on `next_if` on `std::Peekable`. -pub fn accept_if(predicate: impl Fn(char) -> bool, stream: &mut CharStream) -> Option { - let c = stream.peek()?; - if predicate(c) { - stream.advance(); - Some(c) - } else { - None - } -} - /// Structures representing whitespace (trailing whitespace can contain comments) #[cfg_attr(test, derive(PartialEq, Eq))] struct LeadingWhitespace; @@ -114,7 +99,7 @@ struct Comment; /// (which can be used to detect end-of-input). impl Parse for LeadingWhitespace { fn parse(stream: &mut CharStream) -> Parsed { - let eat_space = |stream: &mut _| accept_if(|c| "\t ".contains(c), stream); + let eat_space = |stream: &mut CharStream| stream.next_if(|c| "\t ".contains(c)); while eat_space(stream).is_some() {} if stream.peek().is_some() { @@ -134,9 +119,9 @@ impl Parse for TrailingWhitespace { let _ = LeadingWhitespace::parse(stream); // don't propagate any errors // line continuations - if accept_if(|c| c == '\\', stream).is_some() { + if stream.eat_char('\\') { // do the equivalent of expect_syntax('\n', stream)?, without recursion - if accept_if(|c| c == '\n', stream).is_none() { + if !stream.eat_char('\n') { unrecoverable!(stream, "stray escape sequence") } } else { @@ -151,8 +136,10 @@ impl Parse for TrailingWhitespace { /// Parses a comment impl Parse for Comment { fn parse(stream: &mut CharStream) -> Parsed { - accept_if(|c| c == '#', stream).ok_or(Status::Reject)?; - while accept_if(|c| c != '\n', stream).is_some() {} + if !stream.eat_char('#') { + return Err(Status::Reject); + } + while stream.eat_not_char('\n') {} make(Comment {}) } } @@ -164,7 +151,9 @@ fn skip_trailing_whitespace(stream: &mut CharStream) -> Parsed<()> { /// Adheres to the contract of the [Parse] trait, accepts one character and consumes trailing whitespace. pub fn try_syntax(syntax: char, stream: &mut CharStream) -> Parsed<()> { - accept_if(|c| c == syntax, stream).ok_or(Status::Reject)?; + if !stream.eat_char(syntax) { + return Err(Status::Reject); + } skip_trailing_whitespace(stream)?; make(()) } @@ -237,15 +226,15 @@ impl Parse for T { stream: &mut CharStream, ) -> Parsed { const ESCAPE: char = '\\'; - if T::ALLOW_ESCAPE && accept_if(|c| c == ESCAPE, stream).is_some() { - if let Some(c) = accept_if(T::escaped, stream) { + if T::ALLOW_ESCAPE && stream.eat_char(ESCAPE) { + if let Some(c) = stream.next_if(T::escaped) { Ok(c) } else if pred(ESCAPE) { Ok(ESCAPE) } else { unrecoverable!(stream, "illegal escape sequence") } - } else if let Some(c) = accept_if(pred, stream) { + } else if let Some(c) = stream.next_if(pred) { Ok(c) } else { reject() @@ -325,7 +314,7 @@ where result.push(item); let _ = maybe(Comment::parse(stream)); - if accept_if(|c| c == '\n', stream).is_none() { + if !stream.eat_char('\n') { if parsed_item_ok { let msg = if stream.peek().is_none() { "missing line terminator at end of file" @@ -335,7 +324,7 @@ where let error = |stream: &mut CharStream| unrecoverable!(stream, "{msg}"); result.push(error(stream)); } - while accept_if(|c| c != '\n', stream).is_some() {} + while stream.eat_not_char('\n') {} } } diff --git a/src/sudoers/char_stream.rs b/src/sudoers/char_stream.rs index 63f768bdf..59b314aab 100644 --- a/src/sudoers/char_stream.rs +++ b/src/sudoers/char_stream.rs @@ -15,8 +15,9 @@ impl<'a> CharStream<'a> { } impl CharStream<'_> { - pub fn advance(&mut self) { - match self.iter.next() { + pub fn next_if(&mut self, f: impl FnOnce(char) -> bool) -> Option { + let item = self.iter.next_if(|&c| f(c)); + match item { Some('\n') => { self.line += 1; self.col = 1; @@ -24,6 +25,15 @@ impl CharStream<'_> { Some(_) => self.col += 1, _ => {} } + item + } + + pub fn eat_char(&mut self, expect_char: char) -> bool { + self.next_if(|c| c == expect_char).is_some() + } + + pub fn eat_not_char(&mut self, expect_not_char: char) -> bool { + self.next_if(|c| c != expect_not_char).is_some() } pub fn peek(&mut self) -> Option { @@ -43,12 +53,12 @@ mod test { fn test_iter() { let mut stream = CharStream::new("12\n3\n".chars()); assert_eq!(stream.peek(), Some('1')); - stream.advance(); + assert!(stream.eat_char('1')); assert_eq!(stream.peek(), Some('2')); - stream.advance(); - stream.advance(); + assert!(stream.eat_char('2')); + assert!(stream.eat_char('\n')); assert_eq!(stream.peek(), Some('3')); - stream.advance(); + assert!(stream.eat_char('3')); assert_eq!(stream.get_pos(), (2, 2)); } }