Skip to content

Throw NotImplementedException when a build errorId is not supported#82508

Merged
JoeRobich merged 9 commits intomainfrom
dev/jorobich/fix-error-list
Mar 3, 2026
Merged

Throw NotImplementedException when a build errorId is not supported#82508
JoeRobich merged 9 commits intomainfrom
dev/jorobich/fix-error-list

Conversation

@JoeRobich
Copy link
Member

@JoeRobich JoeRobich commented Feb 24, 2026

In #79964 we removed a check for whether a non-compiler build error should be reported to the Error List. This PR restores that check. It also adds an in-process cache of diagnostic descriptors so that it can synchronously determine whether a diagnostic id is supported.

Fixes #81109
Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2695780

The cache is updated when projects are added/removed from the workspace
or when AnalyzerReferences are added/removed from projects. The cache
is used to answer whether a project can generate a particular diagnostic
id synchronously.
@JoeRobich JoeRobich changed the title Allow non-compiler errors to be added to Build only Error List Throw NotImplementedException when a build errorId is not supported Feb 28, 2026
@JoeRobich
Copy link
Member Author

@jasonmalinowski This is ready for another look.

@JoeRobich
Copy link
Member Author

@jasonmalinowski Ready for another look

@JoeRobich JoeRobich force-pushed the dev/jorobich/fix-error-list branch from e635be4 to cac9a4b Compare March 3, 2026 03:35

internal class VisualStudioDiagnosticIdCache : IWorkspaceService
{
// This dictionary maps ProjectIds to a descriptor list.
Copy link
Member

Choose a reason for hiding this comment

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

Use doc comments.

Copy link
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

Comments are not blocking, :shipit:.

/// Map from project ID to all the possible analyzer diagnostic IDs that can be reported in the project.
/// </summary>
private ImmutableDictionary<ProjectId, AsyncLazy<ImmutableHashSet<string>>> _allDiagnosticIdMap = ImmutableDictionary<ProjectId, AsyncLazy<ImmutableHashSet<string>>>.Empty;
private readonly VisualStudioDiagnosticIdCache _diagnosticIdCache = solution.Workspace.Services.GetRequiredService<VisualStudioDiagnosticIdCache>();
Copy link
Member

Choose a reason for hiding this comment

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

It may be a practical question of whether this line is doing much -- I assume we create new InProgressStates, and this will force the cache to be initialized earlier; in a CPS-project only solution we'll fetch this and then never use it.

Copy link
Member

Choose a reason for hiding this comment

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

And if we delete that then this InProgressState type might have no reason to exist either, but that's a follow-up.

@JoeRobich JoeRobich merged commit 0a6db8d into main Mar 3, 2026
25 checks passed
@JoeRobich
Copy link
Member Author

/backport to release/insiders

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Started backporting to release/insiders (link to workflow run)

@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Mar 3, 2026
phil-allen-msft pushed a commit that referenced this pull request Mar 3, 2026
… is not supported (#82613)

Backport of #82508 to release/insiders

The underlying issue is that we never reported that there were errorIds
we didn't support. We would then only report compiler errors. This mean
analyzer diagnostics and build errors went unreported. This PR adds a
in-process cache of supported diagnostics ids and we now throw
NotImplementedException when we know for certain we do not support the
errorId. In cases where our cache is not populated and we cannot say
that the errorId is unsupported we will report it.

## Customer Impact

## Regression

- [x] Yes
- [ ] No

See https://github.com/dotnet/roslyn/pull/79915/changes#r2277979495

## Testing

Tests were added as well as manual testing.

## Risk

Low - Even in cases where the cache fails and never populates, we would
still report the errors such that they are shown in the Error List.

---------

Co-authored-by: Joey Robichaud <jorobich@microsoft.com>
jjonescz added a commit that referenced this pull request Mar 6, 2026
…ported" (#82638)

Reverts #82508

---------

Co-authored-by: Jan Jones <janjones@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build fails - Error List doesn't show error

3 participants