Skip to content

Commit

Permalink
Redirecting to a variable should be possible (#20381)
Browse files Browse the repository at this point in the history
* add tests

* fix formatting issues

* Make redirection to variable an experimental feature.

Update test to check for experimental feature status.
Set-Variable will still have the -append parameter, but if used when the experimental feature is disabled, a parameter binding error will result.

* Check to be sure provider is not null.

* update to use different FullyQualifiedErrorId

* use Experimental attribute for append parameter rather than runtime check.

* Revert "update to use different FullyQualifiedErrorId"

This reverts commit 8b34af1.

* Update src/Microsoft.PowerShell.Commands.Utility/commands/utility/Var.cs

Co-authored-by: Ilya <darpa@yandex.ru>

* Move remediation steps into resource.

Update Set-Variable -Append to be correct when -name and -value are used.
Add tests for the new behavior in Set-Variable.

* Change expected error for contrained language mode and redirection.

* Update src/Microsoft.PowerShell.Commands.Utility/commands/utility/Var.cs

Co-authored-by: Ilya <darpa@yandex.ru>

* Support redirection of native app.

Add tests to validate.

* testexe needs proper case to run on linux.

* Address codefactor issues 01.

* Update src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs

Co-authored-by: Steve Lee <slee@microsoft.com>

* Update src/Microsoft.PowerShell.Commands.Utility/commands/utility/Var.cs

Co-authored-by: Steve Lee <slee@microsoft.com>

* Update src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs

Co-authored-by: Steve Lee <slee@microsoft.com>

---------

Co-authored-by: Ilya <darpa@yandex.ru>
Co-authored-by: Steve Lee <slee@microsoft.com>
  • Loading branch information
3 people committed Jun 11, 2024
1 parent 7108ae0 commit 167a492
Show file tree
Hide file tree
Showing 7 changed files with 289 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,13 @@ public SwitchParameter PassThru

private bool _passThru;

/// <summary>
/// Gets whether we will append to the variable if it exists.
/// </summary>
[Parameter]
[Experimental(ExperimentalFeature.PSRedirectToVariable, ExperimentAction.Show)]
public SwitchParameter Append { get; set; }

private bool _nameIsFormalParameter;
private bool _valueIsFormalParameter;
#endregion parameters
Expand All @@ -711,6 +718,33 @@ protected override void BeginProcessing()
{
_valueIsFormalParameter = true;
}

if (Append)
{
// create the list here and add to it if it has a value
// but if they have more than one name, produce an error
if (Name.Length != 1)
{
ErrorRecord appendVariableError = new ErrorRecord(new InvalidOperationException(), "SetVariableAppend", ErrorCategory.InvalidOperation, Name);
appendVariableError.ErrorDetails = new ErrorDetails("SetVariableAppend");
appendVariableError.ErrorDetails.RecommendedAction = VariableCommandStrings.UseSingleVariable;
ThrowTerminatingError(appendVariableError);
}

_valueList = new List<object>();
var currentValue = Context.SessionState.PSVariable.Get(Name[0]);
if (currentValue is not null)
{
if (currentValue.Value is IList<object> ilist)
{
_valueList.AddRange(ilist);
}
else
{
_valueList.Add(currentValue.Value);
}
}
}
}

/// <summary>
Expand All @@ -726,6 +760,16 @@ protected override void ProcessRecord()
{
if (_nameIsFormalParameter && _valueIsFormalParameter)
{
if (Append)
{
if (Value != AutomationNull.Value)
{
_valueList ??= new List<object>();

_valueList.Add(Value);
}
}

return;
}

Expand Down Expand Up @@ -756,7 +800,14 @@ protected override void EndProcessing()
{
if (_valueIsFormalParameter)
{
SetVariable(Name, Value);
if (Append)
{
SetVariable(Name, _valueList);
}
else
{
SetVariable(Name, Value);
}
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@
<data name="SetVariableTarget" xml:space="preserve">
<value>Name: {0} Value: {1}</value>
</data>
<data name="UseSingleVariable" xml:space="preserve">
<value>Use a single variable rather than a collection</value>
</data>
<data name="NewVariableAction" xml:space="preserve">
<value>New variable</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public class ExperimentalFeature
internal const string PSFeedbackProvider = "PSFeedbackProvider";
internal const string PSCommandWithArgs = "PSCommandWithArgs";
internal const string PSNativeWindowsTildeExpansion = nameof(PSNativeWindowsTildeExpansion);
internal const string PSRedirectToVariable = "PSRedirectToVariable";

#endregion

Expand Down Expand Up @@ -127,8 +128,10 @@ static ExperimentalFeature()
description: "Enable `-CommandWithArgs` parameter for pwsh"),
new ExperimentalFeature(
name: PSNativeWindowsTildeExpansion,
description: "On windows, expand unquoted tilde (`~`) with the user's current home folder."
)
description: "On windows, expand unquoted tilde (`~`) with the user's current home folder."),
new ExperimentalFeature(
name: PSRedirectToVariable,
description: "Add support for redirecting to the variable drive"),
};

