From cac74f8e430c06d5a0810646e0e322774b1e7c43 Mon Sep 17 00:00:00 2001 From: jdickson Date: Sun, 17 Nov 2024 12:12:11 +0700 Subject: [PATCH] simplify solution --- .../SarifConversionTests.cs | 49 ++++--- .../Agoda.CodeCompass.MSBuild.csproj | 1 + .../Sarif/V1Result.cs | 4 + .../SarifReporter.cs | 129 ++++++++-------- .../TechDebtReportTask.cs | 138 +----------------- 5 files changed, 104 insertions(+), 217 deletions(-) diff --git a/src/Agoda.CodeCompass.MSBuild.Tests/SarifConversionTests.cs b/src/Agoda.CodeCompass.MSBuild.Tests/SarifConversionTests.cs index 6050fb5..4eebd1c 100644 --- a/src/Agoda.CodeCompass.MSBuild.Tests/SarifConversionTests.cs +++ b/src/Agoda.CodeCompass.MSBuild.Tests/SarifConversionTests.cs @@ -2,10 +2,13 @@ using Agoda.CodeCompass.MSBuild.Sarif; using Microsoft.Build.Framework; using Microsoft.VisualStudio.TestPlatform.Utilities; +using Newtonsoft.Json.Serialization; +using Newtonsoft.Json; using NSubstitute; using NUnit.Framework; using NUnit.Framework.Internal; using Shouldly; +using ErrorEventArgs = Newtonsoft.Json.Serialization.ErrorEventArgs; namespace Agoda.CodeCompass.MSBuild.Tests; @@ -15,6 +18,12 @@ public class SarifConversionTests private readonly string _writeSarifPath = "TestData/write.sarif"; private readonly string _sampleSarifPath = "TestData/sample.sarif"; private readonly IBuildEngine _buildEngine = Substitute.For(); + private JsonSerializerSettings _jsonSettings = new JsonSerializerSettings + { + ContractResolver = new CamelCasePropertyNamesContractResolver(), + Error = HandleDeserializationError + + }; [Test] public async Task ConvertSarif_WithValidInput_ShouldAddTechDebtProperties() @@ -34,8 +43,14 @@ public async Task ConvertSarif_WithValidInput_ShouldAddTechDebtProperties() // Assert result.ShouldBeTrue(); + var jsonSettings = new JsonSerializerSettings + { + ContractResolver = new CamelCasePropertyNamesContractResolver(), + Error = HandleDeserializationError + + }; var outputJson = await File.ReadAllTextAsync(outfile); - var output = JsonSerializer.Deserialize(outputJson, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); + var output = JsonConvert.DeserializeObject(outputJson, jsonSettings); output.ShouldNotBeNull(); output.Runs.ShouldNotBeEmpty(); @@ -49,21 +64,6 @@ public async Task ConvertSarif_WithValidInput_ShouldAddTechDebtProperties() firstResult.Properties.TechDebt.Priority.ShouldNotBeNullOrWhiteSpace(); } - [Test] - public void ConvertSarif_WithInvalidPath_ShouldReturnFalse() - { - var task = new TechDebtSarifTask - { - InputPath = "TestData/invalid.sarif", - OutputPath = Guid.NewGuid().ToString(), - BuildEngine = _buildEngine - }; - - var result = task.Execute(); - - result.ShouldBeFalse(); - } - [Test] public async Task ConvertSarif_WithV1FromTestData_ShouldHave1Violation() { @@ -80,14 +80,23 @@ public async Task ConvertSarif_WithV1FromTestData_ShouldHave1Violation() result.ShouldBeTrue(); var outputJson = await File.ReadAllTextAsync(outfile); - var output = JsonSerializer.Deserialize(outputJson, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); + var output = JsonConvert.DeserializeObject(outputJson, _jsonSettings); - output.Runs[0].Results.Count.ShouldBe(1); + output.Runs[0].Results.Length.ShouldBe(1); var results = output.Runs[0].Results; results[0].RuleId.ShouldBe("CA1707"); } + + private static void HandleDeserializationError(object sender, ErrorEventArgs errorArgs) + { + // Log the error but don't throw it + var currentError = errorArgs.ErrorContext.Error.Message; + Console.WriteLine($"Warning during SARIF processing: {currentError}"); + errorArgs.ErrorContext.Handled = true; + } + [Test] public async Task ConvertSarif_WithMultipleRules_ShouldPreserveRuleMetadata() { @@ -108,7 +117,7 @@ public async Task ConvertSarif_WithMultipleRules_ShouldPreserveRuleMetadata() }; await File.WriteAllTextAsync(_writeSarifPath, - JsonSerializer.Serialize(sarif, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase })); + JsonConvert.SerializeObject(sarif, _jsonSettings)); var outfile = "TestData/" + Guid.NewGuid(); var task = new TechDebtSarifTask { @@ -122,7 +131,7 @@ await File.WriteAllTextAsync(_writeSarifPath, // Assert var outputJson = await File.ReadAllTextAsync(outfile); - var output = JsonSerializer.Deserialize(outputJson, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); + var output = JsonConvert.DeserializeObject(outputJson, _jsonSettings); output.ShouldNotBeNull(); output.Runs[0].Results.Count.ShouldBe(2); diff --git a/src/Agoda.CodeCompass.MSBuild/Agoda.CodeCompass.MSBuild.csproj b/src/Agoda.CodeCompass.MSBuild/Agoda.CodeCompass.MSBuild.csproj index b865ea4..b05a6ca 100644 --- a/src/Agoda.CodeCompass.MSBuild/Agoda.CodeCompass.MSBuild.csproj +++ b/src/Agoda.CodeCompass.MSBuild/Agoda.CodeCompass.MSBuild.csproj @@ -24,6 +24,7 @@ + diff --git a/src/Agoda.CodeCompass.MSBuild/Sarif/V1Result.cs b/src/Agoda.CodeCompass.MSBuild/Sarif/V1Result.cs index 41030cd..ba19185 100644 --- a/src/Agoda.CodeCompass.MSBuild/Sarif/V1Result.cs +++ b/src/Agoda.CodeCompass.MSBuild/Sarif/V1Result.cs @@ -1,9 +1,13 @@ +using Newtonsoft.Json; + namespace Agoda.CodeCompass.MSBuild.Sarif; public class V1Result { public string RuleId { get; set; } = string.Empty; public string Level { get; set; } = string.Empty; + + [JsonProperty("message")] public string Message { get; set; } = string.Empty; public V1Location[] Locations { get; set; } = Array.Empty(); public TechDebtProperties Properties { get; set; } = new(); diff --git a/src/Agoda.CodeCompass.MSBuild/SarifReporter.cs b/src/Agoda.CodeCompass.MSBuild/SarifReporter.cs index d2ec33a..c7952a8 100644 --- a/src/Agoda.CodeCompass.MSBuild/SarifReporter.cs +++ b/src/Agoda.CodeCompass.MSBuild/SarifReporter.cs @@ -1,75 +1,82 @@ -using System.Text.Json; using Agoda.CodeCompass.Data; using Agoda.CodeCompass.MSBuild.Sarif; -using Microsoft.CodeAnalysis; -using Location = Agoda.CodeCompass.MSBuild.Sarif.Location; - -namespace Agoda.CodeCompass.MSBuild; +using Agoda.CodeCompass.MSBuild; +using Microsoft.Build.Framework; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using Newtonsoft.Json.Serialization; +using ErrorEventArgs = Newtonsoft.Json.Serialization.ErrorEventArgs; +using Formatting = System.Xml.Formatting; public class SarifReporter { - public static string GenerateSarifReport(IEnumerable diagnostics) + private static void HandleDeserializationError(object sender, ErrorEventArgs errorArgs) + { + // Log the error but don't throw it + var currentError = errorArgs.ErrorContext.Error.Message; + Console.WriteLine($"Warning during SARIF processing: {currentError}"); + errorArgs.ErrorContext.Handled = true; + } + public static string AddTechDebtToSarif(string sarifContent) + { + var jsonSettings = new JsonSerializerSettings + { + ContractResolver = new CamelCasePropertyNamesContractResolver(), + Error = HandleDeserializationError + + }; + + // Detect version + var jObject = JObject.Parse(sarifContent); + var version = jObject["version"]?.ToString(); + + return version switch + { + "1.0.0" => AddTechDebtToSarifV1(sarifContent, jsonSettings), + "2.1.0" => AddTechDebtToSarifV2(sarifContent, jsonSettings), + _ => throw new NotSupportedException($"Unsupported SARIF version: {version}") + }; + } + + private static string AddTechDebtToSarifV1(string sarifContent, JsonSerializerSettings jsonSettings) { - var report = new SarifReport + var report = JsonConvert.DeserializeObject(sarifContent, jsonSettings); + + // Add tech debt properties to results for V1 + foreach (var run in report.Runs) { - Runs = new[] + foreach (var result in run.Results) { - new Run - { - Tool = new Tool - { - Driver = new ToolDriver - { - Rules = diagnostics - .Select(d => d.Descriptor) - .Distinct() - .Select(d => new Rule - { - Id = d.Id, - Name = d.Title.ToString(), - ShortDescription = new Message { Text = d.Title.ToString() }, - FullDescription = new Message { Text = d.Description.ToString() }, - Help = new Message { Text = d.Description.ToString() }, - Properties = GetTechDebtProperties(d.Id) - }) - .ToArray() - } - }, - Results = diagnostics.Select(d => new Result - { - RuleId = d.Id, - Message = new Message { Text = d.GetMessage() }, - Locations = new[] - { - new Location - { - PhysicalLocation = new PhysicalLocation - { - ArtifactLocation = new ArtifactLocation - { - Uri = d.Location.GetLineSpan().Path - }, - Region = new Region - { - StartLine = d.Location.GetLineSpan().StartLinePosition.Line + 1, - StartColumn = d.Location.GetLineSpan().StartLinePosition.Character + 1, - EndLine = d.Location.GetLineSpan().EndLinePosition.Line + 1, - EndColumn = d.Location.GetLineSpan().EndLinePosition.Character + 1 - } - } - } - }, - Properties = GetTechDebtProperties(d.Id) - }).ToList() - } + result.Properties = GetTechDebtProperties(result.RuleId); } - }; + } + + return JsonConvert.SerializeObject(report, jsonSettings); + } - return JsonSerializer.Serialize(report, new JsonSerializerOptions + private static string AddTechDebtToSarifV2(string sarifContent, JsonSerializerSettings jsonSettings) + { + var report = JsonConvert.DeserializeObject(sarifContent, jsonSettings); + + // Add tech debt properties to rules + if (report.Runs?.FirstOrDefault()?.Tool?.Driver?.Rules != null) { - WriteIndented = true, - PropertyNamingPolicy = JsonNamingPolicy.CamelCase - }); + foreach (var rule in report.Runs[0].Tool.Driver.Rules) + { + rule.Properties = GetTechDebtProperties(rule.Id); + } + } + + // Add tech debt properties to results + foreach (var run in report.Runs) + { + foreach (var result in run.Results) + { + result.Properties = GetTechDebtProperties(result.RuleId); + } + } + + return JsonConvert.SerializeObject(report, jsonSettings); } private static TechDebtProperties GetTechDebtProperties(string ruleId) diff --git a/src/Agoda.CodeCompass.MSBuild/TechDebtReportTask.cs b/src/Agoda.CodeCompass.MSBuild/TechDebtReportTask.cs index 9ae7ed9..f05be41 100644 --- a/src/Agoda.CodeCompass.MSBuild/TechDebtReportTask.cs +++ b/src/Agoda.CodeCompass.MSBuild/TechDebtReportTask.cs @@ -25,9 +25,8 @@ public override bool Execute() try { var inputSarif = File.ReadAllText(InputPath); - var inputDiagnostics = ParseSarifDiagnostics(inputSarif); - var techDebtSarif = SarifReporter.GenerateSarifReport(inputDiagnostics); - File.WriteAllText(OutputPath, techDebtSarif); + var outputSarif = SarifReporter.AddTechDebtToSarif(inputSarif); + File.WriteAllText(OutputPath, outputSarif); return true; } catch (InvalidDataException iex) @@ -41,138 +40,5 @@ public override bool Execute() return false; } } - - private Diagnostic CreateDiagnosticFromSarifV2(Result result) - { - if (result.Locations.Length == 0) throw new InvalidDataException("Sarif input was wrong"); - - var lineSpan = result.Locations.FirstOrDefault()?.PhysicalLocation?.Region; - var linePosition = new LinePositionSpan( - new LinePosition( - (lineSpan?.StartLine ?? 1) - 1, - (lineSpan?.StartColumn ?? 1) - 1), - new LinePosition( - (lineSpan?.EndLine ?? 1) - 1, - (lineSpan?.EndColumn ?? 1) - 1) - ); - - var filePath = result.Locations.FirstOrDefault()?.PhysicalLocation?.ArtifactLocation?.Uri ?? ""; - var sourceText = SourceText.From(File.ReadAllText(filePath)); - var syntaxTree = CSharpSyntaxTree.ParseText(sourceText); - - var location = Microsoft.CodeAnalysis.Location.Create( - syntaxTree, - new TextSpan(0, 0)); - - var descriptor = new DiagnosticDescriptor( - id: result.RuleId, - title: result.Message.Text, - messageFormat: result.Message.Text, - category: result.Properties?.TechDebt?.Category ?? "Default", - defaultSeverity: DiagnosticSeverity.Warning, - isEnabledByDefault: true); - - var properties = ImmutableDictionary.CreateBuilder(); - if (result.Properties?.TechDebt != null) - { - properties.Add("techDebtMinutes", result.Properties.TechDebt.Minutes.ToString()); - properties.Add("techDebtCategory", result.Properties.TechDebt.Category); - properties.Add("techDebtPriority", result.Properties.TechDebt.Priority); - properties.Add("techDebtRationale", result.Properties.TechDebt.Rationale); - properties.Add("techDebtRecommendation", result.Properties.TechDebt.Recommendation); - } - - return Diagnostic.Create( - descriptor, - location, - properties.ToImmutable(), - result.Message.Text); - } - - public IEnumerable ParseSarifDiagnostics(string sarifContent) - { - // Detect SARIF version from the JSON - using var doc = JsonDocument.Parse(sarifContent); - var version = doc.RootElement.GetProperty("version").GetString(); - - return version switch - { - "1.0.0" => ParseSarifV1(sarifContent), - "2.1.0" => ParseSarifV2(sarifContent), - _ => throw new NotSupportedException($"Unsupported SARIF version: {version}") - }; - } - - private IEnumerable ParseSarifV1(string sarifContent) - { - var sarif = JsonSerializer.Deserialize(sarifContent, - new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); - return sarif.Runs.SelectMany(r => r.Results) - .Select(r => CreateDiagnosticFromSarifV1(r)); - } - - private IEnumerable ParseSarifV2(string sarifContent) - { - var sarif = JsonSerializer.Deserialize(sarifContent, - new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); - return sarif.Runs.SelectMany(r => r.Results) - .Select(r => CreateDiagnosticFromSarifV2(r)); - } - - private Diagnostic CreateDiagnosticFromSarifV1(V1Result result) - { - var lineSpan = result.Locations.FirstOrDefault()?.ResultFile?.Region; - var linePosition = new LinePositionSpan( - new LinePosition( - (lineSpan?.StartLine ?? 1) - 1, - (lineSpan?.StartColumn ?? 1) - 1), - new LinePosition( - (lineSpan?.EndLine ?? 1) - 1, - (lineSpan?.EndColumn ?? 1) - 1) - ); - - var filePath = result.Locations.FirstOrDefault()?.ResultFile?.Uri ?? ""; - var sourceText = SourceText.From(File.ReadAllText(filePath)); - var syntaxTree = CSharpSyntaxTree.ParseText(sourceText); - var location = Microsoft.CodeAnalysis.Location.Create( - syntaxTree, - new TextSpan(0, 0)); - - var descriptor = new DiagnosticDescriptor( - id: result.RuleId, - title: result.Message, - messageFormat: result.Message, - category: result.Properties?.TechDebt?.Category ?? "Default", - defaultSeverity: MapV1SeverityToDiagnosticSeverity(result.Level), - isEnabledByDefault: true); - - var properties = ImmutableDictionary.CreateBuilder(); - if (result.Properties?.TechDebt != null) - { - properties.Add("techDebtMinutes", result.Properties.TechDebt.Minutes.ToString()); - properties.Add("techDebtCategory", result.Properties.TechDebt.Category); - properties.Add("techDebtPriority", result.Properties.TechDebt.Priority); - properties.Add("techDebtRationale", result.Properties.TechDebt.Rationale); - properties.Add("techDebtRecommendation", result.Properties.TechDebt.Recommendation); - } - - return Diagnostic.Create( - descriptor, - location, - properties.ToImmutable(), - result.Message); - } - - private DiagnosticSeverity MapV1SeverityToDiagnosticSeverity(string level) - { - return level?.ToLowerInvariant() switch - { - "error" => DiagnosticSeverity.Error, - "warning" => DiagnosticSeverity.Warning, - "info" => DiagnosticSeverity.Info, - "note" => DiagnosticSeverity.Hidden, - _ => DiagnosticSeverity.Warning - }; - } }