And remove the out parameter

(discovered that the only place where the evaluation wasn't wrapeed is the dynamic dispatcher, so that one needs to have special handling on the focus to "put it back" as there's no log wapper at that stage - already executed and logged the parameters)

Author: brianpos

src/Hl7.Fhir.Base/FhirPath/Expressions/Closure.cs

@@ -23,36 +23,45 @@ public Closure(EvaluationContext ctx)
         {
             EvaluationContext = ctx;
             Id = ctx.IncrementClosuresCreatedCount();
+            _debugTracerActive = ctx.DebugTracer != null;
         }
 
         public Closure(Closure parent, EvaluationContext ctx)
         {
             Parent = parent;
             EvaluationContext = ctx;
             Id = ctx.IncrementClosuresCreatedCount();
+            _debugTracerActive = ctx.DebugTracer != null;
         }
 
         /// <summary>
         /// When the debug/trace is enabled this property is used to record the focus of the closure.
-        /// It is set in the delegate produced for each node by the evaluator visitor.
-        /// The value is set <b>immediately before</b> returning the result of the evaluation of the node,
-        /// <b>after all</b> it's processing, this must be done as the same context is re-used in many
-        /// cases, and thus needs to be re-set just before it returns from the delegate.
-        /// The debug tracer uses this information in the wrapped delegate to report not only the
-        /// result of the expression, but also the other states of the closure, such as the focus,
-        /// resource, root resource, etc.
-        /// The $this variable doesn't change within a closure object, so it is not set here.
+        /// <br/>VALUE IS NOT USED OUTSIDE DEBUG - without debug/tracer, the value is not consistent.
         /// </summary>
+        /// <remarks>
+        /// It is set in the delegate produced for each node by the evaluator visitor.
+        /// The debug tracer will reset the focus in the closure after calling the delegate it's wrapping.
+        /// ensuring that argument evaluation doesn't impact the focus logged in the debug trace in other
+        /// calls.
+        /// </remarks>
         public IEnumerable<ITypedElement> focus
         {
-            get => _focus;
+            get
+            {
+                if (!_debugTracerActive)
+                    return ElementNode.EmptyList;
+                return _focus;
+            }
             set
             {
+                if (!_debugTracerActive)
+                    return;
                 _focus = value;
             }
         }
 
         private IEnumerable<ITypedElement> _focus;
+        private bool _debugTracerActive = false;
 
         public EvaluationContext EvaluationContext { get; private set; }
 

src/Hl7.Fhir.Base/FhirPath/Expressions/DynaDispatcher.cs

@@ -26,17 +26,18 @@ public DynaDispatcher(string name, SymbolTable scope)
         private readonly string _name;
         private readonly SymbolTable _scope;
 
-        public FocusCollection Dispatcher(Closure context, IEnumerable<Invokee> args, out FocusCollection focus)
+        public FocusCollection Dispatcher(Closure context, IEnumerable<Invokee> args)
         {
             var actualArgs = new List<FocusCollection>();
 
-            focus = args.First()(context, InvokeeFactory.EmptyArgs, out _);
+            var focus = args.First()(context, InvokeeFactory.EmptyArgs);
+            context.focus = focus;
             if (!focus.Any()) return ElementNode.EmptyList;
 
             actualArgs.Add(focus);
             var newCtx = context.Nest(focus);
 
-            actualArgs.AddRange(args.Skip(1).Select(a => a(newCtx, InvokeeFactory.EmptyArgs, out _)));
+            actualArgs.AddRange(args.Skip(1).Select(a => a(newCtx, InvokeeFactory.EmptyArgs)));
             if (actualArgs.Any(aa => !aa.Any())) return ElementNode.EmptyList;
 
             var entry = _scope.DynamicGet(_name, actualArgs);
@@ -48,8 +49,12 @@ public FocusCollection Dispatcher(Closure context, IEnumerable<Invokee> args, ou
                     // The Get() here should never fail, since we already know there's a (dynamic) matching candidate
                     // Need to clean up this duplicate logic later
                     var argFuncs = actualArgs.Select(InvokeeFactory.Return);
+                    var result = entry(context, argFuncs);
+
+                    // Dynamically dispatched function arguments aren't wrapped
+                    // for the debug/trace, so need to manually put the focus back
                     context.focus = focus;
-                    return entry(context, argFuncs, out _);
+                    return result;
                 }
                 catch (TargetInvocationException tie)
                 {

src/Hl7.Fhir.Base/FhirPath/Expressions/EvaluatorVisitor.cs

@@ -21,10 +21,12 @@ private Invokee WrapForDebugTracer(Invokee invokee, Expression expression)
         {
             if (_injectDebugHook)
             {
-                return (Closure context, IEnumerable<Invokee> arguments, out FocusCollection focus) => {
+                return (Closure context, IEnumerable<Invokee> arguments) => {
                     var oldFocus = context.focus;
-                    var result = invokee(context, arguments, out focus);
+                    var result = invokee(context, arguments);
+
                     context.EvaluationContext.DebugTracer?.TraceCall(expression, context.Id, context.focus, context.GetThis(), context.GetIndex()?.FirstOrDefault(), context.GetTotal(), result, context.Variables());
+
                     // restore the original focus to the context
                     context.focus = oldFocus;
                     return result;
@@ -106,19 +108,18 @@ public override Invokee VisitVariableRef(FP.VariableRefExpression expression)
 
             return WrapForDebugTracer(chainResolves, expression);
 
-            FocusCollection chainResolves(Closure context, IEnumerable<Invokee> invokees, out FocusCollection focus)
+            FocusCollection chainResolves(Closure context, IEnumerable<Invokee> invokees)
             {
                 var value = context.ResolveValue(expression.Name);
                 if (value != null)
                 {
                     // this was in the context, so the scope was $this (the context)
-                    focus = context.GetThis();
-                    context.focus = focus;
+                    context.focus = context.GetThis();
                     return value;
                 }
                 else
                 {
-                    return resolve(Symbols, expression.Name, Enumerable.Empty<Type>())(context, [], out focus);
+                    return resolve(Symbols, expression.Name, Enumerable.Empty<Type>())(context, []);
                 }
             }
         }