Skip to content

Commit

Permalink
Fix crash in the preview of a code action that modified an .editorconfig
Browse files Browse the repository at this point in the history
For reasons that aren't entirely clear, when we create a preview of
a code fix, we remove and re-add documents that are being changed when
we produce the PreviewWorkspace. This process had an bug which meant
that the file path of an .editorconfig got dropped, so the resultant
.editorconfig document path was null. This later caused a crash if
the diagnostic engine, when processing the PreviewWorkspace's
new solution, tried asking for a Compilation, since the null path would
get passed to AnalyzerConfig.Parse(), and it would throw.

This was exposed by dotnet#45076; until then if you had a preview of just
an .editorconfig file, nothing would actually be analyzed and so
nothing asked for the Compilation. The crash is "fixed" by dotnet#44331 in
that asking for a Compilation no longer results in a call to
AnalyzerConfig.Parse(); you have to ask for a Compilation and then do
something with a semantic model. By luck, that doesn't happen in the
common repro, but more obscure things absolutely could still fail.

This is being tested by the integration test being added in dotnet#46639. We
don't really have a good unit tests here as far as I can tell, but even
then a unit test really doesn't validate any of the end-to-end here.

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1162464
  • Loading branch information
jasonmalinowski committed Aug 7, 2020
1 parent 6b35d09 commit 0dae2e9
Showing 1 changed file with 22 additions and 9 deletions.
31 changes: 22 additions & 9 deletions src/EditorFeatures/Core.Wpf/Preview/PreviewFactoryService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Diagnostics;
using System.Linq;
Expand Down Expand Up @@ -358,7 +359,7 @@ private async Task<DifferenceViewerPreview> CreateRemovedTextDocumentPreviewView
double zoomLevel,
Func<TDocument, CancellationToken, ValueTask<ITextBuffer>> createBufferAsync,
Func<Solution, DocumentId, Solution> removeTextDocument,
Func<Solution, DocumentId, string, SourceText, Solution> addTextDocument,
Func<Solution, DocumentInfo, Solution> addTextDocument,
Action<Workspace, DocumentId> openTextDocument,
CancellationToken cancellationToken)
where TDocument : TextDocument
Expand All @@ -384,7 +385,13 @@ private async Task<DifferenceViewerPreview> CreateRemovedTextDocumentPreviewView
// so that all IDE services (colorizer, squiggles etc.) light up in this buffer.
var leftDocumentId = DocumentId.CreateNewId(document.Project.Id);
var solutionWithRemovedDocument = removeTextDocument(document.Project.Solution, document.Id);
var leftSolution = addTextDocument(solutionWithRemovedDocument, leftDocumentId, document.Name, oldBuffer.AsTextContainer().CurrentText);
var leftDocumentInfo = DocumentInfo.Create(
leftDocumentId,
document.Name,
loader: TextLoader.From(TextAndVersion.Create(oldBuffer.AsTextContainer().CurrentText, VersionStamp.Create(), document.FilePath)),
filePath: document.FilePath,
folders: document.Folders);
var leftSolution = addTextDocument(solutionWithRemovedDocument, leftDocumentInfo);
var leftDocument = leftSolution.GetTextDocument(leftDocumentId);
var leftWorkspace = new PreviewWorkspace(leftSolution);
openTextDocument(leftWorkspace, leftDocument.Id);
Expand All @@ -400,7 +407,7 @@ public Task<DifferenceViewerPreview> CreateRemovedDocumentPreviewViewAsync(Docum
document, zoomLevel,
createBufferAsync: (textDocument, cancellationToken) => CreateNewBufferAsync(textDocument, cancellationToken),
removeTextDocument: (solution, documentId) => solution.RemoveDocument(documentId),
addTextDocument: (solution, documentId, name, text) => solution.AddDocument(documentId, name, text),
addTextDocument: (solution, documentInfo) => solution.AddDocument(documentInfo),
openTextDocument: (workspace, documentId) => workspace.OpenDocument(documentId),
cancellationToken);
}
Expand All @@ -411,7 +418,7 @@ public Task<DifferenceViewerPreview> CreateRemovedAdditionalDocumentPreviewViewA
document, zoomLevel,
createBufferAsync: CreateNewPlainTextBufferAsync,
removeTextDocument: (solution, documentId) => solution.RemoveAdditionalDocument(documentId),
addTextDocument: (solution, documentId, name, text) => solution.AddAdditionalDocument(documentId, name, text),
addTextDocument: (solution, documentInfo) => solution.AddAdditionalDocument(documentInfo),
openTextDocument: (workspace, documentId) => workspace.OpenAdditionalDocument(documentId),
cancellationToken);
}
Expand All @@ -422,7 +429,7 @@ public Task<DifferenceViewerPreview> CreateRemovedAnalyzerConfigDocumentPreviewV
document, zoomLevel,
createBufferAsync: CreateNewPlainTextBufferAsync,
removeTextDocument: (solution, documentId) => solution.RemoveAnalyzerConfigDocument(documentId),
addTextDocument: (solution, documentId, name, text) => solution.AddAnalyzerConfigDocument(documentId, name, text),
addTextDocument: (solution, documentInfo) => solution.AddAnalyzerConfigDocuments(ImmutableArray.Create(documentInfo)),
openTextDocument: (workspace, documentId) => workspace.OpenAnalyzerConfigDocument(documentId),
cancellationToken);
}
Expand Down Expand Up @@ -528,7 +535,7 @@ private async Task<DifferenceViewerPreview> CreateChangedAdditionalOrAnalyzerCon
TextDocument newDocument,
double zoomLevel,
Func<Solution, DocumentId, Solution> removeTextDocument,
Func<Solution, DocumentId, string, SourceText, Solution> addTextDocument,
Func<Solution, DocumentInfo, Solution> addTextDocument,
Func<Solution, DocumentId, SourceText, PreservationMode, Solution> withDocumentText,
Action<Workspace, DocumentId> openTextDocument,
CancellationToken cancellationToken)
Expand Down Expand Up @@ -571,7 +578,13 @@ private async Task<DifferenceViewerPreview> CreateChangedAdditionalOrAnalyzerCon
// so that all IDE services (colorizer, squiggles etc.) light up in these buffers.
var leftDocumentId = DocumentId.CreateNewId(oldDocument.Project.Id);
var solutionWithRemovedDocument = removeTextDocument(oldDocument.Project.Solution, oldDocument.Id);
var leftSolution = addTextDocument(solutionWithRemovedDocument, leftDocumentId, oldDocument.Name, oldBuffer.AsTextContainer().CurrentText);
var leftDocumentInfo = DocumentInfo.Create(
leftDocumentId,
oldDocument.Name,
loader: TextLoader.From(TextAndVersion.Create(oldBuffer.AsTextContainer().CurrentText, VersionStamp.Create(), oldDocument.FilePath)),
filePath: oldDocument.FilePath,
folders: oldDocument.Folders);
var leftSolution = addTextDocument(solutionWithRemovedDocument, leftDocumentInfo);
var leftWorkspace = new PreviewWorkspace(leftSolution);
openTextDocument(leftWorkspace, leftDocumentId);

