Skip to content

Commit

Permalink
BUGFIX: Adjust Json Serialization field order for ReportingDescriptor…
Browse files Browse the repository at this point in the history 
… and skip emit empty AutomationDetails node (#2420)

* Fix empty AutomationDetails node

* set the order to the exact order before issue happened

* use DataMember.Order property to set json node order

* test remove SerializeIfNotNull(_run.RunAggregates, "runAggregates");

* trigger the same check when manual write json nodes one by one

* fix baseline according to the new Order

* update ReleaseHistory.md

* update change description

* check all other run properties are using the shouldSerialize pattern, add the same logic as this.
  • Loading branch information
@shaopeng-gh
shaopeng-gh authored Jan 10, 2022
1 parent 744f720 commit 4959f73
Show file tree
Hide file tree
Showing 12 changed files with 111 additions and 51 deletions.
1 change: 1 addition & 0 deletions src/ReleaseHistory.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

* BUGFIX: Adjust Json Serialization field order for ReportingDescriptor and skip emit empty AutomationDetails node. [#2420](/~https://github.com/microsoft/sarif-sdk/pull/2420)
* BREAKING: Fix `InvalidOperationException` when using PropertiesDictionary in a multithreaded application, and remove `[Serializable]` from it. Now use of BinaryFormatter on it will result in `SerializationException`: Type `PropertiesDictionary` is not marked as serializable. [#2415](/~https://github.com/microsoft/sarif-sdk/pull/2415)

## **v2.4.12** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.4.12) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.4.12) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.4.12) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.4.12) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.4.12)
Expand Down
2 changes: 0 additions & 2 deletions src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Runtime.InteropServices;
using System.Security;
using System.Threading.Channels;
using System.Threading.Tasks;

Expand Down
33 changes: 18 additions & 15 deletions src/Sarif/Autogenerated/ReportingDescriptor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,91 +37,94 @@ public SarifNodeKind SarifNodeKind

// NOTYETAUTOGENERATED: Jschema needs a mechanism to emit all public methods as virtual
// /~https://github.com/Microsoft/jschema/issues/97
// NOTYETAUTOGENERATED: Order attribute
// /~https://github.com/microsoft/jschema/issues/140
//
/// <summary>
/// A stable, opaque identifier for the report.
/// </summary>
[DataMember(Name = "id", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "id", IsRequired = false, EmitDefaultValue = false, Order = 1)]
public virtual string Id { get; set; }

/// <summary>
/// An array of stable, opaque identifiers by which this report was known in some previous version of the analysis tool.
/// </summary>
[DataMember(Name = "deprecatedIds", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "deprecatedIds", IsRequired = false, EmitDefaultValue = false, Order = 8)]
public virtual IList<string> DeprecatedIds { get; set; }

/// <summary>
/// A unique identifer for the reporting descriptor in the form of a GUID.
/// </summary>
[DataMember(Name = "guid", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "guid", IsRequired = false, EmitDefaultValue = false, Order = 9)]
public virtual string Guid { get; set; }

/// <summary>
/// An array of unique identifies in the form of a GUID by which this report was known in some previous version of the analysis tool.
/// </summary>
[DataMember(Name = "deprecatedGuids", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "deprecatedGuids", IsRequired = false, EmitDefaultValue = false, Order = 10)]
public virtual IList<string> DeprecatedGuids { get; set; }

/// <summary>
/// A report identifier that is understandable to an end user.
/// </summary>
[DataMember(Name = "name", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "name", IsRequired = false, EmitDefaultValue = false, Order = 7)]
public virtual string Name { get; set; }

/// <summary>
/// An array of readable identifiers by which this report was known in some previous version of the analysis tool.
/// </summary>
[DataMember(Name = "deprecatedNames", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "deprecatedNames", IsRequired = false, EmitDefaultValue = false, Order = 11)]
public virtual IList<string> DeprecatedNames { get; set; }

/// <summary>
/// A concise description of the report. Should be a single sentence that is understandable when visible space is limited to a single line of text.
/// </summary>
[DataMember(Name = "shortDescription", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "shortDescription", IsRequired = false, EmitDefaultValue = false, Order = 6)]
public virtual MultiformatMessageString ShortDescription { get; set; }

/// <summary>
/// A description of the report. Should, as far as possible, provide details sufficient to enable resolution of any problem indicated by the result.
/// </summary>
[DataMember(Name = "fullDescription", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "fullDescription", IsRequired = false, EmitDefaultValue = false, Order = 2)]
public virtual MultiformatMessageString FullDescription { get; set; }

/// <summary>
/// A set of name/value pairs with arbitrary names. Each value is a multiformatMessageString object, which holds message strings in plain text and (optionally) Markdown format. The strings can include placeholders, which can be used to construct a message in combination with an arbitrary number of additional string arguments.
/// </summary>
[DataMember(Name = "messageStrings", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "messageStrings", IsRequired = false, EmitDefaultValue = false, Order = 5)]
public virtual IDictionary<string, MultiformatMessageString> MessageStrings { get; set; }

/// <summary>
/// Default reporting configuration information.
/// </summary>
[DataMember(Name = "defaultConfiguration", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "defaultConfiguration", IsRequired = false, EmitDefaultValue = false, Order = 12)]
public virtual ReportingConfiguration DefaultConfiguration { get; set; }

/// <summary>
/// A URI where the primary documentation for the report can be found.
/// </summary>
[DataMember(Name = "helpUri", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "helpUri", IsRequired = false, EmitDefaultValue = false, Order = 3)]
[JsonConverter(typeof(Microsoft.CodeAnalysis.Sarif.Readers.UriConverter))]
public virtual Uri HelpUri { get; set; }

/// <summary>
/// Provides the primary documentation for the report, useful when there is no online documentation.
/// </summary>
[DataMember(Name = "help", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "help", IsRequired = false, EmitDefaultValue = false, Order = 4)]
public virtual MultiformatMessageString Help { get; set; }

/// <summary>
/// An array of objects that describe relationships between this reporting descriptor and others.
/// </summary>
[DataMember(Name = "relationships", IsRequired = false, EmitDefaultValue = false)]
[JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate)]
[DataMember(Name = "relationships", IsRequired = false, EmitDefaultValue = false, Order = 13)]
[JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate, Order = 13)]
public virtual IList<ReportingDescriptorRelationship> Relationships { get; set; }

/// <summary>
/// Key/value pairs that provide additional information about the report.
/// </summary>
[DataMember(Name = "properties", IsRequired = false, EmitDefaultValue = false)]
[JsonProperty(Order = 14)]
[DataMember(Name = "properties", IsRequired = false, EmitDefaultValue = false, Order = 14)]
internal override IDictionary<string, SerializedPropertyInfo> Properties { get; set; }

/// <summary>
Expand Down
33 changes: 18 additions & 15 deletions src/Sarif/NotYetAutoGenerated/ReportingDescriptor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,91 +37,94 @@ public SarifNodeKind SarifNodeKind

// NOTYETAUTOGENERATED: Jschema needs a mechanism to emit all public methods as virtual
// /~https://github.com/Microsoft/jschema/issues/97
// NOTYETAUTOGENERATED: Order attribute
// /~https://github.com/microsoft/jschema/issues/140
//
/// <summary>
/// A stable, opaque identifier for the report.
/// </summary>
[DataMember(Name = "id", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "id", IsRequired = false, EmitDefaultValue = false, Order = 1)]
public virtual string Id { get; set; }

/// <summary>
/// An array of stable, opaque identifiers by which this report was known in some previous version of the analysis tool.
/// </summary>
[DataMember(Name = "deprecatedIds", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "deprecatedIds", IsRequired = false, EmitDefaultValue = false, Order = 8)]
public virtual IList<string> DeprecatedIds { get; set; }

/// <summary>
/// A unique identifer for the reporting descriptor in the form of a GUID.
/// </summary>
[DataMember(Name = "guid", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "guid", IsRequired = false, EmitDefaultValue = false, Order = 9)]
public virtual string Guid { get; set; }

/// <summary>
/// An array of unique identifies in the form of a GUID by which this report was known in some previous version of the analysis tool.
/// </summary>
[DataMember(Name = "deprecatedGuids", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "deprecatedGuids", IsRequired = false, EmitDefaultValue = false, Order = 10)]
public virtual IList<string> DeprecatedGuids { get; set; }

/// <summary>
/// A report identifier that is understandable to an end user.
/// </summary>
[DataMember(Name = "name", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "name", IsRequired = false, EmitDefaultValue = false, Order = 7)]
public virtual string Name { get; set; }

/// <summary>
/// An array of readable identifiers by which this report was known in some previous version of the analysis tool.
/// </summary>
[DataMember(Name = "deprecatedNames", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "deprecatedNames", IsRequired = false, EmitDefaultValue = false, Order = 11)]
public virtual IList<string> DeprecatedNames { get; set; }

/// <summary>
/// A concise description of the report. Should be a single sentence that is understandable when visible space is limited to a single line of text.
/// </summary>
[DataMember(Name = "shortDescription", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "shortDescription", IsRequired = false, EmitDefaultValue = false, Order = 6)]
public virtual MultiformatMessageString ShortDescription { get; set; }

/// <summary>
/// A description of the report. Should, as far as possible, provide details sufficient to enable resolution of any problem indicated by the result.
/// </summary>
[DataMember(Name = "fullDescription", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "fullDescription", IsRequired = false, EmitDefaultValue = false, Order = 2)]
public virtual MultiformatMessageString FullDescription { get; set; }

/// <summary>
/// A set of name/value pairs with arbitrary names. Each value is a multiformatMessageString object, which holds message strings in plain text and (optionally) Markdown format. The strings can include placeholders, which can be used to construct a message in combination with an arbitrary number of additional string arguments.
/// </summary>
[DataMember(Name = "messageStrings", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "messageStrings", IsRequired = false, EmitDefaultValue = false, Order = 5)]
public virtual IDictionary<string, MultiformatMessageString> MessageStrings { get; set; }

/// <summary>
/// Default reporting configuration information.
/// </summary>
[DataMember(Name = "defaultConfiguration", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "defaultConfiguration", IsRequired = false, EmitDefaultValue = false, Order = 12)]
public virtual ReportingConfiguration DefaultConfiguration { get; set; }

/// <summary>
/// A URI where the primary documentation for the report can be found.
/// </summary>
[DataMember(Name = "helpUri", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "helpUri", IsRequired = false, EmitDefaultValue = false, Order = 3)]
[JsonConverter(typeof(Microsoft.CodeAnalysis.Sarif.Readers.UriConverter))]
public virtual Uri HelpUri { get; set; }

/// <summary>
/// Provides the primary documentation for the report, useful when there is no online documentation.
/// </summary>
[DataMember(Name = "help", IsRequired = false, EmitDefaultValue = false)]
[DataMember(Name = "help", IsRequired = false, EmitDefaultValue = false, Order = 4)]
public virtual MultiformatMessageString Help { get; set; }

/// <summary>
/// An array of objects that describe relationships between this reporting descriptor and others.
/// </summary>
[DataMember(Name = "relationships", IsRequired = false, EmitDefaultValue = false)]
[JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate)]
[DataMember(Name = "relationships", IsRequired = false, EmitDefaultValue = false, Order = 13)]
[JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate, Order = 13)]
public virtual IList<ReportingDescriptorRelationship> Relationships { get; set; }

