diff --git a/src/VisualStudio/Core/Def/Workspace/SourceGeneratedFileManager.cs b/src/VisualStudio/Core/Def/Workspace/SourceGeneratedFileManager.cs index 94a4ac734a9f3..94b5c7e2c5d92 100644 --- a/src/VisualStudio/Core/Def/Workspace/SourceGeneratedFileManager.cs +++ b/src/VisualStudio/Core/Def/Workspace/SourceGeneratedFileManager.cs @@ -346,7 +346,7 @@ public async ValueTask RefreshFileAsync(CancellationToken cancellationToken) else { // The file isn't there anymore; do we still have the generator at all? - if (project.AnalyzerReferences.Any(a => a.GetGenerators(project.Language).Any(static (g, self) => SourceGeneratorIdentity.GetGeneratorAssemblyName(g) == self._documentIdentity.Generator.AssemblyName, this))) + if (project.AnalyzerReferences.Any(a => a.FullPath == _documentIdentity.Generator.AssemblyPath)) { windowFrameMessageToShow = string.Format(ServicesVSResources.The_generator_0_that_generated_this_file_has_stopped_generating_this_file, GeneratorDisplayName); windowFrameImageMonikerToShow = KnownMonikers.StatusError; diff --git a/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/SourceGeneratorItem.cs b/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/SourceGeneratorItem.cs index 87dec097f7a55..da6dd13f2a261 100644 --- a/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/SourceGeneratorItem.cs +++ b/src/VisualStudio/Core/Impl/SolutionExplorer/DiagnosticItem/SourceGeneratorItem.cs @@ -19,7 +19,7 @@ public SourceGeneratorItem(ProjectId projectId, ISourceGenerator generator, Anal : base(name: SourceGeneratorIdentity.GetGeneratorTypeName(generator)) { ProjectId = projectId; - Identity = new SourceGeneratorIdentity(generator); + Identity = new SourceGeneratorIdentity(generator, analyzerReference); AnalyzerReference = analyzerReference; } diff --git a/src/Workspaces/Core/Portable/SourceGeneratorTelemetry/ISourceGeneratorTelemetryCollectorWorkspaceService.cs b/src/Workspaces/Core/Portable/SourceGeneratorTelemetry/ISourceGeneratorTelemetryCollectorWorkspaceService.cs index 11687f1346ed0..3de84caecda15 100644 --- a/src/Workspaces/Core/Portable/SourceGeneratorTelemetry/ISourceGeneratorTelemetryCollectorWorkspaceService.cs +++ b/src/Workspaces/Core/Portable/SourceGeneratorTelemetry/ISourceGeneratorTelemetryCollectorWorkspaceService.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.Text; using Microsoft.CodeAnalysis.Host; @@ -11,6 +12,6 @@ namespace Microsoft.CodeAnalysis.SourceGeneratorTelemetry { internal interface ISourceGeneratorTelemetryCollectorWorkspaceService : IWorkspaceService { - void CollectRunResult(GeneratorDriverRunResult driverRunResult, GeneratorDriverTimingInfo driverTimingInfo); + void CollectRunResult(GeneratorDriverRunResult driverRunResult, GeneratorDriverTimingInfo driverTimingInfo, ProjectState project); } } diff --git a/src/Workspaces/Core/Portable/SourceGeneratorTelemetry/SourceGeneratorTelemetryCollectorWorkspaceService.cs b/src/Workspaces/Core/Portable/SourceGeneratorTelemetry/SourceGeneratorTelemetryCollectorWorkspaceService.cs index 48549d468bf09..9052fe6510e63 100644 --- a/src/Workspaces/Core/Portable/SourceGeneratorTelemetry/SourceGeneratorTelemetryCollectorWorkspaceService.cs +++ b/src/Workspaces/Core/Portable/SourceGeneratorTelemetry/SourceGeneratorTelemetryCollectorWorkspaceService.cs @@ -3,8 +3,11 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Immutable; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Internal.Log; using Roslyn.Utilities; @@ -14,15 +17,22 @@ internal class SourceGeneratorTelemetryCollectorWorkspaceService : ISourceGenera { private sealed record GeneratorTelemetryKey { - public GeneratorTelemetryKey(ISourceGenerator generator) + // TODO: mark with SetsRequiredMembers when we have the attributes in place + // [SetsRequiredMembers] + public GeneratorTelemetryKey(ISourceGenerator generator, AnalyzerReference analyzerReference) { - Identity = new SourceGeneratorIdentity(generator); - FileVersion = FileVersionInfo.GetVersionInfo(generator.GetGeneratorType().Assembly.Location).FileVersion ?? "(null)"; + Identity = new SourceGeneratorIdentity(generator, analyzerReference); + FileVersion = "(null)"; + + if (Identity.AssemblyPath != null) + { + FileVersion = FileVersionInfo.GetVersionInfo(Identity.AssemblyPath).FileVersion; + } } // TODO: mark these 'required' when we have the attributes in place - public SourceGeneratorIdentity Identity { get; } - public string FileVersion { get; } + public SourceGeneratorIdentity Identity { get; init; } + public string FileVersion { get; init; } } /// @@ -34,19 +44,19 @@ public GeneratorTelemetryKey(ISourceGenerator generator) private readonly StatisticLogAggregator _elapsedTimeByGenerator = new(); private readonly StatisticLogAggregator _producedFilesByGenerator = new(); - private GeneratorTelemetryKey GetTelemetryKey(ISourceGenerator generator) - => _generatorTelemetryKeys.GetValue(generator, static g => new GeneratorTelemetryKey(g)); + private GeneratorTelemetryKey GetTelemetryKey(ISourceGenerator generator, ProjectState project) + => _generatorTelemetryKeys.GetValue(generator, g => new GeneratorTelemetryKey(g, project.GetAnalyzerReferenceForGenerator(g))); - public void CollectRunResult(GeneratorDriverRunResult driverRunResult, GeneratorDriverTimingInfo driverTimingInfo) + public void CollectRunResult(GeneratorDriverRunResult driverRunResult, GeneratorDriverTimingInfo driverTimingInfo, ProjectState project) { foreach (var generatorTime in driverTimingInfo.GeneratorTimes) { - _elapsedTimeByGenerator.AddDataPoint(GetTelemetryKey(generatorTime.Generator), generatorTime.ElapsedTime); + _elapsedTimeByGenerator.AddDataPoint(GetTelemetryKey(generatorTime.Generator, project), generatorTime.ElapsedTime); } foreach (var generatorResult in driverRunResult.Results) { - _producedFilesByGenerator.AddDataPoint(GetTelemetryKey(generatorResult.Generator), generatorResult.GeneratedSources.Length); + _producedFilesByGenerator.AddDataPoint(GetTelemetryKey(generatorResult.Generator, project), generatorResult.GeneratedSources.Length); } } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs index a5246b53caad1..5d7c18cdb3883 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectState.cs @@ -61,9 +61,9 @@ internal partial class ProjectState private AnalyzerOptions? _lazyAnalyzerOptions; /// - /// Backing field for ; this is a default ImmutableArray if it hasn't been computed yet. + /// The list of source generators and the analyzer reference they came from. /// - private ImmutableArray _lazySourceGenerators; + private ImmutableDictionary? _lazySourceGenerators; private ProjectState( ProjectInfo projectInfo, @@ -796,20 +796,38 @@ public ProjectState WithAnalyzerReferences(IEnumerable analyz return With(projectInfo: ProjectInfo.WithAnalyzerReferences(analyzerReferences).WithVersion(Version.GetNewerVersion())); } - public ImmutableArray SourceGenerators + [MemberNotNull(nameof(_lazySourceGenerators))] + private void EnsureSourceGeneratorsInitialized() { - get + if (_lazySourceGenerators == null) { - if (_lazySourceGenerators.IsDefault) + var builder = ImmutableDictionary.CreateBuilder(); + + foreach (var analyzerReference in AnalyzerReferences) { - var generators = AnalyzerReferences.SelectMany(a => a.GetGenerators(this.Language)).ToImmutableArray(); - ImmutableInterlocked.InterlockedInitialize(ref _lazySourceGenerators, generators); + foreach (var generator in analyzerReference.GetGenerators(Language)) + builder.Add(generator, analyzerReference); } - return _lazySourceGenerators; + Interlocked.CompareExchange(ref _lazySourceGenerators, builder.ToImmutable(), comparand: null); } } + public IEnumerable SourceGenerators + { + get + { + EnsureSourceGeneratorsInitialized(); + return _lazySourceGenerators.Keys; + } + } + + public AnalyzerReference GetAnalyzerReferenceForGenerator(ISourceGenerator generator) + { + EnsureSourceGeneratorsInitialized(); + return _lazySourceGenerators[generator]; + } + public ProjectState AddDocuments(ImmutableArray documents) { Debug.Assert(!documents.Any(d => DocumentStates.Contains(d.Id))); diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationAndGeneratorDriverTranslationAction_Actions.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationAndGeneratorDriverTranslationAction_Actions.cs index 2679b177db7bb..636d8676cdaa1 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationAndGeneratorDriverTranslationAction_Actions.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationAndGeneratorDriverTranslationAction_Actions.cs @@ -330,7 +330,7 @@ public ReplaceGeneratorDriverAction(GeneratorDriver oldGeneratorDriver, ProjectS var generatorDriver = _oldGeneratorDriver.ReplaceAdditionalTexts(_newProjectState.AdditionalDocumentStates.SelectAsArray(static documentState => documentState.AdditionalText)) .WithUpdatedParseOptions(_newProjectState.ParseOptions!) .WithUpdatedAnalyzerConfigOptions(_newProjectState.AnalyzerOptions.AnalyzerConfigOptionsProvider) - .ReplaceGenerators(_newProjectState.SourceGenerators); + .ReplaceGenerators(_newProjectState.SourceGenerators.ToImmutableArray()); return generatorDriver; } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs index 71d30f9cb155e..d66313ca4d7a2 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs @@ -834,7 +834,7 @@ private async Task FinalizeCompilationAsync( generatorInfo = generatorInfo.WithDriver(compilationFactory.CreateGeneratorDriver( this.ProjectState.ParseOptions!, - ProjectState.SourceGenerators, + ProjectState.SourceGenerators.ToImmutableArray(), this.ProjectState.AnalyzerOptions.AnalyzerConfigOptionsProvider, additionalTexts)); } @@ -880,7 +880,7 @@ private async Task FinalizeCompilationAsync( generatorInfo = generatorInfo.WithDriver(generatorInfo.Driver!.RunGenerators(compilationToRunGeneratorsOn, cancellationToken)); - solution.Services.GetService()?.CollectRunResult(generatorInfo.Driver!.GetRunResult(), generatorInfo.Driver!.GetTimingInfo()); + solution.Services.GetService()?.CollectRunResult(generatorInfo.Driver!.GetRunResult(), generatorInfo.Driver!.GetTimingInfo(), ProjectState); var runResult = generatorInfo.Driver!.GetRunResult(); @@ -907,11 +907,14 @@ private async Task FinalizeCompilationAsync( continue; } + var generatorAnalyzerReference = this.ProjectState.GetAnalyzerReferenceForGenerator(generatorResult.Generator); + foreach (var generatedSource in generatorResult.GeneratedSources) { var existing = FindExistingGeneratedDocumentState( generatorInfo.Documents, generatorResult.Generator, + generatorAnalyzerReference, generatedSource.HintName); if (existing != null) @@ -933,7 +936,8 @@ private async Task FinalizeCompilationAsync( ProjectState.Id, generatedSource.HintName, generatorResult.Generator, - generatedSource.SyntaxTree.FilePath); + generatedSource.SyntaxTree.FilePath, + generatorAnalyzerReference); generatedDocumentsBuilder.Add( SourceGeneratedDocumentState.Create( @@ -1002,9 +1006,10 @@ private async Task FinalizeCompilationAsync( static SourceGeneratedDocumentState? FindExistingGeneratedDocumentState( TextDocumentStates states, ISourceGenerator generator, + AnalyzerReference analyzerReference, string hintName) { - var generatorIdentity = new SourceGeneratorIdentity(generator); + var generatorIdentity = new SourceGeneratorIdentity(generator, analyzerReference); foreach (var (_, state) in states.States) { diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocumentIdentity.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocumentIdentity.cs index 0841301656383..4c2f9baef98ec 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocumentIdentity.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocumentIdentity.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Text; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; using Roslyn.Utilities; @@ -21,23 +22,28 @@ internal readonly record struct SourceGeneratedDocumentIdentity { public bool ShouldReuseInSerialization => true; - public static SourceGeneratedDocumentIdentity Generate(ProjectId projectId, string hintName, ISourceGenerator generator, string filePath) + public static SourceGeneratedDocumentIdentity Generate(ProjectId projectId, string hintName, ISourceGenerator generator, string filePath, AnalyzerReference analyzerReference) { // We want the DocumentId generated for a generated output to be stable between Compilations; this is so features that track // a document by DocumentId can find it after some change has happened that requires generators to run again. // To achieve this we'll just do a crytographic hash of the generator name and hint name; the choice of a cryptographic hash // as opposed to a more generic string hash is we actually want to ensure we don't have collisions. - var generatorIdentity = new SourceGeneratorIdentity(generator); + var generatorIdentity = new SourceGeneratorIdentity(generator, analyzerReference); // Combine the strings together; we'll use Encoding.Unicode since that'll match the underlying format; this can be made much // faster once we're on .NET Core since we could directly treat the strings as ReadOnlySpan. var projectIdBytes = projectId.Id.ToByteArray(); - using var _ = ArrayBuilder.GetInstance(capacity: (generatorIdentity.AssemblyName.Length + 1 + generatorIdentity.TypeName.Length + 1 + hintName.Length) * 2 + projectIdBytes.Length, out var hashInput); + + // The assembly path should exist in any normal scenario; the hashing of the name only would apply if the user loaded a + // dynamic assembly they produced at runtime and passed us that via a custom AnalyzerReference. + var assemblyNameToHash = generatorIdentity.AssemblyPath ?? generatorIdentity.AssemblyName; + + using var _ = ArrayBuilder.GetInstance(capacity: (assemblyNameToHash.Length + 1 + generatorIdentity.TypeName.Length + 1 + hintName.Length) * 2 + projectIdBytes.Length, out var hashInput); hashInput.AddRange(projectIdBytes); // Add a null to separate the generator name and hint name; since this is effectively a joining of UTF-16 bytes // we'll use a UTF-16 null just to make sure there's absolutely no risk of collision. - hashInput.AddRange(Encoding.Unicode.GetBytes(generatorIdentity.AssemblyName)); + hashInput.AddRange(Encoding.Unicode.GetBytes(assemblyNameToHash)); hashInput.AddRange(0, 0); hashInput.AddRange(Encoding.Unicode.GetBytes(generatorIdentity.TypeName)); hashInput.AddRange(0, 0); @@ -61,6 +67,7 @@ public void WriteTo(ObjectWriter writer) writer.WriteString(HintName); writer.WriteString(Generator.AssemblyName); + writer.WriteString(Generator.AssemblyPath); writer.WriteString(Generator.AssemblyVersion.ToString()); writer.WriteString(Generator.TypeName); writer.WriteString(FilePath); @@ -72,6 +79,7 @@ internal static SourceGeneratedDocumentIdentity ReadFrom(ObjectReader reader) var hintName = reader.ReadString(); var generatorAssemblyName = reader.ReadString(); + var generatorAssemblyPath = reader.ReadString(); var generatorAssemblyVersion = Version.Parse(reader.ReadString()); var generatorTypeName = reader.ReadString(); var filePath = reader.ReadString(); @@ -79,7 +87,13 @@ internal static SourceGeneratedDocumentIdentity ReadFrom(ObjectReader reader) return new SourceGeneratedDocumentIdentity( documentId, hintName, - new SourceGeneratorIdentity(generatorAssemblyName, generatorAssemblyVersion, generatorTypeName), + new SourceGeneratorIdentity + { + AssemblyName = generatorAssemblyName, + AssemblyPath = generatorAssemblyPath, + AssemblyVersion = generatorAssemblyVersion, + TypeName = generatorTypeName + }, filePath); } } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratorIdentity.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratorIdentity.cs index f347a0c91ef34..9428efad6062c 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratorIdentity.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratorIdentity.cs @@ -3,21 +3,38 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections.Generic; -using System.Text; +using System.Diagnostics.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; namespace Microsoft.CodeAnalysis { - internal record struct SourceGeneratorIdentity(string AssemblyName, Version AssemblyVersion, string TypeName) + /// + /// Assembly path is used as a part of a generator identity to deal with the case that the user accidentally installed the same + /// generator twice from two different paths, or actually has two different generators that just happened to use the same name. + /// In the wild we've seen cases where a user has a broken project or build that results in the same generator being added twice; + /// we aren't going to try to deduplicate those anywhere since currently the compiler does't do any deduplication either: + /// you'll simply get duplicate outputs which might collide and cause compile errors. If https://github.com/dotnet/roslyn/issues/56619 + /// is addressed, we can potentially match the compiler behavior by taking a different approach here. + /// + internal record struct SourceGeneratorIdentity { - public SourceGeneratorIdentity(ISourceGenerator generator) - : this(GetGeneratorAssemblyName(generator), generator.GetGeneratorType().Assembly.GetName().Version!, GetGeneratorTypeName(generator)) - { - } + // TODO: mark these 'required' when we have the attributes in place + public string AssemblyName { get; init; } + public string? AssemblyPath { get; init; } + public Version AssemblyVersion { get; init; } + public string TypeName { get; init; } - public static string GetGeneratorAssemblyName(ISourceGenerator generator) + // TODO: mark with SetsRequiredMembers when we have the attributes in place + // [SetsRequiredMembers] + public SourceGeneratorIdentity(ISourceGenerator generator, AnalyzerReference analyzerReference) { - return generator.GetGeneratorType().Assembly.GetName().Name!; + var generatorType = generator.GetGeneratorType(); + var assembly = generatorType.Assembly; + var assemblyName = assembly.GetName(); + AssemblyName = assemblyName.Name!; + AssemblyPath = analyzerReference.FullPath; + AssemblyVersion = assemblyName.Version!; + TypeName = generatorType.FullName!; } public static string GetGeneratorTypeName(ISourceGenerator generator) diff --git a/src/Workspaces/CoreTest/SolutionTests/SolutionWithSourceGeneratorTests.cs b/src/Workspaces/CoreTest/SolutionTests/SolutionWithSourceGeneratorTests.cs index f4a59254b13e9..4943348df06ad 100644 --- a/src/Workspaces/CoreTest/SolutionTests/SolutionWithSourceGeneratorTests.cs +++ b/src/Workspaces/CoreTest/SolutionTests/SolutionWithSourceGeneratorTests.cs @@ -91,6 +91,28 @@ public async Task WithReferencesMethodCorrectlyUpdatesRunningGenerators() Assert.Single((await project.GetRequiredCompilationAsync(CancellationToken.None)).SyntaxTrees); } + // We only run this test on Release, as the compiler has asserts that trigger in Debug that the type names probably shouldn't be the same. + [ConditionalFact(typeof(IsRelease))] + public async Task GeneratorAddedWithDifferentFilePathsProducesDistinctDocumentIds() + { + using var workspace = CreateWorkspace(); + + // Produce two generator references with different paths, but the same generator by assembly/type. We will still give them separate + // generator instances, because in the "real" analyzer reference case each analyzer reference produces it's own generator objects. + var generatorReference1 = new TestGeneratorReference(new SingleFileTestGenerator("", hintName: "DuplicateFile"), analyzerFilePath: "Z:\\A.dll"); + var generatorReference2 = new TestGeneratorReference(new SingleFileTestGenerator("", hintName: "DuplicateFile"), analyzerFilePath: "Z:\\B.dll"); + + var project = AddEmptyProject(workspace.CurrentSolution) + .AddAnalyzerReferences(new[] { generatorReference1, generatorReference2 }); + + Assert.Equal(2, (await project.GetRequiredCompilationAsync(CancellationToken.None)).SyntaxTrees.Count()); + + var generatedDocuments = (await project.GetSourceGeneratedDocumentsAsync()).ToList(); + Assert.Equal(2, generatedDocuments.Count); + + Assert.NotEqual(generatedDocuments[0].Id, generatedDocuments[1].Id); + } + [Fact] public async Task IncrementalSourceGeneratorInvokedCorrectNumberOfTimes() { diff --git a/src/Workspaces/CoreTestUtilities/TestGeneratorReference.cs b/src/Workspaces/CoreTestUtilities/TestGeneratorReference.cs index fff413c2ac07c..6a7a76a3efed3 100644 --- a/src/Workspaces/CoreTestUtilities/TestGeneratorReference.cs +++ b/src/Workspaces/CoreTestUtilities/TestGeneratorReference.cs @@ -18,7 +18,7 @@ public sealed class TestGeneratorReference : AnalyzerReference, IChecksummedObje private readonly ISourceGenerator _generator; private readonly Checksum _checksum; - public TestGeneratorReference(ISourceGenerator generator) + public TestGeneratorReference(ISourceGenerator generator, string? analyzerFilePath = null) { _generator = generator; Guid = Guid.NewGuid(); @@ -31,14 +31,16 @@ public TestGeneratorReference(ISourceGenerator generator) var checksumArray = Guid.ToByteArray(); Array.Resize(ref checksumArray, Checksum.HashSize); _checksum = Checksum.From(checksumArray); + + FullPath = analyzerFilePath; } - public TestGeneratorReference(IIncrementalGenerator generator) - : this(generator.AsSourceGenerator()) + public TestGeneratorReference(IIncrementalGenerator generator, string? analyzerFilePath = null) + : this(generator.AsSourceGenerator(), analyzerFilePath) { } - public override string? FullPath => null; + public override string? FullPath { get; } public override object Id => this; public Guid Guid { get; }