Check for unsupported "time &" in the proper place

This means we can detect this error also for simple blocks.

While at it do some cleanup in the area.

src/parse_execution.rs

@@ -1585,18 +1585,27 @@ impl<'a> ParseExecutionContext {
             0
         };
 
-        let job_wants_timing = job_node_wants_timing(job_node);
         let job_is_background = job_node.bg.is_some();
-        let job_is_simple_node = self.job_is_simple_block(job_node);
-
-        // Start the timer here to ensure command substitution is measured
-        let _timer = push_timer(job_wants_timing && (job_is_simple_node || !job_is_background));
+        let _timer = {
+            let wants_timing = job_node_wants_timing(job_node);
+            // It's an error to have 'time' in a background job.
+            if wants_timing && job_is_background {
+                return report_error!(
+                    self,
+                    ctx,
+                    STATUS_INVALID_ARGS.unwrap(),
+                    job_node,
+                    ERROR_TIME_BACKGROUND
+                );
+            }
+            wants_timing.then(push_timer)
+        };
 
         // When we encounter a block construct (e.g. while loop) in the general case, we create a "block
         // process" containing its node. This allows us to handle block-level redirections.
         // However, if there are no redirections, then we can just jump into the block directly, which
         // is significantly faster.
-        if job_is_simple_node {
+        if self.job_is_simple_block(job_node) {
             let mut block = None;
             let mut result =
                 self.apply_variable_assignments(ctx, None, &job_node.variables, &mut block);
@@ -1651,17 +1660,6 @@ impl<'a> ParseExecutionContext {
             props.skip_notification =
                 ld.is_subshell || parser.is_block() || ld.is_event != 0 || !parser.is_interactive();
             props.from_event_handler = ld.is_event != 0;
-
-            // It's an error to have 'time' in a background job.
-            if job_wants_timing && job_is_background {
-                return report_error!(
-                    self,
-                    ctx,
-                    STATUS_INVALID_ARGS.unwrap(),
-                    job_node,
-                    ERROR_TIME_BACKGROUND
-                );
-            }
         }
 
         let mut job = Job::new(props, self.node_source(job_node));

src/timer.rs

@@ -30,17 +30,12 @@ struct TimerSnapshot {
     cpu_children: libc::rusage,
 }
 
-/// If `enabled`, create a `TimerSnapshot` and return a `PrintElapsedOnDrop` object that will print
-/// upon being dropped the delta between now and the time that it is dropped at. Otherwise return
-/// `None`.
-pub fn push_timer(enabled: bool) -> Option<PrintElapsedOnDrop> {
-    if !enabled {
-        return None;
-    }
-
-    Some(PrintElapsedOnDrop {
+/// Create a `TimerSnapshot` and return a `PrintElapsedOnDrop` object that will print upon
+/// being dropped the delta between now and the time that it is dropped at.
+pub fn push_timer() -> PrintElapsedOnDrop {
+    PrintElapsedOnDrop {
         start: TimerSnapshot::take(),
-    })
+    }
 }
 
 /// An enumeration of supported libc rusage types used by [`getrusage()`].

tests/checks/syntax-error-location.fish

@@ -68,7 +68,7 @@ $fish -c 'echo {$,}'
 
 echo "bind -M" | $fish
 # CHECKERR: bind: -M: option requires an argument
-# CHECKERR: Standard input (line 1): 
+# CHECKERR: Standard input (line 1):
 # CHECKERR: bind -M
 # CHECKERR: ^
 # CHECKERR: (Type 'help bind' for related documentation)
@@ -118,6 +118,11 @@ $fish -c 'echo (time echo foo &)'
 # CHECKERR: echo (time echo foo &)
 # CHECKERR: ^~~~~~~~~~~~~~~~^
 
+$fish -c 'time begin; end &'
+# CHECKERR: fish: 'time' is not supported for background jobs. Consider using 'command time'.
+# CHECKERR: time begin; end &
+# CHECKERR: ^~~~~~~~~~~~~~~~^
+
 $fish -c 'echo (set -l foo 1 2 3; for $foo in foo; end)'
 # CHECKERR: fish: Unable to expand variable name ''
 # CHECKERR: set -l foo 1 2 3; for $foo in foo; end