Extremely basic virtual host parser

I'm a Rust novice and would appreciate feedback on whether this code (including tests) is idiomatic:

I'm using this to parse virtual host information from approximately 5,000 Apache configuration files, most of which contain virtual host definitions. Note that I'm aware of apachectl -S, but I want the code to be able to obtain this information from configuration files ‘offline’ (without having an Apache instance running). The files are well-formatted, and each contains at most one virtual host definition, so I don't have any edge cases to handle — the code is giving the expected results in its current form.

You could simplify this test

#[test]
fn from_str_none() {
    match VirtualHost::from_str("") {
        None => (),
        _ => panic!("virtual host should not be parsed from empty string"),
    };
}

a little to

#[test]
fn from_str_none() {
    assert!(VirtualHost::from_str("").is_none());
}

I'd also rename the test to something like from_empty_string.

4 Likes

You should run cargo clippy to see what it suggests, for example it might suggest to write this:

            Some(VirtualHost {
                document_root,
                server_name,
                server_alias,
            })

Second - I'd make a parser that returns Result of some sort rather than Option. Currently you are ignoring unexpected values, normally you'd check and complain if something is unexpected.

3 Likes

Is a good practice use Self instead of the type name inside the implementations:

impl VirtualHost {
    /// Parses a `VirtualHost` from the specified Apache configuration text. Returns `None` if the
    /// text does not contain the details of a virtual host.
    pub fn from_str(configuration: &str) -> Option<Self> {
        let mut document_root = None;
        let mut server_name = None;
        let mut server_alias = Vec::new();

        for line in configuration.lines() {
            let mut tokens = line.split_whitespace();
            if let (Some(directive), Some(argument)) = (tokens.next(), tokens.next()) {
                match directive {
                    "DocumentRoot" => document_root = Some(argument.into()),
                    "ServerName" => server_name = Some(argument.to_owned()),
                    "ServerAlias" => server_alias.push(argument.to_owned()),
                    _ => (),
                }
            }
        }

        if let (Some(document_root), Some(server_name)) = (document_root, server_name) {
            Some(Self {
                document_root,
                server_name,
                server_alias,
            })
        } else {
            None
        }
    }
}
1 Like

Oops... I had previously set up rust-analyzer in VSCode to run clippy automatically, but in this particular repository the Rust workspace is two levels deep so it wasn't being automatically discovered. I’ve now configured linkedProjects and can see clippy is not happy with me.

Thank you everyone for your suggestions. I’ve implemented these, along with clippy's suggestion to implement the FromStr trait rather than just having an unrelated from_str method, and I’ve updated the gist with the revised code.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.