More validation of recipes, allow leading shebang

notes

@@ -1,24 +1,29 @@
 notes
 -----
 
+polyglot:
+- recipes can have shebangs on first line
+- complain if there is extra leading whitespace in a non-shebang recipe
+- extract and run script
+- preserve line numbers
+
+- change name to "a polyglot command runner"
 - comment code
 - fix docs (note that shell is invoked with -cu)
 - publish to github and cargo
+- spam facebook, reddit
 
-polyglot:
+wishlist:
-- recipes can have shebangs
+- preludes:
-- extract and run script
+  may be nice to allow all recipes in a given langauge to share
-- preserve line numbers
+  functions, variables, etc. could have a "prelude" recipe
-- special 'prelude recipe"
+  which was included as a prefix to other recipes
-  . allow launching binaries from cargo
+- windows support: currently calling 'sh', which won't work on windows
-  . script until --
-  . all recipes are then in that language?
-
-extras:
 - args can be passed after --, or with some special syntax:
   a: 1 2 3 :
 - should also add an annotation for recipes
   a FOO BAR, export variables FOO and BAR with args
+  fail if doesn't get two arguments
 - indent for line continuation
 - use launch recipes asyncronously
 - ~/.justfile: 

src/lib.rs

@@ -42,21 +42,23 @@ fn re(pattern: &str) -> Regex {
 }
 
 pub struct Recipe<'a> {
-  line:               usize,
+  line_number:        usize,
+  label:              &'a str,
   name:               &'a str,
   leading_whitespace: &'a str,
-  commands:           Vec<&'a str>,
+  lines:              Vec<&'a str>,
   dependencies:       BTreeSet<&'a str>,
