2
\$\begingroup\$

I'm working on a library that, in essence, splits a string at whitespace, with extra rules:

  • Consecutive whitespace is collapsed, and trimmed at the start and end.
  • Whitespace wrapped in quotes is not collapsed or split.
  • Backslash escaping can be used to include a single quote or whitespace character (or any other character after it) as-is, including inside of a pair of quotes.

The source code of this project is on GitHub - though I will outline the parts I am most concerned with here.

The main component users use is the Command struct and its from_str function, which:

  1. Prefixes a single space in front of the string, such as to force the WhitespaceBlock to be the first-used "syntax block" (explained later), and ensure at least one empty "token" (split string segment) is present for characters to be placed in.
     let mut input = String::from(" ");
     input.push_str(s);
  1. Iterates over each character and attempts to parse it, providing a list of available "syntax blocks" that might be useful for this character.
     let mut data = ParserData::new(&input);
     while data.not_empty() {
         handle_or_push(&mut data, &vec![ &EscapeBlock{}, &QuoteBlock{}, &WhitespaceBlock{} ]);
     }
     let mut tokens = data.get_result().clone();
  1. Deletes the last token if it is entirely empty (which it will be if either no valid characters were provided, OR if the input string ends with whitespace.)
     // Prevents whitespace at the end of the command from creating an empty garbage argument.
     if tokens.last().unwrap().is_empty() {
         tokens.pop();
     }
  1. Produces an instance of Command and returns it if it is valid.

The concept of a SyntaxBlock is any sort of rule that might come into effect while reading characters from a string. A SyntaxBlock's consume method checks if the next input character is applicable, and if so, "eat"s (removes) it and continues to "eat" further applicable characters. A consume method can use the handle_or_push method nested within it to allow for nested syntax rules. Because this method is responsible for both checking if it is applicable and then proceeding to consume characters, it will return true if it has consumed characters, and false otherwise (allowing handle_or_push to keep checking other syntax rules before giving up and copying a character verbatim.)

For example, here is QuoteBlock's behavior:

pub struct QuoteBlock;

impl SyntaxBlock for QuoteBlock {
    fn consume(&self, input: &mut ParserData) -> bool {
        // If we see a quote, aka the start of a QuoteBlock...
        if input.peek().unwrap() == '"' {
            // We eat up that quote
            input.eat().unwrap();

            // We eat up the quoted area's contents and try to handle them.
            // The `EscapeBlock` is the only one applicable inside of quotes
            // and everything else should just be eaten and pushed.
            while input.not_empty() && input.peek().unwrap() != '"' {
                handle_or_push(input, &vec![ &EscapeBlock{} ]);
            }

            // We eat the final quote character - unwrap_or_default ensures
            // this doesn't fail if we've actually reached the end of input
            // instead of a closing quote.
            input.eat().unwrap_or_default();

            // Return true because we ate stuff and the next character should
            // be interpreted from the start of the ruleset again
            true
        } else {
            // Return false because we didn't see a quote block
            false
        }
    }
}

handle_or_push takes in a list of applicable SyntaxBlocks and checks if the next character applies to any of them, in order. If not, it just eats the character and pushes it to the output instead.

/// `handle_blocks` iterates over a set of SyntaxBlock objects, attempting to consume data from each
/// one in the given order. If any match, it will stop iterating and return true. If no matches are
/// found, it returns false and does not eat any input.
///
/// This behavior is used to check if any special syntax blocks can be used at the moment.
pub fn handle_blocks(input: &mut ParserData, types: &Vec<&dyn SyntaxBlock>) -> bool {
    if input.not_empty() {
        for t in types {
            if t.consume(input) { return true; }
        }
    }

    false
}

/// `handle_or_push` tests if the given SyntaxBlocks are able to consume available input, in order,
/// stopping when a block successfully eats one or more characters. If no blocks eat characters,
/// `handle_or_push` will instead eat the first available character and push it to the output.
///
/// This behavior is used to handle any nested syntax blocks where plaintext should be pushed.
pub fn handle_or_push(input: &mut ParserData, types: &Vec<&dyn SyntaxBlock>) {
    if !handle_blocks(input, types) {
        input.eat_and_push().unwrap();
    }
}

ParserData stores the input and output, and looks like this:

pub struct ParserData {
    input: String,
    output: Vec<String>,
    offset: usize
}

impl ParserData {
    pub fn new(input: &String) -> ParserData {
        ParserData { input: input.clone(), output: Vec::new(), offset: 0 }
    }

    pub fn eat(&mut self) -> Result<char, OutOfInputError> {
        let result = self.peek()?;
        self.offset += 1;
        Ok(result)
    }

    pub fn eat_and_push(&mut self) -> Result<(), OutOfInputError> {
        let result = self.eat();
        self.push(result?);
        Ok(())
    }

    pub fn peek(&self) -> Result<char, OutOfInputError> {
        self.input.chars().nth(self.offset).ok_or(OutOfInputError)
    }

    pub fn not_empty(&self) -> bool {
        self.offset < self.input.len()
    }

    pub fn new_token(&mut self) {
        self.output.push(String::new());
    }

    pub fn push(&mut self, c: char) {
        self.output.last_mut().unwrap().push(c);
    }

    pub fn get_result(&self) -> &Vec<String> {
        &self.output
    }
}

My main concern in this whole thing is the entry-point and how it has to mess with the input in order for it to be valid, before the actual parsing system takes over. I'm also concerned with the mutable input-output structure I pass around, though I can't think of a better way to do that.

Would you consider this "good code" in rust? I'm new to the language and honestly, some of the things I had to do to make it work for me seem a bit messy. I'm used to separately working with functional and OOP languages but I'm not sure about the way rust is meant to be used, so to speak.

\$\endgroup\$
  • \$\begingroup\$ Whean creating a custom lexer/parser, I strongly advise writing a bunch of unit tests to test each block against all kinds of input. This facilitates writing the parser, but also us reviewing your code :) \$\endgroup\$ – dfhwze Aug 6 '19 at 20:32
  • \$\begingroup\$ @dfhwze Good idea. I excluded quite a few tests from this source, though those tests all provide input to the main component rather than individually testing certain blocks. I should add some block-specific tests. \$\endgroup\$ – Ethan McTague Aug 6 '19 at 20:33
  • \$\begingroup\$ And don't be shy to include unit tests in the question :) \$\endgroup\$ – dfhwze Aug 6 '19 at 20:35
  • \$\begingroup\$ @dfhwze Just wanted to avoid jamming a whole bunch of code into the question. I'm not really a big fan of unit testing (I only code for fun for now and most of my stuff is way too simple to warrant it). I'll be sure to include them in further Q's! \$\endgroup\$ – Ethan McTague Aug 6 '19 at 20:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.