From 737dc7fba23be704d745c58a7b9bda56165a169b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 Sep 2023 11:51:17 -0700 Subject: [PATCH 1/4] Include a bit in DocumentId to indicate if it corresponds to a SG document or not --- .../Shared/AbstractSyntaxIndex_Persistence.cs | 2 +- .../Shared/Extensions/ProjectExtensions.cs | 17 +- .../Portable/Workspace/Solution/DocumentId.cs | 31 ++- .../Portable/Workspace/Solution/Project.cs | 23 +-- .../SourceGeneratedDocumentIdentity.cs | 181 ++++++++++-------- 5 files changed, 122 insertions(+), 132 deletions(-) diff --git a/src/Workspaces/Core/Portable/FindSymbols/Shared/AbstractSyntaxIndex_Persistence.cs b/src/Workspaces/Core/Portable/FindSymbols/Shared/AbstractSyntaxIndex_Persistence.cs index 22d86e27cb15f..5d23b20ea675f 100644 --- a/src/Workspaces/Core/Portable/FindSymbols/Shared/AbstractSyntaxIndex_Persistence.cs +++ b/src/Workspaces/Core/Portable/FindSymbols/Shared/AbstractSyntaxIndex_Persistence.cs @@ -17,7 +17,7 @@ namespace Microsoft.CodeAnalysis.FindSymbols internal partial class AbstractSyntaxIndex : IObjectWritable { private static readonly string s_persistenceName = typeof(TIndex).Name; - private static readonly Checksum s_serializationFormatChecksum = Checksum.Create("37"); + private static readonly Checksum s_serializationFormatChecksum = Checksum.Create("38"); /// /// Cache of ParseOptions to a checksum for the contained diff --git a/src/Workspaces/Core/Portable/Shared/Extensions/ProjectExtensions.cs b/src/Workspaces/Core/Portable/Shared/Extensions/ProjectExtensions.cs index 8488743d5d473..769477242f8c2 100644 --- a/src/Workspaces/Core/Portable/Shared/Extensions/ProjectExtensions.cs +++ b/src/Workspaces/Core/Portable/Shared/Extensions/ProjectExtensions.cs @@ -44,25 +44,12 @@ public static TextDocument GetRequiredAnalyzerConfigDocument(this Project projec => project.GetAnalyzerConfigDocument(documentId) ?? throw new InvalidOperationException(WorkspaceExtensionsResources.The_solution_does_not_contain_the_specified_document); public static TextDocument GetRequiredTextDocument(this Project project, DocumentId documentId) - { - var document = project.GetTextDocument(documentId); - if (document == null) - { - throw new InvalidOperationException(WorkspaceExtensionsResources.The_solution_does_not_contain_the_specified_document); - } - - return document; - } + => project.GetTextDocument(documentId) ?? throw new InvalidOperationException(WorkspaceExtensionsResources.The_solution_does_not_contain_the_specified_document); public static async ValueTask GetRequiredSourceGeneratedDocumentAsync(this Project project, DocumentId documentId, CancellationToken cancellationToken) { var document = await project.GetSourceGeneratedDocumentAsync(documentId, cancellationToken).ConfigureAwait(false); - if (document is null) - { - throw new InvalidOperationException(WorkspaceExtensionsResources.The_solution_does_not_contain_the_specified_document); - } - - return document; + return document ?? throw new InvalidOperationException(WorkspaceExtensionsResources.The_solution_does_not_contain_the_specified_document); } } } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/DocumentId.cs b/src/Workspaces/Core/Portable/Workspace/Solution/DocumentId.cs index 1c063cc6d1f05..48a91190f5896 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/DocumentId.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/DocumentId.cs @@ -15,8 +15,8 @@ namespace Microsoft.CodeAnalysis /// An identifier that can be used to retrieve the same across versions of the /// workspace. /// - [DebuggerDisplay("{GetDebuggerDisplay(),nq}")] [DataContract] + [DebuggerDisplay("{GetDebuggerDisplay(),nq}")] public sealed class DocumentId : IEquatable, IObjectWritable { [DataMember(Order = 0)] @@ -24,12 +24,15 @@ public sealed class DocumentId : IEquatable, IObjectWritable [DataMember(Order = 1)] public Guid Id { get; } [DataMember(Order = 2)] + internal bool IsSourceGenerated { get; } + [DataMember(Order = 3)] private readonly string? _debugName; - private DocumentId(ProjectId projectId, Guid guid, string? debugName) + private DocumentId(ProjectId projectId, Guid guid, bool isSourceGenerated, string? debugName) { this.ProjectId = projectId; this.Id = guid; + this.IsSourceGenerated = isSourceGenerated; _debugName = debugName; } @@ -39,28 +42,20 @@ private DocumentId(ProjectId projectId, Guid guid, string? debugName) /// The project id this document id is relative to. /// An optional name to make this id easier to recognize while debugging. public static DocumentId CreateNewId(ProjectId projectId, string? debugName = null) - { - if (projectId == null) - { - throw new ArgumentNullException(nameof(projectId)); - } - - return new DocumentId(projectId, Guid.NewGuid(), debugName); - } + => CreateFromSerialized(projectId, Guid.NewGuid(), isSourceGenerated: false, debugName); public static DocumentId CreateFromSerialized(ProjectId projectId, Guid id, string? debugName = null) + => CreateFromSerialized(projectId, id, isSourceGenerated: true, debugName); + + internal static DocumentId CreateFromSerialized(ProjectId projectId, Guid id, bool isSourceGenerated, string? debugName) { if (projectId == null) - { throw new ArgumentNullException(nameof(projectId)); - } if (id == Guid.Empty) - { throw new ArgumentException(nameof(id)); - } - return new DocumentId(projectId, id, debugName); + return new DocumentId(projectId, id, isSourceGenerated, debugName); } internal string? DebugName => _debugName; @@ -78,7 +73,7 @@ public bool Equals(DocumentId? other) { // Technically, we don't need to check project id. return - other is object && + other is not null && this.Id == other.Id && this.ProjectId == other.ProjectId; } @@ -100,6 +95,7 @@ void IObjectWritable.WriteTo(ObjectWriter writer) writer.WriteGuid(Id); writer.WriteString(DebugName); + writer.WriteBoolean(IsSourceGenerated); } internal static DocumentId ReadFrom(ObjectReader reader) @@ -108,8 +104,9 @@ internal static DocumentId ReadFrom(ObjectReader reader) var guid = reader.ReadGuid(); var debugName = reader.ReadString(); + var isSourceGenerated = reader.ReadBoolean(); - return CreateFromSerialized(projectId, guid, debugName); + return CreateFromSerialized(projectId, guid, isSourceGenerated, debugName); } } } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Project.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Project.cs index a235517e2f99c..81320bb018144 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Project.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Project.cs @@ -271,9 +271,7 @@ public bool ContainsAnalyzerConfigDocument(DocumentId documentId) { var document = GetDocument(documentId) ?? GetAdditionalDocument(documentId) ?? GetAnalyzerConfigDocument(documentId); if (document != null) - { return document; - } return await GetSourceGeneratedDocumentAsync(documentId, cancellationToken).ConfigureAwait(false); } @@ -297,21 +295,20 @@ internal async ValueTask> GetAllRegularAndSourceGeneratedD public async ValueTask GetSourceGeneratedDocumentAsync(DocumentId documentId, CancellationToken cancellationToken = default) { + if (!documentId.IsSourceGenerated) + return null; + // Quick check first: if we already have created a SourceGeneratedDocument wrapper, we're good if (_idToSourceGeneratedDocumentMap.TryGetValue(documentId, out var sourceGeneratedDocument)) - { return sourceGeneratedDocument; - } // We'll have to run generators if we haven't already and now try to find it. var generatedDocumentStates = await _solution.State.GetSourceGeneratedDocumentStatesAsync(State, cancellationToken).ConfigureAwait(false); var generatedDocumentState = generatedDocumentStates.GetState(documentId); - if (generatedDocumentState != null) - { - return GetOrCreateSourceGeneratedDocument(generatedDocumentState); - } + if (generatedDocumentState is null) + return null; - return null; + return GetOrCreateSourceGeneratedDocument(generatedDocumentState); } internal SourceGeneratedDocument GetOrCreateSourceGeneratedDocument(SourceGeneratedDocumentState state) @@ -327,20 +324,18 @@ internal SourceGeneratedDocument GetOrCreateSourceGeneratedDocument(SourceGenera /// internal SourceGeneratedDocument? TryGetSourceGeneratedDocumentForAlreadyGeneratedId(DocumentId documentId) { + if (!documentId.IsSourceGenerated) + return null; + // Easy case: do we already have the SourceGeneratedDocument created? if (_idToSourceGeneratedDocumentMap.TryGetValue(documentId, out var document)) - { return document; - } // Trickier case now: it's possible we generated this, but we don't actually have the SourceGeneratedDocument for it, so let's go // try to fetch the state. var documentState = _solution.State.TryGetSourceGeneratedDocumentStateForAlreadyGeneratedId(documentId); - if (documentState == null) - { return null; - } return ImmutableHashMapExtensions.GetOrAdd(ref _idToSourceGeneratedDocumentMap, documentId, s_createSourceGeneratedDocumentFunction, (documentState, this)); } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocumentIdentity.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocumentIdentity.cs index 4c2f9baef98ec..0af640d481159 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocumentIdentity.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocumentIdentity.cs @@ -3,98 +3,109 @@ // See the LICENSE file in the project root for more information. 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; -namespace Microsoft.CodeAnalysis +namespace Microsoft.CodeAnalysis; + +/// +/// A small struct that holds the values that define the identity of a source generated document, and don't change +/// as new generations happen. This is mostly for convenience as we are reguarly working with this combination of values. +/// +internal readonly record struct SourceGeneratedDocumentIdentity + : IObjectWritable, IEquatable { - /// - /// A small struct that holds the values that define the identity of a source generated document, and don't change - /// as new generations happen. This is mostly for convenience as we are reguarly working with this combination of values. - /// - internal readonly record struct SourceGeneratedDocumentIdentity - (DocumentId DocumentId, string HintName, SourceGeneratorIdentity Generator, string FilePath) - : IObjectWritable, IEquatable + public bool ShouldReuseInSerialization => true; + + public readonly DocumentId DocumentId; + public readonly string HintName; + public readonly SourceGeneratorIdentity Generator; + public readonly string FilePath; + + public SourceGeneratedDocumentIdentity(DocumentId documentId, string hintName, SourceGeneratorIdentity generator, string filePath) + { + Contract.ThrowIfFalse(documentId.IsSourceGenerated); + DocumentId = documentId; + HintName = hintName; + Generator = generator; + FilePath = 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, 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(); + + // 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(assemblyNameToHash)); + hashInput.AddRange(0, 0); + hashInput.AddRange(Encoding.Unicode.GetBytes(generatorIdentity.TypeName)); + hashInput.AddRange(0, 0); + hashInput.AddRange(Encoding.Unicode.GetBytes(hintName)); + + // The particular choice of crypto algorithm here is arbitrary and can be always changed as necessary. The only requirement + // is it must be collision resistant, and provide enough bits to fill a GUID. + using var crytpoAlgorithm = System.Security.Cryptography.SHA256.Create(); + var hash = crytpoAlgorithm.ComputeHash(hashInput.ToArray()); + Array.Resize(ref hash, 16); + var guid = new Guid(hash); + + var documentId = DocumentId.CreateFromSerialized(projectId, guid, isSourceGenerated: true, hintName); + + return new SourceGeneratedDocumentIdentity(documentId, hintName, generatorIdentity, filePath); + } + + public void WriteTo(ObjectWriter writer) { - public bool ShouldReuseInSerialization => true; - - 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, 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(); - - // 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(assemblyNameToHash)); - hashInput.AddRange(0, 0); - hashInput.AddRange(Encoding.Unicode.GetBytes(generatorIdentity.TypeName)); - hashInput.AddRange(0, 0); - hashInput.AddRange(Encoding.Unicode.GetBytes(hintName)); - - // The particular choice of crypto algorithm here is arbitrary and can be always changed as necessary. The only requirement - // is it must be collision resistant, and provide enough bits to fill a GUID. - using var crytpoAlgorithm = System.Security.Cryptography.SHA256.Create(); - var hash = crytpoAlgorithm.ComputeHash(hashInput.ToArray()); - Array.Resize(ref hash, 16); - var guid = new Guid(hash); - - var documentId = DocumentId.CreateFromSerialized(projectId, guid, hintName); - - return new SourceGeneratedDocumentIdentity(documentId, hintName, generatorIdentity, filePath); - } - - public void WriteTo(ObjectWriter writer) - { - DocumentId.WriteTo(writer); - - writer.WriteString(HintName); - writer.WriteString(Generator.AssemblyName); - writer.WriteString(Generator.AssemblyPath); - writer.WriteString(Generator.AssemblyVersion.ToString()); - writer.WriteString(Generator.TypeName); - writer.WriteString(FilePath); - } - - internal static SourceGeneratedDocumentIdentity ReadFrom(ObjectReader reader) - { - var documentId = DocumentId.ReadFrom(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(); - - return new SourceGeneratedDocumentIdentity( - documentId, - hintName, - new SourceGeneratorIdentity - { - AssemblyName = generatorAssemblyName, - AssemblyPath = generatorAssemblyPath, - AssemblyVersion = generatorAssemblyVersion, - TypeName = generatorTypeName - }, - filePath); - } + DocumentId.WriteTo(writer); + + writer.WriteString(HintName); + writer.WriteString(Generator.AssemblyName); + writer.WriteString(Generator.AssemblyPath); + writer.WriteString(Generator.AssemblyVersion.ToString()); + writer.WriteString(Generator.TypeName); + writer.WriteString(FilePath); + } + + internal static SourceGeneratedDocumentIdentity ReadFrom(ObjectReader reader) + { + var documentId = DocumentId.ReadFrom(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(); + + return new SourceGeneratedDocumentIdentity( + documentId, + hintName, + new SourceGeneratorIdentity + { + AssemblyName = generatorAssemblyName, + AssemblyPath = generatorAssemblyPath, + AssemblyVersion = generatorAssemblyVersion, + TypeName = generatorTypeName + }, + filePath); } } From 4b9032cc88330b6fb86e10ac325ca21d9af611cb Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 Sep 2023 11:56:22 -0700 Subject: [PATCH 2/4] Docs --- .../Core/Portable/Workspace/Solution/Project.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Project.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Project.cs index 81320bb018144..ca5b0afdba85e 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Project.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Project.cs @@ -295,9 +295,15 @@ internal async ValueTask> GetAllRegularAndSourceGeneratedD public async ValueTask GetSourceGeneratedDocumentAsync(DocumentId documentId, CancellationToken cancellationToken = default) { + // Immediately shortcircuit out if we know this is not a doc-id corresponding to an SG document. if (!documentId.IsSourceGenerated) return null; + // User incorrect called into us with a doc id for a different project. Ideally we'd throw here, but we've + // always been resilient to this misuse since the start of roslyn, so we just quick-bail instead. + if (this.Id != documentId.ProjectId) + return null; + // Quick check first: if we already have created a SourceGeneratedDocument wrapper, we're good if (_idToSourceGeneratedDocumentMap.TryGetValue(documentId, out var sourceGeneratedDocument)) return sourceGeneratedDocument; @@ -324,9 +330,15 @@ internal SourceGeneratedDocument GetOrCreateSourceGeneratedDocument(SourceGenera /// internal SourceGeneratedDocument? TryGetSourceGeneratedDocumentForAlreadyGeneratedId(DocumentId documentId) { + // Immediately shortcircuit out if we know this is not a doc-id corresponding to an SG document. if (!documentId.IsSourceGenerated) return null; + // User incorrect called into us with a doc id for a different project. Ideally we'd throw here, but we've + // always been resilient to this misuse since the start of roslyn, so we just quick-bail instead. + if (this.Id != documentId.ProjectId) + return null; + // Easy case: do we already have the SourceGeneratedDocument created? if (_idToSourceGeneratedDocumentMap.TryGetValue(documentId, out var document)) return document; From 2f91eacd74be3a79383dde6113fda91aaefc024e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 Sep 2023 12:05:17 -0700 Subject: [PATCH 3/4] REorder --- .../Core/Portable/Workspace/Solution/DocumentId.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/DocumentId.cs b/src/Workspaces/Core/Portable/Workspace/Solution/DocumentId.cs index 48a91190f5896..5f46b9367568a 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/DocumentId.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/DocumentId.cs @@ -92,19 +92,17 @@ public override int GetHashCode() void IObjectWritable.WriteTo(ObjectWriter writer) { ProjectId.WriteTo(writer); - writer.WriteGuid(Id); - writer.WriteString(DebugName); writer.WriteBoolean(IsSourceGenerated); + writer.WriteString(DebugName); } internal static DocumentId ReadFrom(ObjectReader reader) { var projectId = ProjectId.ReadFrom(reader); - var guid = reader.ReadGuid(); - var debugName = reader.ReadString(); var isSourceGenerated = reader.ReadBoolean(); + var debugName = reader.ReadString(); return CreateFromSerialized(projectId, guid, isSourceGenerated, debugName); } From 961133b1d69848fb8bce3b85f2283104499bbb5f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 14 Sep 2023 12:11:00 -0700 Subject: [PATCH 4/4] mistype --- src/Workspaces/Core/Portable/Workspace/Solution/DocumentId.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/DocumentId.cs b/src/Workspaces/Core/Portable/Workspace/Solution/DocumentId.cs index 5f46b9367568a..31d3e7cec7da9 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/DocumentId.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/DocumentId.cs @@ -45,7 +45,7 @@ public static DocumentId CreateNewId(ProjectId projectId, string? debugName = nu => CreateFromSerialized(projectId, Guid.NewGuid(), isSourceGenerated: false, debugName); public static DocumentId CreateFromSerialized(ProjectId projectId, Guid id, string? debugName = null) - => CreateFromSerialized(projectId, id, isSourceGenerated: true, debugName); + => CreateFromSerialized(projectId, id, isSourceGenerated: false, debugName); internal static DocumentId CreateFromSerialized(ProjectId projectId, Guid id, bool isSourceGenerated, string? debugName) {