From dccf92bae89942c2ba275ea2a59ffb678e42e8df Mon Sep 17 00:00:00 2001
From: ok <59588824+jku20@users.noreply.github.com>
Date: Mon, 12 Aug 2024 16:07:04 -0400
Subject: [PATCH] [fud2] Make Empty Ops an Error (#2257)

Empty ops currently can be created in the new DSL. When exported, this
creates broken ninja code. This is an error of the user, not creating
the desired target, but this class of error can never create the desired
target. Therefore a helpful error was inserted when this happens.
---
 fud2/fud-core/src/run.rs           | 12 +++++++++++-
 fud2/fud-core/src/script/error.rs  | 11 +++++++++++
 fud2/fud-core/src/script/plugin.rs | 10 +++++++++-
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/fud2/fud-core/src/run.rs b/fud2/fud-core/src/run.rs
index aa35b8e7d..0f55d3eae 100644
--- a/fud2/fud-core/src/run.rs
+++ b/fud2/fud-core/src/run.rs
@@ -383,7 +383,17 @@ impl<'a> Run<'a> {
                 }
             }) {
                 let stdout_files =
-                    std::fs::File::open(self.plan.workdir.join(filename))?;
+                    std::fs::File::open(self.plan.workdir.join(filename))
+                        // The output file we're emitting to stdout should exist. If it doesn't,
+                        // some op didn't produce the output file it claimed to.
+                        .map_err(|e| if let std::io::ErrorKind::NotFound = e.kind() {
+                            std::io::Error::new(
+                                e.kind(),
+                                format!("{}\nHint: Check ops actually generate all of their targets.", e)
+                            )
+                        } else {
+                            e
+                })?;
                 std::io::copy(
                     &mut std::io::BufReader::new(stdout_files),
                     &mut std::io::stdout(),
diff --git a/fud2/fud-core/src/script/error.rs b/fud2/fud-core/src/script/error.rs
index 4b8c00cc5..c20b4fe72 100644
--- a/fud2/fud-core/src/script/error.rs
+++ b/fud2/fud-core/src/script/error.rs
@@ -21,6 +21,7 @@ pub(super) enum RhaiSystemErrorKind {
     ExpectedString(String),
     ExpectedShell,
     ExpectedShellDeps,
+    EmptyOp,
 }
 
 impl RhaiSystemError {
@@ -90,6 +91,13 @@ impl RhaiSystemError {
         }
     }
 
+    pub(super) fn empty_op() -> Self {
+        Self {
+            kind: RhaiSystemErrorKind::EmptyOp,
+            position: rhai::Position::NONE,
+        }
+    }
+
     pub(super) fn with_pos(mut self, p: rhai::Position) -> Self {
         self.position = p;
         self
@@ -129,6 +137,9 @@ impl Display for RhaiSystemError {
             RhaiSystemErrorKind::ExpectedShellDeps => {
                 write!(f, "Expected `shell_deps`, got `shell`. Ops may contain only one of `shell` or `shell_deps` calls, not calls to both")
             }
+            RhaiSystemErrorKind::EmptyOp => {
+                write!(f, "Error: Op must contain at least one call to `shell` or `shell_deps`.")
+            }
         }
     }
 }
diff --git a/fud2/fud-core/src/script/plugin.rs b/fud2/fud-core/src/script/plugin.rs
index 8aec4475f..608d0caf0 100644
--- a/fud2/fud-core/src/script/plugin.rs
+++ b/fud2/fud-core/src/script/plugin.rs
@@ -352,8 +352,16 @@ impl ScriptContext {
                         c
                     }
                     Some(ShellCommands::Cmds(c)) => c.clone(),
-                    None => vec![],
+
+                    None => {
+                        // If cmds is empty, then the op doesn't create any of it's targets and
+                        // should be considered erroneous.
+                        return Err(RhaiSystemError::empty_op()
+                            .with_pos(pos)
+                            .into());
+                    }
                 };
+
                 let op_name = name.clone();
                 let config_vars = config_vars.clone();
                 let op_emitter = crate::run::RulesOp {