Skip to content

Commit

Permalink
Remove extra EnsureEditableDocuments calls (dotnet#57725)
Browse files Browse the repository at this point in the history
Remove single check for EnsureEditableDocuments on each document. This was duplicating the work that the call in TryApplyChanges does, and can have conflicts with linked files.

For example:

If file A.cs is renamed to B.cs and is in 2 projects
Each project will go through and apply changes (lets call P1 and P2)
P1 goes through and calls: ApplyTextChange > EnsureEditable > Rename File (File is now B.cs on disk)
P2 goes through to do the same as P1, but A.cs no longer exists so EnsureEditable will fail (can't check status of a file that doesn't exist)
We should be doing all the work to check edit status upfront so it doesn't collide with the actual edits we need to do. TryApplyChanges is already overridden and does that. This change just removes the extra work we were doing that also created problems for renaming files.

Fixes dotnet#56467
  • Loading branch information
ryzngard authored Nov 12, 2021
1 parent 3e792fc commit a4d1d8f
Showing 1 changed file with 0 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1171,7 +1171,6 @@ protected override void ApplyAnalyzerConfigDocumentTextChanged(DocumentId docume

private void ApplyTextDocumentChange(DocumentId documentId, SourceText newText)
{
EnsureEditableDocuments(documentId);
var containedDocument = TryGetContainedDocument(documentId);

if (containedDocument != null)
Expand Down Expand Up @@ -1442,9 +1441,6 @@ public void EnsureEditableDocuments(IEnumerable<DocumentId> documents)
}
}

public void EnsureEditableDocuments(params DocumentId[] documents)
=> this.EnsureEditableDocuments((IEnumerable<DocumentId>)documents);

internal override bool CanAddProjectReference(ProjectId referencingProject, ProjectId referencedProject)
{
_foregroundObject.AssertIsForeground();
Expand Down

0 comments on commit a4d1d8f

Please sign in to comment.