Move to IReadOnlyList from IEnumerable to prevent double enumerations

Seems to be the easiest "clean and safe" way to handle this.

Author: andyleejordan

src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs

@@ -40,7 +40,7 @@ public BreakpointService(
             _debugStateService = debugStateService;
         }
 
-        public async Task<List<Breakpoint>> GetBreakpointsAsync()
+        public async Task<IReadOnlyList<Breakpoint>> GetBreakpointsAsync()
         {
             if (BreakpointApiUtils.SupportsBreakpointApis(_editorServicesHost.CurrentRunspace))
             {
@@ -52,14 +52,12 @@ public async Task<List<Breakpoint>> GetBreakpointsAsync()
 
             // Legacy behavior
             PSCommand psCommand = new PSCommand().AddCommand(@"Microsoft.PowerShell.Utility\Get-PSBreakpoint");
-            IEnumerable<Breakpoint> breakpoints = await _executionService
+            return await _executionService
                 .ExecutePSCommandAsync<Breakpoint>(psCommand, CancellationToken.None)
                 .ConfigureAwait(false);
-
-            return breakpoints.ToList();
         }
 
-        public async Task<IEnumerable<BreakpointDetails>> SetBreakpointsAsync(string escapedScriptPath, IEnumerable<BreakpointDetails> breakpoints)
+        public async Task<IReadOnlyList<BreakpointDetails>> SetBreakpointsAsync(string escapedScriptPath, IReadOnlyList<BreakpointDetails> breakpoints)
         {
             if (BreakpointApiUtils.SupportsBreakpointApis(_editorServicesHost.CurrentRunspace))
             {
@@ -147,7 +145,7 @@ public async Task<IEnumerable<BreakpointDetails>> SetBreakpointsAsync(string esc
             return configuredBreakpoints;
         }
 
-        public async Task<IEnumerable<CommandBreakpointDetails>> SetCommandBreakpointsAsync(IEnumerable<CommandBreakpointDetails> breakpoints)
+        public async Task<IReadOnlyList<CommandBreakpointDetails>> SetCommandBreakpointsAsync(IReadOnlyList<CommandBreakpointDetails> breakpoints)
         {
             if (BreakpointApiUtils.SupportsBreakpointApis(_editorServicesHost.CurrentRunspace))
             {
@@ -216,7 +214,7 @@ public async Task<IEnumerable<CommandBreakpointDetails>> SetCommandBreakpointsAs
             // If no PSCommand was created then there are no breakpoints to set.
             if (psCommand is not null)
             {
-                IEnumerable<Breakpoint> setBreakpoints = await _executionService
+                IReadOnlyList<Breakpoint> setBreakpoints = await _executionService
                     .ExecutePSCommandAsync<Breakpoint>(psCommand, CancellationToken.None)
                     .ConfigureAwait(false);
                 configuredBreakpoints.AddRange(setBreakpoints.Select(CommandBreakpointDetails.Create));

src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs

@@ -130,9 +130,9 @@ public DebugService(
         /// <param name="breakpoints">BreakpointDetails for each breakpoint that will be set.</param>
         /// <param name="clearExisting">If true, causes all existing breakpoints to be cleared before setting new ones.</param>
         /// <returns>An awaitable Task that will provide details about the breakpoints that were set.</returns>
-        public async Task<IEnumerable<BreakpointDetails>> SetLineBreakpointsAsync(
+        public async Task<IReadOnlyList<BreakpointDetails>> SetLineBreakpointsAsync(
             ScriptFile scriptFile,
-            IEnumerable<BreakpointDetails> breakpoints,
+            IReadOnlyList<BreakpointDetails> breakpoints,
             bool clearExisting = true)
         {
             DscBreakpointCapability dscBreakpoints = await _debugContext.GetDscBreakpointCapabilityAsync().ConfigureAwait(false);
@@ -146,15 +146,15 @@ public async Task<IEnumerable<BreakpointDetails>> SetLineBreakpointsAsync(
                 if (!_remoteFileManager.IsUnderRemoteTempPath(scriptPath))
                 {
                     _logger.LogTrace($"Could not set breakpoints for local path '{scriptPath}' in a remote session.");
-                    return Enumerable.Empty<BreakpointDetails>();
+                    return Array.Empty<BreakpointDetails>();
                 }
 
                 scriptPath = _remoteFileManager.GetMappedPath(scriptPath, _psesHost.CurrentRunspace);
             }
             else if (temporaryScriptListingPath?.Equals(scriptPath, StringComparison.CurrentCultureIgnoreCase) == true)
             {
                 _logger.LogTrace($"Could not set breakpoint on temporary script listing path '{scriptPath}'.");
-                return Enumerable.Empty<BreakpointDetails>();
+                return Array.Empty<BreakpointDetails>();
             }
 
             // Fix for issue #123 - file paths that contain wildcard chars [ and ] need to
@@ -182,25 +182,20 @@ public async Task<IEnumerable<BreakpointDetails>> SetLineBreakpointsAsync(
         /// <param name="breakpoints">CommandBreakpointDetails for each command breakpoint that will be set.</param>
         /// <param name="clearExisting">If true, causes all existing function breakpoints to be cleared before setting new ones.</param>
         /// <returns>An awaitable Task that will provide details about the breakpoints that were set.</returns>
-        public async Task<CommandBreakpointDetails[]> SetCommandBreakpointsAsync(
-            CommandBreakpointDetails[] breakpoints,
+        public async Task<IReadOnlyList<CommandBreakpointDetails>> SetCommandBreakpointsAsync(
+            IReadOnlyList<CommandBreakpointDetails> breakpoints,
             bool clearExisting = true)
         {
-            CommandBreakpointDetails[] resultBreakpointDetails = null;
-
             if (clearExisting)
             {
                 // Flatten dictionary values into one list and remove them all.
-                IEnumerable<Breakpoint> existingBreakpoints = await _breakpointService.GetBreakpointsAsync().ConfigureAwait(false);
+                IReadOnlyList<Breakpoint> existingBreakpoints = await _breakpointService.GetBreakpointsAsync().ConfigureAwait(false);
                 await _breakpointService.RemoveBreakpointsAsync(existingBreakpoints.OfType<CommandBreakpoint>()).ConfigureAwait(false);
             }
 
-            if (breakpoints.Length > 0)
-            {
-                resultBreakpointDetails = (await _breakpointService.SetCommandBreakpointsAsync(breakpoints).ConfigureAwait(false)).ToArray();
-            }
-
-            return resultBreakpointDetails ?? Array.Empty<CommandBreakpointDetails>();
+            return breakpoints.Count > 0
+                ? await _breakpointService.SetCommandBreakpointsAsync(breakpoints).ConfigureAwait(false)
+                : Array.Empty<CommandBreakpointDetails>();
         }
 
         /// <summary>

src/PowerShellEditorServices/Services/DebugAdapter/Handlers/BreakpointHandlers.cs

@@ -83,17 +83,17 @@ public async Task<SetBreakpointsResponse> Handle(SetBreakpointsArguments request
             }
 
             // At this point, the source file has been verified as a PowerShell script.
-            IEnumerable<BreakpointDetails> breakpointDetails = request.Breakpoints
+            IReadOnlyList<BreakpointDetails> breakpointDetails = request.Breakpoints
                 .Select((srcBreakpoint) => BreakpointDetails.Create(
                     scriptFile.FilePath,
                     srcBreakpoint.Line,
                     srcBreakpoint.Column,
                     srcBreakpoint.Condition,
                     srcBreakpoint.HitCondition,
-                    srcBreakpoint.LogMessage));
+                    srcBreakpoint.LogMessage)).ToList();
 
             // If this is a "run without debugging (Ctrl+F5)" session ignore requests to set breakpoints.
-            IEnumerable<BreakpointDetails> updatedBreakpointDetails = breakpointDetails;
+            IReadOnlyList<BreakpointDetails> updatedBreakpointDetails = breakpointDetails;
             if (!_debugStateService.NoDebug)
             {
                 await _debugStateService.WaitForSetBreakpointHandleAsync().ConfigureAwait(false);
@@ -125,23 +125,20 @@ await _debugService.SetLineBreakpointsAsync(
 
         public async Task<SetFunctionBreakpointsResponse> Handle(SetFunctionBreakpointsArguments request, CancellationToken cancellationToken)
         {
-            CommandBreakpointDetails[] breakpointDetails = request.Breakpoints
+            IReadOnlyList<CommandBreakpointDetails> breakpointDetails = request.Breakpoints
                 .Select((funcBreakpoint) => CommandBreakpointDetails.Create(
                     funcBreakpoint.Name,
-                    funcBreakpoint.Condition))
-                .ToArray();
+                    funcBreakpoint.Condition)).ToList();
 
             // If this is a "run without debugging (Ctrl+F5)" session ignore requests to set breakpoints.
-            CommandBreakpointDetails[] updatedBreakpointDetails = breakpointDetails;
+            IReadOnlyList<CommandBreakpointDetails> updatedBreakpointDetails = breakpointDetails;
             if (!_debugStateService.NoDebug)
             {
                 await _debugStateService.WaitForSetBreakpointHandleAsync().ConfigureAwait(false);
 
                 try
                 {
-                    updatedBreakpointDetails =
-                        await _debugService.SetCommandBreakpointsAsync(
-                            breakpointDetails).ConfigureAwait(false);
+                    updatedBreakpointDetails = await _debugService.SetCommandBreakpointsAsync(breakpointDetails).ConfigureAwait(false);
                 }
                 catch (Exception e)
                 {
@@ -156,9 +153,7 @@ await _debugService.SetCommandBreakpointsAsync(
 
             return new SetFunctionBreakpointsResponse
             {
-                Breakpoints = updatedBreakpointDetails
-                    .Select(LspDebugUtils.CreateBreakpoint)
-                    .ToArray()
+                Breakpoints = updatedBreakpointDetails.Select(LspDebugUtils.CreateBreakpoint).ToList()
             };
         }
 

src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs

@@ -21,17 +21,18 @@ internal class DscBreakpointCapability
         private string[] dscResourceRootPaths = Array.Empty<string>();
         private readonly Dictionary<string, int[]> breakpointsPerFile = new();
 
-        public async Task<IEnumerable<BreakpointDetails>> SetLineBreakpointsAsync(
+        public async Task<IReadOnlyList<BreakpointDetails>> SetLineBreakpointsAsync(
             IInternalPowerShellExecutionService executionService,
             string scriptPath,
-            IEnumerable<BreakpointDetails> breakpoints)
+            IReadOnlyList<BreakpointDetails> breakpoints)
         {
             // We always get the latest array of breakpoint line numbers
             // so store that for future use
-            if (breakpoints.Any())
+            int[] lineNumbers = breakpoints.Select(b => b.LineNumber).ToArray();
+            if (lineNumbers.Length > 0)
             {
                 // Set the breakpoints for this scriptPath
-                breakpointsPerFile[scriptPath] = breakpoints.Select(b => b.LineNumber).ToArray();
+                breakpointsPerFile[scriptPath] = lineNumbers;
             }
             else
             {

test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs

@@ -203,7 +203,7 @@ await debugService.SetCommandBreakpointsAsync(
         [MemberData(nameof(DebuggerAcceptsScriptArgsTestData))]
         public async Task DebuggerAcceptsScriptArgs(string[] args)
         {
-            IEnumerable<BreakpointDetails> breakpoints = await debugService.SetLineBreakpointsAsync(
+            IReadOnlyList<BreakpointDetails> breakpoints = await debugService.SetLineBreakpointsAsync(
                 oddPathScriptFile,
                 new[] { BreakpointDetails.Create(oddPathScriptFile.FilePath, 3) }).ConfigureAwait(true);
 
@@ -257,15 +257,15 @@ public async Task DebuggerAcceptsScriptArgs(string[] args)
         [Fact]
         public async Task DebuggerSetsAndClearsFunctionBreakpoints()
         {
-            CommandBreakpointDetails[] breakpoints = await debugService.SetCommandBreakpointsAsync(
+            IReadOnlyList<CommandBreakpointDetails> breakpoints = await debugService.SetCommandBreakpointsAsync(
                 new[] {
                     CommandBreakpointDetails.Create("Write-Host"),
                     CommandBreakpointDetails.Create("Get-Date")
                 }).ConfigureAwait(true);
 
-            Assert.Equal(2, breakpoints.Length);
-            Assert.Equal("Write-Host", breakpoints.ElementAt(0).Name);
-            Assert.Equal("Get-Date", breakpoints.ElementAt(1).Name);
+            Assert.Equal(2, breakpoints.Count);
+            Assert.Equal("Write-Host", breakpoints[0].Name);
+            Assert.Equal("Get-Date", breakpoints[1].Name);
 
             breakpoints = await debugService.SetCommandBreakpointsAsync(
                 new[] { CommandBreakpointDetails.Create("Get-Host") }).ConfigureAwait(true);
@@ -281,7 +281,7 @@ public async Task DebuggerSetsAndClearsFunctionBreakpoints()
         [Fact]
         public async Task DebuggerStopsOnFunctionBreakpoints()
         {
-            CommandBreakpointDetails[] breakpoints = await debugService.SetCommandBreakpointsAsync(
+            IReadOnlyList<CommandBreakpointDetails> breakpoints = await debugService.SetCommandBreakpointsAsync(
                 new[] { CommandBreakpointDetails.Create("Write-Host") }).ConfigureAwait(true);
 
             Task _ = ExecuteDebugFileAsync();
@@ -311,7 +311,7 @@ public async Task DebuggerStopsOnFunctionBreakpoints()
         [Fact]
         public async Task DebuggerSetsAndClearsLineBreakpoints()
         {
-            IEnumerable<BreakpointDetails> breakpoints =
+            IReadOnlyList<BreakpointDetails> breakpoints =
                 await debugService.SetLineBreakpointsAsync(
                     debugScriptFile,
                     new[] {
@@ -322,16 +322,16 @@ await debugService.SetLineBreakpointsAsync(
             IReadOnlyList<LineBreakpoint> confirmedBreakpoints = await GetConfirmedBreakpoints(debugScriptFile).ConfigureAwait(true);
 
             Assert.Equal(2, confirmedBreakpoints.Count);
-            Assert.Equal(5, breakpoints.ElementAt(0).LineNumber);
-            Assert.Equal(10, breakpoints.ElementAt(1).LineNumber);
+            Assert.Equal(5, breakpoints[0].LineNumber);
+            Assert.Equal(10, breakpoints[1].LineNumber);
 
             breakpoints = await debugService.SetLineBreakpointsAsync(
                 debugScriptFile,
                 new[] { BreakpointDetails.Create(debugScriptFile.FilePath, 2) }).ConfigureAwait(true);
             confirmedBreakpoints = await GetConfirmedBreakpoints(debugScriptFile).ConfigureAwait(true);
 
             Assert.Single(confirmedBreakpoints);
-            Assert.Equal(2, breakpoints.ElementAt(0).LineNumber);
+            Assert.Equal(2, breakpoints[0].LineNumber);
 
             await debugService.SetLineBreakpointsAsync(
                 debugScriptFile,
@@ -442,7 +442,7 @@ await debugService.SetLineBreakpointsAsync(
         [Fact]
         public async Task DebuggerProvidesMessageForInvalidConditionalBreakpoint()
         {
-            IEnumerable<BreakpointDetails> breakpoints =
+            IReadOnlyList<BreakpointDetails> breakpoints =
                 await debugService.SetLineBreakpointsAsync(
                     debugScriptFile,
                     new[] {
@@ -457,36 +457,36 @@ await debugService.SetLineBreakpointsAsync(
                     }).ConfigureAwait(true);
 
             Assert.Single(breakpoints);
-            // Assert.Equal(5, breakpoints.ElementAt(0).LineNumber);
-            // Assert.True(breakpoints.ElementAt(0).Verified);
-            // Assert.Null(breakpoints.ElementAt(0).Message);
-
-            Assert.Equal(10, breakpoints.ElementAt(0).LineNumber);
-            Assert.False(breakpoints.ElementAt(0).Verified);
-            Assert.NotNull(breakpoints.ElementAt(0).Message);
-            Assert.Contains("Unexpected token '-ez'", breakpoints.ElementAt(0).Message);
+            // Assert.Equal(5, breakpoints[0].LineNumber);
+            // Assert.True(breakpoints[0].Verified);
+            // Assert.Null(breakpoints[0].Message);
+
+            Assert.Equal(10, breakpoints[0].LineNumber);
+            Assert.False(breakpoints[0].Verified);
+            Assert.NotNull(breakpoints[0].Message);
+            Assert.Contains("Unexpected token '-ez'", breakpoints[0].Message);
         }
 
         [Fact]
         public async Task DebuggerFindsParsableButInvalidSimpleBreakpointConditions()
         {
-            IEnumerable<BreakpointDetails> breakpoints =
+            IReadOnlyList<BreakpointDetails> breakpoints =
                 await debugService.SetLineBreakpointsAsync(
                     debugScriptFile,
                     new[] {
                         BreakpointDetails.Create(debugScriptFile.FilePath, 5, column: null, condition: "$i == 100"),
                         BreakpointDetails.Create(debugScriptFile.FilePath, 7, column: null, condition: "$i > 100")
                     }).ConfigureAwait(true);
 
-            Assert.Equal(2, breakpoints.Count());
-            Assert.Equal(5, breakpoints.ElementAt(0).LineNumber);
-            Assert.False(breakpoints.ElementAt(0).Verified);
-            Assert.Contains("Use '-eq' instead of '=='", breakpoints.ElementAt(0).Message);
+            Assert.Equal(2, breakpoints.Count);
+            Assert.Equal(5, breakpoints[0].LineNumber);
+            Assert.False(breakpoints[0].Verified);
+            Assert.Contains("Use '-eq' instead of '=='", breakpoints[0].Message);
 
-            Assert.Equal(7, breakpoints.ElementAt(1).LineNumber);
-            Assert.False(breakpoints.ElementAt(1).Verified);
-            Assert.NotNull(breakpoints.ElementAt(1).Message);
-            Assert.Contains("Use '-gt' instead of '>'", breakpoints.ElementAt(1).Message);
+            Assert.Equal(7, breakpoints[1].LineNumber);
+            Assert.False(breakpoints[1].Verified);
+            Assert.NotNull(breakpoints[1].Message);
+            Assert.Contains("Use '-gt' instead of '>'", breakpoints[1].Message);
         }
 
         [Fact]