Skip to content

Commit ca4fade

Browse files
committed
Make many fixes to the roslyn recipe runner
1 parent 97b050c commit ca4fade

File tree

11 files changed

+223
-102
lines changed

11 files changed

+223
-102
lines changed
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
using DiffPlex.DiffBuilder;
2+
using DiffPlex.DiffBuilder.Model;
3+
4+
namespace Rewrite.MSBuild;
5+
6+
public static class DiffHelper
7+
{
8+
/// <summary>
9+
/// Compares two strings and returns line numbers (1-based) where differences occur.
10+
/// </summary>
11+
/// <param name="oldText">The original text.</param>
12+
/// <param name="newText">The modified text.</param>
13+
/// <returns>A list of line numbers that differ between the two texts.</returns>
14+
public static List<int> GetDifferentLineNumbers(string oldText, string newText)
15+
{
16+
var differ = new SideBySideDiffBuilder(new DiffPlex.Differ());
17+
var diff = differ.BuildDiffModel(oldText, newText);
18+
19+
var differingLines = new List<int>();
20+
21+
for (int i = 0; i < diff.OldText.Lines.Count; i++)
22+
{
23+
var oldLine = diff.OldText.Lines[i];
24+
var newLine = diff.NewText.Lines[i];
25+
26+
if (oldLine.Type != ChangeType.Unchanged || newLine.Type != ChangeType.Unchanged)
27+
{
28+
differingLines.Add(i + 1); // +1 for 1-based line numbers
29+
}
30+
}
31+
32+
return differingLines;
33+
}
34+
}

Rewrite/src/Rewrite.MSBuild/RecipeExecutionContext.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ public Recipe CreateRecipe(params IReadOnlyCollection<RecipeStartInfo> recipeSta
6868
throw new InvalidOperationException("Different recipe types cannot be mixed in same execution context");
6969
}
7070

71-
var recipeType = recipeStartInfos.First().Kind;
71+
var recipeStartInfo = recipeStartInfos.First();
72+
var recipeType = recipeStartInfo.Kind;
7273

7374

7475

