Skip to content

Commit ecd68ce

Browse files
committed
refactor: split MigrateDocumentAsync and skip per-iteration recompile
Extract CollectAndAnnotate + ApplyRewritesAsync helpers so each method stays under Sonar's cognitive-complexity ceiling of 15 (the original MigrateDocumentAsync hit 23). Hoist maxUsingChange computation from the work-item intent before the rewrite loop, so a block-level rewrite that absorbs a sibling annotated node still applies the absorbed item's using-directive change. Guard the final swap with anyRewriteSucceeded so a no-op pass doesn't add a Testably using to a still-TestingHelpers-bound file. Skip the Document round-trip for purely syntactic patterns (most of them) and only sync through a fresh SemanticModel for AccessorAddFile, FilesCtor*, and AddFilesFromEmbeddedNamespace. When a sync is necessary, re-locate the annotated target in the round-tripped tree — Roslyn's WithSyntaxRoot may produce a tree whose nodes are not reference-equal to the input, which would make SemanticModel.GetSymbolInfo reject them.
1 parent d43d9e6 commit ecd68ce

2 files changed

Lines changed: 129 additions & 53 deletions

File tree

Source/Testably.Abstractions.Migration.Analyzers.CodeFixers/SystemIOAbstractionsCodeFixProvider.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1739,6 +1739,17 @@ internal enum UsingChange
17391739
_ => UsingChange.None,
17401740
};
17411741

1742+
/// <summary>
1743+
/// Whether a pattern's pure rewriter requires a <see cref="SemanticModel" />.
1744+
/// Used by the fix-all loop to avoid an O(N) per-iteration document round-trip
1745+
/// when the rewrite is purely syntactic — most patterns fall in that bucket.
1746+
/// </summary>
1747+
internal static bool PatternNeedsSemanticModel(string pattern) => pattern
1748+
is Patterns.AccessorAddFile
1749+
or Patterns.MockFileSystemFilesConstructor
1750+
or Patterns.MockFileSystemFilesOptionsConstructor
1751+
or Patterns.MockFileSystemAddFilesFromEmbeddedNamespace;
1752+
17421753
/// <summary>
17431754
/// Applies a using-directive change to the compilation unit. Called once at the end
17441755
/// of a fix pipeline so that overlapping using edits from multiple rewrites collapse
@@ -1779,9 +1790,8 @@ internal static async Task<Document> ApplySinglePatternAsync(
17791790
.ConfigureAwait(false);
17801791
}
17811792

1782-
Document? result = await SystemIOAbstractionsFixAllProvider
1793+
return await SystemIOAbstractionsFixAllProvider
17831794
.MigrateDocumentAsync(document, siblings, cancellationToken).ConfigureAwait(false);
1784-
return result ?? document;
17851795
}
17861796

17871797
/// <summary>

Source/Testably.Abstractions.Migration.Analyzers.CodeFixers/SystemIOAbstractionsFixAllProvider.cs

Lines changed: 117 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,12 @@ private SystemIOAbstractionsFixAllProvider()
2626
{
2727
}
2828

