Skip to content

Commit

Permalink
Fix a couple of issues (#141)
Browse files Browse the repository at this point in the history
This fixes the following issue:

- #129 missed a case where `None` was special cased in `CompilationData`. That meant it double added `SyntaxTree` items. 
- Potential source of non-determinism as we're iterating a dictionary without sorting keys first.

closes #139
closes #140
  • Loading branch information
jaredpar authored Jun 25, 2024
1 parent c310985 commit 9d78014
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 13 deletions.
7 changes: 4 additions & 3 deletions src/Basic.CompilerLog.UnitTests/CompilationDataTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,11 @@ public void GetCompilationAfterGeneratorsDiagnostics()
Assert.NotEmpty(diagnostics);
}

[Fact]
public void GetGeneratedSyntaxTrees()
[Theory]
[CombinatorialData]
public void GetGeneratedSyntaxTrees(BasicAnalyzerKind basicAnalyzerKind)
{
using var reader = CompilerLogReader.Create(Fixture.Console.Value.CompilerLogPath);
using var reader = CompilerLogReader.Create(Fixture.Console.Value.CompilerLogPath, basicAnalyzerKind);
var data = reader.ReadAllCompilationData().Single();
var trees = data.GetGeneratedSyntaxTrees();
Assert.Single(trees);
Expand Down
2 changes: 1 addition & 1 deletion src/Basic.CompilerLog.Util/BinaryLogUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public List<CompilerCall> GetAllCompilerCalls(MSBuildProjectEvaluationData? eval
var targetFramework = TargetFramework ?? evaluationData?.TargetFramework;
var list = new List<CompilerCall>();

foreach (var data in _taskMap.Values)
foreach (var data in _taskMap.OrderBy(kvp => kvp.Key).Select(kvp => kvp.Value))
{
if (!_targetMap.TryGetValue(data.TargetId, out var compilerCallKind))
{
Expand Down
13 changes: 4 additions & 9 deletions src/Basic.CompilerLog.Util/CompilationData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -152,16 +152,11 @@ public List<SyntaxTree> GetGeneratedSyntaxTrees(
{
var afterCompilation = GetCompilationAfterGenerators(out diagnostics, cancellationToken);

// This is a bit of a hack to get the number of syntax trees before running the generators. It feels
// a bit disjoint that we have to think of the None case differently here. Possible it may be simpler
// to have the None host go back to faking a ISourceGenerator in memory that just adds the files
// directly.
// Generated syntax trees are always added to the end of the list. This is an
// implementation detail of the compiler, but one that is unlikely to ever
// change. Doing so would represent a breaking change as file ordering impacts
// semantics.
var originalCount = Compilation.SyntaxTrees.Count();
if (BasicAnalyzerHost is BasicAnalyzerHostNone none)
{
var generatedCount = none.GeneratedSourceTexts.Length;
originalCount -= generatedCount;
}
return afterCompilation.SyntaxTrees.Skip(originalCount).ToList();
}

Expand Down

0 comments on commit 9d78014

Please sign in to comment.