@@ -137,13 +138,15 @@ private Recipe CreateRoslynRecipe(params IReadOnlyCollection<RecipeStartInfo> re
137138
{
138139
var recipeStartInfo = recipeStartInfos.First();
139140
var solutionFilePath = (string)recipeStartInfo.Arguments[nameof(RoslynRecipe.SolutionFilePath)].Value!;
141+
var dryRun = (bool)recipeStartInfo.Arguments[nameof(RoslynRecipe.DryRun)].Value!;
140142
var diagnosticIds = recipeStartInfos.Select(x => x.Id).ToHashSet();
141143

142144
var roslynRecipe = new RoslynRecipe(_recipeAssemblies, _loggerFactory.CreateLogger<RoslynRecipe>())
143145
{
144146
DiagnosticIds = diagnosticIds,
145147
ApplyFixer = true,
146-
SolutionFilePath = solutionFilePath
148+
SolutionFilePath = solutionFilePath,
149+
DryRun = dryRun
147150
};
148151
return roslynRecipe;
149152
}

Rewrite/src/Rewrite.MSBuild/RecipeExecutionResult.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@ public record RecipeExecutionResult(string SolutionFile, TimeSpan ExecutionTime,
66

77
public record IssueFixResult(string IssueId, TimeSpan ExecutionTime, List<DocumentFixResult> Fixes);
88

9-
public record DocumentFixResult(string FileName);
9+
public record DocumentFixResult(string FileName, List<int> LineNumbers);

Rewrite/src/Rewrite.MSBuild/Rewrite.MSBuild.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
</PropertyGroup>
66

77
<ItemGroup>
8+
<PackageReference Include="DiffPlex" Version="1.7.2" />
89
<PackageReference Include="NMica.Utils" Version="3.0.0" />
910
<PackageReference Include="Serilog" Version="4.2.0" />
1011
<PackageReference Include="Serilog.Sinks.Console" Version="6.0.0" />

Rewrite/src/Rewrite.MSBuild/RoslynRecipe.cs

Lines changed: 149 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Diagnostics;
44
using System.Reflection;
55
using System.Runtime.Loader;
6+
using Microsoft.Build.Exceptions;
67
using Microsoft.CodeAnalysis;
78
using Microsoft.CodeAnalysis.CodeActions;
89
using Microsoft.CodeAnalysis.CodeFixes;
@@ -89,6 +90,7 @@ public async Task<RecipeExecutionResult> Execute(CancellationToken cancellationT
8990
.Cast<CodeFixProvider>()
9091
.Where(x => x.FixableDiagnosticIds.Any())
9192
.SelectMany(fixer => fixer.FixableDiagnosticIds.Select(id => (Id: id, Fixer: fixer)))
93+
.DistinctBy(x => x.Id)
9294
.ToDictionary(x => x.Id, x => x.Fixer);
9395

9496
var analyzersWithFixersById = allAnalyzers
@@ -108,20 +110,39 @@ public async Task<RecipeExecutionResult> Execute(CancellationToken cancellationT
108110
return new RecipeExecutionResult(SolutionFilePath, watch.Elapsed, []);
109111
}
110112

111-
var issuesBeingFixed = analyzersWithFixersById.Select(x =>
112-
{
113-
var diagnostic = x.Value.Analyzer.SupportedDiagnostics.First(y => y.Id == x.Key);
114-
return $"{diagnostic.Id}: {diagnostic.Title}";
115-
});
116-
117-
log.LogDebug("Solution {SolutionFilePath} loaded in {Elapsed}. Fixing {@Issues}", SolutionFilePath, watch.Elapsed, issuesBeingFixed);
118-
119-
var issuesTypesInCodebase = (await GetDiagnostics(solution, analyzersWithFixersById, cancellationToken))
113+
// var issuesBeingFixed = analyzersWithFixersById.Select(x =>
114+
// {
115+
// var diagnostic = x.Value.Analyzer.SupportedDiagnostics.First(y => y.Id == x.Key);
116+
// return $"{diagnostic.Id}: {diagnostic.Title}";
117+
// });
118+
119+
var loadedTime = watch.Elapsed;
120+
121+
var allDiagnostics = await GetDiagnostics(solution, analyzersWithFixersById, cancellationToken);
122+
var issuesTypesInCodebase = allDiagnostics
123+
.Where(x => this.DiagnosticIds.Contains(x.Id))
120124
.Select(x => x.Id)
121125
.ToHashSet();
122126
var analyzersWithFixersByIdForIssuesInCodebase = analyzersWithFixersById
123127
.Where(x => issuesTypesInCodebase.Contains(x.Key))
124128
.ToDictionary(x => x.Key, x => x.Value);
129+
130+
log.LogDebug("Solution {SolutionFilePath} loaded in {Elapsed}", SolutionFilePath, loadedTime);
131+
if (analyzersWithFixersByIdForIssuesInCodebase.Count == 0)
132+
{
133+
log.LogDebug("No fixable issues found in solution");
134+
}
135+
// else
136+
// {
137+
// var issueCounts = allDiagnostics
138+
// .GroupBy(x => x.Id)
139+
// .Select(x => new
140+
// {
141+
// IssueId = $"{x.Key}: {x.First().Descriptor.Title}",
142+
// Occurances = x.Count()
143+
// });
144+
// log.LogDebug("Fixing {@Issues}", issueCounts);
145+
// }
125146

126147
foreach (var (issueId, (analyzer, codeFixProvider)) in analyzersWithFixersByIdForIssuesInCodebase)
127148
{
@@ -134,96 +155,142 @@ public async Task<RecipeExecutionResult> Execute(CancellationToken cancellationT
134155
{issueId, (analyzer, codeFixProvider)}
135156
};
136157
var diagnostics = await GetDiagnostics(solution, analyzersToRun, cancellationToken);
137-
var diagnosticsById = diagnostics.ToLookup(x => x.Id);
138-
var diagnosticsToProcess = diagnosticsById.ToList();
139-
140-
foreach (var diagnosticIssue in diagnosticsToProcess)
158+
diagnostics = diagnostics
159+
.Where(x => x.Id == issueId)
160+
.ToList();
161+
if (diagnostics.Count == 0)
141162
{
142-
var diagnostic = diagnosticIssue.First();
143-
var document = solution.Solution.GetDocument(diagnostic.Location.SourceTree) ?? throw new Exception($"Could not find document associated with {diagnostic.Id} {diagnostic.Descriptor.Title}");
144-
145-
var diagnosticsByDocument = diagnosticIssue
146-
.Select(x => (Diagnostic: x, Document: solution.Solution.GetDocument(x.Location.SourceTree)))
147-
.Where(x => x.Document != null)
148-
.GroupBy(x => x.Document!, x => x.Diagnostic)
149-
.Where(x => x.Key is not SourceGeneratedDocument)
150-
.ToImmutableDictionary(x => x.Key, x => x.ToImmutableArray());
163+
continue;
164+
}
151165

166+
var diagnosticsByDocument = diagnostics
167+
.Select(x => (Diagnostic: x, Document: solution.Solution.GetDocument(x.Location.SourceTree)))
168+
.Where(x => x.Document != null)
169+
.GroupBy(x => x.Document!, x => x.Diagnostic)
170+
.Where(x => x.Key is not SourceGeneratedDocument)
171+
.ToImmutableDictionary(x => x.Key, x => x.ToImmutableArray());
152172

173+
var diagnosticProvider = new FixMultipleDiagnosticProvider(diagnosticsByDocument);
174+
// var codeFixProvider = analyzersWithFixersById[diagnosticIssue.Key].Fixer;
175+
var fixAllProvider = codeFixProvider.GetFixAllProvider();
153176

154-
var diagnosticProvider = new FixMultipleDiagnosticProvider(diagnosticsByDocument);
155-
// var codeFixProvider = analyzersWithFixersById[diagnosticIssue.Key].Fixer;
156-
var fixAllProvider = codeFixProvider.GetFixAllProvider() ?? throw new InvalidOperationException($"Bulk fix provider not available for {diagnosticIssue.Key}: {diagnosticIssue.First().Descriptor.Title}");
177+
if (fixAllProvider == null)
178+
{
179+
fixAllProvider = WellKnownFixAllProviders.BatchFixer;
180+
// throw new InvalidOperationException($"Bulk fix provider not available for {issueId}: {diagnostic.Descriptor.Title}");
181+
}
157182

158-
log.LogDebug("Fixing {DiagnosticId}: '{Title}' using {TypeName} in {DocumentCount} documents ({OccurrenceCount} occurrences)",
159-
diagnosticIssue.Key,
160-
diagnosticIssue.First().Descriptor.Title,
161-
codeFixProvider.GetType().Name,
162-
diagnosticsByDocument.Keys.Count(),
163-
diagnosticsToProcess.SelectMany(x => x).Count()
164-
);
183+
Diagnostic? sampledDiagnostic = null;
184+
string? equivalenceKey = null;
185+
// try to find first viable fixup type for this issue type
186+
foreach(var diagnostic in diagnostics)
187+
{
188+
var document = solution.Solution.GetDocument(diagnostic.Location.SourceTree);
189+
if (document == null)
190+
throw new Exception($"Could not find document associated with {diagnostic.Id} {diagnostic.Descriptor.Title}");
165191

166192
var actions = new List<CodeAction>();
167-
var context = new CodeFixContext(document, diagnostic, (a, d) => actions.Add(a), CancellationToken.None);
168-
await codeFixProvider.RegisterCodeFixesAsync(context);
169-
if (actions.Count == 0)
193+
194+
var context = new CodeFixContext(document!, diagnostic, (a, d) => actions.Add(a), CancellationToken.None);
195+
try
170196
{
171-
log.LogDebug("No code fixes found");
197+
await codeFixProvider.RegisterCodeFixesAsync(context);
198+
}
199+
catch (Exception)
200+
{
201+
// log.LogError(e, "Code fix up for {IssueId} failed due it its own internal logic", issueId);
172202
continue;
173203
}
174204

205+
206+
if (actions.Count == 0)
207+
{
208+
// log.LogDebug("No fixable issues found");
209+
continue;
210+
}
211+
175212
var codeFixAction = actions[0];
213+
214+
//codeFixAction.
176215
if (!codeFixAction.NestedActions.IsEmpty)
177216
{
178-
log.LogWarning("Skipping refactoring of recipe {DiagnosticId} because there's multiple variations of refactoring that can be applied", diagnosticIssue.Key);
217+
// log.LogWarning("Skipping refactoring of recipe {DiagnosticId} because there's multiple variations of refactoring that can be applied", issueId);
179218
continue;
180219
}
220+
sampledDiagnostic = diagnostic;
221+
equivalenceKey = codeFixAction.EquivalenceKey;
222+
break;
223+
}
181224

182-
var fixAllContext = new FixAllContext(
183-
diagnosticsByDocument.Keys.First(),
184-
codeFixProvider,
185-
FixAllScope.Solution,
186-
actions.First().EquivalenceKey,
187-
[diagnosticIssue.Key],
188-
diagnosticProvider,
189-
cancellationToken);
225+
if (sampledDiagnostic == null)
226+
{
227+
// generally we end up here when an analyzer has a fixer associated with it, but fixer determined that it can't actually fix it automatically (didn't register any code actions)
228+
log.LogDebug("No fixable issues found in solution");
229+
break;
230+
}
190231

232+
log.LogDebug("Fixing {DiagnosticId}: '{Title}' using {TypeName} in {DocumentCount} documents ({OccurrenceCount} occurrences)",
233+
issueId,
234+
sampledDiagnostic.Descriptor.Title,
235+
codeFixProvider.GetType().Name,
236+
diagnosticsByDocument.Keys.Count(),
237+
diagnostics.Count()
238+
);
239+
240+
var fixAllContext = new FixAllContext(
241+
diagnosticsByDocument.Keys.First(),
242+
codeFixProvider,
243+
FixAllScope.Solution,
244+
equivalenceKey,
245+
[issueId],
246+
diagnosticProvider,
247+
cancellationToken);
248+
Solution newSolution;
249+
try
250+
{
191251
var codeAction = await fixAllProvider.GetFixAsync(fixAllContext) ?? throw new Exception("Code action was not found");
192-
252+
193253
var operations = await codeAction.GetOperationsAsync(cancellationToken);
194254
var applyChangesOperation = operations.OfType<ApplyChangesOperation>().First();
195-
var newSolution = applyChangesOperation.ChangedSolution;
196-
// var affectedDocumentIds = await GetChangedDocumentsAsync(solution.Solution, newSolution, cancellationToken);
197-
// var affectedDocuments = affectedDocumentIds
198-
// .Select(docId => solution.Solution.GetDocument(docId)?.FilePath)
199-
// .Where(x => x != null)
200-
// .Select(x => ((AbsolutePath)SolutionFilePath).GetRelativePathTo((AbsolutePath)x));
201-
// changedFiles.AddRange(affectedDocuments);
202-
// foreach (var docId in affectedDocuments.Take(1))
203-
// {
204-
// var before = (await solution.Solution.GetDocument(docId)!.GetTextAsync()).ToString();
205-
// var after = (await newSolution.GetDocument(docId)!.GetTextAsync()).ToString();
206-
//
207-
// var diffs = StringDiffer.GetDifferences(before, after);
208-
// Console.WriteLine(solution.Solution.GetDocument(docId).FilePath);
209-
// Console.WriteLine("======");
210-
// Console.WriteLine(diffs);
211-
// }
212-
213-
solution.Solution = newSolution;
214-
var affectedDocumentIds = await GetChangedDocumentsAsync(originalSolution, solution.Solution, cancellationToken);
215-
var affectedDocuments = affectedDocumentIds
216-
.Select(docId => solution.Solution.GetDocument(docId)!)
217-
.ToList();
218-
var issueFixResult = new IssueFixResult(
219-
IssueId: issueId,
220-
ExecutionTime: recipeWatch.Elapsed,
221-
Fixes: affectedDocuments
222-
.Select(x => new DocumentFixResult(x.FilePath!))
223-
.ToList());
224-
fixedIssues.Add(issueFixResult);
225-
recipeWatch.Stop();
255+
newSolution = applyChangesOperation.ChangedSolution;
256+
}
257+
catch (Exception e)
258+
{
259+
log.LogError(e, "Unable to apply {IssueId} do to internal CodeFixup logic error", issueId);
260+
continue;
226261
}
262+
263+
// var affectedDocumentIds = await GetChangedDocumentsAsync(solution.Solution, newSolution, cancellationToken);
264+
// var affectedDocuments = affectedDocumentIds
265+
// .Select(docId => solution.Solution.GetDocument(docId)?.FilePath)
266+
// .Where(x => x != null)
267+
// .Select(x => ((AbsolutePath)SolutionFilePath).GetRelativePathTo((AbsolutePath)x));
268+
// changedFiles.AddRange(affectedDocuments);
269+
// foreach (var docId in affectedDocuments.Take(1))
270+
// {
271+
// var before = (await solution.Solution.GetDocument(docId)!.GetTextAsync()).ToString();
272+
// var after = (await newSolution.GetDocument(docId)!.GetTextAsync()).ToString();
273+
//
274+
// var diffs = StringDiffer.GetDifferences(before, after);
275+
// Console.WriteLine(solution.Solution.GetDocument(docId).FilePath);
276+
// Console.WriteLine("======");
277+
// Console.WriteLine(diffs);
278+
// }
279+
280+
solution.Solution = newSolution;
281+
var affectedDocumentIds = await GetChangedDocumentsAsync(originalSolution, solution.Solution, cancellationToken);
282+
var affectedDocuments = affectedDocumentIds
283+
.Select(x => (Document: solution.Solution.GetDocument(x.DocumentId)!, x.ChangedLineNumbers))
284+
.ToList();
285+
var issueFixResult = new IssueFixResult(
286+
IssueId: issueId,
287+
ExecutionTime: recipeWatch.Elapsed,
288+
Fixes: affectedDocuments
289+
.Select(x => new DocumentFixResult(x.Document.FilePath!, x.ChangedLineNumbers))
290+
.ToList());
291+
fixedIssues.Add(issueFixResult);
292+
recipeWatch.Stop();
293+
227294
}
228295

229296
if (!DryRun)
@@ -263,12 +330,12 @@ private async Task<List<Diagnostic>> GetDiagnostics(
263330
return diagnostics;
264331
}
265332

266-
static async Task<IEnumerable<DocumentId>> GetChangedDocumentsAsync(
333+
static async Task<IEnumerable<(DocumentId DocumentId, List<int> ChangedLineNumbers)>> GetChangedDocumentsAsync(
267334
Solution oldSolution,
268335
Solution newSolution,
269336
CancellationToken cancellationToken = default)
270337
{
271-
var changedDocumentIds = new List<DocumentId>();
338+
var changedDocumentIds = new List<(DocumentId, List<int> ChangedLineNumbers)>();
272339

273340
foreach (var projectId in newSolution.ProjectIds)
274341
{
@@ -291,7 +358,9 @@ static async Task<IEnumerable<DocumentId>> GetChangedDocumentsAsync(
291358

292359
if (!oldText.ContentEquals(newText))
293360
{
294-
changedDocumentIds.Add(documentId);
361+
var diffLineNumbers = DiffHelper.GetDifferentLineNumbers(oldText.ToString(), newText.ToString());
362+
changedDocumentIds.Add((documentId,diffLineNumbers));
363+
295364
}
296365
}
297366
}

Rewrite/src/Rewrite.Server/Commands/ConfigureInterceptor.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using Microsoft.Extensions.DependencyInjection;
1+
using Microsoft.Extensions.Configuration;
2+
using Microsoft.Extensions.DependencyInjection;
23
using Microsoft.Extensions.Hosting;
34
using Microsoft.Extensions.Logging;
45
using Rewrite.Diagnostics;
@@ -16,6 +17,10 @@ public void Intercept(CommandContext context, CommandSettings settings)
1617
Logging.ConfigureLogging(commonSettings.LogFilePath);
1718
builder.Logging.ClearProviders();
1819
builder.Logging.AddSerilog();
20+
builder.Configuration
21+
.AddYamlFile("appsettings.yaml")
22+
.AddYamlFile($"appsettings.{builder.Environment.EnvironmentName}.yaml", optional: true)
23+
.AddEnvironmentVariables();
1924
builder.Services.AddSingleton<RecipeManager>();
2025
builder.Services.AddSingleton<NuGet.Common.ILogger, NugetLogger>();
2126
builder.Services.AddSingleton(commonSettings);

0 commit comments

Comments
 (0)