+  shebang:            bool,
 }
 
 impl<'a> Display for Recipe<'a> {
   fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-    try!(writeln!(f, "{}:", self.name));
+    try!(writeln!(f, "{}", self.label));
-    for (i, command) in self.commands.iter().enumerate() {
+    for (i, line) in self.lines.iter().enumerate() {
-      if i + 1 < self.commands.len() {
+      if i + 1 < self.lines.len() {
-        try!(writeln!(f, "    {}", command));
+        try!(writeln!(f, "    {}", line));
       } {
-        try!(write!(f, "    {}", command));
+        try!(write!(f, "    {}", line));
       }
     }
     Ok(())
@@ -79,7 +81,8 @@ fn error_from_signal<'a>(recipe: &'a str, exit_status: process::ExitStatus) -> R
 
 impl<'a> Recipe<'a> {
   fn run(&self) -> Result<(), RunError<'a>> {
-    for command in &self.commands {
+    // TODO: if shebang, run as script
+    for command in &self.lines {
       let mut command = *command;
       if !command.starts_with("@") {
         warn!("{}", command);
@@ -126,7 +129,7 @@ fn resolve<'a>(
         if seen.contains(dependency.name) {
           let first = stack[0];
           stack.push(first);
-          return Err(error(text, recipe.line, ErrorKind::CircularDependency {
+          return Err(error(text, recipe.line_number, ErrorKind::CircularDependency {
             circle: stack.iter()
               .skip_while(|name| **name != dependency.name)
               .cloned().collect()
@@ -134,7 +137,7 @@ fn resolve<'a>(
         }
         return resolve(text, recipes, resolved, seen, stack, dependency);
       },
-      None => return Err(error(text, recipe.line, ErrorKind::UnknownDependency {
+      None => return Err(error(text, recipe.line_number, ErrorKind::UnknownDependency {
         name:    recipe.name,
         unknown: dependency_name
       })),
@@ -160,8 +163,10 @@ enum ErrorKind<'a> {
   DuplicateRecipe{first: usize, name: &'a str},
   TabAfterSpace{whitespace: &'a str},
   MixedLeadingWhitespace{whitespace: &'a str},
+  ExtraLeadingWhitespace,
   InconsistentLeadingWhitespace{expected: &'a str, found: &'a str},
-  Shebang,
+  OuterShebang,
+  NonLeadingShebang{recipe: &'a str},
   UnknownDependency{name: &'a str, unknown: &'a str},
   Unparsable,
   UnparsableDependencies,
@@ -228,14 +233,20 @@ impl<'a> Display for Error<'a> {
           show_whitespace(whitespace)
         ));
       }
+      ErrorKind::ExtraLeadingWhitespace => {
+        try!(writeln!(f, "line has extra leading whitespace"));
+      }
       ErrorKind::InconsistentLeadingWhitespace{expected, found} => {
         try!(writeln!(f,
           "inconsistant leading whitespace: recipe started with {} but found line with {}:",
           show_whitespace(expected), show_whitespace(found)
         ));
       }
-      ErrorKind::Shebang => {
+      ErrorKind::OuterShebang => {
-        try!(writeln!(f, "shebang \"#!\" is reserved syntax"))
+        try!(writeln!(f, "a shebang \"#!\" is reserved syntax outside of recipes"))
+      }
+      ErrorKind::NonLeadingShebang{..} => {
+        try!(writeln!(f, "a shebang \"#!\" may only appear on the first line of a recipe"))
       }
       ErrorKind::UnknownDependency{name, unknown} => {
         try!(writeln!(f, "recipe {} has unknown dependency {}", name, unknown));
@@ -266,7 +277,7 @@ impl<'a> Justfile<'a> {
     let mut first: Option<&Recipe<'a>> = None;
     for (_, recipe) in self.recipes.iter() {
       if let Some(first_recipe) = first {
-        if recipe.line < first_recipe.line {
+        if recipe.line_number < first_recipe.line_number {
           first = Some(recipe)
         }
       } else {
@@ -374,8 +385,6 @@ pub fn parse<'a>(text: &'a str) -> Result<Justfile, Error> {
   for (i, line) in text.lines().enumerate() {
     if blank_re.is_match(line) {
       continue;
-    } else if shebang_re.is_match(line) {
-      return Err(error(text, i, ErrorKind::Shebang));
     }
 
     if let Some(mut recipe) = current_recipe {
@@ -399,7 +408,7 @@ pub fn parse<'a>(text: &'a str) -> Result<Justfile, Error> {
               found:    leading_whitespace,
             }));
           }
-          recipe.commands.push(line.split_at(recipe.leading_whitespace.len()).1);
+          recipe.lines.push(line.split_at(recipe.leading_whitespace.len()).1);
           current_recipe = Some(recipe);
           continue;
         },
@@ -412,6 +421,8 @@ pub fn parse<'a>(text: &'a str) -> Result<Justfile, Error> {
     
     if comment_re.is_match(line) {
       // ignore
+    } else if shebang_re.is_match(line) {
+      return Err(error(text, i, ErrorKind::OuterShebang));
     } else if let Some(captures) = label_re.captures(line) {
       let name = captures.at(1).unwrap();
       if !name_re.is_match(name) {
@@ -421,7 +432,7 @@ pub fn parse<'a>(text: &'a str) -> Result<Justfile, Error> {
       }
       if let Some(recipe) = recipes.get(name) {
         return Err(error(text, i, ErrorKind::DuplicateRecipe {
-          first: recipe.line,
+          first: recipe.line_number,
           name: name,
         }));
       }
@@ -442,11 +453,13 @@ pub fn parse<'a>(text: &'a str) -> Result<Justfile, Error> {
       }
 
       current_recipe = Some(Recipe{
-        line:               i,
+        line_number:        i,
+        label:              line,
         name:               name,
         leading_whitespace: "",
-        commands:           vec![],
+        lines:              vec![],
-        dependencies:      dependencies,
+        dependencies:       dependencies,
+        shebang:            false,
       });
     } else {
       return Err(error(text, i, ErrorKind::Unparsable));
@@ -457,6 +470,24 @@ pub fn parse<'a>(text: &'a str) -> Result<Justfile, Error> {
     recipes.insert(recipe.name, recipe);
   }
 
+  let leading_whitespace_re = re(r"^\s+");
+
+  for recipe in recipes.values_mut() {
+    for (i, line) in recipe.lines.iter().enumerate() {
+      let line_number = recipe.line_number + 1 + i;
+      if shebang_re.is_match(line) {
+        if i == 0 {
+          recipe.shebang = true;
+        } else {
+          return Err(error(text, line_number, ErrorKind::NonLeadingShebang{recipe: recipe.name}));
+        }
+      }
+      if !recipe.shebang && leading_whitespace_re.is_match(line) {
+        return Err(error(text, line_number, ErrorKind::ExtraLeadingWhitespace));
+      }
+    }
+  }
+
   let mut resolved = HashSet::new();
   let mut seen     = HashSet::new();
   let mut stack    = vec![];

src/tests.rs

@@ -22,7 +22,7 @@ fn check_recipe(
   name: &str,
   line: usize,
   leading_whitespace: &str,
-  commands: &[&str],
+  lines: &[&str],
   dependencies: &[&str]
 ) {
   let recipe = match justfile.recipes.get(name) {
@@ -30,9 +30,9 @@ fn check_recipe(
     None => panic!("Justfile had no recipe \"{}\"", name),
   };
   assert_eq!(recipe.name, name);
-  assert_eq!(recipe.line, line);
+  assert_eq!(recipe.line_number, line);
   assert_eq!(recipe.leading_whitespace, leading_whitespace);
-  assert_eq!(recipe.commands, commands);
+  assert_eq!(recipe.lines, lines);
   assert_eq!(recipe.dependencies.iter().cloned().collect::<Vec<_>>(), dependencies);
 }
 
@@ -87,8 +87,8 @@ fn inconsistent_leading_whitespace() {
 
 #[test]
 fn shebang() {
-  expect_error("#!/bin/sh", 0, ErrorKind::Shebang);
+  expect_error("#!/bin/sh", 0, ErrorKind::OuterShebang);
-  expect_error("a:\n #!/bin/sh", 1, ErrorKind::Shebang);
+  expect_error("a:\n echo hello\n #!/bin/sh", 2, ErrorKind::NonLeadingShebang{recipe:"a"});
 }
 
 #[test]
@@ -96,6 +96,12 @@ fn unknown_dependency() {
   expect_error("a: b", 0, ErrorKind::UnknownDependency{name: "a", unknown: "b"});
 }
 
+#[test]
+fn extra_whitespace() {
+  expect_error("a:\n blah\n  blarg", 2, ErrorKind::ExtraLeadingWhitespace);
+  expect_success("a:\n #!\n  print(1)");
+}
+
 #[test]
 fn unparsable() {
   expect_error("hello", 0, ErrorKind::Unparsable);