PSEval can be called as $PSEval, @PSEval or %PSEval, but null/empty returns only make sense for $PSEval

jimbobmcgee

Consider the arbitrary OtterScript...

set @fileLines = @PSEval(Get-Content -LiteralPath 'X:\PathTo\File')
if $ListCount(@fileLines) != 0 {
   Log-Debug "OK"
}

If the Get-Content PowerShell cmdlet returns no lines, then the current approach is to attempt to set the return value of @PSEval to be an empty string, rather than an empty array. This in turn causes Otter to throw a rather nasty exception:

Unhandled exception: System.ArgumentException: Cannot assign a Scalar value to a Vector variable.
   at Inedo.ExecutionEngine.Executer.ExecuterThread.InitializeVariable(RuntimeVariableName name, RuntimeValue value, VariableAssignmentMode mode)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteAsync(AssignVariableStatement assignVariableStatement)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteNextAsync()

From an end-user perspective, trying to resolve whether a PSEval would return scalar, vector or hash is therefore fraught -- you have to know in advance whether the script will return 0, 1 or many items, and cannot safely deduce this by simply calling in vector context and testing the resulting length.

The best I can think of is try { set global @r = @PSEval(...); } catch { set global @r = @(); }, which technically proceeds, but the exception is still thrown and the overall job still results in an error state (assuming no subsequent force normal).

Ideally, PSEval should behave appropriately on a null/empty value, depending on whether it was called as $PSEval, @PSEval or %PSEval -- in scalar context, it is fine to return the empty string; in vector context, it would be better if it returned an empty vector; in hash context, an empty hash.

I appreciate that this might not be resolvable straight away, as you probably need assistance from whatever parses and invokes the script line upstream (maybe ExecuterThread?) to pass the desired RuntimeValueType to you. I expect it either needs to be suppled by extending IVariableFunctionContext (which may have side-effects on implementors of that interface); or—more likely—be introduced as a protected internal settable property of VariableFunction itself.

Alternatively, if it is possible to alter RuntimeValue so it has a dedicated constructor/static sentinel value for empty values, ExecuterThread.InitializeVariable might be better-placed to handle the scalar/vector/hash decision, and you can just return the sentinel from EvaluateAsync.

(Reposted from Github on request)

dean-houston

@jimbobmcgee thanks for reposting this here as well

Working with PowerShell output variables is a very long-standing challenge, in particular because PowerShell has very inconsistent returns based on Windows, Windows Core, LinuxCore. We made some fixes not too long ago, but it's still not perfect and was a ton of effort that ended up breaking some user scripts.

And as you probably saw poking around the execution engine code, a variable prefix ($, @, %) is more of a convenience/convention, and the prefix isn't really available in any useful context. I'm almost certain you can do stuff like $MyVar = @(1,2,3) for example. This is very likely not something we will want to change.

Keep in mind that OtterScript was never designed as a general-purpose scripting language, but as a light-weight orchestration script to run other scripts. So these limitations happen.

I will make a note of this on our long-term roadmap, but it's likely we won't take action on it due to sensitivity of all this and not wanting to break existing scripts.

jimbobmcgee

@dean-houston said in PSEval can be called as $PSEval, @PSEval or %PSEval, but null/empty returns only make sense for $PSEval:

a variable prefix ($, @, %) is more of a convenience/convention, and the prefix isn't really available in any useful context. I'm almost certain you can do stuff like $MyVar = @(1,2,3) for example.

For what it is worth, if this is the intent, then it does not match what actually occurs. The execution engine throws exceptions when you mismatch the variable types:

