Skip to content

Commit a6b0254

Browse files
committed
Enhances the error message you get when you provide an invalid, simple breakpoint conditon message like $i == 3 or $i > 5.
1 parent 3abde2b commit a6b0254

File tree

2 files changed

+113
-1
lines changed

2 files changed

+113
-1
lines changed

src/PowerShellEditorServices/Debugging/DebugService.cs

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,17 @@ public async Task<BreakpointDetails[]> SetLineBreakpoints(
133133
{
134134
ScriptBlock actionScriptBlock = ScriptBlock.Create(breakpoint.Condition);
135135

136+
// Check for simple, common errors that ScriptBlock parsing will not catch
137+
// e.g. $i == 3 and $i > 3
138+
string message;
139+
if (!ValidateBreakpointConditionAst(actionScriptBlock.Ast, out message))
140+
{
141+
breakpoint.Verified = false;
142+
breakpoint.Message = message;
143+
resultBreakpointDetails.Add(breakpoint);
144+
continue;
145+
}
146+
136147
// Check for "advanced" condition syntax i.e. if the user has specified
137148
// a "break" or "continue" statement anywhere in their scriptblock,
138149
// pass their scriptblock through to the Action parameter as-is.
@@ -156,7 +167,7 @@ public async Task<BreakpointDetails[]> SetLineBreakpoints(
156167
// Failed to create conditional breakpoint likely because the user provided an
157168
// invalid PowerShell expression. Let the user know why.
158169
breakpoint.Verified = false;
159-
breakpoint.Message = ex.Message;
170+
breakpoint.Message = ExtractAndScrubParseExceptionMessage(ex, breakpoint.Condition);
160171
resultBreakpointDetails.Add(breakpoint);
161172
continue;
162173
}
@@ -536,6 +547,85 @@ private async Task FetchStackFrames()
536547
}
537548
}
538549

550+
private bool ValidateBreakpointConditionAst(Ast conditionAst, out string message)
551+
{
552+
message = string.Empty;
553+
554+
// We are only inspecting a few simple scenarios in the EndBlock only.
555+
ScriptBlockAst scriptBlockAst = conditionAst as ScriptBlockAst;
556+
if ((scriptBlockAst != null) &&
557+
(scriptBlockAst.BeginBlock == null) &&
558+
(scriptBlockAst.ProcessBlock == null) &&
559+
(scriptBlockAst.EndBlock != null) &&
560+
(scriptBlockAst.EndBlock.Statements.Count == 1))
561+
{
562+
StatementAst statementAst = scriptBlockAst.EndBlock.Statements[0];
563+
string condition = statementAst.Extent.Text;
564+
565+
if (statementAst is AssignmentStatementAst)
566+
{
567+
message = FormatInvalidBreakpointConditionMessage(condition, "Use '-eq' instead of '=='.");
568+
return false;
569+
}
570+
571+
PipelineAst pipelineAst = statementAst as PipelineAst;
572+
if ((pipelineAst != null) && (pipelineAst.PipelineElements.Count == 1) &&
573+
(pipelineAst.PipelineElements[0].Redirections.Count > 0))
574+
{
575+
message = FormatInvalidBreakpointConditionMessage(condition, "Use '-gt' instead of '>'.");
576+
return false;
577+
}
578+
}
579+
580+
return true;
581+
}
582+
583+
private string ExtractAndScrubParseExceptionMessage(ParseException parseException, string condition)
584+
{
585+
string[] messageLines = parseException.Message.Split('\n');
586+
587+
// Skip first line - it is a location indicator "At line:1 char: 4"
588+
for (int i = 1; i < messageLines.Length; i++)
589+
{
590+
string line = messageLines[i];
591+
if (line.StartsWith("+"))
592+
{
593+
continue;
594+
}
595+
596+
if (!string.IsNullOrWhiteSpace(line))
597+
{
598+
// Note '==' and '>" do not generate parse errors
599+
if (line.Contains("'!='"))
600+
{
601+
line += " Use operator '-ne' instead of '!='.";
602+
}
603+
else if (line.Contains("'<'") && condition.Contains("<="))
604+
{
605+
line += " Use operator '-le' instead of '<='.";
606+
}
607+
else if (line.Contains("'<'"))
608+
{
609+
line += " Use operator '-lt' instead of '<'.";
610+
}
611+
else if (condition.Contains(">="))
612+
{
613+
line += " Use operator '-ge' instead of '>='.";
614+
}
615+
616+
return FormatInvalidBreakpointConditionMessage(condition, line);
617+
}
618+
}
619+
620+
// If the message format isn't in a form we expect, just return the whole message.
621+
return FormatInvalidBreakpointConditionMessage(condition, parseException.Message);
622+
}
623+
624+
private string FormatInvalidBreakpointConditionMessage(string condition, string message)
625+
{
626+
return $"'{condition}' is not a valid PowerShell expression. {message}";
627+
}
628+
539629
#endregion
540630

541631
#region Events

test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,28 @@ await this.debugService.SetLineBreakpoints(
279279
Assert.Contains("Unexpected token '-ez'", breakpoints[1].Message);
280280
}
281281

282+
[Fact]
283+
public async Task DebuggerFindsParseableButInvalidSimpleBreakpointConditions()
284+
{
285+
BreakpointDetails[] breakpoints =
286+
await this.debugService.SetLineBreakpoints(
287+
this.debugScriptFile,
288+
new[] {
289+
BreakpointDetails.Create("", 5, column: null, condition: "$i == 100"),
290+
BreakpointDetails.Create("", 7, column: null, condition: "$i > 100")
291+
});
292+
293+
Assert.Equal(2, breakpoints.Length);
294+
Assert.Equal(5, breakpoints[0].LineNumber);
295+
Assert.False(breakpoints[0].Verified);
296+
Assert.Contains("Use '-eq' instead of '=='", breakpoints[0].Message);
297+
298+
Assert.Equal(7, breakpoints[1].LineNumber);
299+
Assert.False(breakpoints[1].Verified);
300+
Assert.NotNull(breakpoints[1].Message);
301+
Assert.Contains("Use '-gt' instead of '>'", breakpoints[1].Message);
302+
}
303+
282304
[Fact]
283305
public async Task DebuggerBreaksWhenRequested()
284306
{

0 commit comments

Comments
 (0)