Skip to content

Commit bdb4dbe

Browse files
Fix issue where analyzers are missing when computing diagnostics OOP (#79821)
If you were to have the same analyzer added as both a host analyzer and a project analyzer, we might end up removing it from the "project analyzer" set when we create the project CompilationWithAnalyzers. This confuses later code since it expects the analyzers to be present. This was regressed in c860af4, where a mechanical refactoring took us from having a single CompilationWithAnalyzers to two, one for project analyzers and one for host analyzers. The loop in the method was copied into two, but the set wasn't cleared between the two loops. I believe the intent was to clear out the set (rather than the set persist to deal with duplicates between the host and project sets), given the comment implies we do want duplicates to overwrite in the analyzerMap that we're also producing. I also split the two analyzer maps, one for project analyzers and one for host analyzers. If you don't do this, it means that a conflict where the same analyzer existing in both sets will overwrite each other, later code will then return a project analyzer which might be a different instance than the host analyzer equivalent. Fixes #79706 Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2325694
2 parents 0039763 + 3fcabbd commit bdb4dbe

File tree

4 files changed

+215
-30
lines changed

4 files changed

+215
-30
lines changed

src/Compilers/Test/Core/Diagnostics/CommonDiagnosticAnalyzers.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2487,6 +2487,9 @@ public override void Initialize(AnalysisContext context)
24872487
}
24882488
}
24892489