/// <summary>
/// Key/value pairs that provide additional information about the report.
/// </summary>
[DataMember(Name = "properties", IsRequired = false, EmitDefaultValue = false)]
[JsonProperty(Order = 14)]
[DataMember(Name = "properties", IsRequired = false, EmitDefaultValue = false, Order = 14)]
internal override IDictionary<string, SerializedPropertyInfo> Properties { get; set; }

/// <summary>
Expand Down
29 changes: 21 additions & 8 deletions src/Sarif/Writers/ResultLogJsonWriter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ public void CompleteRun()
}

if ((_writeConditions & Conditions.InvocationsWritten) != Conditions.InvocationsWritten &&
_run.Invocations?.Count > 0)
_run.ShouldSerializeInvocations())
{
WriteInvocations(_run.Invocations);
}
Expand All @@ -307,29 +307,42 @@ public void CompleteRun()
SerializeIfNotNull(_run.OriginalUriBaseIds, "originalUriBaseIds");

if ((_writeConditions & Conditions.FilesWritten) != Conditions.FilesWritten &&
_run.Artifacts != null)
_run.ShouldSerializeArtifacts())
{
WriteArtifacts(_run.Artifacts);
}

if ((_writeConditions & Conditions.LogicalLocationsWritten) != Conditions.LogicalLocationsWritten &&
_run.LogicalLocations != null)
_run.ShouldSerializeLogicalLocations())
{
WriteLogicalLocations(_run.LogicalLocations);
}

