Skip to content
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

Jump to bottom

Only provide fallback options for host analyzers #75317

Merged
merged 2 commits into from
Oct 10, 2024
Merged

Only provide fallback options for host analyzers #75317

merged 2 commits into from
Oct 10, 2024

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented Oct 1, 2024 •
edited
Loading

  • Separate analysis passes into "project" analyzers (aka NuGet-installed analyzers) and "solution" analyzers (aka host- or VSIX-installed analyzers)
  • Only provide fallback options from the host for host-installed analyzers

Items not covered by this pull request

  • Redirect Code Style analyzers to Features analyzers when loaded in Visual Studio. This is addressed separately by Do not load CodeStyle analyzers added by the SDK #75250.
  • Treat Features analyzers as project analyzers when they are being used in place of Code Style analyzers.
  • NuGet-installed code fixes and refactorings should not use host options, but currently they do. Only source generators and analyzers are corrected by this pull request.

Known issues introduced by this change

@sharwell sharwell requested review from a team as code owners October 1, 2024 13:38
@sharwell sharwell marked this pull request as draft October 1, 2024 13:38
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 1, 2024
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

This PR modifies public API files. Please follow the instructions at https://github.com/dotnet/roslyn/blob/main/docs/contributing/API%20Review%20Process.md for ensuring all public APIs are reviewed before merging.

@dotnet-policy-service dotnet-policy-service bot added the Needs API Review Needs to be reviewed by the API review council label Oct 1, 2024
@build-analysis build-analysis bot mentioned this pull request Oct 1, 2024
Open
3 tasks
@build-analysis build-analysis bot mentioned this pull request Oct 2, 2024
Open
3 tasks
tmat
tmat reviewed Oct 2, 2024
View reviewed changes
src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs
foreach (var reference in _project.Solution.AnalyzerReferences.Concat(_project.AnalyzerReferences))
using var _1 = ArrayBuilder<DiagnosticAnalyzer>.GetInstance(out var projectAnalyzerBuilder);
using var _2 = ArrayBuilder<DiagnosticAnalyzer>.GetInstance(out var hostAnalyzerBuilder);
foreach (var reference in _project.Solution.AnalyzerReferences)
{
Copy link
Member

@tmat tmat Oct 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Perhaps the loop can be factored to a local function and AppendAnalyzerMap can be inlined.

Copy link
Member Author

Choose a reason for hiding this comment

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

➡️ This is small enough that I'll leave it for now.

@arunchndr
Copy link
Member

What do we need before this can be published?

@sharwell sharwell changed the base branch from main to release/dev17.12 October 4, 2024 18:02
@sharwell sharwell mentioned this pull request Oct 4, 2024
Closed
@sharwell sharwell marked this pull request as ready for review October 9, 2024 14:42
tmat
tmat reviewed Oct 9, 2024
View reviewed changes
src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs
@@ -333,14 +360,84 @@ public AnalyzerConfigData GetAnalyzerOptionsForPath(string path, CancellationTok
}

internal sealed class ProjectAnalyzerConfigOptionsProvider(ProjectState projectState) : AnalyzerConfigOptionsProvider
{
private AnalyzerConfigOptionsCache.Value GetCache()
Copy link
Member

Choose a reason for hiding this comment

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

Deduplicate code in a follow up. E.g. use a common base class

tmat
tmat approved these changes Oct 9, 2024
View reviewed changes
Copy link
Member

@tmat tmat left a comment

Choose a reason for hiding this comment

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

Approved assuming a clean-up will be done in a follow up PR.

@sharwell sharwell merged commit cf4dbdf into dotnet:release/dev17.12 Oct 10, 2024
25 checks passed
@sharwell sharwell deleted the host-options branch October 10, 2024 14:27
This was referenced Oct 24, 2024
Closed
Closed
Closed
@sharwell sharwell mentioned this pull request Nov 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmat tmat tmat approved these changes

Assignees
No one assigned
Labels
Area-Analyzers Needs API Review Needs to be reviewed by the API review council untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sharwell @arunchndr @tmat