29-
protected override Task<Document?> FixAllAsync(
29+
protected override async Task<Document?> FixAllAsync(
3030
FixAllContext fixAllContext,
3131
Document document,
3232
ImmutableArray<Diagnostic> diagnostics)
33-
=> MigrateDocumentAsync(document, diagnostics, fixAllContext.CancellationToken);
33+
=> await MigrateDocumentAsync(document, diagnostics, fixAllContext.CancellationToken)
34+
.ConfigureAwait(false);
3435

3536
/// <summary>
3637
/// Migrates every diagnostic in <paramref name="diagnostics" /> within
@@ -41,7 +42,7 @@ private SystemIOAbstractionsFixAllProvider()
4142
/// the per-diagnostic Fix re-discovers all sibling diagnostics in the document
4243
/// and routes through here so one click migrates the whole file (Mockolate-style).
4344
/// </summary>
44-
internal static async Task<Document?> MigrateDocumentAsync(
45+
internal static async Task<Document> MigrateDocumentAsync(
4546
Document document,
4647
ImmutableArray<Diagnostic> diagnostics,
4748
CancellationToken cancellationToken)
@@ -58,10 +59,51 @@ private SystemIOAbstractionsFixAllProvider()
5859
return document;
5960
}
6061

61-
// Source-order pass: annotate each diagnostic's target node so we can locate it
62-
// after prior rewrites have transformed its surroundings. The same diagnostic id
63-
// may legitimately appear on overlapping spans (e.g. nested initializers); a
64-
// fresh annotation per work item keeps them addressable independently.
62+
(CompilationUnitSyntax annotatedCu, List<WorkItem> work) = CollectAndAnnotate(originalCu, diagnostics);
63+
if (work.Count == 0)
64+
{
65+
return document;
66+
}
67+
68+
// Compute the using-directive change up front from work-item intent rather than
69+
// from rewrite success. A block-level rewrite (e.g. FilesCtor initializer
70+
// expansion) can absorb a sibling annotated node so that its later dispatch
71+
// returns null — but the absorbed item's using-change contribution must still
72+
// land, otherwise the file would be left half-migrated.
73+
SystemIOAbstractionsCodeFixProvider.UsingChange maxUsingChange =
74+
SystemIOAbstractionsCodeFixProvider.UsingChange.None;
75+
foreach (WorkItem item in work)
76+
{
77+
SystemIOAbstractionsCodeFixProvider.UsingChange change =
78+
SystemIOAbstractionsCodeFixProvider.GetUsingChange(item.Pattern);
79+
if (change > maxUsingChange)
80+
{
81+
maxUsingChange = change;
82+
}
83+
}
84+
85+
(CompilationUnitSyntax finalCu, bool anyRewriteSucceeded) = await ApplyRewritesAsync(
86+
document, annotatedCu, work, cancellationToken).ConfigureAwait(false);
87+
88+
// Suppress the using swap when no rewrite landed. Otherwise we'd add the Testably
89+
// using to a file whose code still references TestingHelpers — non-compiling.
90+
if (anyRewriteSucceeded
91+
&& maxUsingChange != SystemIOAbstractionsCodeFixProvider.UsingChange.None)
92+
{
93+
finalCu = SystemIOAbstractionsCodeFixProvider.ApplyUsingChange(finalCu, maxUsingChange);
94+
}
95+
96+
return document.WithSyntaxRoot(finalCu);
97+
}
98+
99+
/// <summary>
100+
/// Walks the diagnostics in source order, finds each one's target node in the
101+
/// original compilation unit, and annotates every target so subsequent rewrites
102+
/// can locate it via <see cref="SyntaxNode.GetAnnotatedNodes(SyntaxAnnotation)" />.
103+
/// </summary>
104+
private static (CompilationUnitSyntax AnnotatedCu, List<WorkItem> Work) CollectAndAnnotate(
105+
CompilationUnitSyntax originalCu, ImmutableArray<Diagnostic> diagnostics)
106+
{
65107
List<WorkItem> work = [];
66108
Dictionary<SyntaxNode, List<SyntaxAnnotation>> nodeToAnnotations = new();
67109
foreach (Diagnostic diagnostic in diagnostics.OrderBy(d => d.Location.SourceSpan.Start))
@@ -91,75 +133,99 @@ private SystemIOAbstractionsFixAllProvider()
91133

92134
if (work.Count == 0)
93135
{
94-
return document;
136+
return (originalCu, work);
95137
}
96138

97139
CompilationUnitSyntax annotatedCu = originalCu.ReplaceNodes(
98140
nodeToAnnotations.Keys,
99141
(original, _) => original.WithAdditionalAnnotations([..nodeToAnnotations[original]]));
142+
return (annotatedCu, work);
143+
}
100144

