Set $? correctly for command expression with redirections (#16046)

src/System.Management.Automation/engine/parser/Compiler.cs

@@ -3279,9 +3279,9 @@ namespace System.Management.Automation.Language
         /// <returns>True is the compiler should add the success setting, false otherwise.</returns>
         private bool ShouldSetExecutionStatusToSuccess(PipelineAst pipelineAst)
         {
-            ExpressionAst expressionAst = pipelineAst.GetPureExpression();
+            ExpressionAst expressionAst = GetSingleExpressionFromPipeline(pipelineAst);
 
-            // If the pipeline is not a simple expression, it will set $?
+            // If the pipeline is not a single expression, it will set $?
             if (expressionAst == null)
             {
                 return false;
@@ -3291,6 +3291,22 @@ namespace System.Management.Automation.Language
             return ShouldSetExecutionStatusToSuccess(expressionAst);
         }
 
+        /// <summary>
+        /// If the pipeline contains a single expression, the expression is returned, otherwise null is returned.
+        /// This method is different from <see cref="PipelineAst.GetPureExpression"/> in that it allows the single
+        /// expression to have redirections.
+        /// </summary>
+        private static ExpressionAst GetSingleExpressionFromPipeline(PipelineAst pipelineAst)
+        {
+            var pipelineElements = pipelineAst.PipelineElements;
+            if (pipelineElements.Count == 1 && pipelineElements[0] is CommandExpressionAst expr)
+            {
+                return expr.Expression;
+            }
+
+            return null;
+        }
+
         /// <summary>
         /// Determines whether an assignment statement must have an explicit setting
         /// for $? = $true after it by the compiler.

test/powershell/Language/Scripting/DollarHook.Tests.ps1

@@ -85,4 +85,84 @@ Describe 'Tests for setting $? for execution success' -Tag 'CI' {
         $script:hookValues | Should -Be $HookResults
         $output | Should -Be $PipelineResults
     }
+
+    It 'Sets $? correctly for single expression with redirection ''<Expression>''' -TestCases @(
+        @{ Expression = 'Write-Error "Bad"; Hook $?; "b" > $null; Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; "b" >> $null; Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; "b" > TESTDRIVE:\out.txt; Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; "b" >> TESTDRIVE:\out.txt; Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; "b" 2> $null; Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; "b" 2>> $null; Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; "b" 2> TESTDRIVE:\out.txt; Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; "b" 2>> TESTDRIVE:\out.txt; Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; "b" 2>&1; Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; "b" 2>&1; Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; "b" 2>&1 > $null; Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; "b" 2>&1 > TESTDRIVE:\out.txt; Hook $?'; HookResults = $($false, $true); }
+
+        @{ Expression = 'Write-Error "Bad"; Hook $?; ("b" > $null); Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; ("b" >> $null); Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; ("b" > TESTDRIVE:\out.txt); Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; ("b" >> TESTDRIVE:\out.txt); Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; ("b" 2> $null); Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; ("b" 2>> $null); Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; ("b" 2> TESTDRIVE:\out.txt); Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; ("b" 2>> TESTDRIVE:\out.txt); Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; ("b" 2>&1); Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; ("b" 2>&1 > $null); Hook $?'; HookResults = $($false, $true); }
+        @{ Expression = 'Write-Error "Bad"; Hook $?; ("b" 2>&1 > TESTDRIVE:\out.txt); Hook $?'; HookResults = $($false, $true); }
+    ) {
+        param([string]$Expression, [object[]]$HookResults)
+
+        Invoke-Expression $Expression 2>&1 >$null
+
+        $script:hookValues | Should -Be $HookResults
+    }
+
+    Context 'Validate $? with potential terminating error' {
+
+        ## Script execution directly in Pester tests will be enclosed in try/catch by the Pester,
+        ## and therefore, general exceptions thrown from an expression like "1/0" will be turned
+        ## into a terminating exception, which will stop the execution of remaining scripts.
+        ##
+        ## For those test cases, we have to use a PowerShell instance, so as to keep the default
+        ## error handling behavior for the general exceptions.
+
+        BeforeAll {
+            $pwsh = [powershell]::Create()
+
+            function Invoke([string] $script)
+            {
+                $pwsh.Commands.Clear()
+                $pwsh.Streams.ClearStreams()
+                $pwsh.AddScript($script).Invoke()
+            }
+
+            $root = Join-Path ([System.IO.Path]::GetTempPath()) ([guid]::NewGuid().ToString())
+            $null = Invoke "New-PSDrive -Name TESTDRIVE -PSProvider FileSystem -Root $root"
+        }
+
+        Afterall {
+            $null = Invoke "Remove-PSDrive -Name TESTDRIVE -PSProvider FileSystem -Force"
+            $pwsh.Dispose()
+        }
+
+        It 'Sets $? correctly for single expression with redirection ''<Expression>''' -TestCases @(
+            @{ Expression = '1/0 > $null; $?'; Result = $false; }
+            @{ Expression = '1/0 >> $null; $?'; Result = $false; }
+            @{ Expression = '1/0 > TESTDRIVE:\out.txt; $?'; Result = $false; }
+            @{ Expression = '1/0 >> TESTDRIVE:\out.txt; $?'; Result = $false; }
+            @{ Expression = '1/0 2>&1; $?'; Result = $false; }
+            @{ Expression = '1/0 2>&1 > $null; $?'; Result = $false; }
+            @{ Expression = '1/0 2>&1 > TESTDRIVE:\out.txt; $?'; Result = $false; }
+
+            @{ Expression = '"b" > NonExistDrive:\nowhere.txt; $?'; Result = $false; }
+            @{ Expression = '"b" >> NonExistDrive:\nowhere.txt; $?'; Result = $false; }
+            @{ Expression = '"b" 2>&1 > NonExistDrive:\nowhere.txt; $?'; Result = $false; }
+        ) {
+            param([string]$Expression, $Result)
+
+            Invoke $Expression | Should -Be $Result
+        }
+    }
 }