Repository: casey/just
Pull request #1527 (2023-01-25 02:15:40)

Analyzer::analyze borrows rather than clones Ast
I was looking at the code around the Analyzer and I realized it was cloning the entire `Ast` object, which didn't seem to be necessary.

casey: Nice! I removed the `'a` lifetime since I think it isn't necessary.

Patch:
diff --git a/src/analyzer.rs b/src/analyzer.rs
index c8ee53ee..172b5b20 100644
--- a/src/analyzer.rs
+++ b/src/analyzer.rs
@@ -4,40 +4,39 @@ const VALID_ALIAS_ATTRIBUTES: [Attribute; 1] = [Attribute::Private];
 
 #[derive(Default)]
 pub(crate) struct Analyzer<'src> {
-  recipes: Table<'src, UnresolvedRecipe<'src>>,
   assignments: Table<'src, Assignment<'src>>,
   aliases: Table<'src, Alias<'src, Name<'src>>>,
   sets: Table<'src, Set<'src>>,
 }
 
 impl<'src> Analyzer<'src> {
-  pub(crate) fn analyze(ast: Ast<'src>) -> CompileResult<'src, Justfile> {
+  pub(crate) fn analyze(ast: &Ast<'src>) -> CompileResult<'src, Justfile<'src>> {
     Analyzer::default().justfile(ast)
   }
 
-  pub(crate) fn justfile(mut self, ast: Ast<'src>) -> CompileResult<'src, Justfile<'src>> {
+  fn justfile(mut self, ast: &Ast<'src>) -> CompileResult<'src, Justfile<'src>> {
     let mut recipes = Vec::new();
 
-    for item in ast.items {
+    for item in &ast.items {
       match item {
         Item::Alias(alias) => {
-          self.analyze_alias(&alias)?;
-          self.aliases.insert(alias);
+          self.analyze_alias(alias)?;
+          self.aliases.insert(alias.clone());
         }
         Item::Assignment(assignment) => {
-          self.analyze_assignment(&assignment)?;
-          self.assignments.insert(assignment);
+          self.analyze_assignment(assignment)?;
+          self.assignments.insert(assignment.clone());
         }
         Item::Comment(_) => (),
         Item::Recipe(recipe) => {
           if recipe.enabled() {
-            Self::analyze_recipe(&recipe)?;
+            Self::analyze_recipe(recipe)?;
             recipes.push(recipe);
           }
         }
         Item::Set(set) => {
-          self.analyze_set(&set)?;
-          self.sets.insert(set);
+          self.analyze_set(set)?;
+          self.sets.insert(set.clone());
         }
       }
     }
@@ -81,10 +80,12 @@ impl<'src> Analyzer<'src> {
 
     let assignments = self.assignments;
 
+    let mut recipe_table: Table<'src, UnresolvedRecipe<'src>> = Default::default();
+
     AssignmentResolver::resolve_assignments(&assignments)?;
 
     for recipe in recipes {
-      if let Some(original) = self.recipes.get(recipe.name.lexeme()) {
+      if let Some(original) = recipe_table.get(recipe.name.lexeme()) {
         if !settings.allow_duplicate_recipes {
           return Err(recipe.name.token().error(DuplicateRecipe {
             recipe: original.name(),
@@ -92,10 +93,10 @@ impl<'src> Analyzer<'src> {
           }));
         }
       }
-      self.recipes.insert(recipe);
+      recipe_table.insert(recipe.clone());
     }
 
-    let recipes = RecipeResolver::resolve_recipes(self.recipes, &assignments)?;
+    let recipes = RecipeResolver::resolve_recipes(recipe_table, &assignments)?;
 
     let mut aliases = Table::new();
     while let Some(alias) = self.aliases.pop() {
@@ -103,7 +104,7 @@ impl<'src> Analyzer<'src> {
     }
 
     Ok(Justfile {
-      warnings: ast.warnings,
+      warnings: ast.warnings.clone(),
       first: recipes
         .values()
         .fold(None, |accumulator, next| match accumulator {
diff --git a/src/compiler.rs b/src/compiler.rs
index 2164856a..cd944a72 100644
--- a/src/compiler.rs
+++ b/src/compiler.rs
@@ -8,6 +8,6 @@ impl Compiler {
 
     let ast = Parser::parse(&tokens)?;
 
-    Analyzer::analyze(ast)
+    Analyzer::analyze(&ast)
   }
 }
diff --git a/src/subcommand.rs b/src/subcommand.rs
index 499b541c..e95a3fee 100644
--- a/src/subcommand.rs
+++ b/src/subcommand.rs
@@ -181,7 +181,7 @@ impl Subcommand {
 
     let tokens = Lexer::lex(src)?;
     let ast = Parser::parse(&tokens)?;
-    let justfile = Analyzer::analyze(ast.clone())?;
+    let justfile = Analyzer::analyze(&ast)?;
 
     if config.verbosity.loud() {
       for warning in &justfile.warnings {


Test patch:
diff --git a/src/testing.rs b/src/testing.rs
index 22eaaeb7..1e420856 100644
--- a/src/testing.rs
+++ b/src/testing.rs
@@ -64,7 +64,7 @@ pub(crate) fn analysis_error(
 
   let ast = Parser::parse(&tokens).expect("Parsing failed in analysis test...");
 
-  match Analyzer::analyze(ast) {
+  match Analyzer::analyze(&ast) {
     Ok(_) => panic!("Analysis unexpectedly succeeded"),
     Err(have) => {
       let want = CompileError {