Skip to content

Commit 22502d8

Browse files
kceiwkceiw
authored
Fix the way to parse the example (#22178)
* Use PredictionContext to get the command ast. - The existing way of using Parser.ParseInput doesn't create the CommandAst accurately especially when the input include `@{}`. So we change to use PredictionContext that is also used by PSReadLine. * Add unit test * Unify the way to get command name from the example and the user input * Remove one test case since that is not what we have in the example
1 parent 5009870 commit 22502d8

File tree

8 files changed

+248
-69
lines changed

8 files changed

+248
-69
lines changed

‎tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/CommandLineTests.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
// limitations under the License.
1313
// ----------------------------------------------------------------------------------
1414

15+
using Microsoft.Azure.PowerShell.Tools.AzPredictor.Test.Mocks;
1516
using Microsoft.Azure.PowerShell.Tools.AzPredictor.Utilities;
17+
using System.Collections.Generic;
1618
using System.Linq;
1719
using System.Management.Automation.Language;
1820
using Xunit;
@@ -82,10 +84,11 @@ public void VerifyMaskCommandLine(string expected, string input)
8284
/// </summary>
8385
[Theory]
8486
[InlineData("Set-AzContext -Subscription 'xxxx-xxxx-xxxx-xxxx' -Tenant <String>", null)]
85-
[InlineData("Set-AzContext -Subscription 'xxxx-xxxx-xxxx-xxxx' -Tenant {String}", null)]
87+
[InlineData("Set-AzContext -Subscription 'xxxx-xxxx-xxxx-xxxx' -Tenant `<String`>", null)]
8688
[InlineData("Set-AzContext -Subscription $subscription -Tenant [[String]]", null)]
8789
[InlineData("Set-AzContext -Subscription 'xxxx-xxxx-xxxx-xxxx' -Tenant TenantName", null)]
8890
[InlineData("Get-AzContext | Set-AzContext", "Set-AzContext")]
91+
[InlineData("Get-AzDiskEncryptionSet -ResourceGroupName rg1 -Name enc1;", "Get-AzDiskEncryptionSet -ResourceGroupName rg1 -Name enc1")]
8992
public void VerifyGetCommandAst(string input, string expected)
9093
{
9194
if (expected == null)

‎tools/Az.Tools.Predictor/Az.Tools.Predictor.Test/ParameterSetTests.cs

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
// ----------------------------------------------------------------------------------
1414

1515
using Microsoft.Azure.PowerShell.Tools.AzPredictor.Test.Mocks;
16-
using Microsoft.Azure.PowerShell.Tools.AzPredictor.Utilities;
1716
using System;
1817
using System.Collections.Generic;
1918
using System.Linq;
@@ -204,6 +203,25 @@ public void VerifyIncompleteParameterInMiddel(string inputData)
204203
Assert.Throws<CommandLineException>(() => new ParameterSet(commandAst, _azContext));
205204
}
206205

206+
/// <summary>
207+
/// Verify that we can parse the - in the parameter value.
208+
/// </summary>
209+
[Fact]
210+
public void VerifyMinusInParameterValue()
211+
{
212+
var inputData = "Set-Date -Adjust -0:10:0 -DisplayHint Time";
213+
var predictionContext = PredictionContext.Create(inputData);
214+
var commandAst = predictionContext.RelatedAsts.OfType<CommandAst>().LastOrDefault();
215+
var expected = new List<Parameter>()
216+
{
217+
new Parameter("Adjust", "-0:10:0", false),
218+
new Parameter("DisplayHint", "Time", false),
219+
};
220+
221+
var parameterSet = new ParameterSet(commandAst, _azContext);
222+
Assert.Equal(expected, parameterSet.Parameters);
223+
}
224+
207225
/// <summary>
208226
/// Verify that the one positional parameter can be parsed.
209227
/// </summary>
@@ -298,5 +316,90 @@ public void VerifyPositionalParametersAfterNamedParameters()
298316
var commandAst = predictionContext.RelatedAsts.OfType<CommandAst>().LastOrDefault();
299317
Assert.Throws<CommandLineException>(() => new ParameterSet(commandAst, _azContext));
300318
}
319+
320+
/// <summary>
321+
/// Verify that we can parse the @ in the parameter names/values.
322+
/// </summary>
323+
[Fact]
324+
public void VerifyParameterValuesWithAtSymbol()
325+
{
326+
// We should be able to parse the @ symbol when it's used as the parameter value.
327+
var inputData = "New-AzWebApp -Location westus -ResourceGroupName webappGroup -Name webappname -AppServicePlan appServicePlan -Force -Tag @{ Name1 = \"value\"; Name2 = \"Value2\" }";
328+
var predictionContext = PredictionContext.Create(inputData);
329+
var commandAst = predictionContext.RelatedAsts.OfType<CommandAst>().LastOrDefault();
330+
var expected = new List<Parameter>()
331+
{
332+
new Parameter("Location", "westus", false),
333+
new Parameter("ResourceGroupName", "webappGroup", false),
334+
new Parameter("Name", "webappname", false),
335+
new Parameter("AppServicePlan", "appServicePlan", false),
336+
new Parameter("Force", null, false),
337+
new Parameter("Tag", "@{ Name1 = \"value\"; Name2 = \"Value2\" }", false),
338+
};
339+
340+
var parameterSet = new ParameterSet(commandAst, _azContext);
341+
Assert.Equal(expected, parameterSet.Parameters);
342+
343+
// We should be able to parse the @ symbol when it's used as the parameter value.
344+
inputData = "Invoke-AzMLWorkspaceDiagnose -ResourceGroupName ml-rg-test -Name mlworkspace-cli01 -ApplicationInsightId @{'key1'=\"/subscriptions/xxxx-xxxxx-xxxxxxxxx-xxxx/resourceGroups/ml-rg-test/providers/Microsoft.insights/components/xxxxxxxxxxx\"}";
345+
predictionContext = PredictionContext.Create(inputData);
346+
commandAst = predictionContext.RelatedAsts.OfType<CommandAst>().LastOrDefault();
347+
expected = new List<Parameter>()
348+
{
349+
new Parameter("ResourceGroupName", "ml-rg-test", false),
350+
new Parameter("Name", "mlworkspace-cli01", false),
351+
new Parameter("ApplicationInsightId", "@{'key1'=\"/subscriptions/xxxx-xxxxx-xxxxxxxxx-xxxx/resourceGroups/ml-rg-test/providers/Microsoft.insights/components/xxxxxxxxxxx\"}", false),
352+
};
353+
354+
parameterSet = new ParameterSet(commandAst, _azContext);
355+
Assert.Equal(expected, parameterSet.Parameters);
356+
357+
inputData = "New-AzActivityLogAlertActionGroupObject -Id $ActionGroupResourceId -WebhookProperty @{\"sampleWebhookProperty\"=\"SamplePropertyValue\"}";
358+
predictionContext = PredictionContext.Create(inputData);
359+
commandAst = predictionContext.RelatedAsts.OfType<CommandAst>().LastOrDefault();
360+
expected = new List<Parameter>()
361+
{
362+
new Parameter("Id", "$ActionGroupResourceId", false),
363+
new Parameter("WebhookProperty", "@{\"sampleWebhookProperty\"=\"SamplePropertyValue\"}", false),
364+
};
365+
366+
parameterSet = new ParameterSet(commandAst, _azContext);
367+
Assert.Equal(expected, parameterSet.Parameters);
368+
369+
inputData = "New-AzAutoscaleWebhookNotificationObject -Property @{} -ServiceUri \"http://myservice.com\"";
370+
predictionContext = PredictionContext.Create(inputData);
371+
commandAst = predictionContext.RelatedAsts.OfType<CommandAst>().LastOrDefault();
372+
expected = new List<Parameter>()
373+
{
374+
new Parameter("Property", "@{}", false),
375+
new Parameter("ServiceUri", "\"http://myservice.com\"", false),
376+
};
377+
378+
parameterSet = new ParameterSet(commandAst, _azContext);
379+
Assert.Equal(expected, parameterSet.Parameters);
380+
381+
inputData = "New-AzCommunicationServiceKey -CommunicationServiceName ContosoAcsResource1 -ResourceGroupName ContosoResourceProvider1 -Parameter @{KeyType=\"Primary\"}";
382+
predictionContext = PredictionContext.Create(inputData);
383+
commandAst = predictionContext.RelatedAsts.OfType<CommandAst>().LastOrDefault();
384+
expected = new List<Parameter>()
385+
{
386+
new Parameter("CommunicationServiceName", "ContosoAcsResource1", false),
387+
new Parameter("ResourceGroupName", "ContosoResourceProvider1", false),
388+
new Parameter("Parameter", "@{KeyType=\"Primary\"}", false),
389+
};
390+
391+
parameterSet = new ParameterSet(commandAst, _azContext);
392+
Assert.Equal(expected, parameterSet.Parameters);
393+
394+
parameterSet = new ParameterSet(commandAst, _azContext);
395+
Assert.Equal(expected, parameterSet.Parameters);
396+
397+
// When @ is used to indicate the variable for the parameter name/value pair, we throw the exception since we
398+
// don't want to provide that as the suggestion.
399+
inputData = "New-ScriptFileInfo @newScriptInfo";
400+
predictionContext = PredictionContext.Create(inputData);
401+
commandAst = predictionContext.RelatedAsts.OfType<CommandAst>().LastOrDefault();
402+
Assert.Throws<CommandLineException>(() => new ParameterSet(commandAst, _azContext));
403+
}
301404
}
302405
}

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/AzPredictorService.cs

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -148,17 +148,7 @@ public virtual CommandLineSuggestion GetSuggestion(PredictionContext context, in
148148
Validation.CheckArgument<ArgumentOutOfRangeException>(suggestionCount > 0, $"{nameof(suggestionCount)} must be larger than 0.");
149149
Validation.CheckArgument<ArgumentOutOfRangeException>(maxAllowedCommandDuplicate > 0, $"{nameof(maxAllowedCommandDuplicate)} must be larger than 0.");
150150

151-
var relatedAsts = context.RelatedAsts;
152-
CommandAst commandAst = null;
153-
154-
for (var i = relatedAsts.Count - 1; i >= 0; --i)
155-
{
156-
if (relatedAsts[i] is CommandAst c)
157-
{
158-
commandAst = c;
159-
break;
160-
}
161-
}
151+
CommandAst commandAst = CommandLineUtilities.GetCommandAst(context);
162152

163153
CommandLineSuggestion earlyReturnResult = new()
164154
{

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/CommandLinePredictor.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ public CommandLinePredictor(IList<PredictiveCommand> modelPredictions, Parameter
6565

6666
_telemetryClient = telemetryClient;
6767
_parameterValuePredictor = parameterValuePredictor;
68-
var commnadLines = new List<CommandLine>();
6968
var errors = new HashSet<string>();
7069

7170
if (modelPredictions != null)

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/ParameterSet.cs

Lines changed: 54 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -80,65 +80,77 @@ public ParameterSet(CommandAst commandAst, IAzContext azContext = null)
8080
// Loop through all the parameters. The first element of CommandElements is the command name, so skip it.
8181
bool hasSeenNamedParameter = false;
8282
bool hasSeenIncompleteParameter = false;
83-
for (var i = 1; i < commandAst.CommandElements.Count(); ++i)
83+
try
8484
{
85-
var elem = commandAst.CommandElements[i];
86-
87-
if (elem is CommandParameterAst p)
85+
for (var i = 1; i < commandAst.CommandElements.Count(); ++i)
8886
{
89-
if (hasSeenIncompleteParameter)
90-
{
91-
throw new CommandLineException("'-' is in the middle of the parameter list.");
92-
}
87+
var elem = commandAst.CommandElements[i];
9388

94-
hasSeenNamedParameter = true;
95-
AddNamedParameter(param, arg);
96-
// In case there is a switch parameter, we store the parameter name/value and add them when we see the next pair.
97-
param = p;
98-
arg = null;
99-
}
100-
else if (AzPredictorConstants.ParameterIndicator == elem?.ToString().Trim().FirstOrDefault())
101-
{
102-
// We have an incomplete command line such as
103-
// `New-AzResourceGroup -Name ResourceGroup01 -Location WestUS -`
104-
// We'll ignore the incomplete parameter.
105-
AddNamedParameter(param, arg);
106-
param = null;
107-
arg = null;
108-
hasSeenIncompleteParameter = true;
109-
parameters.Add(new Parameter(AzPredictorConstants.DashParameterName, null, false));
110-
}
111-
else
112-
{
113-
if (hasSeenIncompleteParameter || (hasSeenNamedParameter && param == null))
89+
if (elem is null)
11490
{
115-
throw new CommandLineException("Positional parameters must be before named parameters.");
91+
continue;
11692
}
11793

118-
if (param == null)
94+
if (elem is CommandParameterAst p)
11995
{
120-
// This is a positional parameter.
121-
var pair = BoundParameters.First((pair) => pair.Value.Value == elem);
96+
if (hasSeenIncompleteParameter)
97+
{
98+
throw new CommandLineException("'-' is in the middle of the parameter list.");
99+
}
122100

123-
var parameterName = pair.Key;
124-
var parameterValue = CommandLineUtilities.EscapePredictionText(pair.Value.Value.ToString());
125-
parameters.Add(new Parameter(parameterName, parameterValue, true));
126-
BoundParameters.Remove(pair); // Remove it so that we can match another parameter with the same value.
101+
hasSeenNamedParameter = true;
102+
AddNamedParameter(param, arg);
103+
// In case there is a switch parameter, we store the parameter name/value and add them when we see the next pair.
104+
param = p;
105+
arg = null;
127106
}
128-
else
107+
else if (elem?.ToString()?.Trim()?.Length == 1 && (AzPredictorConstants.ParameterIndicator == elem.ToString().Trim().FirstOrDefault()))
129108
{
130-
arg = elem;
109+
// We have an incomplete command line such as
110+
// `New-AzResourceGroup -Name ResourceGroup01 -Location WestUS -`
111+
// We'll ignore the incomplete parameter.
131112
AddNamedParameter(param, arg);
132113
param = null;
133114
arg = null;
115+
hasSeenIncompleteParameter = true;
116+
parameters.Add(new Parameter(AzPredictorConstants.DashParameterName, null, false));
117+
}
118+
else
119+
{
120+
if (hasSeenIncompleteParameter || (hasSeenNamedParameter && param == null))
121+
{
122+
throw new CommandLineException("Positional parameters must be before named parameters.");
123+
}
124+
125+
if (param == null)
126+
{
127+
// This is a positional parameter.
128+
var pair = BoundParameters.First((pair) => pair.Value.Value == elem);
129+
130+
var parameterName = pair.Key;
131+
var parameterValue = pair.Value.Value.ToString();
132+
parameters.Add(new Parameter(parameterName, parameterValue, true));
133+
BoundParameters.Remove(pair); // Remove it so that we can match another parameter with the same value.
134+
}
135+
else
136+
{
137+
arg = elem;
138+
AddNamedParameter(param, arg);
139+
param = null;
140+
arg = null;
141+
}
134142
}
135143
}
136-
}
137144

138-
Validation.CheckInvariant<CommandLineException>((param != null) || (arg == null));
139-
AddNamedParameter(param, arg);
145+
Validation.CheckInvariant<CommandLineException>((param != null) || (arg == null));
146+
AddNamedParameter(param, arg);
140147

141-
Parameters = parameters;
148+
Parameters = parameters;
149+
}
150+
catch (Exception e) when (!(e is CommandLineException))
151+
{
152+
throw new CommandLineException("There are errors in parsing the parameters.", e);
153+
}
142154

143155
void AddNamedParameter(CommandParameterAst parameter, Ast parameterValue)
144156
{

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/AzPredictorTelemetryClient.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ private void ProcessTelemetryData(GetSuggestionTelemetryData telemetryData)
462462
}
463463

464464
var maskedUserInput = telemetryData.IsSupported ?
465-
CommandLineUtilities.MaskCommandLine(CommandLineUtilities.GetCommandAst(telemetryData.UserInput)) :
465+
CommandLineUtilities.MaskCommandLine(telemetryData.UserInput) :
466466
AzPredictorConstants.CommandPlaceholder;
467467

468468
var suggestionSession = new SuggestionSession()

‎tools/Az.Tools.Predictor/Az.Tools.Predictor/Telemetry/GetSuggestionTelemetryData.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public sealed class GetSuggestionTelemetryData : ITelemetryData
6060
/// <summary>
6161
/// Gets the user input.
6262
/// </summary>
63-
public Ast UserInput { get; }
63+
public CommandAst UserInput { get; }
6464

6565
/// <summary>
6666
/// Gets whether the command in <see cref="UserInput" /> is supported or not.
@@ -100,7 +100,7 @@ public sealed class GetSuggestionTelemetryData : ITelemetryData
100100
/// <param name="suggestion">The suggestions returned for the <paramref name="userInput"/>.</param>
101101
/// <param name="isCancellationRequested">Indicates if the cancellation has been requested.</param>
102102
/// <param name="exception">The exception that is thrown if there is an error.</param>
103-
public GetSuggestionTelemetryData(PredictionClient client, uint suggestionSessionId, Ast userInput, bool isSupported, CommandLineSuggestion suggestion, bool isCancellationRequested, Exception exception)
103+
public GetSuggestionTelemetryData(PredictionClient client, uint suggestionSessionId, CommandAst userInput, bool isSupported, CommandLineSuggestion suggestion, bool isCancellationRequested, Exception exception)
104104
{
105105
Client = client;
106106
SuggestionSessionId = suggestionSessionId;

0 commit comments

Comments
 (0)