Move to new label parser in server

Cargo.lock

@@ -1508,6 +1508,7 @@ dependencies = [
  "lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "openssl 0.10.16 (registry+https://github.com/rust-lang/crates.io-index)",
+ "parser 0.1.0",
  "regex 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "reqwest 0.9.9 (registry+https://github.com/rust-lang/crates.io-index)",
  "rocket 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",

Cargo.toml

@@ -5,7 +5,6 @@ authors = ["Mark Rousskov <mark.simulacrum@gmail.com>"]
 edition = "2018"
 
 [workspace]
-members = ["parser"]
 
 [dependencies]
 rocket = "0.4"
@@ -20,6 +19,7 @@ log = "0.4"
 failure = "0.1"
 hex = "0.3.2"
 env_logger = "0.6"
+parser = { path = "parser" }
 
 [dependencies.serde]
 version = "1"

parser/src/command.rs

@@ -9,8 +9,9 @@ pub fn find_commmand_start(input: &str, bot: &str) -> Option<usize> {
 }
 
 #[derive(Debug)]
-pub enum Command {
-    Label(label::LabelCommand),
+pub enum Command<'a> {
+    Label(Result<label::LabelCommand, Error<'a>>),
+    None,
 }
 
 #[derive(Debug)]
@@ -31,10 +32,10 @@ impl<'a> Input<'a> {
         }
     }
 
-    pub fn parse_command(&mut self) -> Result<Option<Command>, Error<'a>> {
+    pub fn parse_command(&mut self) -> Command<'a> {
         let start = match find_commmand_start(&self.all[self.parsed..], self.bot) {
             Some(pos) => pos,
-            None => return Ok(None),
+            None => return Command::None,
         };
         self.parsed += start;
         let mut tok = Tokenizer::new(&self.all[self.parsed..]);
@@ -45,15 +46,18 @@ impl<'a> Input<'a> {
 
         let mut success = vec![];
 
+        let original_tokenizer = tok.clone();
+
         {
-            let mut lc = tok.clone();
-            let res = label::LabelCommand::parse(&mut lc)?;
+            let mut tok = original_tokenizer.clone();
+            let res = label::LabelCommand::parse(&mut tok);
             match res {
-                None => {}
-                Some(cmd) => {
-                    // save tokenizer off
-                    tok = lc;
-                    success.push(Command::Label(cmd));
+                Ok(None) => {}
+                Ok(Some(cmd)) => {
+                    success.push((tok, Command::Label(Ok(cmd))));
+                }
+                Err(err) => {
+                    success.push((tok, Command::Label(Err(err))));
                 }
             }
         }
@@ -70,12 +74,39 @@ impl<'a> Input<'a> {
             .code
             .overlaps_code((self.parsed)..(self.parsed + tok.position()))
         {
-            return Ok(None);
+            return Command::None;
         }
 
-        self.parsed += tok.position();
+        match success.pop() {
+            Some((mut tok, c)) => {
+                // if we errored out while parsing the command do not move the input forwards
+                if c.is_ok() {
+                    self.parsed += tok.position();
+                }
+                c
+            }
+            None => Command::None,
+        }
+    }
+}
 
-        Ok(success.pop())
+impl<'a> Command<'a> {
+    pub fn is_ok(&self) -> bool {
+        match self {
+            Command::Label(r) => r.is_ok(),
+            Command::None => true,
+        }
+    }
+
+    pub fn is_err(&self) -> bool {
+        !self.is_ok()
+    }
+
+    pub fn is_none(&self) -> bool {
+        match self {
+            Command::None => true,
+            _ => false,
+        }
     }
 }
 
@@ -91,7 +122,7 @@ fn errors_outside_command_are_fine() {
 fn code_1() {
     let input = "`@bot modify labels: +bug.`";
     let mut input = Input::new(input, "bot");
-    assert!(input.parse_command().unwrap().is_none());
+    assert!(input.parse_command().is_none());
 }
 
 #[test]
@@ -100,14 +131,15 @@ fn code_2() {
     @bot modify labels: +bug.
     ```";
     let mut input = Input::new(input, "bot");
-    assert!(input.parse_command().unwrap().is_none());
+    assert!(input.parse_command().is_none());
 }
 
 #[test]
 fn move_input_along() {
     let input = "@bot modify labels: +bug. Afterwards, delete the world.";
     let mut input = Input::new(input, "bot");
-    assert!(input.parse_command().unwrap().is_some());
+    let parsed = input.parse_command();
+    assert!(parsed.is_ok());
     assert_eq!(&input.all[input.parsed..], " Afterwards, delete the world.");
 }
 

parser/src/command/label.rs

@@ -30,7 +30,7 @@ use std::error::Error as _;
 use std::fmt;
 
 #[derive(Debug)]
-pub struct LabelCommand(Vec<LabelDelta>);
+pub struct LabelCommand(pub Vec<LabelDelta>);
 
 #[derive(Debug, PartialEq, Eq)]
 pub enum LabelDelta {

src/github.rs

@@ -133,13 +133,23 @@ impl RequestSend for RequestBuilder {
 
 #[derive(Clone)]
 pub struct GithubClient {
+    username: String,
     token: String,
     client: Client,
 }
 
 impl GithubClient {
     pub fn new(c: Client, token: String) -> Self {
-        GithubClient { client: c, token }
+        // XXX: configuration for username
+        GithubClient {
+            client: c,
+            token,
+            username: String::from("rust-highfive"),
+        }
+    }
+
+    pub fn username(&self) -> &str {
+        self.username.as_str()
     }
 
     fn get(&self, url: &str) -> RequestBuilder {

src/handlers/label.rs

@@ -1,20 +1,20 @@
 //! Purpose: Allow any user to modify issue labels on GitHub via comments.
 //!
-//! The current syntax allows adding labels (+labelname or just labelname) following the
-//! `/label` prefix. Users can also remove labels with -labelname.
-//!
 //! Labels are checked against the labels in the project; the bot does not support creating new
 //! labels.
 //!
-//! There will be no feedback beyond the label change to reduce notification noise.
+//! Parsing is done in the `parser::command::label` module.
+//!
+//! If the command was successful, there will be no feedback beyond the label change to reduce
+//! notification noise.
 
 use crate::{
-    github::{GithubClient, Label},
+    github::{self, GithubClient},
     registry::{Event, Handler},
 };
 use failure::Error;
-use lazy_static::lazy_static;
-use regex::Regex;
+use parser::command::label::{LabelCommand, LabelDelta};
+use parser::command::{Command, Input};
 
 pub struct LabelHandler {
     pub client: GithubClient,
@@ -30,36 +30,34 @@ impl Handler for LabelHandler {
             return Ok(());
         };
 
-        lazy_static! {
-            static ref LABEL_RE: Regex = Regex::new(r#"/label (\S+\s*)+"#).unwrap();
-        }
-
         let mut issue_labels = event.issue.labels().to_owned();
 
+        let mut input = Input::new(&event.comment.body, self.client.username());
+        let deltas = match input.parse_command() {
+            Command::Label(Ok(LabelCommand(deltas))) => deltas,
+            Command::Label(Err(_)) => {
+                // XXX: inform user of error
+                return Ok(());
+            }
+            _ => return Ok(()),
+        };
+
         let mut changed = false;
-        for label_block in LABEL_RE.find_iter(&event.comment.body) {
-            let label_block = &label_block.as_str()["label: ".len()..]; // guaranteed to start with this
-            for label in label_block.split_whitespace() {
-                if label.starts_with('-') {
-                    if let Some(label) = issue_labels.iter().position(|el| el.name == &label[1..]) {
+        for delta in &deltas {
+            match delta {
+                LabelDelta::Add(label) => {
+                    if !issue_labels.iter().any(|l| l.name == label.as_str()) {
+                        changed = true;
+                        issue_labels.push(github::Label {
+                            name: label.to_string(),
+                        });
+                    }
+                }
+                LabelDelta::Remove(label) => {
+                    if let Some(pos) = issue_labels.iter().position(|l| l.name == label.as_str()) {
                         changed = true;
-                        issue_labels.remove(label);
-                    } else {
-                        // do nothing, if the user attempts to remove a label that's not currently
-                        // set simply skip it
+                        issue_labels.remove(pos);
                     }
-                } else if label.starts_with('+') {
-                    // add this label, but without the +
-                    changed = true;
-                    issue_labels.push(Label {
-                        name: label[1..].to_string(),
-                    });
-                } else {
-                    // add this label (literally)
-                    changed = true;
-                    issue_labels.push(Label {
-                        name: label.to_string(),
-                    });
                 }
             }
         }