SerializeIfNotNull(_run.Graphs, "graphs");
// All ShouldSerialize() will not be triggered automatically when manually write node by node.
// To make sure the same logic applied we should call the same method here, if it is defined.
if (_run.ShouldSerializeGraphs())
{
Serialize(_run.Graphs, "graphs");
}

// Results go here in schema order

SerializeIfNotNull(_run.AutomationDetails, "automationDetails");
if (_run.ShouldSerializeAutomationDetails())
{
Serialize(_run.AutomationDetails, "automationDetails");
}
SerializeIfNotNull(_run.RunAggregates, "runAggregates");
SerializeIfNotNull(_run.BaselineGuid, "baselineGuid");
SerializeIfNotNull(_run.RedactionTokens, "redactionTokens");
SerializeIfNotNull(_run.DefaultEncoding, "defaultEncoding");
SerializeIfNotNull(_run.DefaultSourceLanguage, "defaultSourceLanguage");
SerializeIfNotNull(_run.NewlineSequences, "newlineSequences");
SerializeIfNotNull(_run.ColumnKind == ColumnKind.UnicodeCodePoints ? "unicodeCodePoints" : "utf16CodeUnits", "columnKind");
if (_run.ShouldSerializeNewlineSequences())
{
Serialize(_run.NewlineSequences, "newlineSequences");
}
if (_run.ShouldSerializeColumnKind())
{
Serialize(_run.ColumnKind == ColumnKind.UnicodeCodePoints ? "unicodeCodePoints" : "utf16CodeUnits", "columnKind");
}
SerializeIfNotNull(_run.ExternalPropertyFileReferences, "externalPropertyFileReferences");
SerializeIfNotNull(_run.ThreadFlowLocations, "threadFlowLocations");
SerializeIfNotNull(_run.Taxonomies, "taxonomies");
Expand Down
2 changes: 1 addition & 1 deletion ...a/Multitool/ValidateCommand/ExpectedOutputs/SARIF2012.ProvideRuleProperties_Invalid.sarif
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
"index": 0
},
"region": {
"startLine": 21,
"startLine": 22,
"startColumn": 46
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...tool/ValidateCommand/ExpectedOutputs/SARIF2014.ProvideDynamicMessageContent_Invalid.sarif
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
"index": 0
},
"region": {
"startLine": 18,
"startLine": 19,
"startColumn": 74
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...tool/ValidateCommand/ExpectedOutputs/SARIF2015.EnquoteDynamicMessageContent_Invalid.sarif
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
"index": 0
},
"region": {
"startLine": 18,
"startLine": 19,
"startColumn": 91
}
}
Expand Down
4 changes: 2 additions & 2 deletions ...f/TestData/Multitool/ValidateCommand/Inputs/SARIF2012.ProvideRuleProperties_Invalid.sarif
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
},
{
"id": "TEST0002",
"name": "This isn't pascal case",
"helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html"
"helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html",
"name": "This isn't pascal case"
}
]
}
Expand Down
4 changes: 2 additions & 2 deletions ...ata/Multitool/ValidateCommand/Inputs/SARIF2014.ProvideDynamicMessageContent_Invalid.sarif
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
"fullDescription": {
"text": "This is a test."
},
"helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html",
"messageStrings": {
"NoPlaceholders": {
"text": "This message does not contain dynamic content."
}
},
"helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html"
}
}
]
}
Expand Down
4 changes: 2 additions & 2 deletions ...ata/Multitool/ValidateCommand/Inputs/SARIF2015.EnquoteDynamicMessageContent_Invalid.sarif
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
"fullDescription": {
"text": "This is a test."
},
"helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html",
"messageStrings": {
"NotEnquoted": {
"text": "This message contains dynamic content {0} that is not enquoted."
}
},
"helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html"
}
}
]
}
Expand Down
Loading

0 comments on commit 4959f73

Please sign in to comment.