Expand All @@ -591,7 +604,7 @@ public Task<DifferenceViewerPreview> CreateChangedAdditionalDocumentPreviewViewA
return CreateChangedAdditionalOrAnalyzerConfigDocumentPreviewViewAsync(
oldDocument, newDocument, zoomLevel,
removeTextDocument: (solution, documentId) => solution.RemoveAdditionalDocument(documentId),
addTextDocument: (solution, documentId, name, text) => solution.AddAdditionalDocument(documentId, name, text),
addTextDocument: (solution, documentInfo) => solution.AddAdditionalDocument(documentInfo),
withDocumentText: (solution, documentId, newText, mode) => solution.WithAdditionalDocumentText(documentId, newText, mode),
openTextDocument: (workspace, documentId) => workspace.OpenAdditionalDocument(documentId),
cancellationToken);
Expand All @@ -602,7 +615,7 @@ public Task<DifferenceViewerPreview> CreateChangedAnalyzerConfigDocumentPreviewV
return CreateChangedAdditionalOrAnalyzerConfigDocumentPreviewViewAsync(
oldDocument, newDocument, zoomLevel,
removeTextDocument: (solution, documentId) => solution.RemoveAnalyzerConfigDocument(documentId),
addTextDocument: (solution, documentId, name, text) => solution.AddAnalyzerConfigDocument(documentId, name, text),
addTextDocument: (solution, documentInfo) => solution.AddAnalyzerConfigDocuments(ImmutableArray.Create(documentInfo)),
withDocumentText: (solution, documentId, newText, mode) => solution.WithAnalyzerConfigDocumentText(documentId, newText, mode),
openTextDocument: (workspace, documentId) => workspace.OpenAnalyzerConfigDocument(documentId),
cancellationToken);
Expand Down

0 comments on commit 0dae2e9

Please sign in to comment.