From 28a57d98288761c3c732dea407cf7a6e1eef9c9c Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Sat, 18 Nov 2017 01:18:04 -0800 Subject: [PATCH] Refactor evaluators and resolvers into common form (#258) --- src/assignment_evaluator.rs | 50 +++++++-------- src/assignment_resolver.rs | 42 ++++++------- src/justfile.rs | 4 +- src/main.rs | 3 + src/parser.rs | 6 +- src/recipe_resolver.rs | 118 ++++++++++++++++++------------------ 6 files changed, 111 insertions(+), 112 deletions(-) diff --git a/src/assignment_evaluator.rs b/src/assignment_evaluator.rs index 333ecd4..9183465 100644 --- a/src/assignment_evaluator.rs +++ b/src/assignment_evaluator.rs @@ -2,31 +2,6 @@ use common::*; use brev; -pub fn evaluate_assignments<'a>( - assignments: &Map<&'a str, Expression<'a>>, - overrides: &Map<&str, &str>, - quiet: bool, - shell: &'a str, - dry_run: bool, -) -> RunResult<'a, Map<&'a str, String>> { - let mut evaluator = AssignmentEvaluator { - assignments: assignments, - evaluated: empty(), - exports: &empty(), - overrides: overrides, - quiet: quiet, - scope: &empty(), - shell: shell, - dry_run: dry_run, - }; - - for name in assignments.keys() { - evaluator.evaluate_assignment(name)?; - } - - Ok(evaluator.evaluated) -} - pub struct AssignmentEvaluator<'a: 'b, 'b> { pub assignments: &'b Map<&'a str, Expression<'a>>, pub evaluated: Map<&'a str, String>, @@ -39,6 +14,31 @@ pub struct AssignmentEvaluator<'a: 'b, 'b> { } impl<'a, 'b> AssignmentEvaluator<'a, 'b> { + pub fn evaluate_assignments( + assignments: &Map<&'a str, Expression<'a>>, + overrides: &Map<&str, &str>, + quiet: bool, + shell: &'a str, + dry_run: bool, + ) -> RunResult<'a, Map<&'a str, String>> { + let mut evaluator = AssignmentEvaluator { + assignments: assignments, + evaluated: empty(), + exports: &empty(), + overrides: overrides, + quiet: quiet, + scope: &empty(), + shell: shell, + dry_run: dry_run, + }; + + for name in assignments.keys() { + evaluator.evaluate_assignment(name)?; + } + + Ok(evaluator.evaluated) + } + pub fn evaluate_line( &mut self, line: &[Fragment<'a>], diff --git a/src/assignment_resolver.rs b/src/assignment_resolver.rs index c163263..a745def 100644 --- a/src/assignment_resolver.rs +++ b/src/assignment_resolver.rs @@ -2,27 +2,7 @@ use common::*; use CompilationErrorKind::*; -pub fn resolve_assignments<'a>( - assignments: &Map<&'a str, Expression<'a>>, - assignment_tokens: &Map<&'a str, Token<'a>>, -) -> CompilationResult<'a, ()> { - - let mut resolver = AssignmentResolver { - assignments: assignments, - assignment_tokens: assignment_tokens, - stack: empty(), - seen: empty(), - evaluated: empty(), - }; - - for name in assignments.keys() { - resolver.resolve_assignment(name)?; - } - - Ok(()) -} - -struct AssignmentResolver<'a: 'b, 'b> { +pub struct AssignmentResolver<'a: 'b, 'b> { assignments: &'b Map<&'a str, Expression<'a>>, assignment_tokens: &'b Map<&'a str, Token<'a>>, stack: Vec<&'a str>, @@ -31,6 +11,26 @@ struct AssignmentResolver<'a: 'b, 'b> { } impl<'a: 'b, 'b> AssignmentResolver<'a, 'b> { + pub fn resolve_assignments( + assignments: &Map<&'a str, Expression<'a>>, + assignment_tokens: &Map<&'a str, Token<'a>>, + ) -> CompilationResult<'a, ()> { + + let mut resolver = AssignmentResolver { + assignments: assignments, + assignment_tokens: assignment_tokens, + stack: empty(), + seen: empty(), + evaluated: empty(), + }; + + for name in assignments.keys() { + resolver.resolve_assignment(name)?; + } + + Ok(()) + } + fn resolve_assignment(&mut self, name: &'a str) -> CompilationResult<'a, ()> { if self.evaluated.contains(name) { return Ok(()); diff --git a/src/justfile.rs b/src/justfile.rs index 7bfd889..a86dd02 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -1,8 +1,6 @@ use common::*; use edit_distance::edit_distance; -use assignment_evaluator::evaluate_assignments; -use range_ext::RangeExt; pub struct Justfile<'a> { pub recipes: Map<&'a str, Recipe<'a>>, @@ -55,7 +53,7 @@ impl<'a, 'b> Justfile<'a> where 'a: 'b { return Err(RuntimeError::UnknownOverrides{overrides: unknown_overrides}); } - let scope = evaluate_assignments( + let scope = AssignmentEvaluator::evaluate_assignments( &self.assignments, &configuration.overrides, configuration.quiet, diff --git a/src/main.rs b/src/main.rs index 93279d7..f69cf4e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -53,6 +53,7 @@ mod common { pub use tempdir::TempDir; pub use assignment_evaluator::AssignmentEvaluator; + pub use assignment_resolver::AssignmentResolver; pub use command_ext::CommandExt; pub use compilation_error::{CompilationError, CompilationErrorKind, CompilationResult}; pub use configuration::Configuration; @@ -63,7 +64,9 @@ mod common { pub use misc::{default, empty}; pub use parameter::Parameter; pub use parser::Parser; + pub use range_ext::RangeExt; pub use recipe::Recipe; + pub use recipe_resolver::RecipeResolver; pub use runtime_error::{RuntimeError, RunResult}; pub use shebang::Shebang; pub use token::{Token, TokenKind}; diff --git a/src/parser.rs b/src/parser.rs index 1e7d9a8..edcbb16 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3,8 +3,6 @@ use common::*; use itertools; use TokenKind::*; use CompilationErrorKind::*; -use recipe_resolver::resolve_recipes; -use assignment_resolver::resolve_assignments; pub struct Parser<'a> { text: &'a str, @@ -350,7 +348,7 @@ impl<'a> Parser<'a> { })) } - resolve_recipes(&self.recipes, &self.assignments, self.text)?; + RecipeResolver::resolve_recipes(&self.recipes, &self.assignments, self.text)?; for recipe in self.recipes.values() { for parameter in &recipe.parameters { @@ -371,7 +369,7 @@ impl<'a> Parser<'a> { } } - resolve_assignments(&self.assignments, &self.assignment_tokens)?; + AssignmentResolver::resolve_assignments(&self.assignments, &self.assignment_tokens)?; Ok(Justfile { recipes: self.recipes, diff --git a/src/recipe_resolver.rs b/src/recipe_resolver.rs index 434cd5b..fac5ce7 100644 --- a/src/recipe_resolver.rs +++ b/src/recipe_resolver.rs @@ -2,63 +2,7 @@ use common::*; use CompilationErrorKind::*; -pub fn resolve_recipes<'a>( - recipes: &Map<&'a str, Recipe<'a>>, - assignments: &Map<&'a str, Expression<'a>>, - text: &'a str, -) -> CompilationResult<'a, ()> { - let mut resolver = RecipeResolver { - seen: empty(), - stack: empty(), - resolved: empty(), - recipes: recipes, - }; - - for recipe in recipes.values() { - resolver.resolve(recipe)?; - resolver.seen = empty(); - } - - for recipe in recipes.values() { - for line in &recipe.lines { - for fragment in line { - if let Fragment::Expression{ref expression, ..} = *fragment { - for variable in expression.variables() { - let name = variable.lexeme; - let undefined = !assignments.contains_key(name) - && !recipe.parameters.iter().any(|p| p.name == name); - if undefined { - // There's a borrow issue here that seems too difficult to solve. - // The error derived from the variable token has too short a lifetime, - // so we create a new error from its contents, which do live long - // enough. - // - // I suspect the solution here is to give recipes, pieces, and expressions - // two lifetime parameters instead of one, with one being the lifetime - // of the struct, and the second being the lifetime of the tokens - // that it contains - let error = variable.error(UndefinedVariable{variable: name}); - return Err(CompilationError { - text: text, - index: error.index, - line: error.line, - column: error.column, - width: error.width, - kind: UndefinedVariable { - variable: &text[error.index..error.index + error.width.unwrap()], - } - }); - } - } - } - } - } - } - - Ok(()) -} - -struct RecipeResolver<'a: 'b, 'b> { +pub struct RecipeResolver<'a: 'b, 'b> { stack: Vec<&'a str>, seen: Set<&'a str>, resolved: Set<&'a str>, @@ -66,7 +10,63 @@ struct RecipeResolver<'a: 'b, 'b> { } impl<'a, 'b> RecipeResolver<'a, 'b> { - fn resolve(&mut self, recipe: &Recipe<'a>) -> CompilationResult<'a, ()> { + pub fn resolve_recipes( + recipes: &Map<&'a str, Recipe<'a>>, + assignments: &Map<&'a str, Expression<'a>>, + text: &'a str, + ) -> CompilationResult<'a, ()> { + let mut resolver = RecipeResolver { + seen: empty(), + stack: empty(), + resolved: empty(), + recipes: recipes, + }; + + for recipe in recipes.values() { + resolver.resolve_recipe(recipe)?; + resolver.seen = empty(); + } + + for recipe in recipes.values() { + for line in &recipe.lines { + for fragment in line { + if let Fragment::Expression{ref expression, ..} = *fragment { + for variable in expression.variables() { + let name = variable.lexeme; + let undefined = !assignments.contains_key(name) + && !recipe.parameters.iter().any(|p| p.name == name); + if undefined { + // There's a borrow issue here that seems too difficult to solve. + // The error derived from the variable token has too short a lifetime, + // so we create a new error from its contents, which do live long + // enough. + // + // I suspect the solution here is to give recipes, pieces, and expressions + // two lifetime parameters instead of one, with one being the lifetime + // of the struct, and the second being the lifetime of the tokens + // that it contains + let error = variable.error(UndefinedVariable{variable: name}); + return Err(CompilationError { + text: text, + index: error.index, + line: error.line, + column: error.column, + width: error.width, + kind: UndefinedVariable { + variable: &text[error.index..error.index + error.width.unwrap()], + } + }); + } + } + } + } + } + } + + Ok(()) + } + + fn resolve_recipe(&mut self, recipe: &Recipe<'a>) -> CompilationResult<'a, ()> { if self.resolved.contains(recipe.name) { return Ok(()) } @@ -85,7 +85,7 @@ impl<'a, 'b> RecipeResolver<'a, 'b> { .cloned().collect() })); } - self.resolve(dependency)?; + self.resolve_recipe(dependency)?; }, None => return Err(dependency_token.error(UnknownDependency { recipe: recipe.name,