From 9d78014c6e8920dfb62d2c09116ae9850e9dacc6 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Mon, 24 Jun 2024 23:47:26 -0700 Subject: [PATCH] Fix a couple of issues (#141) 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 --- .../CompilationDataTests.cs | 7 ++++--- src/Basic.CompilerLog.Util/BinaryLogUtil.cs | 2 +- src/Basic.CompilerLog.Util/CompilationData.cs | 13 ++++--------- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/Basic.CompilerLog.UnitTests/CompilationDataTests.cs b/src/Basic.CompilerLog.UnitTests/CompilationDataTests.cs index 5331bda..a99d9c5 100644 --- a/src/Basic.CompilerLog.UnitTests/CompilationDataTests.cs +++ b/src/Basic.CompilerLog.UnitTests/CompilationDataTests.cs @@ -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); diff --git a/src/Basic.CompilerLog.Util/BinaryLogUtil.cs b/src/Basic.CompilerLog.Util/BinaryLogUtil.cs index c96a3ff..6c93e2e 100644 --- a/src/Basic.CompilerLog.Util/BinaryLogUtil.cs +++ b/src/Basic.CompilerLog.Util/BinaryLogUtil.cs @@ -63,7 +63,7 @@ public List GetAllCompilerCalls(MSBuildProjectEvaluationData? eval var targetFramework = TargetFramework ?? evaluationData?.TargetFramework; var list = new List(); - 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)) { diff --git a/src/Basic.CompilerLog.Util/CompilationData.cs b/src/Basic.CompilerLog.Util/CompilationData.cs index d1cce28..f89b943 100644 --- a/src/Basic.CompilerLog.Util/CompilationData.cs +++ b/src/Basic.CompilerLog.Util/CompilationData.cs @@ -152,16 +152,11 @@ public List 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(); }