Rework passing array literal to native commands (#5301)

This commit is contained in:
Jason Shirk
2017-11-06 17:05:26 -08:00
committed by Dongbo Wang
parent a674c518b1
commit 8391b9e155
5 changed files with 77 additions and 77 deletions
@@ -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; }
}
/// <summary>
/// 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.
/// </summary>
internal bool ArrayIsSingleArgumentForNativeCommand
{
get { return _argument != null ? _argument.arrayIsSingleArgumentForNativeCommand : false; }
}
/// <summary>
/// Set the argument value and ast.
/// </summary>
@@ -175,12 +164,10 @@ namespace System.Management.Automation
/// <param name="value">The argument value.</param>
/// <param name="ast">The ast of the argument value in the script.</param>
/// <param name="splatted">True if the argument value is to be splatted, false otherwise.</param>
/// <param name="arrayIsSingleArgumentForNativeCommand">If the command is native, pass the string with commas instead of multiple arguments</param>
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
/// <param name="argumentAst">The ast of the argument value in the script.</param>
/// <param name="value">The argument value.</param>
/// <param name="spaceAfterParameter">Used in native commands to correctly handle -foo:bar vs. -foo: bar</param>
/// <param name="arrayIsSingleArgumentForNativeCommand">If the command is native, pass the string with commas instead of multiple arguments</param>
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
};
}
@@ -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);
}
}
}
@@ -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
/// </summary>
/// <param name="context">Execution context instance</param>
/// <param name="obj">The object to append</param>
/// <param name="separator">A space or comma used when obj is enumerable</param>
/// <param name="argArrayAst">If the argument was an array literal, the Ast, otherwise null</param>
/// <param name="sawVerbatimArgumentMarker">true if the argument occurs after --%</param>
/// <param name="usedQuotes">True if the argument was a quoted string (single or double)</param>
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 " , ";
}
/// <summary>
/// The native command to bind to
@@ -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,
@@ -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: <arguments>" -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])>"
}
}
}