Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mitigation in 17.7 for cases where local and remote host disagree on source generated files. #69965

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/EditorFeatures/Core.Wpf/Peek/PeekableItemSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ private static async IAsyncEnumerable<IPeekableItem> GetPeekableItemsForNavigabl
workspace, document.Id, item.SourceSpan.Start, cancellationToken).ConfigureAwait(false))
{
var text = await document.GetTextAsync(project.Solution, cancellationToken).ConfigureAwait(false);
if (text is null)
continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Can we put this in CanNavigateToPositionAsync instead?


var linePositionSpan = text.Lines.GetLinePositionSpan(item.SourceSpan);
if (document.FilePath != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,9 @@ internal async Task<ImmutableArray<CodeDefinitionWindowLocation>> GetContextFrom
if (await navigationService.CanNavigateToSpanAsync(workspace, item.Document.Id, item.SourceSpan, cancellationToken).ConfigureAwait(false))
{
var text = await item.Document.GetTextAsync(document.Project.Solution, cancellationToken).ConfigureAwait(false);
if (text is null)
continue;

var linePositionSpan = text.Lines.GetLinePositionSpan(item.SourceSpan);

if (item.Document.FilePath != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ public async Task<ImmutableArray<DocumentHighlights>> GetDocumentHighlightsAsync
return ImmutableArray<DocumentHighlights>.Empty;
}

return await result.Value.SelectAsArrayAsync(h => h.RehydrateAsync(solution)).ConfigureAwait(false);
var highlights = await result.Value.SelectAsArrayAsync(h => h.RehydrateAsync(solution, cancellationToken)).ConfigureAwait(false);
return highlights.WhereNotNull();
}

return await GetDocumentHighlightsInCurrentProcessAsync(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,17 @@ public SerializableDocumentHighlights(DocumentId documentId, ImmutableArray<High
HighlightSpans = highlightSpans;
}

public async ValueTask<DocumentHighlights> RehydrateAsync(Solution solution)
=> new(await solution.GetRequiredDocumentAsync(DocumentId, includeSourceGenerated: true).ConfigureAwait(false), HighlightSpans);
public async ValueTask<DocumentHighlights?> RehydrateAsync(Solution solution, CancellationToken cancellationToken)
{
// https://github.com/dotnet/roslyn/issues/69964
//
// Remove this once we solve root cause issue of the hosts disagreeing on source generated documents.
var document = await solution.GetRequiredDocumentIncludingSourceGeneratedAsync(DocumentId, throwForMissingSourceGenerated: false, cancellationToken).ConfigureAwait(false);
if (document is null)
return null;

return new(document, HighlightSpans);
}

public static SerializableDocumentHighlights Dehydrate(DocumentHighlights highlights)
=> new(highlights.Document.Id, highlights.HighlightSpans);
Expand Down
10 changes: 8 additions & 2 deletions src/Features/Core/Portable/NavigateTo/RoslynNavigateToItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,14 @@ public RoslynNavigateToItem(
}
else
{
var document = await solution.GetRequiredDocumentAsync(
DocumentId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);
// https://github.com/dotnet/roslyn/issues/69964
//
// Remove this once we solve root cause issue of the hosts disagreeing on source generated documents.
var document = await solution.GetRequiredDocumentIncludingSourceGeneratedAsync(
DocumentId, throwForMissingSourceGenerated: false, cancellationToken).ConfigureAwait(false);
if (document == null)
return null;

return new NavigateToSearchResult(this, document, activeDocument);
}
}
Expand Down
18 changes: 15 additions & 3 deletions src/Features/Core/Portable/Navigation/INavigableItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,30 @@ public static NavigableDocument FromDocument(Document document)
/// this navigable item. The document is required to exist within the solution, e.g. a case where the
/// navigable item was constructed during a Find Symbols operation on the same solution instance.
/// </summary>
internal ValueTask<Document> GetRequiredDocumentAsync(Solution solution, CancellationToken cancellationToken)
=> solution.GetRequiredDocumentAsync(Id, includeSourceGenerated: IsSourceGeneratedDocument, cancellationToken);
internal async ValueTask<Document?> GetRequiredDocumentAsync(Solution solution, CancellationToken cancellationToken)
{
if (!IsSourceGeneratedDocument)
return solution.GetRequiredDocument(Id);

// https://github.com/dotnet/roslyn/issues/69964
//
// Remove this once we solve root cause issue of the hosts disagreeing on source generated documents.
return await solution.GetRequiredDocumentIncludingSourceGeneratedAsync(
Id, throwForMissingSourceGenerated: false, cancellationToken).ConfigureAwait(false);
}

/// <summary>
/// Get the <see cref="SourceText"/> of the <see cref="CodeAnalysis.Document"/> within
/// <paramref name="solution"/> which is referenced by this navigable item. The document is required to
/// exist within the solution, e.g. a case where the navigable item was constructed during a Find Symbols
/// operation on the same solution instance.
/// </summary>
internal async ValueTask<SourceText> GetTextAsync(Solution solution, CancellationToken cancellationToken)
internal async ValueTask<SourceText?> GetTextAsync(Solution solution, CancellationToken cancellationToken)
{
var document = await GetRequiredDocumentAsync(solution, cancellationToken).ConfigureAwait(false);
if (document is null)
return null;

return await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,12 @@ public AbstractGoToDefinitionHandler(IMetadataAsSourceFileService metadataAsSour
continue;
}

var definitionDoc = await definition.Document.GetRequiredDocumentAsync(document.Project.Solution, cancellationToken).ConfigureAwait(false);
if (definitionDoc is null)
continue;

var location = await ProtocolConversions.TextSpanToLocationAsync(
await definition.Document.GetRequiredDocumentAsync(document.Project.Solution, cancellationToken).ConfigureAwait(false),
definitionDoc,
definition.SourceSpan,
definition.IsStale,
cancellationToken).ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ public LSPNavigateToCallback(
public async Task AddItemAsync(Project project, INavigateToSearchResult result, CancellationToken cancellationToken)
{
var document = await result.NavigableItem.Document.GetRequiredDocumentAsync(project.Solution, cancellationToken).ConfigureAwait(false);
if (document is null)
return;

var location = await ProtocolConversions.TextSpanToLocationAsync(
document, result.NavigableItem.SourceSpan, result.NavigableItem.IsStale, _context, cancellationToken).ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.Navigation;
using Microsoft.CodeAnalysis.GoToDefinition;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.ExternalAccess.OmniSharp.GoToDefinition
{
Expand All @@ -17,10 +18,18 @@ internal static async Task<ImmutableArray<OmniSharpNavigableItem>> FindDefinitio
{
var service = document.GetRequiredLanguageService<IFindDefinitionService>();
var result = await service.FindDefinitionsAsync(document, position, cancellationToken).ConfigureAwait(false);
return await result.NullToEmpty().SelectAsArrayAsync(
async (original, solution, cancellationToken) => new OmniSharpNavigableItem(original.DisplayTaggedParts, await original.Document.GetRequiredDocumentAsync(solution, cancellationToken).ConfigureAwait(false), original.SourceSpan),
var items = await result.NullToEmpty().SelectAsArrayAsync(
async (original, solution, cancellationToken) =>
{
var document = await original.Document.GetRequiredDocumentAsync(solution, cancellationToken).ConfigureAwait(false);
if (document is null)
return (OmniSharpNavigableItem?)null;

return new OmniSharpNavigableItem(original.DisplayTaggedParts, document, original.SourceSpan);
},
document.Project.Solution,
cancellationToken).ConfigureAwait(false);
return items.WhereNotNull();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ public OmniSharpNavigateToCallbackImpl(OmniSharpNavigateToCallback callback)
public async Task AddItemAsync(Project project, INavigateToSearchResult result, CancellationToken cancellationToken)
{
var document = await result.NavigableItem.Document.GetRequiredDocumentAsync(project.Solution, cancellationToken).ConfigureAwait(false);
if (document is null)
return;

var omniSharpResult = new OmniSharpNavigateToSearchResult(
result.AdditionalInformation,
result.Kind,
Expand Down
3 changes: 3 additions & 0 deletions src/VisualStudio/Core/Def/Progression/GraphBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,9 @@ public void AddLink(GraphNode from, GraphCategory category, GraphNode to, Cancel
public async Task<GraphNode?> CreateNodeAsync(Solution solution, INavigateToSearchResult result, CancellationToken cancellationToken)
{
var document = await result.NavigableItem.Document.GetRequiredDocumentAsync(solution, cancellationToken).ConfigureAwait(false);
if (document is null)
return null;

var project = document.Project;

// If it doesn't belong to a document or project we can navigate to, then ignore entirely.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,11 @@ public async Task<bool> CanNavigateToPositionAsync(Workspace workspace, Document
return true;
}

var document = workspace.CurrentSolution.GetRequiredDocument(documentId);
var document = await workspace.CurrentSolution.GetRequiredDocumentIncludingSourceGeneratedAsync(
documentId, throwForMissingSourceGenerated: false, cancellationToken).ConfigureAwait(false);
if (document is null)
return false;

var text = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false);

var boundedPosition = GetPositionWithinDocumentBounds(position, text.Length);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,9 @@ public GoToDefinitionHandler(IMetadataAsSourceFileService metadataAsSourceFileSe
foreach (var item in items)
{
var document = await item.Document.GetRequiredDocumentAsync(context.Solution, cancellationToken).ConfigureAwait(false);
if (document is null)
continue;

var location = await ProtocolConversions.TextSpanToLocationAsync(
document, item.SourceSpan, item.IsStale, cancellationToken).ConfigureAwait(false);
locations.AddIfNotNull(location);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,15 @@ async Task AddMatchingTypesAsync(
cancellationToken.ThrowIfCancellationRequested();

Debug.Assert(infos.Count > 0);
var document = await solution.GetRequiredDocumentAsync(documentId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);

// https://github.com/dotnet/roslyn/issues/69964
//
// Remove this once we solve root cause issue of the hosts disagreeing on source generated documents.
var document = await solution.GetRequiredDocumentIncludingSourceGeneratedAsync(
documentId, throwForMissingSourceGenerated: false, cancellationToken).ConfigureAwait(false);
if (document is null)
continue;

var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
cachedModels.Add(semanticModel);

Expand All @@ -276,7 +284,14 @@ async Task AddSourceTypesThatDeriveFromNameAsync(SymbolSet result, string name)
{
cancellationToken.ThrowIfCancellationRequested();

var document = await solution.GetRequiredDocumentAsync(documentId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);
// https://github.com/dotnet/roslyn/issues/69964
//
// Remove this once we solve root cause issue of the hosts disagreeing on source generated documents.
var document = await solution.GetRequiredDocumentIncludingSourceGeneratedAsync(
documentId, throwForMissingSourceGenerated: false, cancellationToken).ConfigureAwait(false);
if (document is null)
continue;

var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
cachedModels.Add(semanticModel);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace Microsoft.CodeAnalysis.FindSymbols
internal partial class AbstractSyntaxIndex<TIndex> : 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");

/// <summary>
/// Cache of ParseOptions to a checksum for the <see cref="ParseOptions.PreprocessorSymbolNames"/> contained
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,27 @@ public ValueTask OnCompletedAsync(CancellationToken cancellationToken)

public async ValueTask OnFindInDocumentStartedAsync(DocumentId documentId, CancellationToken cancellationToken)
{
var document = await solution.GetRequiredDocumentAsync(documentId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);
// https://github.com/dotnet/roslyn/issues/69964
//
// Remove this once we solve root cause issue of the hosts disagreeing on source generated documents.
var document = await solution.GetRequiredDocumentIncludingSourceGeneratedAsync(
documentId, throwForMissingSourceGenerated: false, cancellationToken).ConfigureAwait(false);
if (document is null)
return;

await progress.OnFindInDocumentStartedAsync(document, cancellationToken).ConfigureAwait(false);
}

public async ValueTask OnFindInDocumentCompletedAsync(DocumentId documentId, CancellationToken cancellationToken)
{
var document = await solution.GetRequiredDocumentAsync(documentId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);
// https://github.com/dotnet/roslyn/issues/69964
//
// Remove this once we solve root cause issue of the hosts disagreeing on source generated documents.
var document = await solution.GetRequiredDocumentIncludingSourceGeneratedAsync(
documentId, throwForMissingSourceGenerated: false, cancellationToken).ConfigureAwait(false);
if (document is null)
return;

await progress.OnFindInDocumentCompletedAsync(document, cancellationToken).ConfigureAwait(false);
}

Expand Down Expand Up @@ -102,10 +116,11 @@ public async ValueTask OnReferenceFoundAsync(
}
}

var referenceLocation = await reference.RehydrateAsync(
solution, cancellationToken).ConfigureAwait(false);
var referenceLocation = await reference.RehydrateAsync(solution, cancellationToken).ConfigureAwait(false);
if (referenceLocation is null)
return;

await progress.OnReferenceFoundAsync(symbolGroup, symbol, referenceLocation, cancellationToken).ConfigureAwait(false);
await progress.OnReferenceFoundAsync(symbolGroup, symbol, referenceLocation.Value, cancellationToken).ConfigureAwait(false);
}
}
}
Expand Down
11 changes: 9 additions & 2 deletions src/Workspaces/Core/Portable/Remote/RemoteArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,17 @@ public static SerializableReferenceLocation Dehydrate(
referenceLocation.CandidateReason);
}

public async ValueTask<ReferenceLocation> RehydrateAsync(
public async ValueTask<ReferenceLocation?> RehydrateAsync(
Solution solution, CancellationToken cancellationToken)
{
var document = await solution.GetRequiredDocumentAsync(this.Document, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);
// https://github.com/dotnet/roslyn/issues/69964
//
// Remove this once we solve root cause issue of the hosts disagreeing on source generated documents.
var document = await solution.GetRequiredDocumentIncludingSourceGeneratedAsync(
this.Document, throwForMissingSourceGenerated: false, cancellationToken).ConfigureAwait(false);
if (document is null)
return null;

var syntaxTree = await document.GetRequiredSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
var aliasSymbol = await RehydrateAliasAsync(solution, cancellationToken).ConfigureAwait(false);
var additionalProperties = this.AdditionalProperties;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ internal partial class SymbolicRenameLocations
serializableLocations.Options,
fallbackOptions,
locations,
implicitLocations,
implicitLocations.WhereNotNull(),
referencedSymbols);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ private LightweightRenameLocations(
Options,
FallbackOptions,
Locations,
implicitLocations,
implicitLocations.WhereNotNull(),
referencedSymbols);
}

Expand Down
Loading
Loading