Skip to content

Commit 8958a72

Browse files
Log a warning when skipping workspace folders without a URI
Restore the per-folder warning that the previous refactor dropped. Silently discarding malformed workspace folders makes it hard for server operators to diagnose why a client's folder didn't take effect, and it matches the behavior the PR description advertises. Logging each skipped folder requires the explicit loop, so revert the pure `GetValidWorkspaceFolders` helper (and its dedicated tests); the existing `AddWorkspaceFoldersIgnoresNullAndUrilessFolders` regression test still covers null input, URI-less/null folders, and the downstream dereference. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5a9a5ce commit 8958a72

2 files changed

Lines changed: 14 additions & 37 deletions

File tree

src/PowerShellEditorServices/Services/Workspace/WorkspaceService.cs

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -118,23 +118,25 @@ public WorkspaceService(ILoggerFactory factory)
118118
/// </remarks>
119119
public void AddWorkspaceFolders(IEnumerable<WorkspaceFolder> workspaceFolders)
120120
{
121-
foreach (WorkspaceFolder workspaceFolder in GetValidWorkspaceFolders(workspaceFolders))
121+
if (workspaceFolders is null)
122122
{
123+
return;
124+
}
125+
126+
foreach (WorkspaceFolder workspaceFolder in workspaceFolders)
127+
{
128+
// Some LSP clients send folders without a URI; skip them (and warn) so we never
129+
// store a folder whose URI would later be dereferenced and throw.
130+
if (workspaceFolder?.Uri is null)
131+
{
132+
logger.LogWarning($"Ignoring workspace folder without a URI: {workspaceFolder?.Name}");
133+
continue;
134+
}
135+
123136
WorkspaceFolders.Add(workspaceFolder);
124137
}
125138
}
126139

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-
138140
/// <summary>
139141
/// Gets an open file in the workspace. If the file isn't open but exists on the filesystem, load and return it.
140142
/// <para>IMPORTANT: Not all documents have a backing file e.g. untitled: scheme documents. Consider using

test/PowerShellEditorServices.Test/Session/WorkspaceTests.cs

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -112,31 +112,6 @@ 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-
140115
[Fact]
141116
public void HasDefaultForWorkspacePaths()
142117
{

0 commit comments

Comments
 (0)