EngineExperimentalFeatures = new ReadOnlyCollection<ExperimentalFeature>(engineFeatures);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1090,10 +1090,23 @@ public override string ToString()
// dir > out
internal override void Bind(PipelineProcessor pipelineProcessor, CommandProcessorBase commandProcessor, ExecutionContext context)
{
// Check first to see if File is a variable path. If so, we'll not create the FileBytePipe
bool redirectToVariable = false;
if (ExperimentalFeature.IsEnabled(ExperimentalFeature.PSRedirectToVariable))
{
ProviderInfo p;
context.SessionState.Path.GetUnresolvedProviderPathFromPSPath(File, out p, out _);
if (p != null && p.NameEquals(context.ProviderNames.Variable))
{
redirectToVariable = true;
}
}

if (commandProcessor is NativeCommandProcessor nativeCommand
&& nativeCommand.CommandRuntime.ErrorMergeTo is not MshCommandRuntime.MergeDataStream.Output
&& FromStream is RedirectionStream.Output
&& !string.IsNullOrWhiteSpace(File))
&& !string.IsNullOrWhiteSpace(File)
&& !redirectToVariable)
{
nativeCommand.StdOutDestination = FileBytePipe.Create(File, Appending);
return;
Expand Down Expand Up @@ -1208,26 +1221,51 @@ internal Pipe GetRedirectionPipe(ExecutionContext context, PipelineProcessor par
return new Pipe { NullPipe = true };
}

CommandProcessorBase commandProcessor = context.CreateCommand("out-file", false);
Diagnostics.Assert(commandProcessor != null, "CreateCommand returned null");

// Previously, we mandated Unicode encoding here
// Now, We can take what ever has been set if PSDefaultParameterValues
// Unicode is still the default, but now may be overridden
// determine whether we're trying to set a variable by inspecting the file path
// if we can determine that it's a variable, we'll use Set-Variable rather than Out-File
ProviderInfo p;
PSDriveInfo d;
CommandProcessorBase commandProcessor;
var name = context.SessionState.Path.GetUnresolvedProviderPathFromPSPath(File, out p, out d);

var cpi = CommandParameterInternal.CreateParameterWithArgument(
/*parameterAst*/null, "Filepath", "-Filepath:",
/*argumentAst*/null, File,
false);
commandProcessor.AddParameter(cpi);
if (ExperimentalFeature.IsEnabled(ExperimentalFeature.PSRedirectToVariable) && p != null && p.NameEquals(context.ProviderNames.Variable))
{
commandProcessor = context.CreateCommand("Set-Variable", false);
Diagnostics.Assert(commandProcessor != null, "CreateCommand returned null");
var cpi = CommandParameterInternal.CreateParameterWithArgument(
/*parameterAst*/null, "Name", "-Name:",
/*argumentAst*/null, name,
false);
commandProcessor.AddParameter(cpi);

if (this.Appending)
if (this.Appending)
{
commandProcessor.AddParameter(CommandParameterInternal.CreateParameter("Append", "-Append", null));
}
}
else
{
cpi = CommandParameterInternal.CreateParameterWithArgument(
/*parameterAst*/null, "Append", "-Append:",
/*argumentAst*/null, true,
commandProcessor = context.CreateCommand("out-file", false);
Diagnostics.Assert(commandProcessor != null, "CreateCommand returned null");

// Previously, we mandated Unicode encoding here
// Now, We can take what ever has been set if PSDefaultParameterValues
// Unicode is still the default, but now may be overridden

var cpi = CommandParameterInternal.CreateParameterWithArgument(
/*parameterAst*/null, "Filepath", "-Filepath:",
/*argumentAst*/null, File,
false);
commandProcessor.AddParameter(cpi);

if (this.Appending)
{
cpi = CommandParameterInternal.CreateParameterWithArgument(
/*parameterAst*/null, "Append", "-Append:",
/*argumentAst*/null, true,
false);
commandProcessor.AddParameter(cpi);
}
}

PipelineProcessor = new PipelineProcessor();
Expand All @@ -1243,9 +1281,15 @@ internal Pipe GetRedirectionPipe(ExecutionContext context, PipelineProcessor par
// is more specific tp the redirection operation...
if (rte.ErrorRecord.Exception is System.ArgumentException)
{
throw InterpreterError.NewInterpreterExceptionWithInnerException(null,
typeof(RuntimeException), null, "RedirectionFailed", ParserStrings.RedirectionFailed,
rte.ErrorRecord.Exception, File, rte.ErrorRecord.Exception.Message);
throw InterpreterError.NewInterpreterExceptionWithInnerException(
null,
typeof(RuntimeException),
null,
"RedirectionFailed",
ParserStrings.RedirectionFailed,
rte.ErrorRecord.Exception,
File,
rte.ErrorRecord.Exception.Message);
}

throw;
Expand Down
122 changes: 122 additions & 0 deletions test/powershell/Language/Parser/RedirectionOperator.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,125 @@ Describe "File redirection should have 'DoComplete' called on the underlying pip
$errorContent | Should -Match "CommandNotFoundException,Microsoft.PowerShell.Commands.GetCommandCommand"
}
}

Describe "Redirection and Set-Variable -append tests" -tags CI {
Context "variable redirection should work" {
BeforeAll {
if ( $EnabledExperimentalFeatures -contains "PSRedirectToVariable" ) {
$skipTest = $false
}
else {
$skipTest = $true
}
$testCases = @{ Name = "Variable should be created"; scriptBlock = { 1..3>variable:a }; Validation = { ($a -join "") | Should -Be ((1..3) -join "") } },
@{ Name = "variable should be appended"; scriptBlock = {1..3>variable:a; 4..6>>variable:a}; Validation = { ($a -join "") | Should -Be ((1..6) -join "")}},
@{ Name = "variable should maintain type"; scriptBlock = {@{one=1}>variable:a};Validation = {$a | Should -BeOfType [hashtable]}},
@{
Name = "variable should maintain type for multiple objects"
scriptBlock = {@{one=1}>variable:a;@{two=2}>>variable:a;1>>variable:a;"string">>variable:a}
Validation = {
$a.count | Should -Be 4
,$a | Should -BeOfType [array]
$a[0] | Should -BeOfType [hashtable]
$a[1] | Should -BeOfType [hashtable]
$a[2] | Should -BeOfType [int]
$a[3] | Should -BeOfType [string]
$a[0].one | Should -Be 1
$a[1].two | Should -Be 2
$a[2] | Should -Be 1
$a[3] | Should -Be "string"
}
},
@{ Name = "Error stream should be redirectable"
scriptBlock = { write-error bad 2>variable:a}
validation = {
$a |Should -BeOfType [System.Management.Automation.ErrorRecord]
$a.Exception.Message | Should -BeExactly "bad"
}
},
@{ Name = "Warning stream should be redirectable"
scriptBlock = { write-warning warn 3>variable:a}
validation = {
$a |Should -BeOfType [System.Management.Automation.WarningRecord]
$a.Message | Should -BeExactly "warn"
}
},
@{ Name = "Verbose stream should be redirectable"
scriptBlock = { write-verbose -verbose verb 4>variable:a}
validation = {
$a |Should -BeOfType [System.Management.Automation.VerboseRecord]
$a.Message | Should -BeExactly "verb"
}
},
@{ Name = "Debug stream should be redirectable"
scriptBlock = { write-debug -debug deb 5>variable:a}
validation = {
$a |Should -BeOfType [System.Management.Automation.DebugRecord]
$a.Message | Should -BeExactly "deb"
}
},
@{ Name = "Information stream should be redirectable"
scriptBlock = { write-information info 6>variable:a}
validation = {
$a |Should -BeOfType [System.Management.Automation.InformationRecord]
$a.MessageData | Should -BeExactly "info"
}
},
@{ Name = "Complex redirection should be supported"
scriptBlock = {
. {
write-error bad
write-information info
} *>variable:a
}
validation = {
$a.Count | Should -Be 2
$a[0] | Should -BeOfType [System.Management.Automation.ErrorRecord]
$a[0].Exception.Message | Should -Be "bad"
$a[1] |Should -BeOfType [System.Management.Automation.InformationRecord]
$a[1].MessageData | Should -BeExactly "info"
}
},
@{
Name = "multiple redirections should work"
scriptBlock = {
. {
write-error bad
write-information info
} 2>variable:e 6>variable:i
}
validation = {
$e | Should -BeOfType [System.Management.Automation.ErrorRecord]
$e.Exception.Message | Should -Be "bad"
$i | Should -BeOfType [System.Management.Automation.InformationRecord]
$i.MessageData | Should -BeExactly "info"
}
}


}
It "<name>" -TestCases $testCases -skip:$skipTest {
param ( $scriptBlock, $validation )
. $scriptBlock
. $validation
}

It 'Redirection of a native application is correct' {
$expected = @('Arg 0 is <hi>','Arg 1 is <bye>')
testexe -echoargs hi bye > variable:observed
$observed | Should -Be $expected
}

It 'Redirection while in variable provider is correct' {
$expected = @('Arg 0 is <hi>','Arg 1 is <bye>')
try {
Push-Location variable:
testexe -echoargs hi bye > observed
}
finally {
Pop-Location
}
$observed | Should -Be $expected
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ try
$rs.Open()
$pl = $rs.CreatePipeline('"Hello" > c:\temp\foo.txt')

$e = { $pl.Invoke() } | Should -Throw -ErrorId "CmdletInvocationException"
$e = { $pl.Invoke() } | Should -Throw -ErrorId "DriveNotFoundException"

$rs.Dispose()
}
Expand Down
Loading

0 comments on commit 167a492

Please sign in to comment.