Skip to content

Clean up breakpoint files #1701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 10, 2022
Merged

Clean up breakpoint files #1701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 42 additions & 60 deletions src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,9 @@ internal class BreakpointService
private readonly DebugStateService _debugStateService;

// TODO: This needs to be managed per nested session
internal readonly Dictionary<string, HashSet<Breakpoint>> BreakpointsPerFile =
new Dictionary<string, HashSet<Breakpoint>>();
internal readonly Dictionary<string, HashSet<Breakpoint>> BreakpointsPerFile = new();

internal readonly HashSet<Breakpoint> CommandBreakpoints =
new HashSet<Breakpoint>();
internal readonly HashSet<Breakpoint> CommandBreakpoints = new();

public BreakpointService(
ILoggerFactory factory,
Expand All @@ -51,9 +49,11 @@ public async Task<List<Breakpoint>> GetBreakpointsAsync()
}

// Legacy behavior
PSCommand psCommand = new PSCommand()
.AddCommand(@"Microsoft.PowerShell.Utility\Get-PSBreakpoint");
IEnumerable<Breakpoint> breakpoints = await _executionService.ExecutePSCommandAsync<Breakpoint>(psCommand, CancellationToken.None).ConfigureAwait(false);
PSCommand psCommand = new PSCommand().AddCommand(@"Microsoft.PowerShell.Utility\Get-PSBreakpoint");
IEnumerable<Breakpoint> breakpoints = await _executionService
.ExecutePSCommandAsync<Breakpoint>(psCommand, CancellationToken.None)
.ConfigureAwait(false);

return breakpoints.ToList();
}

Expand All @@ -73,13 +73,12 @@ public async Task<IEnumerable<BreakpointDetails>> SetBreakpointsAsync(string esc
breakpointDetails.Verified = false;
}
}

return breakpoints;
}

// Legacy behavior
PSCommand psCommand = null;
List<BreakpointDetails> configuredBreakpoints = new List<BreakpointDetails>();
List<BreakpointDetails> configuredBreakpoints = new();
foreach (BreakpointDetails breakpoint in breakpoints)
{
ScriptBlock actionScriptBlock = null;
Expand All @@ -106,7 +105,7 @@ public async Task<IEnumerable<BreakpointDetails>> SetBreakpointsAsync(string esc

// On first iteration psCommand will be null, every subsequent
// iteration will need to start a new statement.
if (psCommand == null)
if (psCommand is null)
{
psCommand = new PSCommand();
}
Expand All @@ -121,61 +120,61 @@ public async Task<IEnumerable<BreakpointDetails>> SetBreakpointsAsync(string esc
.AddParameter("Line", breakpoint.LineNumber);

// Check if the user has specified the column number for the breakpoint.
if (breakpoint.ColumnNumber.HasValue && breakpoint.ColumnNumber.Value > 0)
if (breakpoint.ColumnNumber > 0)
{
// It bums me out that PowerShell will silently ignore a breakpoint
// where either the line or the column is invalid. I'd rather have an
// error or warning message I could relay back to the client.
psCommand.AddParameter("Column", breakpoint.ColumnNumber.Value);
}

if (actionScriptBlock != null)
if (actionScriptBlock is not null)
{
psCommand.AddParameter("Action", actionScriptBlock);
}
}

// If no PSCommand was created then there are no breakpoints to set.
if (psCommand != null)
if (psCommand is not null)
{
IEnumerable<Breakpoint> setBreakpoints =
await _executionService.ExecutePSCommandAsync<Breakpoint>(psCommand, CancellationToken.None).ConfigureAwait(false);
configuredBreakpoints.AddRange(
setBreakpoints.Select((breakpoint) => BreakpointDetails.Create(breakpoint))
);
IEnumerable<Breakpoint> setBreakpoints = await _executionService
.ExecutePSCommandAsync<Breakpoint>(psCommand, CancellationToken.None)
.ConfigureAwait(false);
configuredBreakpoints.AddRange(setBreakpoints.Select((breakpoint) => BreakpointDetails.Create(breakpoint)));
}

return configuredBreakpoints;
}

public async Task<IEnumerable<CommandBreakpointDetails>> SetCommandBreakpoints(IEnumerable<CommandBreakpointDetails> breakpoints)
public async Task<IEnumerable<CommandBreakpointDetails>> SetCommandBreakpointsAsync(IEnumerable<CommandBreakpointDetails> breakpoints)
{
if (BreakpointApiUtils.SupportsBreakpointApis(_editorServicesHost.CurrentRunspace))
{
foreach (CommandBreakpointDetails commandBreakpointDetails in breakpoints)
{
try
{
BreakpointApiUtils.SetBreakpoint(_editorServicesHost.Runspace.Debugger, commandBreakpointDetails, _debugStateService.RunspaceId);
BreakpointApiUtils.SetBreakpoint(
_editorServicesHost.Runspace.Debugger,
commandBreakpointDetails,
_debugStateService.RunspaceId);
}
catch(InvalidOperationException e)
catch (InvalidOperationException e)
{
commandBreakpointDetails.Message = e.Message;
commandBreakpointDetails.Verified = false;
}
}

return breakpoints;
}

// Legacy behavior
PSCommand psCommand = null;
List<CommandBreakpointDetails> configuredBreakpoints = new List<CommandBreakpointDetails>();
List<CommandBreakpointDetails> configuredBreakpoints = new();
foreach (CommandBreakpointDetails breakpoint in breakpoints)
{
// On first iteration psCommand will be null, every subsequent
// iteration will need to start a new statement.
if (psCommand == null)
if (psCommand is null)
{
psCommand = new PSCommand();
}
Expand Down Expand Up @@ -208,20 +207,18 @@ public async Task<IEnumerable<CommandBreakpointDetails>> SetCommandBreakpoints(I
configuredBreakpoints.Add(breakpoint);
continue;
}

psCommand.AddParameter("Action", actionScriptBlock);
}
}

// If no PSCommand was created then there are no breakpoints to set.
if (psCommand != null)
if (psCommand is not null)
{
IEnumerable<Breakpoint> setBreakpoints =
await _executionService.ExecutePSCommandAsync<Breakpoint>(psCommand, CancellationToken.None).ConfigureAwait(false);
configuredBreakpoints.AddRange(
setBreakpoints.Select(CommandBreakpointDetails.Create));
IEnumerable<Breakpoint> setBreakpoints = await _executionService
.ExecutePSCommandAsync<Breakpoint>(psCommand, CancellationToken.None)
.ConfigureAwait(false);
configuredBreakpoints.AddRange(setBreakpoints.Select(CommandBreakpointDetails.Create));
}

return configuredBreakpoints;
}

