Skip to content

Fix failing tests on main branch #2

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

Open
HowardvanRooijen wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix failing tests on main branch #2

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
75 changes: 46 additions & 29 deletions src/CSharpLanguageServer/State/ServerState.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,46 +452,63 @@ let processServerEvent (logger: ILogger) state postSelf msg : Async<ServerState>
PushDiagnosticsDocumentBacklog = newBacklog }

let wf, docForUri = docUri |> workspaceDocument state.Workspace AnyDocument
let wfPathToUri = workspaceFolderPathToUri wf.Value

match wf, docForUri with
| Some wf, None ->
let cshtmlPath = workspaceFolderUriToPath wf docUri |> _.Value

match! solutionGetRazorDocumentForPath wf.Solution.Value cshtmlPath with
| Some(_, compilation, cshtmlTree) ->
let semanticModelMaybe = compilation.GetSemanticModel cshtmlTree |> Option.ofObj

match semanticModelMaybe with
// Only try to process as a .cshtml file if it actually is one
match workspaceFolderUriToPath wf docUri with
| Some cshtmlPath when cshtmlPath.EndsWith(".cshtml", StringComparison.OrdinalIgnoreCase) ->
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

Inconsistent extension checking: The code uses cshtmlPath.EndsWith(".cshtml", StringComparison.OrdinalIgnoreCase) here, but elsewhere in the codebase (e.g., Workspace.fs:360, 392), the pattern is to use uri.EndsWith ".cshtml" without the case-insensitive comparison. Consider using a consistent approach throughout the codebase. Note that the check should ideally be on the URI (like line 360) before converting to a path, for consistency.

Copilot uses AI. Check for mistakes.
Comment on lines +459 to +460
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

Inefficient ordering of checks: The code converts the URI to a path and then checks if it's a .cshtml file. It would be more efficient to check the URI extension first (like the pattern used in Workspace.fs:360) before attempting the path conversion, since the path conversion might be an expensive operation that could fail.

Copilot uses AI. Check for mistakes.
match wf.Solution with
| None ->
Error(Exception "could not GetSemanticModelAsync")
|> PushDiagnosticsDocumentDiagnosticsResolution
|> postSelf

| Some semanticModel ->
let diagnostics =
semanticModel.GetDiagnostics()
|> Seq.map (Diagnostic.fromRoslynDiagnostic (workspaceFolderPathToUri wf))
|> Seq.filter (fun (_, uri) -> uri = docUri)
|> Seq.map fst
|> Array.ofSeq

Ok(docUri, None, diagnostics)
|> PushDiagnosticsDocumentDiagnosticsResolution
|> postSelf

| None ->
// could not find document for this enqueued uri
// Solution not loaded yet, rebuild backlog and try again later
postSelf PushDiagnosticsDocumentBacklogUpdate
postSelf PushDiagnosticsProcessPendingDocuments
Comment on lines 462 to +465
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

Potential infinite retry loop: When the solution is not loaded, this code rebuilds the backlog and retries processing. However, if the solution never loads or takes a long time to load, this will cause the same document to be retried indefinitely. Consider adding a retry counter or timeout mechanism to prevent infinite retries. Additionally, since the document was already removed from the backlog at line 452, rebuilding the backlog will re-add it, which may not be the intended behavior.

Copilot uses AI. Check for mistakes.
| Some solution ->
match! solutionGetRazorDocumentForPath solution cshtmlPath with
| Some(_, compilation, cshtmlTree) ->
let semanticModelMaybe = compilation.GetSemanticModel cshtmlTree |> Option.ofObj

match semanticModelMaybe with
| None ->
Error(Exception "could not GetSemanticModelAsync")
|> PushDiagnosticsDocumentDiagnosticsResolution
|> postSelf

| Some semanticModel ->
let diagnostics =
semanticModel.GetDiagnostics()
|> Seq.map (Diagnostic.fromRoslynDiagnostic (workspaceFolderPathToUri wf))
|> Seq.filter (fun (_, uri) -> uri = docUri)
|> Seq.map fst
|> Array.ofSeq

Ok(docUri, None, diagnostics)
|> PushDiagnosticsDocumentDiagnosticsResolution
|> postSelf

| None ->
logger.LogDebug(
"PushDiagnosticsProcessPendingDocuments: could not find razor document for \"{cshtmlPath}\"",
cshtmlPath
)
// Continue with next document
postSelf PushDiagnosticsProcessPendingDocuments

| _ ->
// Not a .cshtml file or couldn't convert URI
// This can happen if solution hasn't loaded yet - rebuild backlog for retry
logger.LogDebug(
"PushDiagnosticsProcessPendingDocuments: could not find document w/ uri \"{docUri}\"",
"PushDiagnosticsProcessPendingDocuments: could not find document w/ uri \"{docUri}\", will retry",
string docUri
)

()
postSelf PushDiagnosticsDocumentBacklogUpdate
postSelf PushDiagnosticsProcessPendingDocuments
Comment on lines +497 to +505
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

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

Potential infinite retry loop: Similar to the solution not loaded case, when a document is not found, this code rebuilds the entire backlog and retries processing. This can cause the same document to be retried indefinitely if it remains unfound (e.g., if the file was deleted or the URI is invalid). Consider adding a mechanism to track failed documents and avoid retrying them indefinitely, or at least add a maximum retry count.

Copilot uses AI. Check for mistakes.

return newState

| Some wf, Some doc ->
let wfPathToUri = workspaceFolderPathToUri wf

let resolveDocumentDiagnostics () : Task = task {
let! semanticModelMaybe = doc.GetSemanticModelAsync()

Expand Down