From c2ceb4757f01a06d7dccd28b878410718e3354bc Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Fri, 7 Dec 2018 08:36:27 -0800 Subject: [PATCH 1/5] Emit column kind default explicitly for Windows SDK SARIF emit. --- .../v2/SpecExamples/Comprehensive.sarif | 1 + src/Sarif.UnitTests/Core/RunTests.cs | 51 +++ src/Sarif/Autogenerated/Run.cs | 4 + src/Sarif/CodeGenHints.json | 9 +- src/Sarif/Core/Run.cs | 18 + src/Sarif/NotYetAutoGenerated/Run.cs | 414 ++++++++++++++++++ src/Sarif/Readers/EnumConverter.cs | 1 - src/Sarif/Schemata/sarif-schema.json | 3 +- .../PrereleaseCompatibilityTransformer.cs | 10 + 9 files changed, 508 insertions(+), 3 deletions(-) create mode 100644 src/Sarif.UnitTests/Core/RunTests.cs create mode 100644 src/Sarif/NotYetAutoGenerated/Run.cs diff --git a/src/Sarif.FunctionalTests/v2/SpecExamples/Comprehensive.sarif b/src/Sarif.FunctionalTests/v2/SpecExamples/Comprehensive.sarif index a542c3628..851e27858 100644 --- a/src/Sarif.FunctionalTests/v2/SpecExamples/Comprehensive.sarif +++ b/src/Sarif.FunctionalTests/v2/SpecExamples/Comprehensive.sarif @@ -8,6 +8,7 @@ "baselineInstanceGuid": "0A106451-C9B1-4309-A7EE-06988B95F723", "automationLogicalId": "Build-14.0.1.2-Release-20160716-13:22:18", "architecture": "x86", + "columnKind": "utf16CodeUnits", "tool": { "name": "CodeScanner", "fullName": "CodeScanner 1.1 for Unix (en-US)", diff --git a/src/Sarif.UnitTests/Core/RunTests.cs b/src/Sarif.UnitTests/Core/RunTests.cs new file mode 100644 index 000000000..296c1a84c --- /dev/null +++ b/src/Sarif.UnitTests/Core/RunTests.cs @@ -0,0 +1,51 @@ +// Copyright (c) Microsoft. All Rights Reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using Xunit; +using Newtonsoft.Json; +using FluentAssertions; + +namespace Microsoft.CodeAnalysis.Sarif +{ + public class RunTests + { + [Fact] + public void Run_ColumnKindSerializesProperly() + { + // In our Windows-specific SDK, if no one has explicitly set ColumnKind, we + // will set it to the windows-specific value of Utf16CodeUnits. Otherwise, + // the SARIF file will pick up the ColumnKind default value of + // UnicodeCodePoints, which is not appropriate for Windows frameworks. + RoundTripColumnKind(persistedValue: ColumnKind.None, expectedRoundTrippedValue: ColumnKind.Utf16CodeUnits); + + // When explicitly set, we should always preserve that setting + RoundTripColumnKind(persistedValue: ColumnKind.Utf16CodeUnits, expectedRoundTrippedValue: ColumnKind.Utf16CodeUnits); + RoundTripColumnKind(persistedValue: ColumnKind.UnicodeCodePoints, expectedRoundTrippedValue: ColumnKind.UnicodeCodePoints); + + } + + private void RoundTripColumnKind(ColumnKind persistedValue, ColumnKind expectedRoundTrippedValue) + { + var sarifLog = new SarifLog + { + Runs = new[] + { + new Run + { + Tool = new Tool { Name = "Test tool"}, + ColumnKind = persistedValue + } + } + }; + + string json = JsonConvert.SerializeObject(sarifLog); + + // We should never see the default value persisted to JSON + json.Contains("unicodeCodePoints").Should().BeFalse(); + + sarifLog = JsonConvert.DeserializeObject(json); + sarifLog.Runs[0].ColumnKind.Should().Be(expectedRoundTrippedValue); + } + } +} diff --git a/src/Sarif/Autogenerated/Run.cs b/src/Sarif/Autogenerated/Run.cs index 0caaf7a62..95956ac65 100644 --- a/src/Sarif/Autogenerated/Run.cs +++ b/src/Sarif/Autogenerated/Run.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Runtime.Serialization; using Microsoft.CodeAnalysis.Sarif.Readers; +using Newtonsoft.Json; namespace Microsoft.CodeAnalysis.Sarif { @@ -138,6 +139,8 @@ public SarifNodeKind SarifNodeKind /// Specifies the unit in which the tool measures columns. /// [DataMember(Name = "columnKind", IsRequired = false, EmitDefaultValue = false)] + [JsonConverter(typeof(EnumConverter))] + [System.ComponentModel.DefaultValue(ColumnKind.UnicodeCodePoints)] public ColumnKind ColumnKind { get; set; } /// @@ -151,6 +154,7 @@ public SarifNodeKind SarifNodeKind /// public Run() { + this.ColumnKind = ColumnKind.UnicodeCodePoints; } /// diff --git a/src/Sarif/CodeGenHints.json b/src/Sarif/CodeGenHints.json index 2063c51a4..437848a5d 100644 --- a/src/Sarif/CodeGenHints.json +++ b/src/Sarif/CodeGenHints.json @@ -656,6 +656,14 @@ "unicodeCodePoints" ] } + }, + { + "kind": "AttributeHint", + "arguments": { + "namespaceName": "Newtonsoft.Json", + "typeName": "JsonConverter", + "arguments": [ "typeof(EnumConverter)" ] + } } ], "Run.Files": [ @@ -738,7 +746,6 @@ "arguments": [ "typeof(Microsoft.CodeAnalysis.Sarif.Readers.SarifVersionConverter)" ] } } - ], "stack": [ { diff --git a/src/Sarif/Core/Run.cs b/src/Sarif/Core/Run.cs index a3d7b94ba..5a8bd8ae6 100644 --- a/src/Sarif/Core/Run.cs +++ b/src/Sarif/Core/Run.cs @@ -13,6 +13,24 @@ public partial class Run private static Invocation EmptyInvocation = new Invocation(); private static LogicalLocation EmptyLogicalLocation = new LogicalLocation(); + public bool ShouldSerializeColumnKind() + { + // This serialization helper does two things. + // + // First, if ColumnKind has not been + // explicitly set, we will set it to the value that works for the Microsoft + // platform (which is not the specified SARIF default). This makes sure that + // the value is set appropriate for code running on the Microsoft platform, + // even if the SARIF producer is not aware of this rather obscure value. + if (this.ColumnKind == ColumnKind.None) + { + this.ColumnKind = ColumnKind.Utf16CodeUnits; + } + + // Second, we do not persist this value if it is set to its default. + return this.ColumnKind != ColumnKind.UnicodeCodePoints; + } + public bool ShouldSerializeFiles() { return this.Files != null && this.Files.Values.Any(); } public bool ShouldSerializeGraphs() { return this.Graphs != null && this.Graphs.Values.Any(); } diff --git a/src/Sarif/NotYetAutoGenerated/Run.cs b/src/Sarif/NotYetAutoGenerated/Run.cs new file mode 100644 index 000000000..95956ac65 --- /dev/null +++ b/src/Sarif/NotYetAutoGenerated/Run.cs @@ -0,0 +1,414 @@ +// Copyright (c) Microsoft. All Rights Reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.CodeDom.Compiler; +using System.Collections.Generic; +using System.Runtime.Serialization; +using Microsoft.CodeAnalysis.Sarif.Readers; +using Newtonsoft.Json; + +namespace Microsoft.CodeAnalysis.Sarif +{ + /// + /// Describes a single run of an analysis tool, and contains the output of that run. + /// + [DataContract] + [GeneratedCode("Microsoft.Json.Schema.ToDotNet", "0.58.0.0")] + public partial class Run : PropertyBagHolder, ISarifNode + { + public static IEqualityComparer ValueComparer => RunEqualityComparer.Instance; + + public bool ValueEquals(Run other) => ValueComparer.Equals(this, other); + public int ValueGetHashCode() => ValueComparer.GetHashCode(this); + + /// + /// Gets a value indicating the type of object implementing . + /// + public SarifNodeKind SarifNodeKind + { + get + { + return SarifNodeKind.Run; + } + } + + /// + /// Information about the tool or tool pipeline that generated the results in this run. A run can only contain results produced by a single tool or tool pipeline. A run can aggregate results from multiple log files, as long as context around the tool run (tool command-line arguments and the like) is identical for all aggregated files. + /// + [DataMember(Name = "tool", IsRequired = true)] + public Tool Tool { get; set; } + + /// + /// Describes the invocation of the analysis tool. + /// + [DataMember(Name = "invocations", IsRequired = false, EmitDefaultValue = false)] + public IList Invocations { get; set; } + + /// + /// A conversion object that describes how a converter transformed an analysis tool's native output format into the SARIF format. + /// + [DataMember(Name = "conversion", IsRequired = false, EmitDefaultValue = false)] + public Conversion Conversion { get; set; } + + /// + /// Specifies the revision in version control of the files that were scanned. + /// + [DataMember(Name = "versionControlProvenance", IsRequired = false, EmitDefaultValue = false)] + public IList VersionControlProvenance { get; set; } + + /// + /// The file location specified by each uriBaseId symbol on the machine where the tool originally ran. + /// + [DataMember(Name = "originalUriBaseIds", IsRequired = false, EmitDefaultValue = false)] + public IDictionary OriginalUriBaseIds { get; set; } + + /// + /// A dictionary, each of whose keys is a URI and each of whose values is a file object. + /// + [DataMember(Name = "files", IsRequired = false, EmitDefaultValue = false)] + public IDictionary Files { get; set; } + + /// + /// A dictionary, each of whose keys specifies a logical location such as a namespace, type or function. + /// + [DataMember(Name = "logicalLocations", IsRequired = false, EmitDefaultValue = false)] + public IDictionary LogicalLocations { get; set; } + + /// + /// A dictionary, each of whose keys is the id of a graph and each of whose values is a 'graph' object with that id. + /// + [DataMember(Name = "graphs", IsRequired = false, EmitDefaultValue = false)] + public IDictionary Graphs { get; set; } + + /// + /// The set of results contained in an SARIF log. The results array can be omitted when a run is solely exporting rules metadata. It must be present (but may be empty) if a log file represents an actual scan. + /// + [DataMember(Name = "results", IsRequired = false, EmitDefaultValue = false)] + public IList Results { get; set; } + + /// + /// Items that can be localized, such as message strings and rule metadata. + /// + [DataMember(Name = "resources", IsRequired = false, EmitDefaultValue = false)] + public Resources Resources { get; set; } + + /// + /// Automation details that describe this run. + /// + [DataMember(Name = "id", IsRequired = false, EmitDefaultValue = false)] + public RunAutomationDetails Id { get; set; } + + /// + /// Automation details that describe the aggregate of runs to which this run belongs. + /// + [DataMember(Name = "aggregateIds", IsRequired = false, EmitDefaultValue = false)] + public IList AggregateIds { get; set; } + + /// + /// The 'instanceGuid' property of a previous SARIF 'run' that comprises the baseline that was used to compute result 'baselineState' properties for the run. + /// + [DataMember(Name = "baselineInstanceGuid", IsRequired = false, EmitDefaultValue = false)] + public string BaselineInstanceGuid { get; set; } + + /// + /// The MIME type of all rich text message properties in the run. Default: "text/markdown;variant=GFM" + /// + [DataMember(Name = "richMessageMimeType", IsRequired = false, EmitDefaultValue = false)] + public string RichMessageMimeType { get; set; } + + /// + /// The string used to replace sensitive information in a redaction-aware property. + /// + [DataMember(Name = "redactionToken", IsRequired = false, EmitDefaultValue = false)] + public string RedactionToken { get; set; } + + /// + /// Specifies the default encoding for any file object that refers to a text file. + /// + [DataMember(Name = "defaultFileEncoding", IsRequired = false, EmitDefaultValue = false)] + public string DefaultFileEncoding { get; set; } + + /// + /// An ordered list of character sequences that were treated as line breaks when computing region information for the run. + /// + [DataMember(Name = "newlineSequences", IsRequired = false, EmitDefaultValue = false)] + public IList NewlineSequences { get; set; } + + /// + /// Specifies the unit in which the tool measures columns. + /// + [DataMember(Name = "columnKind", IsRequired = false, EmitDefaultValue = false)] + [JsonConverter(typeof(EnumConverter))] + [System.ComponentModel.DefaultValue(ColumnKind.UnicodeCodePoints)] + public ColumnKind ColumnKind { get; set; } + + /// + /// Key/value pairs that provide additional information about the run. + /// + [DataMember(Name = "properties", IsRequired = false, EmitDefaultValue = false)] + internal override IDictionary Properties { get; set; } + + /// + /// Initializes a new instance of the class. + /// + public Run() + { + this.ColumnKind = ColumnKind.UnicodeCodePoints; + } + + /// + /// Initializes a new instance of the class from the supplied values. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + /// + /// An initialization value for the property. + /// + public Run(Tool tool, IEnumerable invocations, Conversion conversion, IEnumerable versionControlProvenance, IDictionary originalUriBaseIds, IDictionary files, IDictionary logicalLocations, IDictionary graphs, IEnumerable results, Resources resources, RunAutomationDetails id, IEnumerable aggregateIds, string baselineInstanceGuid, string richMessageMimeType, string redactionToken, string defaultFileEncoding, IEnumerable newlineSequences, ColumnKind columnKind, IDictionary properties) + { + Init(tool, invocations, conversion, versionControlProvenance, originalUriBaseIds, files, logicalLocations, graphs, results, resources, id, aggregateIds, baselineInstanceGuid, richMessageMimeType, redactionToken, defaultFileEncoding, newlineSequences, columnKind, properties); + } + + /// + /// Initializes a new instance of the class from the specified instance. + /// + /// + /// The instance from which the new instance is to be initialized. + /// + /// + /// Thrown if is null. + /// + public Run(Run other) + { + if (other == null) + { + throw new ArgumentNullException(nameof(other)); + } + + Init(other.Tool, other.Invocations, other.Conversion, other.VersionControlProvenance, other.OriginalUriBaseIds, other.Files, other.LogicalLocations, other.Graphs, other.Results, other.Resources, other.Id, other.AggregateIds, other.BaselineInstanceGuid, other.RichMessageMimeType, other.RedactionToken, other.DefaultFileEncoding, other.NewlineSequences, other.ColumnKind, other.Properties); + } + + ISarifNode ISarifNode.DeepClone() + { + return DeepCloneCore(); + } + + /// + /// Creates a deep copy of this instance. + /// + public Run DeepClone() + { + return (Run)DeepCloneCore(); + } + + private ISarifNode DeepCloneCore() + { + return new Run(this); + } + + private void Init(Tool tool, IEnumerable invocations, Conversion conversion, IEnumerable versionControlProvenance, IDictionary originalUriBaseIds, IDictionary files, IDictionary logicalLocations, IDictionary graphs, IEnumerable results, Resources resources, RunAutomationDetails id, IEnumerable aggregateIds, string baselineInstanceGuid, string richMessageMimeType, string redactionToken, string defaultFileEncoding, IEnumerable newlineSequences, ColumnKind columnKind, IDictionary properties) + { + if (tool != null) + { + Tool = new Tool(tool); + } + + if (invocations != null) + { + var destination_0 = new List(); + foreach (var value_0 in invocations) + { + if (value_0 == null) + { + destination_0.Add(null); + } + else + { + destination_0.Add(new Invocation(value_0)); + } + } + + Invocations = destination_0; + } + + if (conversion != null) + { + Conversion = new Conversion(conversion); + } + + if (versionControlProvenance != null) + { + var destination_1 = new List(); + foreach (var value_1 in versionControlProvenance) + { + if (value_1 == null) + { + destination_1.Add(null); + } + else + { + destination_1.Add(new VersionControlDetails(value_1)); + } + } + + VersionControlProvenance = destination_1; + } + + if (originalUriBaseIds != null) + { + OriginalUriBaseIds = new Dictionary(); + foreach (var value_2 in originalUriBaseIds) + { + OriginalUriBaseIds.Add(value_2.Key, new FileLocation(value_2.Value)); + } + } + + if (files != null) + { + Files = new Dictionary(); + foreach (var value_3 in files) + { + Files.Add(value_3.Key, new FileData(value_3.Value)); + } + } + + if (logicalLocations != null) + { + LogicalLocations = new Dictionary(); + foreach (var value_4 in logicalLocations) + { + LogicalLocations.Add(value_4.Key, new LogicalLocation(value_4.Value)); + } + } + + if (graphs != null) + { + Graphs = new Dictionary(); + foreach (var value_5 in graphs) + { + Graphs.Add(value_5.Key, new Graph(value_5.Value)); + } + } + + if (results != null) + { + var destination_2 = new List(); + foreach (var value_6 in results) + { + if (value_6 == null) + { + destination_2.Add(null); + } + else + { + destination_2.Add(new Result(value_6)); + } + } + + Results = destination_2; + } + + if (resources != null) + { + Resources = new Resources(resources); + } + + if (id != null) + { + Id = new RunAutomationDetails(id); + } + + if (aggregateIds != null) + { + var destination_3 = new List(); + foreach (var value_7 in aggregateIds) + { + if (value_7 == null) + { + destination_3.Add(null); + } + else + { + destination_3.Add(new RunAutomationDetails(value_7)); + } + } + + AggregateIds = destination_3; + } + + BaselineInstanceGuid = baselineInstanceGuid; + RichMessageMimeType = richMessageMimeType; + RedactionToken = redactionToken; + DefaultFileEncoding = defaultFileEncoding; + if (newlineSequences != null) + { + var destination_4 = new List(); + foreach (var value_8 in newlineSequences) + { + destination_4.Add(value_8); + } + + NewlineSequences = destination_4; + } + + ColumnKind = columnKind; + if (properties != null) + { + Properties = new Dictionary(properties); + } + } + } +} \ No newline at end of file diff --git a/src/Sarif/Readers/EnumConverter.cs b/src/Sarif/Readers/EnumConverter.cs index cbe93c4f6..718eb6709 100644 --- a/src/Sarif/Readers/EnumConverter.cs +++ b/src/Sarif/Readers/EnumConverter.cs @@ -21,7 +21,6 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist { throw new ArgumentNullException(nameof(reader)); } - string value = (string)reader.Value; return Enum.Parse(objectType, ConvertToPascalCase(value)); } diff --git a/src/Sarif/Schemata/sarif-schema.json b/src/Sarif/Schemata/sarif-schema.json index 8e2e689f8..a837dd536 100644 --- a/src/Sarif/Schemata/sarif-schema.json +++ b/src/Sarif/Schemata/sarif-schema.json @@ -1585,7 +1585,8 @@ "columnKind": { "description": "Specifies the unit in which the tool measures columns.", - "enum": [ "utf16CodeUnits", "unicodeCodePoints" ] + "enum": [ "utf16CodeUnits", "unicodeCodePoints" ], + "default": "unicodeCodePoints" }, "properties": { diff --git a/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs b/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs index 877c77cdd..4f9216f4f 100644 --- a/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs +++ b/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs @@ -122,6 +122,16 @@ private static bool ApplyChangesFromTC25ThroughTC28(JObject sarifLog) JObject resources = (JObject)run["resources"]; modifiedLog |= RemapRuleDefaultLevelFromOpenToNote(resources); + + // Specify columnKind as ColumndKind.Utf16CodeUnits in cases where the enum is missing from + // the SARIF file. Moving forward, the absence of this enum will be interpreted as + // the new default, which is ColumnKind.UnicodeCodePoints. + // https://github.com/oasis-tcs/sarif-spec/issues/188 + JProperty columnKind = (JProperty)run["columnKind"]; + if (columnKind == null) + { + run["columnKind"] = "utf16CodeUnits"; + } } } From b5e17973b73c602f8cd2f5f81e4b384e891b7d1e Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Fri, 7 Dec 2018 10:03:11 -0800 Subject: [PATCH 2/5] Update release notes --- src/ReleaseHistory.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md index dfb1256af..1a4ff1683 100644 --- a/src/ReleaseHistory.md +++ b/src/ReleaseHistory.md @@ -226,4 +226,5 @@ * API BREAKING: remove 'run.architecture' https://github.com/oasis-tcs/sarif-spec/issues/262 * API BREAKING: 'result.message' is now a required property https://github.com/oasis-tcs/sarif-spec/issues/283 * API BREAKING: rename 'tool.fileVersion' to 'tool.dottedQuadFileVersion' https://github.com/oasis-tcs/sarif-spec/issues/274 -* API BREAKING: remove 'open' from valid rule default configuration levels. https://github.com/oasis-tcs/sarif-spec/issues/288. The transformer remaps this value to 'note'. \ No newline at end of file +* API BREAKING: remove 'open' from valid rule default configuration levels. https://github.com/oasis-tcs/sarif-spec/issues/288. The transformer remaps this value to 'note'. +* API BREAKING: 'run.columnKind' default value is now 'unicodeCodePoints'. https://github.com/Microsoft/sarif-sdk/pull/1160. The transformer will inject 'utf16CodeUnits', however, when this property is absent, as this value is a more appropriate default for the Windows platform. \ No newline at end of file From d0ea69d6ae91745c5414e6b8c03be24dbcfb0d5d Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Fri, 7 Dec 2018 13:45:10 -0800 Subject: [PATCH 3/5] More column kind test fixes --- .../v1/RestoreFromPropertyBag.sarif | 2 +- src/Sarif/Visitors/SarifVersionOneToCurrentVisitor.cs | 3 ++- src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Sarif.UnitTests/TestData/SarifVersionOneToCurrentVisitor/v1/RestoreFromPropertyBag.sarif b/src/Sarif.UnitTests/TestData/SarifVersionOneToCurrentVisitor/v1/RestoreFromPropertyBag.sarif index 9184ae0f3..9ffcac26e 100644 --- a/src/Sarif.UnitTests/TestData/SarifVersionOneToCurrentVisitor/v1/RestoreFromPropertyBag.sarif +++ b/src/Sarif.UnitTests/TestData/SarifVersionOneToCurrentVisitor/v1/RestoreFromPropertyBag.sarif @@ -9,7 +9,7 @@ }, "results": [], "properties": { - "sarifv2/run": {"tool":{"name":"CodeScanner","semanticVersion":"2.1.0"},"results":[]} + "sarifv2/run": {"tool":{"name":"CodeScanner","semanticVersion":"2.1.0"},"results":[],"columnKind":"utf16CodeUnits"} } } ] diff --git a/src/Sarif/Visitors/SarifVersionOneToCurrentVisitor.cs b/src/Sarif/Visitors/SarifVersionOneToCurrentVisitor.cs index 7db286186..22e576ae4 100644 --- a/src/Sarif/Visitors/SarifVersionOneToCurrentVisitor.cs +++ b/src/Sarif/Visitors/SarifVersionOneToCurrentVisitor.cs @@ -917,7 +917,8 @@ internal Run CreateRun(RunVersionOne v1Run) AggregateIds = aggregateIds, BaselineInstanceGuid = v1Run.BaselineId, Properties = v1Run.Properties, - Tool = CreateTool(v1Run.Tool) + Tool = CreateTool(v1Run.Tool), + ColumnKind = ColumnKind.Utf16CodeUnits }; _currentRun = run; diff --git a/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs b/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs index 4f9216f4f..c08ab6f74 100644 --- a/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs +++ b/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs @@ -127,7 +127,7 @@ private static bool ApplyChangesFromTC25ThroughTC28(JObject sarifLog) // the SARIF file. Moving forward, the absence of this enum will be interpreted as // the new default, which is ColumnKind.UnicodeCodePoints. // https://github.com/oasis-tcs/sarif-spec/issues/188 - JProperty columnKind = (JProperty)run["columnKind"]; + JValue columnKind = (JValue)run["columnKind"]; if (columnKind == null) { run["columnKind"] = "utf16CodeUnits"; From 3d50d89b981ab2564111fee64f31a4a8213af31f Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Fri, 7 Dec 2018 15:11:27 -0800 Subject: [PATCH 4/5] Change behavior to always serialize column kind. --- src/Sarif.FunctionalTests/UpdateBaselines.ps1 | 2 +- src/Sarif.UnitTests/Core/RunTests.cs | 4 ---- src/Sarif/Autogenerated/Run.cs | 1 - src/Sarif/Core/Run.cs | 7 ++++--- src/Sarif/NotYetAutoGenerated/Run.cs | 1 - src/Sarif/Writers/ResultLogJsonWriter.cs | 7 +++++++ 6 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/Sarif.FunctionalTests/UpdateBaselines.ps1 b/src/Sarif.FunctionalTests/UpdateBaselines.ps1 index 11a503abc..c90a4f496 100644 --- a/src/Sarif.FunctionalTests/UpdateBaselines.ps1 +++ b/src/Sarif.FunctionalTests/UpdateBaselines.ps1 @@ -56,10 +56,10 @@ function Build-Baselines($toolName) $toolDirectory = Join-Path "$PSScriptRoot\v2\ConverterTestData" $toolName $sourceExtension = "*.$sourceExtension" Get-ChildItem $toolDirectory -Filter $sourceExtension | ForEach-Object { - Write-Host " $_ -> $_.sarif" $input = $_.FullName $output = "$input.sarif" $outputTemp = "$output.temp" + Write-Host "$utility convert "$input" --tool $toolName --output "$outputTemp" --pretty-print" # Actually run the converter Remove-Item $outputTemp -ErrorAction SilentlyContinue diff --git a/src/Sarif.UnitTests/Core/RunTests.cs b/src/Sarif.UnitTests/Core/RunTests.cs index 296c1a84c..67c3f9b78 100644 --- a/src/Sarif.UnitTests/Core/RunTests.cs +++ b/src/Sarif.UnitTests/Core/RunTests.cs @@ -22,7 +22,6 @@ public void Run_ColumnKindSerializesProperly() // When explicitly set, we should always preserve that setting RoundTripColumnKind(persistedValue: ColumnKind.Utf16CodeUnits, expectedRoundTrippedValue: ColumnKind.Utf16CodeUnits); RoundTripColumnKind(persistedValue: ColumnKind.UnicodeCodePoints, expectedRoundTrippedValue: ColumnKind.UnicodeCodePoints); - } private void RoundTripColumnKind(ColumnKind persistedValue, ColumnKind expectedRoundTrippedValue) @@ -41,9 +40,6 @@ private void RoundTripColumnKind(ColumnKind persistedValue, ColumnKind expectedR string json = JsonConvert.SerializeObject(sarifLog); - // We should never see the default value persisted to JSON - json.Contains("unicodeCodePoints").Should().BeFalse(); - sarifLog = JsonConvert.DeserializeObject(json); sarifLog.Runs[0].ColumnKind.Should().Be(expectedRoundTrippedValue); } diff --git a/src/Sarif/Autogenerated/Run.cs b/src/Sarif/Autogenerated/Run.cs index 95956ac65..75773fc25 100644 --- a/src/Sarif/Autogenerated/Run.cs +++ b/src/Sarif/Autogenerated/Run.cs @@ -154,7 +154,6 @@ public SarifNodeKind SarifNodeKind /// public Run() { - this.ColumnKind = ColumnKind.UnicodeCodePoints; } /// diff --git a/src/Sarif/Core/Run.cs b/src/Sarif/Core/Run.cs index 5a8bd8ae6..0ac29664d 100644 --- a/src/Sarif/Core/Run.cs +++ b/src/Sarif/Core/Run.cs @@ -26,9 +26,10 @@ public bool ShouldSerializeColumnKind() { this.ColumnKind = ColumnKind.Utf16CodeUnits; } - - // Second, we do not persist this value if it is set to its default. - return this.ColumnKind != ColumnKind.UnicodeCodePoints; + + // Second, we will always explicitly serialize this value. Otherwise, we can't easily + // distinguish between earlier versions of the format for which this property was typically absent. + return true; } public bool ShouldSerializeFiles() { return this.Files != null && this.Files.Values.Any(); } diff --git a/src/Sarif/NotYetAutoGenerated/Run.cs b/src/Sarif/NotYetAutoGenerated/Run.cs index 95956ac65..75773fc25 100644 --- a/src/Sarif/NotYetAutoGenerated/Run.cs +++ b/src/Sarif/NotYetAutoGenerated/Run.cs @@ -154,7 +154,6 @@ public SarifNodeKind SarifNodeKind /// public Run() { - this.ColumnKind = ColumnKind.UnicodeCodePoints; } /// diff --git a/src/Sarif/Writers/ResultLogJsonWriter.cs b/src/Sarif/Writers/ResultLogJsonWriter.cs index 11346cc37..0af07a32d 100644 --- a/src/Sarif/Writers/ResultLogJsonWriter.cs +++ b/src/Sarif/Writers/ResultLogJsonWriter.cs @@ -63,6 +63,13 @@ public void Initialize(Run run) throw new ArgumentNullException(nameof(run.Tool)); } + // For this Windows-relevant SDK, if the column kind + // isn't explicitly specified, we will set it to Utf16CodeUnits + if (run.ColumnKind == ColumnKind.None) + { + run.ColumnKind = ColumnKind.Utf16CodeUnits; + } + this.EnsureStateNotAlreadySet(Conditions.Disposed | Conditions.RunInitialized); SarifVersion sarifVersion = SarifVersion.Current; From 83df6bdbbd465977342167beb87df29be6a83baf Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Fri, 7 Dec 2018 15:40:18 -0800 Subject: [PATCH 5/5] Always serialize column kind --- .../PropertyBagConverterTests.cs | 1 + src/Sarif/Autogenerated/Run.cs | 1 - src/Sarif/NotYetAutoGenerated/Run.cs | 413 ------------------ 3 files changed, 1 insertion(+), 414 deletions(-) delete mode 100644 src/Sarif/NotYetAutoGenerated/Run.cs diff --git a/src/Sarif.FunctionalTests/PropertyBagConverterTests.cs b/src/Sarif.FunctionalTests/PropertyBagConverterTests.cs index 54e4c5b8f..79350034f 100644 --- a/src/Sarif.FunctionalTests/PropertyBagConverterTests.cs +++ b/src/Sarif.FunctionalTests/PropertyBagConverterTests.cs @@ -23,6 +23,7 @@ public void PropertyBagConverter_RoundTripsStringPropertyWithEscapedCharacters() ""tool"": { ""name"": ""CodeScanner"" }, + ""columnKind"": ""utf16CodeUnits"", ""properties"": { ""int"": 42, ""string"": ""'\""\\'"" diff --git a/src/Sarif/Autogenerated/Run.cs b/src/Sarif/Autogenerated/Run.cs index 75773fc25..a712e9e96 100644 --- a/src/Sarif/Autogenerated/Run.cs +++ b/src/Sarif/Autogenerated/Run.cs @@ -140,7 +140,6 @@ public SarifNodeKind SarifNodeKind /// [DataMember(Name = "columnKind", IsRequired = false, EmitDefaultValue = false)] [JsonConverter(typeof(EnumConverter))] - [System.ComponentModel.DefaultValue(ColumnKind.UnicodeCodePoints)] public ColumnKind ColumnKind { get; set; } /// diff --git a/src/Sarif/NotYetAutoGenerated/Run.cs b/src/Sarif/NotYetAutoGenerated/Run.cs deleted file mode 100644 index 75773fc25..000000000 --- a/src/Sarif/NotYetAutoGenerated/Run.cs +++ /dev/null @@ -1,413 +0,0 @@ -// Copyright (c) Microsoft. All Rights Reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System; -using System.CodeDom.Compiler; -using System.Collections.Generic; -using System.Runtime.Serialization; -using Microsoft.CodeAnalysis.Sarif.Readers; -using Newtonsoft.Json; - -namespace Microsoft.CodeAnalysis.Sarif -{ - /// - /// Describes a single run of an analysis tool, and contains the output of that run. - /// - [DataContract] - [GeneratedCode("Microsoft.Json.Schema.ToDotNet", "0.58.0.0")] - public partial class Run : PropertyBagHolder, ISarifNode - { - public static IEqualityComparer ValueComparer => RunEqualityComparer.Instance; - - public bool ValueEquals(Run other) => ValueComparer.Equals(this, other); - public int ValueGetHashCode() => ValueComparer.GetHashCode(this); - - /// - /// Gets a value indicating the type of object implementing . - /// - public SarifNodeKind SarifNodeKind - { - get - { - return SarifNodeKind.Run; - } - } - - /// - /// Information about the tool or tool pipeline that generated the results in this run. A run can only contain results produced by a single tool or tool pipeline. A run can aggregate results from multiple log files, as long as context around the tool run (tool command-line arguments and the like) is identical for all aggregated files. - /// - [DataMember(Name = "tool", IsRequired = true)] - public Tool Tool { get; set; } - - /// - /// Describes the invocation of the analysis tool. - /// - [DataMember(Name = "invocations", IsRequired = false, EmitDefaultValue = false)] - public IList Invocations { get; set; } - - /// - /// A conversion object that describes how a converter transformed an analysis tool's native output format into the SARIF format. - /// - [DataMember(Name = "conversion", IsRequired = false, EmitDefaultValue = false)] - public Conversion Conversion { get; set; } - - /// - /// Specifies the revision in version control of the files that were scanned. - /// - [DataMember(Name = "versionControlProvenance", IsRequired = false, EmitDefaultValue = false)] - public IList VersionControlProvenance { get; set; } - - /// - /// The file location specified by each uriBaseId symbol on the machine where the tool originally ran. - /// - [DataMember(Name = "originalUriBaseIds", IsRequired = false, EmitDefaultValue = false)] - public IDictionary OriginalUriBaseIds { get; set; } - - /// - /// A dictionary, each of whose keys is a URI and each of whose values is a file object. - /// - [DataMember(Name = "files", IsRequired = false, EmitDefaultValue = false)] - public IDictionary Files { get; set; } - - /// - /// A dictionary, each of whose keys specifies a logical location such as a namespace, type or function. - /// - [DataMember(Name = "logicalLocations", IsRequired = false, EmitDefaultValue = false)] - public IDictionary LogicalLocations { get; set; } - - /// - /// A dictionary, each of whose keys is the id of a graph and each of whose values is a 'graph' object with that id. - /// - [DataMember(Name = "graphs", IsRequired = false, EmitDefaultValue = false)] - public IDictionary Graphs { get; set; } - - /// - /// The set of results contained in an SARIF log. The results array can be omitted when a run is solely exporting rules metadata. It must be present (but may be empty) if a log file represents an actual scan. - /// - [DataMember(Name = "results", IsRequired = false, EmitDefaultValue = false)] - public IList Results { get; set; } - - /// - /// Items that can be localized, such as message strings and rule metadata. - /// - [DataMember(Name = "resources", IsRequired = false, EmitDefaultValue = false)] - public Resources Resources { get; set; } - - /// - /// Automation details that describe this run. - /// - [DataMember(Name = "id", IsRequired = false, EmitDefaultValue = false)] - public RunAutomationDetails Id { get; set; } - - /// - /// Automation details that describe the aggregate of runs to which this run belongs. - /// - [DataMember(Name = "aggregateIds", IsRequired = false, EmitDefaultValue = false)] - public IList AggregateIds { get; set; } - - /// - /// The 'instanceGuid' property of a previous SARIF 'run' that comprises the baseline that was used to compute result 'baselineState' properties for the run. - /// - [DataMember(Name = "baselineInstanceGuid", IsRequired = false, EmitDefaultValue = false)] - public string BaselineInstanceGuid { get; set; } - - /// - /// The MIME type of all rich text message properties in the run. Default: "text/markdown;variant=GFM" - /// - [DataMember(Name = "richMessageMimeType", IsRequired = false, EmitDefaultValue = false)] - public string RichMessageMimeType { get; set; } - - /// - /// The string used to replace sensitive information in a redaction-aware property. - /// - [DataMember(Name = "redactionToken", IsRequired = false, EmitDefaultValue = false)] - public string RedactionToken { get; set; } - - /// - /// Specifies the default encoding for any file object that refers to a text file. - /// - [DataMember(Name = "defaultFileEncoding", IsRequired = false, EmitDefaultValue = false)] - public string DefaultFileEncoding { get; set; } - - /// - /// An ordered list of character sequences that were treated as line breaks when computing region information for the run. - /// - [DataMember(Name = "newlineSequences", IsRequired = false, EmitDefaultValue = false)] - public IList NewlineSequences { get; set; } - - /// - /// Specifies the unit in which the tool measures columns. - /// - [DataMember(Name = "columnKind", IsRequired = false, EmitDefaultValue = false)] - [JsonConverter(typeof(EnumConverter))] - [System.ComponentModel.DefaultValue(ColumnKind.UnicodeCodePoints)] - public ColumnKind ColumnKind { get; set; } - - /// - /// Key/value pairs that provide additional information about the run. - /// - [DataMember(Name = "properties", IsRequired = false, EmitDefaultValue = false)] - internal override IDictionary Properties { get; set; } - - /// - /// Initializes a new instance of the class. - /// - public Run() - { - } - - /// - /// Initializes a new instance of the class from the supplied values. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - /// - /// An initialization value for the property. - /// - public Run(Tool tool, IEnumerable invocations, Conversion conversion, IEnumerable versionControlProvenance, IDictionary originalUriBaseIds, IDictionary files, IDictionary logicalLocations, IDictionary graphs, IEnumerable results, Resources resources, RunAutomationDetails id, IEnumerable aggregateIds, string baselineInstanceGuid, string richMessageMimeType, string redactionToken, string defaultFileEncoding, IEnumerable newlineSequences, ColumnKind columnKind, IDictionary properties) - { - Init(tool, invocations, conversion, versionControlProvenance, originalUriBaseIds, files, logicalLocations, graphs, results, resources, id, aggregateIds, baselineInstanceGuid, richMessageMimeType, redactionToken, defaultFileEncoding, newlineSequences, columnKind, properties); - } - - /// - /// Initializes a new instance of the class from the specified instance. - /// - /// - /// The instance from which the new instance is to be initialized. - /// - /// - /// Thrown if is null. - /// - public Run(Run other) - { - if (other == null) - { - throw new ArgumentNullException(nameof(other)); - } - - Init(other.Tool, other.Invocations, other.Conversion, other.VersionControlProvenance, other.OriginalUriBaseIds, other.Files, other.LogicalLocations, other.Graphs, other.Results, other.Resources, other.Id, other.AggregateIds, other.BaselineInstanceGuid, other.RichMessageMimeType, other.RedactionToken, other.DefaultFileEncoding, other.NewlineSequences, other.ColumnKind, other.Properties); - } - - ISarifNode ISarifNode.DeepClone() - { - return DeepCloneCore(); - } - - /// - /// Creates a deep copy of this instance. - /// - public Run DeepClone() - { - return (Run)DeepCloneCore(); - } - - private ISarifNode DeepCloneCore() - { - return new Run(this); - } - - private void Init(Tool tool, IEnumerable invocations, Conversion conversion, IEnumerable versionControlProvenance, IDictionary originalUriBaseIds, IDictionary files, IDictionary logicalLocations, IDictionary graphs, IEnumerable results, Resources resources, RunAutomationDetails id, IEnumerable aggregateIds, string baselineInstanceGuid, string richMessageMimeType, string redactionToken, string defaultFileEncoding, IEnumerable newlineSequences, ColumnKind columnKind, IDictionary properties) - { - if (tool != null) - { - Tool = new Tool(tool); - } - - if (invocations != null) - { - var destination_0 = new List(); - foreach (var value_0 in invocations) - { - if (value_0 == null) - { - destination_0.Add(null); - } - else - { - destination_0.Add(new Invocation(value_0)); - } - } - - Invocations = destination_0; - } - - if (conversion != null) - { - Conversion = new Conversion(conversion); - } - - if (versionControlProvenance != null) - { - var destination_1 = new List(); - foreach (var value_1 in versionControlProvenance) - { - if (value_1 == null) - { - destination_1.Add(null); - } - else - { - destination_1.Add(new VersionControlDetails(value_1)); - } - } - - VersionControlProvenance = destination_1; - } - - if (originalUriBaseIds != null) - { - OriginalUriBaseIds = new Dictionary(); - foreach (var value_2 in originalUriBaseIds) - { - OriginalUriBaseIds.Add(value_2.Key, new FileLocation(value_2.Value)); - } - } - - if (files != null) - { - Files = new Dictionary(); - foreach (var value_3 in files) - { - Files.Add(value_3.Key, new FileData(value_3.Value)); - } - } - - if (logicalLocations != null) - { - LogicalLocations = new Dictionary(); - foreach (var value_4 in logicalLocations) - { - LogicalLocations.Add(value_4.Key, new LogicalLocation(value_4.Value)); - } - } - - if (graphs != null) - { - Graphs = new Dictionary(); - foreach (var value_5 in graphs) - { - Graphs.Add(value_5.Key, new Graph(value_5.Value)); - } - } - - if (results != null) - { - var destination_2 = new List(); - foreach (var value_6 in results) - { - if (value_6 == null) - { - destination_2.Add(null); - } - else - { - destination_2.Add(new Result(value_6)); - } - } - - Results = destination_2; - } - - if (resources != null) - { - Resources = new Resources(resources); - } - - if (id != null) - { - Id = new RunAutomationDetails(id); - } - - if (aggregateIds != null) - { - var destination_3 = new List(); - foreach (var value_7 in aggregateIds) - { - if (value_7 == null) - { - destination_3.Add(null); - } - else - { - destination_3.Add(new RunAutomationDetails(value_7)); - } - } - - AggregateIds = destination_3; - } - - BaselineInstanceGuid = baselineInstanceGuid; - RichMessageMimeType = richMessageMimeType; - RedactionToken = redactionToken; - DefaultFileEncoding = defaultFileEncoding; - if (newlineSequences != null) - { - var destination_4 = new List(); - foreach (var value_8 in newlineSequences) - { - destination_4.Add(value_8); - } - - NewlineSequences = destination_4; - } - - ColumnKind = columnKind; - if (properties != null) - { - Properties = new Dictionary(properties); - } - } - } -} \ No newline at end of file