Remove 'open file only' concept from analyzer driver #74340
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dotnet-issue-labeler
bot
added
Area-Analyzers
untriaged
CyrusNajmabadi
commented
Jul 10, 2024
| @@ -945,24 +945,6 @@ public override void Initialize(AnalysisContext context) | |||
| } | |||
| } | |||
|
|
|||
| private class OpenFileOnlyAnalyzer : DiagnosticAnalyzer, IBuiltInAnalyzer | |||
There was a problem hiding this comment.
never referenced.
CyrusNajmabadi
commented
Jul 10, 2024
| @@ -23,9 +23,6 @@ public static bool IsWorkspaceDiagnosticAnalyzer(this DiagnosticAnalyzer analyze | |||
| public static bool IsBuiltInAnalyzer(this DiagnosticAnalyzer analyzer) | |||
| => analyzer is IBuiltInAnalyzer || analyzer.IsWorkspaceDiagnosticAnalyzer() || analyzer.IsCompilerAnalyzer(); | |||
|
|
|||
| public static bool IsOpenFileOnly(this DiagnosticAnalyzer analyzer, SimplifierOptions? options) | |||
There was a problem hiding this comment.
now only returns false. was inlined as appropriate into callsites.
CyrusNajmabadi
commented
Jul 10, 2024
| @@ -33,12 +33,8 @@ internal partial class DiagnosticIncrementalAnalyzer | |||
| var version = await GetDiagnosticVersionAsync(project, cancellationToken).ConfigureAwait(false); | |||
| var existingData = await ProjectAnalysisData.CreateAsync(project, stateSets, avoidLoadingData, cancellationToken).ConfigureAwait(false); | |||
|
|
|||
| // We can't return here if we have open file only analyzers since saved data for open file only analyzer | |||
| // is incomplete -- it only contains info on open files rather than whole project. | |||
| if (existingData.Version == version && !CompilationHasOpenFileOnlyAnalyzers(compilationWithAnalyzers, ideOptions.SimplifierOptions)) | |||
There was a problem hiding this comment.
this method only returns false now. so inlined and updated as appropriate.
CyrusNajmabadi
commented
Jul 10, 2024
| @@ -213,8 +191,7 @@ private static bool CompilationHasOpenFileOnlyAnalyzers(CompilationWithAnalyzers | |||
| foreach (var analyzer in existingAnalyzers) | |||
| { | |||
| if (existing.TryGetValue(analyzer, out var analysisResult) && | |||
| analysisResult.Version == version && | |||
| !analyzer.IsOpenFileOnly(ideOptions.SimplifierOptions)) | |||
| analysisResult.Version == version) | |||
There was a problem hiding this comment.
inlined and elided.
CyrusNajmabadi
commented
Jul 10, 2024
| @@ -195,8 +193,7 @@ public async Task<ImmutableArray<Diagnostic>> GetSourceGeneratorDiagnosticsAsync | |||
|
|
|||
| var ideOptions = ((WorkspaceAnalyzerOptions)compilationWithAnalyzers.AnalysisOptions.Options!).IdeOptions; | |||
|
|
|||
| var analyzers = documentAnalysisScope?.Analyzers ?? | |||
| compilationWithAnalyzers.Analyzers.Where(a => forceExecuteAllAnalyzers || !a.IsOpenFileOnly(ideOptions.SimplifierOptions)); | |||
There was a problem hiding this comment.
the right side of the || became true. So this Where was unnecessary. which made forceExecuteAllAnalyzers unnecessary. which led to the callsite updates.
|
@tmat ptal. |
tmat
approved these changes
Jul 10, 2024
This was referenced Jul 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This logic existed at a time when we unilaterally ran all analyzers all the time. This was far too slow for some of these analyzers, and so we had a special back-door to have them opt out of running.
We no longer need this as our move to pull diagnostics (with separate categories for analyzers vs compiler errors), and our user options around how/when analyzers and compiler diagnostics should run supersedes this.