Skip to content

Commit c167ade

Browse files
authored
[release/dev17.14] Fix issue where analyzers are missing when computing diagnostics OOP (#79861)
Backport of #79821 to release/dev17.14 /cc @jasonmalinowski ## Customer Impact ## Regression - [ ] Yes - [ ] No [If yes, specify when the regression was introduced. Provide the PR or commit if known.] ## Testing [How was the fix verified? How was the issue missed previously? What tests were added?] ## Risk [High/Medium/Low. Justify the indication by mentioning how risks were measured and addressed.]
2 parents a466013 + 5f20df3 commit c167ade

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;
@@ -710,6 +711,166 @@ internal async Task TestOnlyRequiredAnalyzerExecutedDuringDiagnosticComputation(
710711
Assert.Empty(diagnosticMap.Other);
711712
}
712713

714+
[Theory, CombinatorialData]
715+
[WorkItem("https://github.com/dotnet/roslyn/issues/79706")]
716+
internal async Task TestAnalysisWithAnalyzerInBothProjectAndHost_SameAnalyzerInstance(bool documentAnalysis)
717+
{
718+
using var workspace = TestWorkspace.CreateCSharp("class A { }");
719+
720+
var analyzer = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, DiagnosticSeverity.Warning, throwOnAllNamedTypes: false);
721+
var analyzerId = analyzer.GetAnalyzerId();
722+
var analyzerReference = new AnalyzerImageReference([analyzer]);
723+
724+
workspace.TryApplyChanges(
725+
workspace.CurrentSolution.WithAnalyzerReferences([analyzerReference])
726+
.Projects.Single().AddAnalyzerReference(analyzerReference).Solution);
727+
728+
var project = workspace.CurrentSolution.Projects.Single();
729+
var document = documentAnalysis ? project.Documents.Single() : null;
730+
var diagnosticsMapResults = await DiagnosticComputer.GetDiagnosticsAsync(
731+
document, project, Checksum.Null, span: null, projectAnalyzerIds: [analyzerId], [analyzerId],
732+
AnalysisKind.Semantic, new DiagnosticAnalyzerInfoCache(), workspace.Services,
733+
isExplicit: false, logPerformanceInfo: false, getTelemetryInfo: false,
734+
cancellationToken: CancellationToken.None);
735+
736+
// In this case, since the analyzer identity is identical, we ran it once
737+
var analyzerResults = diagnosticsMapResults.Diagnostics.Single();
738+
Assert.Equal(analyzerId, analyzerResults.Item1);
739+
Assert.Equal(1, analyzerResults.Item2.Semantic.Length);
740+
}
741+
742+
[Theory, CombinatorialData]
743+
[WorkItem("https://github.com/dotnet/roslyn/issues/79706")]
744+
internal async Task TestAnalysisWithAnalyzerInBothProjectAndHost_DifferentAnalyzerInstancesFromNonEqualReferences(bool documentAnalysis)
745+
{
746+
using var workspace = TestWorkspace.CreateCSharp("class A { }");
747+
748+
var analyzerProject = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, DiagnosticSeverity.Warning, throwOnAllNamedTypes: false);
749+
var analyzerProjectId = analyzerProject.GetAnalyzerId();
750+
var analyzerProjectReference = new AnalyzerImageReference([analyzerProject]);
751+
752+
var analyzerHost = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, DiagnosticSeverity.Warning, throwOnAllNamedTypes: false);
753+
var analyzerHostId = analyzerHost.GetAnalyzerId();
754+
var analyzerHostReference = new AnalyzerImageReference([analyzerHost]);
755+
756+
// AnalyzerImageReference will create a separate AnalyzerImageReference.Id for each instance created, so these will be different.
757+
Assert.NotEqual(analyzerProjectReference.Id, analyzerHostReference.Id);
758+
Assert.Equal(analyzerProjectId, analyzerHostId);
759+
760+
workspace.TryApplyChanges(
761+
workspace.CurrentSolution.WithAnalyzerReferences([analyzerHostReference])
762+
.Projects.Single().AddAnalyzerReference(analyzerProjectReference).Solution);
763+
764+
var project = workspace.CurrentSolution.Projects.Single();
765+
var document = documentAnalysis ? project.Documents.Single() : null;
766+
var diagnosticsMapResults = await DiagnosticComputer.GetDiagnosticsAsync(
767+
document, project, Checksum.Null, span: null, projectAnalyzerIds: [analyzerProjectId], [analyzerHostId],
768+
AnalysisKind.Semantic, new DiagnosticAnalyzerInfoCache(), workspace.Services,
769+
isExplicit: false, logPerformanceInfo: false, getTelemetryInfo: false,
770+
cancellationToken: CancellationToken.None);
771+
772+
// In this case, since the analyzer reference identity is identical, we ran it once
773+
var analyzerResults = diagnosticsMapResults.Diagnostics.Single();
774+
Assert.Equal(analyzerHostId, analyzerResults.Item1);
775+
Assert.Equal(1, analyzerResults.Item2.Semantic.Length);
776+
}
777+
778+
[Theory, CombinatorialData]
779+
[WorkItem("https://github.com/dotnet/roslyn/issues/79706")]
780+
internal async Task TestAnalysisWithAnalyzerInBothProjectAndHost_DifferentAnalyzerInstancesFromEqualReferences(bool documentAnalysis, bool includeExtraProjectReference, bool includeExtraHostReference)
781+
{
782+
using var workspace = TestWorkspace.CreateCSharp("class A { }");
783+
784+
var analyzerProject = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, DiagnosticSeverity.Warning, throwOnAllNamedTypes: false);
785+
var analyzerProjectId = analyzerProject.GetAnalyzerId();
786+
var analyzerProjectReference = CreateAnalyzerReferenceWithSameId(analyzerProject);
787+
788+
var analyzerHost = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, DiagnosticSeverity.Warning, throwOnAllNamedTypes: false);
789+
var analyzerHostId = analyzerHost.GetAnalyzerId();
790+
var analyzerHostReference = CreateAnalyzerReferenceWithSameId(analyzerHost);
791+
792+
Assert.Equal(analyzerProjectReference.Id, analyzerHostReference.Id);
793+
Assert.Equal(analyzerProjectId, analyzerHostId);
794+
795+
workspace.TryApplyChanges(
796+
workspace.CurrentSolution.WithAnalyzerReferences(AddExtraReferenceIfNeeded(analyzerHostReference, includeExtraHostReference))
797+
.Projects.Single().AddAnalyzerReferences(AddExtraReferenceIfNeeded(analyzerProjectReference, includeExtraProjectReference)).Solution);
798+
799+
var project = workspace.CurrentSolution.Projects.Single();
800+
var document = documentAnalysis ? project.Documents.Single() : null;
801+
var diagnosticsMapResults = await DiagnosticComputer.GetDiagnosticsAsync(
802+
document, project, Checksum.Null, span: null, projectAnalyzerIds: [analyzerProjectId], [analyzerHostId],
803+
AnalysisKind.Semantic, new DiagnosticAnalyzerInfoCache(), workspace.Services,
804+
isExplicit: false, logPerformanceInfo: false, getTelemetryInfo: false,
805+
cancellationToken: CancellationToken.None);
806+
807+
// In this case, the analyzers are ran twice. This appears to be a bug in SkippedHostAnalyzersInfo.Create, because it calls
808+
// HostDiagnosticAnalyzers.CreateProjectDiagnosticAnalyzersPerReference which already filters out references, it doesn't return any
809+
// references to skip.
810+
Assert.Equal(2, diagnosticsMapResults.Diagnostics.Length);
811+
812+
static AnalyzerReference CreateAnalyzerReferenceWithSameId(DiagnosticAnalyzer analyzer)
813+
{
814+
var map = new Dictionary<string, ImmutableArray<DiagnosticAnalyzer>>()
815+
{
816+
{ LanguageNames.CSharp, [analyzer] }
817+
};
818+
819+
return new TestAnalyzerReferenceByLanguage(map);
820+
}
821+
822+
static ImmutableArray<AnalyzerReference> AddExtraReferenceIfNeeded(AnalyzerReference mainReference, bool addExtraReference)
823+
{
824+
if (addExtraReference)
825+
{
826+
return [mainReference, new AnalyzerImageReference([new FieldAnalyzer("FA1234", syntaxTreeAction: false)])];
827+
}
828+
else
829+
{
830+
return [mainReference];
831+
}
832+
}
833+
}
834+
835+
[Theory, CombinatorialData]
836+
[WorkItem("https://github.com/dotnet/roslyn/issues/79706")]
837+
internal async Task TestAnalysisWithAnalyzerInBothProjectAndHost_SameAnalyzerInstancesFromEqualReferences(bool documentAnalysis)
838+
{
839+
using var workspace = TestWorkspace.CreateCSharp("class A { }");
840+
841+
var analyzer = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, DiagnosticSeverity.Warning, throwOnAllNamedTypes: false);
842+
var analyzerProjectReference = CreateAnalyzerReferenceWithSameId(analyzer);
843+
var analyzerHostReference = CreateAnalyzerReferenceWithSameId(analyzer);
844+
845+
var analyzerId = analyzer.GetAnalyzerId();
846+
847+
Assert.Equal(analyzerProjectReference.Id, analyzerHostReference.Id);
848+
849+
workspace.TryApplyChanges(
850+
workspace.CurrentSolution.WithAnalyzerReferences([analyzerHostReference])
851+
.Projects.Single().AddAnalyzerReference(analyzerProjectReference).Solution);
852+
853+
var project = workspace.CurrentSolution.Projects.Single();
854+
var document = documentAnalysis ? project.Documents.Single() : null;
855+
var diagnosticsMapResults = await DiagnosticComputer.GetDiagnosticsAsync(
856+
document, project, Checksum.Null, span: null, projectAnalyzerIds: [analyzerId], [analyzerId],
857+
AnalysisKind.Semantic, new DiagnosticAnalyzerInfoCache(), workspace.Services,
858+
isExplicit: false, logPerformanceInfo: false, getTelemetryInfo: false,
859+
cancellationToken: CancellationToken.None);
860+
861+
Assert.Equal(1, diagnosticsMapResults.Diagnostics.Length);
862+
863+
static AnalyzerReference CreateAnalyzerReferenceWithSameId(DiagnosticAnalyzer analyzer)
864+
{
865+
var map = new Dictionary<string, ImmutableArray<DiagnosticAnalyzer>>()
866+
{
867+
{ LanguageNames.CSharp, [analyzer] }
868+
};
869+
870+
return new TestAnalyzerReferenceByLanguage(map);
871+
}
872+
}
873+
713874
[Theory, WorkItem(67257, "https://github.com/dotnet/roslyn/issues/67257")]
714875
[CombinatorialData]
715876
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
@@ -128,6 +128,13 @@ public static async Task<ImmutableDictionary<DiagnosticAnalyzer, DiagnosticAnaly
128128
var builder = ImmutableDictionary.CreateBuilder<DiagnosticAnalyzer, DiagnosticAnalysisResultBuilder>();
129129
foreach (var analyzer in projectAnalyzers.ConcatFast(hostAnalyzers))
130130
{
131+
if (builder.ContainsKey(analyzer))
132+
{
133+
// If we already have a result for this analyzer, we had a duplicate. We already processed the results
134+
// for this so no reason to process it a second time.
135+
continue;
136+
}
137+
131138
cancellationToken.ThrowIfCancellationRequested();
132139

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

0 commit comments

Comments
 (0)