Expand All @@ -238,30 +235,26 @@ public async Task RemoveAllBreakpointsAsync(string scriptPath = null)
_editorServicesHost.Runspace.Debugger,
_debugStateService.RunspaceId))
{
if (scriptPath == null || scriptPath == breakpoint.Script)
if (scriptPath is null || scriptPath == breakpoint.Script)
{
BreakpointApiUtils.RemoveBreakpoint(
_editorServicesHost.Runspace.Debugger,
breakpoint,
_debugStateService.RunspaceId);
}
}

return;
}

// Legacy behavior

PSCommand psCommand = new PSCommand();
psCommand.AddCommand(@"Microsoft.PowerShell.Utility\Get-PSBreakpoint");
var psCommand = new PSCommand().AddCommand(@"Microsoft.PowerShell.Utility\Get-PSBreakpoint");

if (!string.IsNullOrEmpty(scriptPath))
{
psCommand.AddParameter("Script", scriptPath);
}

psCommand.AddCommand(@"Microsoft.PowerShell.Utility\Remove-PSBreakpoint");

await _executionService.ExecutePSCommandAsync<object>(psCommand, CancellationToken.None).ConfigureAwait(false);
}
catch (Exception e)
Expand All @@ -281,37 +274,26 @@ public async Task RemoveBreakpointsAsync(IEnumerable<Breakpoint> breakpoints)
breakpoint,
_debugStateService.RunspaceId);

switch (breakpoint)
_ = breakpoint switch
{
case CommandBreakpoint commandBreakpoint:
CommandBreakpoints.Remove(commandBreakpoint);
break;
case LineBreakpoint lineBreakpoint:
if (BreakpointsPerFile.TryGetValue(lineBreakpoint.Script, out HashSet<Breakpoint> bps))
{
bps.Remove(lineBreakpoint);
}
break;
default:
throw new ArgumentException("Unsupported breakpoint type.");
}
CommandBreakpoint commandBreakpoint => CommandBreakpoints.Remove(commandBreakpoint),
LineBreakpoint lineBreakpoint =>
BreakpointsPerFile.TryGetValue(lineBreakpoint.Script, out HashSet<Breakpoint> bps) && bps.Remove(lineBreakpoint),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little too sparse for me.

_ => throw new NotImplementedException("Other breakpoints not supported yet"),
};
}

return;
}

// Legacy behavior
var breakpointIds = breakpoints.Select(b => b.Id).ToArray();
if(breakpointIds.Length > 0)
var breakpointIds = breakpoints.Select(b => b.Id);
if (breakpointIds.Any())
{
PSCommand psCommand = new PSCommand();
psCommand.AddCommand(@"Microsoft.PowerShell.Utility\Remove-PSBreakpoint");
psCommand.AddParameter("Id", breakpoints.Select(b => b.Id).ToArray());

PSCommand psCommand = new PSCommand()
.AddCommand(@"Microsoft.PowerShell.Utility\Remove-PSBreakpoint")
.AddParameter("Id", breakpoints.Select(b => b.Id).ToArray());
await _executionService.ExecutePSCommandAsync<object>(psCommand, CancellationToken.None).ConfigureAwait(false);
}
}


}
}
2 changes: 1 addition & 1 deletion src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ public async Task<CommandBreakpointDetails[]> SetCommandBreakpointsAsync(

if (breakpoints.Length > 0)
{
resultBreakpointDetails = (await _breakpointService.SetCommandBreakpoints(breakpoints).ConfigureAwait(false)).ToArray();
resultBreakpointDetails = (await _breakpointService.SetCommandBreakpointsAsync(breakpoints).ConfigureAwait(false)).ToArray();
}

return resultBreakpointDetails ?? Array.Empty<CommandBreakpointDetails>();
Expand Down
43 changes: 24 additions & 19 deletions src/PowerShellEditorServices/Services/DebugAdapter/Debugging/BreakpointApiUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,17 +134,24 @@ public static Breakpoint SetBreakpoint(Debugger debugger, BreakpointDetailsBase
}
}

