diff --git a/src/Features/CSharp/Portable/Debugging/DataTipInfoGetter.cs b/src/Features/CSharp/Portable/Debugging/DataTipInfoGetter.cs index 86b1ba4a94e65..7b82550248997 100644 --- a/src/Features/CSharp/Portable/Debugging/DataTipInfoGetter.cs +++ b/src/Features/CSharp/Portable/Debugging/DataTipInfoGetter.cs @@ -39,11 +39,11 @@ internal static async Task GetInfoAsync(Document document, int if (expression is LiteralExpressionSyntax) { - // If the user hovers over a literal, give them a DataTip for the type of the - // literal they're hovering over. - // Partial semantics should always be sufficient because the (unconverted) type - // of a literal can always easily be determined. - var (_, semanticModel) = await document.GetFullOrPartialSemanticModelAsync(cancellationToken).ConfigureAwait(false); + // If the user hovers over a literal, give them a DataTip for the type of the literal they're hovering + // over. Partial semantics should always be sufficient because the (unconverted) type of a literal can + // always easily be determined. + document = document.WithFrozenPartialSemantics(cancellationToken); + var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); var type = semanticModel.GetTypeInfo(expression, cancellationToken).Type; return type == null ? default diff --git a/src/Features/Core/Portable/Completion/CompletionService.cs b/src/Features/Core/Portable/Completion/CompletionService.cs index af9865823b7b9..c3bfc799cf6e5 100644 --- a/src/Features/Core/Portable/Completion/CompletionService.cs +++ b/src/Features/Core/Portable/Completion/CompletionService.cs @@ -215,7 +215,8 @@ public virtual TextSpan GetDefaultCompletionListSpan(SourceText text, int caretP var extensionManager = document.Project.Solution.Services.GetRequiredService(); // We don't need SemanticModel here, just want to make sure it won't get GC'd before CompletionProviders are able to get it. - (document, var semanticModel) = await GetDocumentWithFrozenPartialSemanticsAsync(document, cancellationToken).ConfigureAwait(false); + document = GetDocumentWithFrozenPartialSemantics(document, cancellationToken); + var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); var description = await extensionManager.PerformFunctionAsync( provider, cancellationToken => provider.GetDescriptionAsync(document, item, options, displayOptions, cancellationToken), @@ -246,7 +247,8 @@ public virtual async Task GetChangeAsync( var extensionManager = document.Project.Solution.Services.GetRequiredService(); // We don't need SemanticModel here, just want to make sure it won't get GC'd before CompletionProviders are able to get it. - (document, var semanticModel) = await GetDocumentWithFrozenPartialSemanticsAsync(document, cancellationToken).ConfigureAwait(false); + document = GetDocumentWithFrozenPartialSemantics(document, cancellationToken); + var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); var change = await extensionManager.PerformFunctionAsync( provider, diff --git a/src/Features/Core/Portable/Completion/CompletionService_GetCompletions.cs b/src/Features/Core/Portable/Completion/CompletionService_GetCompletions.cs index f65263b38a224..72c8a8966e827 100644 --- a/src/Features/Core/Portable/Completion/CompletionService_GetCompletions.cs +++ b/src/Features/Core/Portable/Completion/CompletionService_GetCompletions.cs @@ -69,7 +69,8 @@ internal virtual async Task GetCompletionsAsync( CancellationToken cancellationToken = default) { // We don't need SemanticModel here, just want to make sure it won't get GC'd before CompletionProviders are able to get it. - (document, var semanticModel) = await GetDocumentWithFrozenPartialSemanticsAsync(document, cancellationToken).ConfigureAwait(false); + document = GetDocumentWithFrozenPartialSemantics(document, cancellationToken); + var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); var text = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false); var completionListSpan = GetDefaultCompletionListSpan(text, caretPosition); @@ -177,14 +178,12 @@ static async Task> GetAugmentingProvidersAsyn /// In most cases we'd still end up with complete document, but we'd consider it an acceptable trade-off even when /// we get into this transient state. /// - private async Task<(Document document, SemanticModel? semanticModel)> GetDocumentWithFrozenPartialSemanticsAsync(Document document, CancellationToken cancellationToken) + private Document GetDocumentWithFrozenPartialSemantics(Document document, CancellationToken cancellationToken) { if (_suppressPartialSemantics) - { - return (document, await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false)); - } + return document; - return await document.GetFullOrPartialSemanticModelAsync(cancellationToken).ConfigureAwait(false); + return document.WithFrozenPartialSemantics(cancellationToken); } private static bool ValidatePossibleTriggerCharacterSet(CompletionTriggerKind completionTriggerKind, IEnumerable triggeredProviders, diff --git a/src/Features/Core/Portable/Completion/Providers/AbstractMemberInsertingCompletionProvider.cs b/src/Features/Core/Portable/Completion/Providers/AbstractMemberInsertingCompletionProvider.cs index 25e3bb8a6e34d..aed35da65eafb 100644 --- a/src/Features/Core/Portable/Completion/Providers/AbstractMemberInsertingCompletionProvider.cs +++ b/src/Features/Core/Portable/Completion/Providers/AbstractMemberInsertingCompletionProvider.cs @@ -80,9 +80,11 @@ private async Task DetermineNewDocumentAsync( var tree = await document.GetRequiredSyntaxTreeAsync(cancellationToken).ConfigureAwait(false); var token = GetToken(completionItem, tree, cancellationToken); var annotatedRoot = tree.GetRoot(cancellationToken).ReplaceToken(token, token.WithAdditionalAnnotations(_otherAnnotation)); - // Make sure the new document is frozen before we try to get the semantic model. This is to - // avoid trigger source generator, which is expensive and not needed for calculating the change. - document = document.WithSyntaxRoot(annotatedRoot).WithFrozenPartialSemantics(cancellationToken); + + // Make sure the new document is frozen before we try to get the semantic model. This is to avoid trigger source + // generator, which is expensive and not needed for calculating the change. Pass in 'forceFreeze: true' to + // ensure all further transformations we make do not run generators either. + document = document.WithSyntaxRoot(annotatedRoot).WithFrozenPartialSemantics(forceFreeze: true, cancellationToken); var memberContainingDocument = await GenerateMemberAndUsingsAsync(document, completionItem, line, cancellationToken).ConfigureAwait(false); if (memberContainingDocument == null) diff --git a/src/Features/Core/Portable/Completion/Providers/Snippets/AbstractSnippetCompletionProvider.cs b/src/Features/Core/Portable/Completion/Providers/Snippets/AbstractSnippetCompletionProvider.cs index ecc4b1fedba12..6be105ab1e421 100644 --- a/src/Features/Core/Portable/Completion/Providers/Snippets/AbstractSnippetCompletionProvider.cs +++ b/src/Features/Core/Portable/Completion/Providers/Snippets/AbstractSnippetCompletionProvider.cs @@ -128,9 +128,10 @@ public override async Task ProvideCompletionsAsync(CompletionContext context) var textChange = new TextChange(span, string.Empty); originalText = originalText.WithChanges(textChange); - // The document might not be frozen, so make sure we freeze it here to avoid triggering source generator - // which is not needed for snippet completion and will cause perf issue. - var newDocument = document.WithText(originalText).WithFrozenPartialSemantics(cancellationToken); + // The document might not be frozen, so make sure we freeze it here to avoid triggering source generator which + // is not needed for snippet completion and will cause perf issue. Pass in 'forceFreeze: true' to ensure all + // further transformations we make do not run generators either. + var newDocument = document.WithText(originalText).WithFrozenPartialSemantics(forceFreeze: true, cancellationToken); return (newDocument, span.Start); } } diff --git a/src/Workspaces/Core/Portable/Shared/Extensions/DocumentExtensions.cs b/src/Workspaces/Core/Portable/Shared/Extensions/DocumentExtensions.cs index 6a41bf456a50a..dc01cc80dc577 100644 --- a/src/Workspaces/Core/Portable/Shared/Extensions/DocumentExtensions.cs +++ b/src/Workspaces/Core/Portable/Shared/Extensions/DocumentExtensions.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.FindSymbols; @@ -23,28 +22,6 @@ public static async ValueTask GetSyntaxTreeIndexAsync(this Docu public static ValueTask GetSyntaxTreeIndexAsync(this Document document, bool loadOnly, CancellationToken cancellationToken) => SyntaxTreeIndex.GetIndexAsync(document, loadOnly, cancellationToken); - /// - /// Returns the semantic model for this document that may be produced from partial semantics. The semantic model - /// is only guaranteed to contain the syntax tree for and nothing else. - /// - public static async Task<(Document document, SemanticModel? semanticModel)> GetFullOrPartialSemanticModelAsync(this Document document, CancellationToken cancellationToken) - { - if (document.Project.TryGetCompilation(out var compilation)) - { - // We already have a compilation, so at this point it's fastest to just get a SemanticModel - var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - - // Make sure the compilation is kept alive so that GetSemanticModelAsync() doesn't become expensive - GC.KeepAlive(compilation); - return (document, semanticModel); - } - else - { - var frozenDocument = document.WithFrozenPartialSemantics(cancellationToken); - return (frozenDocument, await frozenDocument.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false)); - } - } - internal static Document WithSolutionOptions(this Document document, OptionSet options) => document.Project.Solution.WithOptions(options).GetRequiredDocument(document.Id); } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Document.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Document.cs index 068122537e3e3..2e77e388f6c74 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Document.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Document.cs @@ -487,18 +487,35 @@ public ImmutableArray GetLinkedDocumentIds() return filteredDocumentIds.Remove(this.Id); } + /// + internal Document WithFrozenPartialSemantics(CancellationToken cancellationToken) + => WithFrozenPartialSemantics(forceFreeze: false, cancellationToken); + /// - /// Creates a branched version of this document that has its semantic model frozen in whatever state it is available at the time, - /// assuming a background process is constructing the semantics asynchronously. Repeated calls to this method may return - /// documents with increasingly more complete semantics. + /// Creates a branched version of this document that has its semantic model frozen in whatever state it is available + /// at the time, assuming a background process is constructing the semantics asynchronously. Repeated calls to this + /// method may return documents with increasingly more complete semantics. /// /// Use this method to gain access to potentially incomplete semantics quickly. - /// Note: this will give back a solution where this 's project will not run - /// generators when getting its compilation. However, all other projects will still run generators when their - /// compilations are requested. + /// Note: this will give back a solution where this 's project will not run generators + /// when getting its compilation. However, all other projects will still run generators when their compilations are + /// requested. /// - internal virtual Document WithFrozenPartialSemantics(CancellationToken cancellationToken) + /// If then a forked document will be returned no matter what. This + /// should be used when the caller wants to ensure that further forks of that document will remain frozen and will + /// not run generators/skeletons. For example, if it is about to transform the document many times, and is fine with + /// the original semantic information they started with. If then this same document may be + /// returned if the compilation for its was already produced. In this case, generators and + /// skeletons will already have been run, so returning the same instance will be fast when getting semantics. + /// However, this does mean that future forks of this instance will continue running generators/skeletons. This + /// should be used for most clients that intend to just query for semantic information and do not intend to make any + /// further changes. + /// + internal virtual Document WithFrozenPartialSemantics(bool forceFreeze, CancellationToken cancellationToken) { + if (!forceFreeze && this.Project.TryGetCompilation(out _)) + return this; + var solution = this.Project.Solution; // only produce doc with frozen semantics if this workspace has support for that, as without diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocument.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocument.cs index 2a2a9d97a6668..ba4bb1339eff8 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocument.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SourceGeneratedDocument.cs @@ -26,7 +26,7 @@ internal SourceGeneratedDocument(Project project, SourceGeneratedDocumentState s internal DateTime GenerationDateTime => State.GenerationDateTime; - internal override Document WithFrozenPartialSemantics(CancellationToken cancellationToken) + internal override Document WithFrozenPartialSemantics(bool forceFreeze, CancellationToken cancellationToken) { // For us to implement frozen partial semantics here with a source generated document, // we'd need to potentially deal with the combination where that happens on a snapshot that was already diff --git a/src/Workspaces/CoreTest/SolutionTests/SolutionWithSourceGeneratorTests.cs b/src/Workspaces/CoreTest/SolutionTests/SolutionWithSourceGeneratorTests.cs index 122e24500d06d..0ab0a08695b10 100644 --- a/src/Workspaces/CoreTest/SolutionTests/SolutionWithSourceGeneratorTests.cs +++ b/src/Workspaces/CoreTest/SolutionTests/SolutionWithSourceGeneratorTests.cs @@ -325,7 +325,8 @@ static async Task AssertCompilationContainsGeneratedFilesAsync(Project project, } [Theory, CombinatorialData] - public async Task PartialCompilationsIncludeGeneratedFilesAfterFullGeneration(TestHost testHost) + public async Task PartialCompilationsIncludeGeneratedFilesAfterFullGeneration( + TestHost testHost, bool forceFreeze) { using var workspace = CreateWorkspaceWithPartialSemantics(testHost); var analyzerReference = new TestGeneratorReference(new GenerateFileForEachAdditionalFileWithContentsCommented()); @@ -338,8 +339,15 @@ public async Task PartialCompilationsIncludeGeneratedFilesAfterFullGeneration(Te Assert.Equal(2, fullCompilation.SyntaxTrees.Count()); - var partialProject = project.Documents.Single().WithFrozenPartialSemantics(CancellationToken.None).Project; - Assert.NotSame(partialProject, project); + var partialProject = project.Documents.Single().WithFrozenPartialSemantics(forceFreeze, CancellationToken.None).Project; + + // If we're forcing the freeze, we must get a different project instance. If we're not, we'll get the same + // project since the compilation was already available. + if (forceFreeze) + Assert.NotSame(partialProject, project); + else + Assert.Same(partialProject, project); + var partialCompilation = await partialProject.GetRequiredCompilationAsync(CancellationToken.None); Assert.Same(fullCompilation, partialCompilation); @@ -729,7 +737,7 @@ public async Task FreezingSolutionEnsuresGeneratorsDoNotRun(bool forkBeforeFreez } [Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/56702")] - public async Task ForkAfterFreezeNoLongerRunsGenerators(TestHost testHost) + public async Task ForkAfterForceFreezeNoLongerRunsGenerators(TestHost testHost) { using var workspace = CreateWorkspaceWithPartialSemantics(testHost); var generatorRan = false; @@ -744,9 +752,10 @@ public async Task ForkAfterFreezeNoLongerRunsGenerators(TestHost testHost) Assert.True(generatorRan); generatorRan = false; - var document = project.Documents.Single().WithFrozenPartialSemantics(CancellationToken.None); + var document = project.Documents.Single().WithFrozenPartialSemantics(forceFreeze: true, CancellationToken.None); - // And fork with new contents; we'll ensure the contents of this tree are different, but the generator will still not be ran + // And fork with new contents; we'll ensure the contents of this tree are different, but the generator will not + // run since we explicitly force froze. document = document.WithText(SourceText.From("// Something else")); var compilation = await document.Project.GetRequiredCompilationAsync(CancellationToken.None); @@ -756,6 +765,35 @@ public async Task ForkAfterFreezeNoLongerRunsGenerators(TestHost testHost) Assert.Equal("// Something else", (await document.GetRequiredSyntaxRootAsync(CancellationToken.None)).ToFullString()); } + [Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/56702")] + public async Task ForkAfterFreezeOfCompletedDocumentStillRunsGenerators(TestHost testHost) + { + using var workspace = CreateWorkspaceWithPartialSemantics(testHost); + var generatorRan = false; + var analyzerReference = new TestGeneratorReference(new CallbackGenerator(_ => { }, onExecute: _ => { generatorRan = true; }, source: "// Hello World!")); + var project = AddEmptyProject(workspace.CurrentSolution) + .AddAnalyzerReference(analyzerReference) + .AddDocument("RegularDocument.cs", "// Source File", filePath: "RegularDocument.cs").Project; + + // Ensure generators are ran + var objectReference = await project.GetCompilationAsync(); + + Assert.True(generatorRan); + generatorRan = false; + + var document = project.Documents.Single().WithFrozenPartialSemantics(CancellationToken.None); + + // And fork with new contents; we'll ensure the contents of this tree are different, but the generator will run + // since we didn't force freeze, and we got the frozen document after its compilation was already computed. + document = document.WithText(SourceText.From("// Something else")); + + var compilation = await document.Project.GetRequiredCompilationAsync(CancellationToken.None); + Assert.Equal(2, compilation.SyntaxTrees.Count()); + Assert.True(generatorRan); + + Assert.Equal("// Something else", (await document.GetRequiredSyntaxRootAsync(CancellationToken.None)).ToFullString()); + } + [Theory, CombinatorialData] public async Task LinkedDocumentOfFrozenShouldNotRunSourceGenerator(TestHost testHost) {