Fix/84847 source generated rename conflict by AdmiralSnyder · Pull Request #84849 · dotnet/roslyn

AdmiralSnyder · 2026-08-11T13:25:51Z

Fixes #84847
Claude generated fixes for #84847 :

include source generated files (razor) in rename operations,
use GetRequiredDocumentAsync instead of GetRequiredDocument
a test was also added, it contains a comment quite large, but LLMs are wordy...
... and feels a little incomplete since it only tests that that rename didn't modify
... and shouldn't the field in the Unrelated class have the original name (i.e. Type) to test that the rename operation didn't mistakenly change it?

PRing untested in coordination with @CyrusNajmabadi after conversation in discord.

just creating the PR for now, it's easier to talk about stuff that is there.

Microsoft Reviewers: Open in CodeFlow

Added a regression test for renaming with unrelated generated documents to ensure proper handling of source-generated DocumentIds.

azure-pipelines · 2026-08-11T13:26:12Z

Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run.

CyrusNajmabadi · 2026-08-11T13:26:45Z

+            ' documents being annotated/renamed, but it remains part of the full per-project document set that is
+            ' consulted when looking for declaration conflicts. That path used to call the DocumentId overload of
+            ' GetRequiredDocument, which throws for source-generated DocumentIds instead of returning the document -
+            ' this crashed with "GetRequiredDocument was given a source-generated DocumentId".


Drop comment

CyrusNajmabadi · 2026-08-11T13:27:04Z

-                        var newDocument = conflictResolution.CurrentSolution.GetRequiredDocument(documentId);
+                        // documentIdsForConflictResolution may contain ids for source-generated documents (for
+                        // example, a rename location that lives in Razor-generated code), so we must use the
+                        // overload that knows how to look those up instead of throwing on them.


Drop commnent

CyrusNajmabadi · 2026-08-11T13:27:59Z

We generally don't comment this way. As the failing test if you change it is better explanation for why it is needed.

Copilot

Pull request overview

This PR fixes a crash in Roslyn’s Rename conflict resolution pipeline when the per-project document set includes source-generated DocumentIds (e.g., Razor/source generator outputs). It updates the conflict identification step to retrieve documents via the async APIs that can resolve source-generated documents, and adds a regression test covering the reported scenario.

Changes:

