diff --git a/src/System.Management.Automation/engine/CommandParameter.cs b/src/System.Management.Automation/engine/CommandParameter.cs index 7aec069f39..ecd23002ed 100644 --- a/src/System.Management.Automation/engine/CommandParameter.cs +++ b/src/System.Management.Automation/engine/CommandParameter.cs @@ -24,7 +24,6 @@ namespace System.Management.Automation internal Ast ast; internal object value; internal bool splatted; - internal bool arrayIsSingleArgumentForNativeCommand; } private Parameter _parameter; @@ -113,16 +112,6 @@ namespace System.Management.Automation get { return _argument != null ? _argument.splatted : false; } } - /// - /// If an argument was specified and it was an array literal with no spaces around the - /// commas, the value should be passed a single argument with it's commas if the command is - /// a native command. - /// - internal bool ArrayIsSingleArgumentForNativeCommand - { - get { return _argument != null ? _argument.arrayIsSingleArgumentForNativeCommand : false; } - } - /// /// Set the argument value and ast. /// @@ -175,12 +164,10 @@ namespace System.Management.Automation /// The argument value. /// The ast of the argument value in the script. /// True if the argument value is to be splatted, false otherwise. - /// If the command is native, pass the string with commas instead of multiple arguments internal static CommandParameterInternal CreateArgument( object value, Ast ast = null, - bool splatted = false, - bool arrayIsSingleArgumentForNativeCommand = false) + bool splatted = false) { return new CommandParameterInternal { @@ -189,7 +176,6 @@ namespace System.Management.Automation value = value, ast = ast, splatted = splatted, - arrayIsSingleArgumentForNativeCommand = arrayIsSingleArgumentForNativeCommand } }; } @@ -210,20 +196,18 @@ namespace System.Management.Automation /// The ast of the argument value in the script. /// The argument value. /// Used in native commands to correctly handle -foo:bar vs. -foo: bar - /// If the command is native, pass the string with commas instead of multiple arguments internal static CommandParameterInternal CreateParameterWithArgument( Ast parameterAst, string parameterName, string parameterText, Ast argumentAst, object value, - bool spaceAfterParameter, - bool arrayIsSingleArgumentForNativeCommand = false) + bool spaceAfterParameter) { return new CommandParameterInternal { _parameter = new Parameter { ast = parameterAst, parameterName = parameterName, parameterText = parameterText }, - _argument = new Argument { ast = argumentAst, value = value, arrayIsSingleArgumentForNativeCommand = arrayIsSingleArgumentForNativeCommand }, + _argument = new Argument { ast = argumentAst, value = value }, _spaceAfterParameter = spaceAfterParameter }; } diff --git a/src/System.Management.Automation/engine/MinishellParameterBinderController.cs b/src/System.Management.Automation/engine/MinishellParameterBinderController.cs index 5dd0e1f5ba..deb745ede1 100644 --- a/src/System.Management.Automation/engine/MinishellParameterBinderController.cs +++ b/src/System.Management.Automation/engine/MinishellParameterBinderController.cs @@ -214,7 +214,7 @@ namespace System.Management.Automation parameters[i] = CommandParameterInternal.CreateParameterWithArgument( parameter.ArgumentAst, EncodedCommandParameter, "-" + EncodedCommandParameter, parameter.ArgumentAst, encodedScript, - spaceAfterParameter: true, arrayIsSingleArgumentForNativeCommand: false); + spaceAfterParameter: true); } } } diff --git a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs index e35e37316f..a3df920e77 100644 --- a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs +++ b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs @@ -105,28 +105,27 @@ namespace System.Management.Automation if (argValue != AutomationNull.Value && argValue != UnboundParameter.Value) { - // ArrayIsSingleArgumentForNativeCommand is true when a comma is used in the - // command line, e.g. + // ArrayLiteralAst is used to reconstruct the correct argument, e.g. // windbg -k com:port=\\devbox\pipe\debug,pipe,resets=0,reconnect // The parser produced an array of strings but marked the parameter so we // can properly reconstruct the correct command line. bool usedQuotes = false; - var pAst = parameter.ArgumentAst; - if (pAst != null) + ArrayLiteralAst arrayLiteralAst = null; + switch (parameter?.ArgumentAst) { - if (pAst is StringConstantExpressionAst sce) - { - usedQuotes = sce.StringConstantType != StringConstantType.BareWord; - } - else if (pAst is ExpandableStringExpressionAst ese) - { - usedQuotes = ese.StringConstantType != StringConstantType.BareWord; - } + case StringConstantExpressionAst sce: + usedQuotes = sce.StringConstantType != StringConstantType.BareWord; + break; + case ExpandableStringExpressionAst ese: + usedQuotes = ese.StringConstantType != StringConstantType.BareWord; + break; + case ArrayLiteralAst ala: + arrayLiteralAst = ala; + break; } + appendOneNativeArgument(Context, argValue, - parameter.ArrayIsSingleArgumentForNativeCommand ? ',' : ' ', - sawVerbatimArgumentMarker, - usedQuotes); + arrayLiteralAst, sawVerbatimArgumentMarker, usedQuotes); } } } @@ -158,14 +157,19 @@ namespace System.Management.Automation /// /// Execution context instance /// The object to append - /// A space or comma used when obj is enumerable + /// If the argument was an array literal, the Ast, otherwise null /// true if the argument occurs after --% /// True if the argument was a quoted string (single or double) - private void appendOneNativeArgument(ExecutionContext context, object obj, char separator, bool sawVerbatimArgumentMarker, bool usedQuotes) + private void appendOneNativeArgument(ExecutionContext context, object obj, ArrayLiteralAst argArrayAst, bool sawVerbatimArgumentMarker, bool usedQuotes) { IEnumerator list = LanguagePrimitives.GetEnumerator(obj); - bool needSeparator = false; + Diagnostics.Assert(argArrayAst == null + || obj is object[] && ((object[])obj).Length == argArrayAst.Elements.Count, + "array argument and ArrayLiteralAst differ in number of elements"); + + int currentElement = -1; + string separator = ""; do { string arg; @@ -180,18 +184,17 @@ namespace System.Management.Automation break; } arg = PSObject.ToStringParser(context, ParserOps.Current(null, list)); + + currentElement += 1; + if (currentElement != 0) + { + separator = GetEnumerableArgSeparator(argArrayAst, currentElement); + } } if (!String.IsNullOrEmpty(arg)) { - if (needSeparator) - { - _arguments.Append(separator); - } - else - { - needSeparator = true; - } + _arguments.Append(separator); if (sawVerbatimArgumentMarker) { @@ -349,6 +352,30 @@ namespace System.Management.Automation return needQuotes; } + static private string GetEnumerableArgSeparator(ArrayLiteralAst arrayLiteralAst, int index) + { + if (arrayLiteralAst == null) return " "; + + // index points to the *next* element, so we're looking for space between + // it and the previous element. + + var next = arrayLiteralAst.Elements[index]; + var prev = arrayLiteralAst.Elements[index - 1]; + + var arrayExtent = arrayLiteralAst.Extent; + var afterPrev = prev.Extent.EndOffset; + var beforeNext = next.Extent.StartOffset - 1; + + if (afterPrev == beforeNext) return ","; + + var arrayText = arrayExtent.Text; + afterPrev -= arrayExtent.StartOffset; + beforeNext -= arrayExtent.StartOffset; + + if (arrayText[afterPrev] == ',') return ", "; + if (arrayText[beforeNext] == ',') return " ,"; + return " , "; + } /// /// The native command to bind to diff --git a/src/System.Management.Automation/engine/parser/Compiler.cs b/src/System.Management.Automation/engine/parser/Compiler.cs index e782a38220..65b9443672 100644 --- a/src/System.Management.Automation/engine/parser/Compiler.cs +++ b/src/System.Management.Automation/engine/parser/Compiler.cs @@ -3443,13 +3443,11 @@ namespace System.Management.Automation.Language splatted = variableExpression.Splatted; } - bool arrayIsSingleArgumentForNativeCommand = ArgumentIsNotReallyArrayIfCommandIsNative(element); elementExprs[i] = Expression.Call(CachedReflectionInfo.CommandParameterInternal_CreateArgument, Expression.Convert(GetCommandArgumentExpression(element), typeof(object)), Expression.Constant(element), - ExpressionCache.Constant(splatted), - ExpressionCache.Constant(arrayIsSingleArgumentForNativeCommand)); + ExpressionCache.Constant(splatted)); } } @@ -3519,31 +3517,6 @@ namespace System.Management.Automation.Language return CallAddPipe(expr, s_getCurrentPipe); } - private bool ArgumentIsNotReallyArrayIfCommandIsNative(Ast arg) - { - var arrayLiteralAst = arg as ArrayLiteralAst; - if (arrayLiteralAst == null) - { - return false; - } - - Diagnostics.Assert(arrayLiteralAst.Elements.Count > 1, "Single dimension array arguments are surrounded with parens if the value is an argument"); - var previousElement = arrayLiteralAst.Elements[0]; - for (int index = 1; index < arrayLiteralAst.Elements.Count; index++) - { - var element = arrayLiteralAst.Elements[index]; - // EndOffset is 1 past the end which puts it on the comma, so if +1 is not the next element, - // there is whitespace between elements - if (previousElement.Extent.EndOffset + 1 != element.Extent.StartOffset) - { - return false; - } - previousElement = element; - } - - return true; - } - public object VisitCommandParameter(CommandParameterAst commandParameterAst) { var arg = commandParameterAst.Argument; @@ -3552,15 +3525,13 @@ namespace System.Management.Automation.Language { bool spaceAfterParameter = (errorPos.EndLineNumber != arg.Extent.StartLineNumber || errorPos.EndColumnNumber != arg.Extent.StartColumnNumber); - bool arrayIsSingleArgumentForNativeCommand = ArgumentIsNotReallyArrayIfCommandIsNative(arg); return Expression.Call(CachedReflectionInfo.CommandParameterInternal_CreateParameterWithArgument, Expression.Constant(commandParameterAst), Expression.Constant(commandParameterAst.ParameterName), Expression.Constant(errorPos.Text), Expression.Constant(arg), Expression.Convert(GetCommandArgumentExpression(arg), typeof(object)), - ExpressionCache.Constant(spaceAfterParameter), - ExpressionCache.Constant(arrayIsSingleArgumentForNativeCommand)); + ExpressionCache.Constant(spaceAfterParameter)); } return Expression.Call(CachedReflectionInfo.CommandParameterInternal_CreateParameter, diff --git a/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 b/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 index aaa82eb40c..8fd95be9e6 100644 --- a/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 +++ b/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 @@ -43,4 +43,22 @@ Describe "Native Command Arguments" -tags "CI" { $lines[$i] | Should Be "Arg $i is <$($expected[$i])>" } } + + It "Should handle PowerShell arrays with or without spaces correctly: " -TestCases @( + @{arguments = "1,2"; expected = @("1,2")} + @{arguments = "1,2,3"; expected = @("1,2,3")} + @{arguments = "1, 2"; expected = "1,", "2"} + @{arguments = "1 ,2"; expected = "1", ",2"} + @{arguments = "1 , 2"; expected = "1", ",", "2"} + @{arguments = "1, 2,3"; expected = "1,", "2,3"} + @{arguments = "1 ,2,3"; expected = "1", ",2,3"} + @{arguments = "1 , 2,3"; expected = "1", ",", "2,3"} + ) { + param($arguments, $expected) + $lines = @(Invoke-Expression "testexe -echoargs $arguments") + $lines.Count | Should Be $expected.Count + for ($i = 0; $i -lt $expected.Count; $i++) { + $lines[$i] | Should Be "Arg $i is <$($expected[$i])>" + } + } }