Skip to content

Commit 5a9a5ce

Browse files
Refactor workspace folder guard into a pure, testable filter
Extract the null/URI-less filtering from `AddWorkspaceFolders` into a pure `GetValidWorkspaceFolders` helper. `WorkspaceService` remains the single chokepoint where client-supplied folders are ingested, so every downstream `Uri` dereference stays protected, but the filtering logic is now trivially unit-testable without constructing a service or logger. Drop the per-folder warning so the filter is a simple, allocation-free expression, and add direct tests covering null, empty, and mixed (null folder / URI-less folder / valid folder) inputs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1191301 commit 5a9a5ce

2 files changed

Lines changed: 37 additions & 12 deletions

File tree

src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -118,23 +118,23 @@ public WorkspaceService(ILoggerFactory factory)
118118
/// </remarks>
119119
public void AddWorkspaceFolders(IEnumerable<WorkspaceFolder> workspaceFolders)
120120
{
121-
if (workspaceFolders is null)
121+
foreach (WorkspaceFolder workspaceFolder in GetValidWorkspaceFolders(workspaceFolders))
122122
{
123-
return;
124-
}
125-
126-
foreach (WorkspaceFolder workspaceFolder in workspaceFolders)
127-
{
128-
if (workspaceFolder?.Uri is null)
129-
{
130-
logger.LogWarning("Ignored workspace folder without a URI: " + workspaceFolder?.Name);
131-
continue;
132-
}
133-
134123
WorkspaceFolders.Add(workspaceFolder);
135124
}
136125
}
137126

127+
/// <summary>
128+
/// Filters workspace folders down to those usable by the service: non-null folders with a
129+
/// non-null URI. The <c>workspaceFolders</c> field is optional in LSP, so the collection
130+
/// itself may also be null.
131+
/// </summary>
132+
/// <param name="workspaceFolders">The workspace folders from the initialize parameters.</param>
133+
/// <returns>The folders that have a non-null URI, or an empty sequence.</returns>
134+
internal static IEnumerable<WorkspaceFolder> GetValidWorkspaceFolders(IEnumerable<WorkspaceFolder> workspaceFolders)
135+
=> workspaceFolders?.Where(static folder => folder?.Uri is not null)
136+
?? Enumerable.Empty<WorkspaceFolder>();
137+
138138
/// <summary>
139139
/// Gets an open file in the workspace. If the file isn't open but exists on the filesystem, load and return it.
140140
/// <para>IMPORTANT: Not all documents have a backing file e.g. untitled: scheme documents. Consider using

test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,31 @@ public void AddWorkspaceFoldersIgnoresNullAndUrilessFolders()
112112
Assert.Equal(s_workspacePath, Assert.Single(workspace.WorkspacePaths));
113113
}
114114

115+
[Fact]
116+
public void GetValidWorkspaceFoldersReturnsEmptyWhenNull()
117+
=> Assert.Empty(WorkspaceService.GetValidWorkspaceFolders(null));
118+
119+
[Fact]
120+
public void GetValidWorkspaceFoldersReturnsEmptyWhenEmpty()
121+
=> Assert.Empty(WorkspaceService.GetValidWorkspaceFolders(new Container<WorkspaceFolder>()));
122+
123+
[Fact]
124+
public void GetValidWorkspaceFoldersSkipsNullFoldersAndNullUris()
125+
{
126+
WorkspaceFolder valid = new()
127+
{
128+
Uri = DocumentUri.FromFileSystemPath(s_workspacePath),
129+
Name = "valid"
130+
};
131+
132+
Container<WorkspaceFolder> folders = new(
133+
null,
134+
new WorkspaceFolder { Name = "missing-uri" },
135+
valid);
136+
137+
Assert.Equal(valid, Assert.Single(WorkspaceService.GetValidWorkspaceFolders(folders)));
138+
}
139+
115140
[Fact]
116141
public void HasDefaultForWorkspacePaths()
117142
{

0 commit comments

Comments
 (0)