Switch conflict-resolution document lookup from GetRequiredDocument(DocumentId) to GetRequiredDocumentAsync(..., includeSourceGenerated: true, ...) in ConflictResolver.Session.IdentifyConflictsAsync.
Ensure both new and old solutions use source-generated-aware document retrieval when mapping declaration-conflict spans.
Add a regression test exercising rename when an unrelated source-generated document contains the replacement text (work item #84847).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolver.Session.cs	Uses source-generated-aware document retrieval during conflict identification to avoid `GetRequiredDocument` failures on source-generated `DocumentId`s.
src/EditorFeatures/Test2/Rename/CSharp/SourceGeneratorTests.vb	Adds a regression test ensuring rename succeeds (and does not crash) when a source-generated document is included in conflict analysis due to containing the replacement text.

Remove outdated comments from RenameWithUnrelatedGeneratedDocumentContainingReplacementText test.

Removed comments regarding source-generated documents in conflict resolution.

AdmiralSnyder · 2026-08-11T13:35:54Z

I edited the comments out of those files and committed. apparently github knows that those commits should be added to the PR. nice.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

src/EditorFeatures/Test2/Rename/CSharp/SourceGeneratorTests.vb:119

This test currently only verifies the rename succeeds without throwing, but it doesn't assert that an unrelated source-generated document containing the original identifier text remains unchanged. Adding the original identifier (Type) into the generated document and asserting it still exists in the generated output after rename would make the regression test able to catch accidental edits to unrelated generated documents.

        Public Sub RenameWithUnrelatedGeneratedDocumentContainingReplacementText(host As RenameTestHost)
            Using result = RenameEngineResult.Create(_outputHelper,
                    <Workspace>
                        <Project Language="C#" AssemblyName="ClassLibrary1" CommonReferences="true">
                            <Document>
public class TypeData
{
    public int [|$$Type|];
}
                            </Document>
                            <DocumentFromSourceGenerator>
public class Unrelated
{
    public int Value;
}
                            </DocumentFromSourceGenerator>
                        </Project>
                    </Workspace>, host:=host, renameTo:="Value")

akhera99 · 2026-08-11T17:33:46Z

+        <Theory, CombinatorialData>
+        <WorkItem("https://github.com/dotnet/roslyn/issues/84847")>
+        Public Sub RenameWithUnrelatedGeneratedDocumentContainingReplacementText(host As RenameTestHost)
+            Using result = RenameEngineResult.Create(_outputHelper,


out of curiosity, can you run this test before/after the resolver changes?

hey, sorry, i can't. As i said in the PR: i don't have a roslyn dev environment, and so cannot do any testing for it. @CyrusNajmabadi explicitly told me to do this PR nonetheless - i also mentioned my doubts regarding the usefullness/exhaustiveness of this additional test - which copilot k*inda agrees?

I can get a dev environment up, but that will have to wait for the weekend :-|

ooh I see, sorry I missed that comment. I can do that bit and will suggest another test, since it does not seem like it exhausts the changed code

in lieu of modifying this PR, I'll just create a follow-up with the test. approving, thank you for the contribution!

* upstream/main: (730 commits) Improve recovery for repeated partial type modifiers (#84934) Add standalone C# LSP telemetry (#84874) Suppress AI artifact audit failure issues (#84925) Remove empty ExternalAccessAspNetCoreResources.resx (#84939) Update helix job monitor version (#84940) Add HasPendingUpdates to HotReloadService.Updates to be used in dotnet-watch (#84891) Add LocalizableBranches parameter for OneLocBuild (#84933) Pin version of Microsoft.CodeAnalysis.Analyzers (#84924) Centralize record and union keyword checks (#84928) Caching compiler: support binary additional texts (#84916) Remove unused AdditionalTextComparer (#84917) Remove Try-Both matching mode for unions. (#84897) Update CodeStyleAnalyzerVersion to 5.9.0 (#84919) Fix/84847 source generated rename conflict (#84849) Return MethodNotFound for unsupported LSP method dispatch (#84892) [main] Update dependencies from dotnet/arcade (#84896) [main] Update dependencies from dotnet/arcade (#84882) Track status for feature "Type Parameter Inference from Constraints" (#84869) [main] Source code updates from dotnet/dotnet (#84879) Stop labeling Loc PRs as community (#84878) ...

AdmiralSnyder added 2 commits August 11, 2026 15:10

Fix rename crash on source-generated DocumentId in conflict resolution

b36c88b

Add regression test for rename with unrelated generated document

ccebd0c

Added a regression test for renaming with unrelated generated documents to ensure proper handling of source-generated DocumentIds.

Copilot AI lite review requested due to automatic review settings August 11, 2026 13:25

AdmiralSnyder requested a review from a team as a code owner August 11, 2026 13:25

Copilot started reviewing on behalf of AdmiralSnyder August 11, 2026 13:26 View session

CyrusNajmabadi reviewed Aug 11, 2026

View reviewed changes

dotnet-policy-service Bot added Community The pull request was submitted by a contributor who is not a Microsoft employee. VSCode labels Aug 11, 2026

CyrusNajmabadi approved these changes Aug 11, 2026

View reviewed changes

Copilot AI reviewed Aug 11, 2026

View reviewed changes

Clean up comments in Rename test method

23d418b

Remove outdated comments from RenameWithUnrelatedGeneratedDocumentContainingReplacementText test.

Copilot AI review requested due to automatic review settings August 11, 2026 13:33

Copilot started reviewing on behalf of AdmiralSnyder August 11, 2026 13:33 View session

Remove comments about source-generated document handling

2437d0a

Removed comments regarding source-generated documents in conflict resolution.

Copilot AI reviewed Aug 11, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 11, 2026 13:37

Copilot started reviewing on behalf of AdmiralSnyder August 11, 2026 13:37 View session

Copilot AI reviewed Aug 11, 2026

View reviewed changes

Comment thread src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolver.Session.cs

Comment thread src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolver.Session.cs

akhera99 reviewed Aug 11, 2026

View reviewed changes

jjonescz added Area-IDE Feature - Source Generators Source Generators labels Aug 13, 2026

akhera99 approved these changes Aug 17, 2026

View reviewed changes

akhera99 merged commit c138cb0 into dotnet:main Aug 17, 2026
25 checks passed

This was referenced Aug 18, 2026

[main] Source code updates from dotnet/roslyn dotnet/dotnet#8363

Merged

[release/11.0.1xx] Source code updates from dotnet/roslyn dotnet/dotnet#8411

Merged

AdmiralSnyder deleted the fix/84847-source-generated-rename-conflict branch August 23, 2026 00:16

davidwengier mentioned this pull request Aug 24, 2026

Update roslyn to 5.11.0-1.26424.9 dotnet/vscode-csharp#9701

Merged

jjonescz added this to the 18.11 milestone Aug 25, 2026

Conversation

AdmiralSnyder commented Aug 11, 2026 • edited by davidwengier Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

azure-pipelines Bot commented Aug 11, 2026

Uh oh!

CyrusNajmabadi Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi commented Aug 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

AdmiralSnyder commented Aug 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

akhera99 Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

AdmiralSnyder Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

akhera99 Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

akhera99 Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AdmiralSnyder commented Aug 11, 2026 •

edited by davidwengier

Loading