# mixed sigils
{
    set $ok = ""; set $no = "";
    try { set $a = "blah";          set $ok = $ok: scalar;            } catch { set $no = $no: scalar;           force normal; }
    try { set $b = @(1,2,3);        set $ok = $ok: vector-as-scalar;  } catch { set $no = $no: vector-as-scalar; force normal; }
    try { set $c = %(a: 1, b: 2);   set $ok = $ok: map-as-scalar;     } catch { set $no = $no: map-as-scalar;    force normal; }
    try { set @d = "blah";          set $ok = $ok: scalar-as-vector;  } catch { set $no = $no: scalar-as-vector; force normal; }
    try { set @e = @(1,2,3);        set $ok = $ok: vector;            } catch { set $no = $no: vector;           force normal; }
    try { set @f = %(a: 1, b: 2);   set $ok = $ok: map-as-vector;     } catch { set $no = $no: map-as-vector;    force normal; }
    try { set %g = "blah";          set $ok = $ok: scalar-as-map;     } catch { set $no = $no: scalar-as-map;    force normal; }
    try { set %h = @(1,2,3);        set $ok = $ok: vector-as-map;     } catch { set $no = $no: vector-as-map;    force normal; }
    try { set %i = %(a: 1, b: 2);   set $ok = $ok: map;               } catch { set $no = $no: map;              force normal; }

    Log-Information Mixed sigils: success${ok}, fail${no};
}

DEBUG: Beginning execution run...
ERROR: Unhandled exception: System.ArgumentException: Cannot assign a Vector value to a Scalar variable.
   at Inedo.ExecutionEngine.Executer.ExecuterThread.InitializeVariable(RuntimeVariableName name, RuntimeValue value, VariableAssignmentMode mode)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteAsync(AssignVariableStatement assignVariableStatement)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteNextAsync()
ERROR: Unhandled exception: System.ArgumentException: Cannot assign a Map value to a Scalar variable.
   at Inedo.ExecutionEngine.Executer.ExecuterThread.InitializeVariable(RuntimeVariableName name, RuntimeValue value, VariableAssignmentMode mode)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteAsync(AssignVariableStatement assignVariableStatement)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteNextAsync()
ERROR: Unhandled exception: System.ArgumentException: Cannot assign a Scalar value to a Vector variable.
   at Inedo.ExecutionEngine.Executer.ExecuterThread.InitializeVariable(RuntimeVariableName name, RuntimeValue value, VariableAssignmentMode mode)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteAsync(AssignVariableStatement assignVariableStatement)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteNextAsync()
ERROR: Unhandled exception: System.ArgumentException: Cannot assign a Map value to a Vector variable.
   at Inedo.ExecutionEngine.Executer.ExecuterThread.InitializeVariable(RuntimeVariableName name, RuntimeValue value, VariableAssignmentMode mode)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteAsync(AssignVariableStatement assignVariableStatement)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteNextAsync()
ERROR: Unhandled exception: System.ArgumentException: Cannot assign a Scalar value to a Map variable.
   at Inedo.ExecutionEngine.Executer.ExecuterThread.InitializeVariable(RuntimeVariableName name, RuntimeValue value, VariableAssignmentMode mode)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteAsync(AssignVariableStatement assignVariableStatement)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteNextAsync()
ERROR: Unhandled exception: System.ArgumentException: Cannot assign a Vector value to a Map variable.
   at Inedo.ExecutionEngine.Executer.ExecuterThread.InitializeVariable(RuntimeVariableName name, RuntimeValue value, VariableAssignmentMode mode)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteAsync(AssignVariableStatement assignVariableStatement)
   at Inedo.ExecutionEngine.Executer.ExecuterThread.ExecuteNextAsync()
INFO : Mixed sigils: success: scalar: vector: map, fail: vector-as-scalar: map-as-scalar: scalar-as-vector: map-as-vector: scalar-as-map: vector-as-map

There are also syntax elements which require specific context, such as foreach requiring a @vec (Iteration source must be a vector value).

I can certainly understand why you would not want to update the base classes so they provide the context (scalar, vector, map) to implementations but I expect it is probably the safest solution for maintaining backwards compatibility with existing authored scripts (if that information is available to you at parse-time).

The alternative is to let the script author pass it along as an optional property to $PSEval() (similar to $GetVariableValue()), but this introduces another character which would then need to be escaped within the embedded Powershell (i.e. ,), and that probably would break authored scripts.