2490+
/// <summary>
2491+
/// An analyzer that reports a diagnostic on every single named type.
2492+
/// </summary>
24902493
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
24912494
public sealed class NamedTypeAnalyzerWithConfigurableEnabledByDefault : DiagnosticAnalyzer
24922495
{

src/EditorFeatures/Test/Diagnostics/DiagnosticAnalyzerServiceTests.cs

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#nullable disable
66

77
using System;
8+
using System.Collections.Generic;
89
using System.Collections.Immutable;
910
using System.Linq;
1011
using System.Threading;
@@ -652,6 +653,166 @@ internal async Task TestOnlyRequiredAnalyzerExecutedDuringDiagnosticComputation(
652653
Assert.Empty(diagnosticMap.Other);
653654
}
654655

656+
[Theory, CombinatorialData]
657+
[WorkItem("https://github.com/dotnet/roslyn/issues/79706")]
658+
internal async Task TestAnalysisWithAnalyzerInBothProjectAndHost_SameAnalyzerInstance(bool documentAnalysis)
659+
{
660+
using var workspace = TestWorkspace.CreateCSharp("class A { }");
661+
662+
var analyzer = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, DiagnosticSeverity.Warning, throwOnAllNamedTypes: false);
663+
var analyzerId = analyzer.GetAnalyzerId();
664+
var analyzerReference = new AnalyzerImageReference([analyzer]);
665+
666+
workspace.TryApplyChanges(
667+
workspace.CurrentSolution.WithAnalyzerReferences([analyzerReference])
668+
.Projects.Single().AddAnalyzerReference(analyzerReference).Solution);
669+
670+
var project = workspace.CurrentSolution.Projects.Single();
671+
var document = documentAnalysis ? project.Documents.Single() : null;
672+
var diagnosticsMapResults = await DiagnosticComputer.GetDiagnosticsAsync(
673+
document, project, Checksum.Null, span: null, projectAnalyzerIds: [analyzerId], [analyzerId],
674+
AnalysisKind.Semantic, new DiagnosticAnalyzerInfoCache(), workspace.Services,
675+
logPerformanceInfo: false, getTelemetryInfo: false,
676+
cancellationToken: CancellationToken.None);
677+
678+
// In this case, since the analyzer identity is identical, we ran it once
679+
var analyzerResults = diagnosticsMapResults.Diagnostics.Single();
680+
Assert.Equal(analyzerId, analyzerResults.Item1);
681+
Assert.Equal(1, analyzerResults.Item2.Semantic.Length);
682+
}
683+
684+
[Theory, CombinatorialData]
685+
[WorkItem("https://github.com/dotnet/roslyn/issues/79706")]
686+
internal async Task TestAnalysisWithAnalyzerInBothProjectAndHost_DifferentAnalyzerInstancesFromNonEqualReferences(bool documentAnalysis)
687+
{
688+
using var workspace = TestWorkspace.CreateCSharp("class A { }");
689+
690+
var analyzerProject = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, DiagnosticSeverity.Warning, throwOnAllNamedTypes: false);
691+
var analyzerProjectId = analyzerProject.GetAnalyzerId();
692+
var analyzerProjectReference = new AnalyzerImageReference([analyzerProject]);
693+
694+
var analyzerHost = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, DiagnosticSeverity.Warning, throwOnAllNamedTypes: false);
695+
var analyzerHostId = analyzerHost.GetAnalyzerId();
696+
var analyzerHostReference = new AnalyzerImageReference([analyzerHost]);
697+
698+
// AnalyzerImageReference will create a separate AnalyzerImageReference.Id for each instance created, so these will be different.
699+
Assert.NotEqual(analyzerProjectReference.Id, analyzerHostReference.Id);
700+
Assert.Equal(analyzerProjectId, analyzerHostId);
701+
702+
workspace.TryApplyChanges(
703+
workspace.CurrentSolution.WithAnalyzerReferences([analyzerHostReference])
704+
.Projects.Single().AddAnalyzerReference(analyzerProjectReference).Solution);
705+
706+
var project = workspace.CurrentSolution.Projects.Single();
707+
var document = documentAnalysis ? project.Documents.Single() : null;
708+
var diagnosticsMapResults = await DiagnosticComputer.GetDiagnosticsAsync(
709+
document, project, Checksum.Null, span: null, projectAnalyzerIds: [analyzerProjectId], [analyzerHostId],
710+
AnalysisKind.Semantic, new DiagnosticAnalyzerInfoCache(), workspace.Services,
711+
logPerformanceInfo: false, getTelemetryInfo: false,
712+
cancellationToken: CancellationToken.None);
713+
714+
// In this case, since the analyzer reference identity is identical, we ran it once
715+
var analyzerResults = diagnosticsMapResults.Diagnostics.Single();
716+
Assert.Equal(analyzerHostId, analyzerResults.Item1);
717+
Assert.Equal(1, analyzerResults.Item2.Semantic.Length);
718+
}
719+
720+
[Theory, CombinatorialData]
721+
[WorkItem("https://github.com/dotnet/roslyn/issues/79706")]
722+
internal async Task TestAnalysisWithAnalyzerInBothProjectAndHost_DifferentAnalyzerInstancesFromEqualReferences(bool documentAnalysis, bool includeExtraProjectReference, bool includeExtraHostReference)
723+
{
724+
using var workspace = TestWorkspace.CreateCSharp("class A { }");
725+
726+
var analyzerProject = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, DiagnosticSeverity.Warning, throwOnAllNamedTypes: false);
727+
var analyzerProjectId = analyzerProject.GetAnalyzerId();
728+
var analyzerProjectReference = CreateAnalyzerReferenceWithSameId(analyzerProject);
729+
730+
var analyzerHost = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, DiagnosticSeverity.Warning, throwOnAllNamedTypes: false);
731+
var analyzerHostId = analyzerHost.GetAnalyzerId();
732+
var analyzerHostReference = CreateAnalyzerReferenceWithSameId(analyzerHost);
733+
734+
Assert.Equal(analyzerProjectReference.Id, analyzerHostReference.Id);
735+
Assert.Equal(analyzerProjectId, analyzerHostId);
736+
737+
workspace.TryApplyChanges(
738+
workspace.CurrentSolution.WithAnalyzerReferences(AddExtraReferenceIfNeeded(analyzerHostReference, includeExtraHostReference))
739+
.Projects.Single().AddAnalyzerReferences(AddExtraReferenceIfNeeded(analyzerProjectReference, includeExtraProjectReference)).Solution);
740+
741+
var project = workspace.CurrentSolution.Projects.Single();
742+
var document = documentAnalysis ? project.Documents.Single() : null;
743+
var diagnosticsMapResults = await DiagnosticComputer.GetDiagnosticsAsync(
744+
document, project, Checksum.Null, span: null, projectAnalyzerIds: [analyzerProjectId], [analyzerHostId],
745+
AnalysisKind.Semantic, new DiagnosticAnalyzerInfoCache(), workspace.Services,
746+
logPerformanceInfo: false, getTelemetryInfo: false,
747+
cancellationToken: CancellationToken.None);
748+
749+
// In this case, the analyzers are ran twice. This appears to be a bug in SkippedHostAnalyzersInfo.Create, because it calls
750+
// HostDiagnosticAnalyzers.CreateProjectDiagnosticAnalyzersPerReference which already filters out references, it doesn't return any
751+
// references to skip.
752+
Assert.Equal(2, diagnosticsMapResults.Diagnostics.Length);
753+
754+
static AnalyzerReference CreateAnalyzerReferenceWithSameId(DiagnosticAnalyzer analyzer)
755+
{
756+
var map = new Dictionary<string, ImmutableArray<DiagnosticAnalyzer>>()
757+
{
758+
{ LanguageNames.CSharp, [analyzer] }
759+
};
760+
761+
return new TestAnalyzerReferenceByLanguage(map);
762+
}
763+
764+
static ImmutableArray<AnalyzerReference> AddExtraReferenceIfNeeded(AnalyzerReference mainReference, bool addExtraReference)
765+
{
766+
if (addExtraReference)
767+
{
768+
return [mainReference, new AnalyzerImageReference([new FieldAnalyzer("FA1234", syntaxTreeAction: false)])];
769+
}
770+
else
771+
{
772+
return [mainReference];
773+
}
774+
}
775+
}
776+
777+
[Theory, CombinatorialData]
778+
[WorkItem("https://github.com/dotnet/roslyn/issues/79706")]
779+
internal async Task TestAnalysisWithAnalyzerInBothProjectAndHost_SameAnalyzerInstancesFromEqualReferences(bool documentAnalysis)
780+
{
781+
using var workspace = TestWorkspace.CreateCSharp("class A { }");
782+
783+
var analyzer = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, DiagnosticSeverity.Warning, throwOnAllNamedTypes: false);
784+
var analyzerProjectReference = CreateAnalyzerReferenceWithSameId(analyzer);
785+
var analyzerHostReference = CreateAnalyzerReferenceWithSameId(analyzer);
786+
787+
var analyzerId = analyzer.GetAnalyzerId();
788+
789+
Assert.Equal(analyzerProjectReference.Id, analyzerHostReference.Id);
790+
791+
workspace.TryApplyChanges(
792+
workspace.CurrentSolution.WithAnalyzerReferences([analyzerHostReference])
793+
.Projects.Single().AddAnalyzerReference(analyzerProjectReference).Solution);
794+
795+
var project = workspace.CurrentSolution.Projects.Single();
796+
var document = documentAnalysis ? project.Documents.Single() : null;
797+
var diagnosticsMapResults = await DiagnosticComputer.GetDiagnosticsAsync(
798+
document, project, Checksum.Null, span: null, projectAnalyzerIds: [analyzerId], [analyzerId],
799+
AnalysisKind.Semantic, new DiagnosticAnalyzerInfoCache(), workspace.Services,
800+
logPerformanceInfo: false, getTelemetryInfo: false,
801+
cancellationToken: CancellationToken.None);
802+
803+
Assert.Equal(1, diagnosticsMapResults.Diagnostics.Length);
804+
805+
static AnalyzerReference CreateAnalyzerReferenceWithSameId(DiagnosticAnalyzer analyzer)
806+
{
807+
var map = new Dictionary<string, ImmutableArray<DiagnosticAnalyzer>>()
808+
{
809+
{ LanguageNames.CSharp, [analyzer] }
810+
};
811+
812+
return new TestAnalyzerReferenceByLanguage(map);
813+
}
814+
}
815+
655816
[Theory, WorkItem(67257, "https://github.com/dotnet/roslyn/issues/67257")]
656817
[CombinatorialData]
657818
public async Task TestFilterSpanOnContextAsync(FilterSpanTestAnalyzer.AnalysisKind kind)

src/Workspaces/Core/Portable/Diagnostics/Extensions.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,13 @@ public static async Task<ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnaly
130130
var builder = ImmutableDictionary.CreateBuilder<DiagnosticAnalyzer, DiagnosticAnalysisResultBuilder>();
131131
foreach (var analyzer in projectAnalyzers.ConcatFast(hostAnalyzers))
132132
{
133+
if (builder.ContainsKey(analyzer))
134+
{
135+
// If we already have a result for this analyzer, we had a duplicate. We already processed the results
136+
// for this so no reason to process it a second time.
137+
continue;
138+
}
139+
133140
cancellationToken.ThrowIfCancellationRequested();
134141

135142
if (skippedAnalyzersInfo.SkippedAnalyzers.Contains(analyzer))

0 commit comments

Comments
 (0)