switch (breakpoint)
return breakpoint switch
{
case BreakpointDetails lineBreakpoint:
return SetLineBreakpointDelegate(debugger, lineBreakpoint.Source, lineBreakpoint.LineNumber, lineBreakpoint.ColumnNumber ?? 0, actionScriptBlock, runspaceId);

case CommandBreakpointDetails commandBreakpoint:
return SetCommandBreakpointDelegate(debugger, commandBreakpoint.Name, null, null, runspaceId);

default:
throw new NotImplementedException("Other breakpoints not supported yet");
}
BreakpointDetails lineBreakpoint => SetLineBreakpointDelegate(
debugger,
lineBreakpoint.Source,
lineBreakpoint.LineNumber,
lineBreakpoint.ColumnNumber ?? 0,
actionScriptBlock,
runspaceId),

CommandBreakpointDetails commandBreakpoint => SetCommandBreakpointDelegate(debugger,
commandBreakpoint.Name,
null,
null,
runspaceId),

_ => throw new NotImplementedException("Other breakpoints not supported yet"),
};
}

public static List<Breakpoint> GetBreakpoints(Debugger debugger, int? runspaceId = null)
Expand Down Expand Up @@ -173,20 +180,20 @@ public static ScriptBlock GetBreakpointActionScriptBlock(string condition, strin

try
{
StringBuilder builder = new StringBuilder(
StringBuilder builder = new(
string.IsNullOrEmpty(logMessage)
? "break"
: $"Microsoft.PowerShell.Utility\\Write-Host \"{logMessage.Replace("\"","`\"")}\"");

// If HitCondition specified, parse and verify it.
if (!(string.IsNullOrWhiteSpace(hitCondition)))
if (!string.IsNullOrWhiteSpace(hitCondition))
{
if (!int.TryParse(hitCondition, out int parsedHitCount))
{
throw new InvalidOperationException("Hit Count was not a valid integer.");
}

if(string.IsNullOrWhiteSpace(condition))
if (string.IsNullOrWhiteSpace(condition))
{
// In the HitCount only case, this is simple as we can just use the HitCount
// property on the breakpoint object which is represented by $_.
Expand Down Expand Up @@ -217,8 +224,7 @@ public static ScriptBlock GetBreakpointActionScriptBlock(string condition, strin
// Check for "advanced" condition syntax i.e. if the user has specified
// a "break" or "continue" statement anywhere in their scriptblock,
// pass their scriptblock through to the Action parameter as-is.
if (parsed.Ast.Find(ast =>
(ast is BreakStatementAst || ast is ContinueStatementAst), true) != null)
if (parsed.Ast.Find(ast => ast is BreakStatementAst || ast is ContinueStatementAst, true) is not null)
{
return parsed;
}
Expand Down Expand Up @@ -247,10 +253,9 @@ private static bool ValidateBreakpointConditionAst(Ast conditionAst, out string

// We are only inspecting a few simple scenarios in the EndBlock only.
if (conditionAst is ScriptBlockAst scriptBlockAst &&
scriptBlockAst.BeginBlock == null &&
scriptBlockAst.ProcessBlock == null &&
scriptBlockAst.EndBlock != null &&
scriptBlockAst.EndBlock.Statements.Count == 1)
scriptBlockAst.BeginBlock is null &&
scriptBlockAst.ProcessBlock is null &&
scriptBlockAst.EndBlock?.Statements.Count == 1)
{
StatementAst statementAst = scriptBlockAst.EndBlock.Statements[0];
string condition = statementAst.Extent.Text;
Expand Down
6 changes: 3 additions & 3 deletions src/PowerShellEditorServices/Services/DebugAdapter/Debugging/BreakpointDetails.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ internal class BreakpointDetails : BreakpointDetailsBase
public int LineNumber { get; private set; }

/// <summary>
/// Gets the column number at which the breakpoint is set. If null, the default of 1 is used.
/// Gets the column number at which the breakpoint is set.
/// </summary>
public int? ColumnNumber { get; private set; }

Expand Down Expand Up @@ -83,9 +83,9 @@ internal static BreakpointDetails Create(
Breakpoint breakpoint,
BreakpointUpdateType updateType = BreakpointUpdateType.Set)
{
Validate.IsNotNull("breakpoint", breakpoint);
Validate.IsNotNull(nameof(breakpoint), breakpoint);

if (!(breakpoint is LineBreakpoint lineBreakpoint))
if (breakpoint is not LineBreakpoint lineBreakpoint)
{
throw new ArgumentException(
"Unexpected breakpoint type: " + breakpoint.GetType().Name);
Expand Down