101-
Document currentDoc = document.WithSyntaxRoot(annotatedCu);
102-
SystemIOAbstractionsCodeFixProvider.UsingChange maxUsingChange =
103-
SystemIOAbstractionsCodeFixProvider.UsingChange.None;
145+
/// <summary>
146+
/// Applies each pattern's pure rewriter to <paramref name="annotatedCu" /> in
147+
/// sequence. Stays in-memory between iterations for purely-syntactic patterns and
148+
/// only round-trips through a fresh <see cref="Document" /> /
149+
/// <see cref="SemanticModel" /> when the pattern actually needs semantic info —
150+
/// that avoids an O(N) re-compile per rewrite for the (common) syntactic patterns.
151+
/// When a round-trip is necessary, both the rewrite target and the semantic model
152+
/// are taken from the round-tripped tree, since <see cref="Document.WithSyntaxRoot" />
153+
/// may produce a tree whose nodes are not reference-equal to the input.
154+
/// </summary>
155+
private static async Task<(CompilationUnitSyntax FinalCu, bool AnyRewriteSucceeded)> ApplyRewritesAsync(
156+
Document originalDocument,
157+
CompilationUnitSyntax annotatedCu,
158+
List<WorkItem> work,
159+
CancellationToken cancellationToken)
160+
{
161+
CompilationUnitSyntax currentCu = annotatedCu;
162+
bool anyRewriteSucceeded = false;
104163

105164
foreach (WorkItem item in work)
106165
{
107-
SyntaxNode? curRoot = await currentDoc
108-
.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
109-
if (curRoot is not CompilationUnitSyntax curCu)
166+
CompilationUnitSyntax? rewritten;
167+
if (SystemIOAbstractionsCodeFixProvider.PatternNeedsSemanticModel(item.Pattern))
110168
{
111-
continue;
169+
rewritten = await DispatchWithSemanticModelAsync(
170+
originalDocument, currentCu, item, cancellationToken).ConfigureAwait(false);
112171
}
113-
114-
// Annotations propagate through ReplaceNode/ReplaceNodes automatically. A null
115-
// here means a prior block-level rewrite (AddFile/FilesCtor initializer
116-
// expansion) replaced an enclosing statement and the annotated node was
117-
// dropped — skip it; the surrounding rewrite already covered it.
118-
SyntaxNode? currentTarget = curCu.GetAnnotatedNodes(item.Annotation).FirstOrDefault();
119-
if (currentTarget is null)
172+
else
120173
{
121-
continue;
174+
SyntaxNode? currentTarget = currentCu.GetAnnotatedNodes(item.Annotation).FirstOrDefault();
175+
if (currentTarget is null)
176+
{
177+
continue;
178+
}
179+
180+
rewritten = SystemIOAbstractionsCodeFixProvider.DispatchPure(
181+
item.Pattern, currentCu, currentTarget, semanticModel: null, cancellationToken);
122182
}
123183

124-
// Re-acquire the semantic model from the live document. Earlier rewrites
125-
// change the syntax tree; the original-document semantic model would return
126-
// stale or null symbol info for nodes in the new tree.
127-
SemanticModel? semanticModel = await currentDoc
128-
.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
129-
130-
CompilationUnitSyntax? rewritten = SystemIOAbstractionsCodeFixProvider.DispatchPure(
131-
item.Pattern, curCu, currentTarget, semanticModel, cancellationToken);
132184
if (rewritten is null)
133185
{
134186
continue;
135187
}
136188

137-
currentDoc = currentDoc.WithSyntaxRoot(rewritten);
189+
currentCu = rewritten;
190+
anyRewriteSucceeded = true;
191+
}
138192

139-
SystemIOAbstractionsCodeFixProvider.UsingChange change =
140-
SystemIOAbstractionsCodeFixProvider.GetUsingChange(item.Pattern);
141-
if (change > maxUsingChange)
142-
{
143-
maxUsingChange = change;
144-
}
193+
return (currentCu, anyRewriteSucceeded);
194+
}
195+
196+
/// <summary>
197+
/// Round-trips <paramref name="currentCu" /> through a document so the semantic
198+
/// model binds to the live tree, then re-locates the work item's annotated target
199+
/// in that round-tripped tree and dispatches the pattern's pure rewriter against
200+
/// it. Returns the new compilation unit (still annotation-bearing) or
201+
/// <see langword="null" /> if the target was absorbed or the rewrite is not
202+
/// applicable.
203+
/// </summary>
204+
private static async Task<CompilationUnitSyntax?> DispatchWithSemanticModelAsync(
205+
Document originalDocument,
206+
CompilationUnitSyntax currentCu,
207+
WorkItem item,
208+
CancellationToken cancellationToken)
209+
{
210+
Document syncedDoc = originalDocument.WithSyntaxRoot(currentCu);
211+
SyntaxNode? syncedRoot = await syncedDoc
212+
.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
213+
if (syncedRoot is not CompilationUnitSyntax syncedCu)
214+
{
215+
return null;
145216
}
146217

147-
// Apply the using-directive change once, after every syntax rewrite. Doing it
148-
// here (instead of inside each pattern's rewrite) is the entire point of this
149-
// provider: it keeps the analyzer firing on remaining diagnostics during the
150-
// pass, and produces a single deterministic text change at the using line.
151-
if (maxUsingChange != SystemIOAbstractionsCodeFixProvider.UsingChange.None)
218+
SyntaxNode? syncedTarget = syncedCu.GetAnnotatedNodes(item.Annotation).FirstOrDefault();
219+
if (syncedTarget is null)
152220
{
153-
SyntaxNode? finalRoot = await currentDoc
154-
.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
155-
if (finalRoot is CompilationUnitSyntax finalCu)
156-
{
157-
finalCu = SystemIOAbstractionsCodeFixProvider.ApplyUsingChange(finalCu, maxUsingChange);
158-
currentDoc = currentDoc.WithSyntaxRoot(finalCu);
159-
}
221+
return null;
160222
}
161223

162-
return currentDoc;
224+
SemanticModel? semanticModel = await syncedDoc
225+
.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
226+
227+
return SystemIOAbstractionsCodeFixProvider.DispatchPure(
228+
item.Pattern, syncedCu, syncedTarget, semanticModel, cancellationToken);
163229
}
164230

165231
private readonly struct WorkItem

0 commit comments

Comments
 (0)