Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorporate schema changes for versionControlDetails.mappedTo and rule.deprecatedIds #1198

Merged
merged 4 commits into from
Jan 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/ReleaseHistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@
* FEATURE: Fortify FPR converter improvements.
* API Non-BREAKING: Remove uniqueness requirement from 'result.locations'.
* API NON-BREAKING: Add 'run.newlineSequences' to schema. https://github.com/oasis-tcs/sarif-spec/issues/169
* API NON-BREAKING: Add 'rule.deprecatedIds' to schema. https://github.com/oasis-tcs/sarif-spec/issues/293
* API NON-BREAKING: Add 'versionControlDetails.mappedTo'. https://github.com/oasis-tcs/sarif-spec/issues/248
* API NON-BREAKING: Add result.rank'. Add 'ruleConfiguration.defaultRank'.
* 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
Expand Down
2 changes: 2 additions & 0 deletions src/Sarif.Driver.UnitTests/Sdk/TestSkimmerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ public abstract class TestSkimmerBase : SkimmerBase<TestAnalysisContext>

public override string Id => throw new NotImplementedException();

public override IList<string> DeprecatedIds => throw new NotImplementedException();

public override Message FullDescription => throw new NotImplementedException();

protected override ResourceManager ResourceManager => throw new NotImplementedException();
Expand Down
25 changes: 10 additions & 15 deletions src/Sarif.Driver.UnitTests/TestRuleBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Collections.Generic;
using Microsoft.CodeAnalysis.Sarif.Readers;

namespace Microsoft.CodeAnalysis.Sarif.Driver
{
Expand All @@ -22,22 +21,19 @@ public virtual SupportedPlatform SupportedPlatforms
public Uri HelpUri { get; set; }

public abstract string Id { get; }
public virtual IList<string> DeprecatedIds => null;
Copy link
Author

@ghost ghost Jan 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DeprecatedIds [](start = 37, length = 13)

This is the change. The rest is C#7 housekeeping. #ByDesign


public virtual ResultLevel DefaultLevel { get { return ResultLevel.Warning; } }
public virtual ResultLevel DefaultLevel => ResultLevel.Warning;

public virtual Message Name { get { return new Message { Text = this.GetType().Name }; } }
public virtual Message Name => new Message { Text = this.GetType().Name };

public virtual Message FullDescription { get { return new Message { Text = this.GetType().Name + " full description." }; } }
public virtual Message FullDescription => new Message { Text = this.GetType().Name + " full description." };

public virtual Message ShortDescription { get { return new Message { Text = this.GetType().Name + " short description." }; } }
public virtual Message ShortDescription => new Message { Text = this.GetType().Name + " short description." };

public IDictionary<string, string> MessageFormats
{
get
{
return new Dictionary<string, string> { { nameof(SdkResources.NotApplicable_InvalidMetadata), SdkResources.NotApplicable_InvalidMetadata } };
}
}
=> new Dictionary<string, string> { { nameof(SdkResources.NotApplicable_InvalidMetadata), SdkResources.NotApplicable_InvalidMetadata } };


internal override IDictionary<string, SerializedPropertyInfo> Properties { get; set; }

Expand All @@ -54,11 +50,11 @@ public RuleConfiguration Configuration
}
}

public IDictionary<string, string> MessageStrings { get { return new Dictionary<string, string>(); } }
public IDictionary<string, string> MessageStrings => new Dictionary<string, string>();

public IDictionary<string, string> RichMessageStrings { get { return new Dictionary<string, string>(); } }
public IDictionary<string, string> RichMessageStrings => new Dictionary<string, string>();

public Message Help { get { return new Message() { Text = "[Empty]" }; } }
public Message Help => new Message() { Text = "[Empty]" };

public abstract void Analyze(TestAnalysisContext context);

Expand All @@ -70,7 +66,6 @@ public virtual AnalysisApplicability CanAnalyze(TestAnalysisContext context, out

public virtual void Initialize(TestAnalysisContext context)
{

}
}
}
3 changes: 3 additions & 0 deletions src/Sarif.Driver/Sdk/SkimmerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ private Dictionary<string, string> InitializeRichMessageStrings()

abstract public string Id { get; }

// This one isn't abstract because there's no point in forcing every skimmer to implement it.
public virtual IList<string> DeprecatedIds => null;

abstract public Message FullDescription { get; }

public virtual Message ShortDescription
Expand Down
18 changes: 9 additions & 9 deletions src/Sarif.FunctionalTests/v2/SpecExamples/Comprehensive.sarif
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@
"branch": "master",
"tag": "beta1",
"revisionId": "abcdabcdabcdabcdabcdabcdabcdabcdabcdabcd",
"timestamp": "2016-07-16T00:00:00Z"
"timestamp": "2016-07-16T00:00:00Z",
"mappedTo": {
"uri": "/example",
"uriBaseId": "SRCROOT"
}
}
],
"originalUriBaseIds": { "TOOLS_ROOT": "file:///bin/tools/" },
Expand Down Expand Up @@ -401,14 +405,6 @@
]
}
],
"provenance": {
"firstDetectionTimeUtc": "2018-07-15T14:20:42Z",
"firstDetectionRunInstanceGuid": "8F62D8A0-C14F-4516-9959-1A663BA6FB99",
"lastDetectionTimeUtc": "2018-07-16T14:20:42Z",
"lastDetectionRunInstanceGuid": "BC650830-A9FE-44CB-8818-AD6C387279A0",
"invocationIndex": 0,
"conversionSources": []
},
"fixes": [
{
"description": {
Expand Down Expand Up @@ -439,6 +435,10 @@
"rules": {
"C2001": {
"id": "C2001",
"deprecatedIds": [
"C0001",
"C1001"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the only change you should retain in this file.

],
"shortDescription": {
"text": "A variable was used without being initialized."
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,16 @@
]
}
],
"versionControlProvenance": [
{
"repositoryUri": "https://github.com/example-corp/browser",
"revisionId": "de67ef7",
"mappedTo": {
"uri": "file:///C:/src/file.c",
"uriBaseId": "%SRCROOT%"
}
}
],
"properties": {
"expectedResults": {
"resultLocationPointers": [
Expand All @@ -266,7 +276,8 @@
"runs[0].results[0].relatedLocations[0].physicalLocation.fileLocation.uri",
"runs[0].results[0].codeFlows[0].threadFlows[0].locations[0].location.physicalLocation.fileLocation.uri",
"runs[0].results[0].stacks[0].frames[0].location.physicalLocation.fileLocation.uri",
"runs[0].results[0].fixes[0].fileChanges[0].fileLocation.uri"
"runs[0].results[0].fixes[0].fileChanges[0].fileLocation.uri",
"runs[0].versionControlProvenance[0].mappedTo.uri"
]
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,16 @@
]
}
],
"versionControlProvenance": [
{
"repositoryUri": "https://github.com/example-corp/browser",
"revisionId": "de67ef7",
"mappedTo": {
"uri": "/browser",
"uriBaseId": "%SRCROOT%"
}
}
],
"properties": {
"expectedResults": {
"resultLocationPointers": []
Expand Down
5 changes: 5 additions & 0 deletions src/Sarif.Multitool/Rules/SarifValidationSkimmerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,11 @@ private void Visit(Tool tool, string toolPointer)
private void Visit(VersionControlDetails versionControlDetails, string versionControlDetailsPointer)
{
Analyze(versionControlDetails, versionControlDetailsPointer);

if (versionControlDetails.MappedTo != null)
{
Visit(versionControlDetails.MappedTo, versionControlDetailsPointer.AtProperty(SarifPropertyName.MappedTo));
}
}

private Region GetRegionFromJPointer(string jPointer)
Expand Down
1 change: 1 addition & 0 deletions src/Sarif.Multitool/SarifPropertyName.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public static class SarifPropertyName
public const string Location = "location";
public const string Locations = "locations";
public const string LogicalLocations = "logicalLocations";
public const string MappedTo = "mappedTo";
public const string Message = "message";
public const string MessageStrings = "messageStrings";
public const string Nodes = "nodes";
Expand Down
4 changes: 2 additions & 2 deletions src/Sarif.UnitTests/Visitors/RebaseUriVisitorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public void RebaseUriVisitor_VisitFileData_RebasesAllTheThings()
visitor.FileDataKeys.Where(k => k != null && k.StartsWith(uriRootText)).Count().Should().Be(3);
visitor.FileDataKeys.Where(k => k != null && k.StartsWith("#" + toolsRootBaseId + "#")).Count().Should().Be(1);

int uriCount = 16;
int uriCount = 17;
Copy link
Author

@ghost ghost Jan 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

17 [](start = 27, length = 2)

Because we fixed that bug in the comprehensive sample that had the effect of adding another fileLocation object with a uri.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand. The prerelease transformer should have created file locations from everything in the comprehensive sarif file. Are you masking a bug here because you brought the comprehensive sarif up to date in this regard?


In reply to: 244756655 [](ancestors = 244756655)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the additional file location is the one that you introduced in the 'mappedTo' property. the transformer is doing the right thing.


In reply to: 244871107 [](ancestors = 244871107,244756655)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see.


In reply to: 244874264 [](ancestors = 244874264,244871107,244756655)


visitor.FileLocationUriBaseIds.Count.Should().Be(uriCount);
visitor.FileLocationUriBaseIds.Where(u => u == null).Count().Should().Be(13);
Expand Down Expand Up @@ -240,7 +240,7 @@ public void RebaseUriVisitor_VisitFileData_RebasesAllTheThings()
visitor.FileLocationUriBaseIds.Where(u => u == toolsRootBaseId).Count().Should().Be(3);
visitor.FileLocationUriBaseIds.Where(u => u == agentRootBaseId).Count().Should().Be(11);

visitor.FileLocationUris.Count.Should().Be(17);
visitor.FileLocationUris.Count.Should().Be(uriCount + 1);

// The AGENT_ROOT originalUriBaseId is the last thing that will include the uriRootText value
visitor.FileLocationUris.Where(u => u != null && u.StartsWith(uriRootText)).Count().Should().Be(1);
Expand Down
5 changes: 5 additions & 0 deletions src/Sarif/Autogenerated/IRule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ public partial interface IRule
/// </summary>
string Id { get; }

/// <summary>
/// An array of stable, opaque identifiers by which this rule was known in some previous version of the analysis tool.
/// </summary>
IList<string> DeprecatedIds { get; }

/// <summary>
/// A rule identifier that is understandable to an end user.
/// </summary>
Expand Down
28 changes: 24 additions & 4 deletions src/Sarif/Autogenerated/Rule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ public SarifNodeKind SarifNodeKind
[DataMember(Name = "id", IsRequired = false, EmitDefaultValue = false)]
public string Id { get; set; }

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

/// <summary>
/// A rule identifier that is understandable to an end user.
/// </summary>
Expand Down Expand Up @@ -107,6 +113,9 @@ public Rule()
/// <param name="id">
/// An initialization value for the <see cref="P: Id" /> property.
/// </param>
/// <param name="deprecatedIds">
/// An initialization value for the <see cref="P: DeprecatedIds" /> property.
/// </param>
/// <param name="name">
/// An initialization value for the <see cref="P: Name" /> property.
/// </param>
Expand Down Expand Up @@ -134,9 +143,9 @@ public Rule()
/// <param name="properties">
/// An initialization value for the <see cref="P: Properties" /> property.
/// </param>
public Rule(string id, Message name, Message shortDescription, Message fullDescription, IDictionary<string, string> messageStrings, IDictionary<string, string> richMessageStrings, RuleConfiguration configuration, Uri helpUri, Message help, IDictionary<string, SerializedPropertyInfo> properties)
public Rule(string id, IEnumerable<string> deprecatedIds, Message name, Message shortDescription, Message fullDescription, IDictionary<string, string> messageStrings, IDictionary<string, string> richMessageStrings, RuleConfiguration configuration, Uri helpUri, Message help, IDictionary<string, SerializedPropertyInfo> properties)
{
Init(id, name, shortDescription, fullDescription, messageStrings, richMessageStrings, configuration, helpUri, help, properties);
Init(id, deprecatedIds, name, shortDescription, fullDescription, messageStrings, richMessageStrings, configuration, helpUri, help, properties);
}

/// <summary>
Expand All @@ -155,7 +164,7 @@ public Rule(Rule other)
throw new ArgumentNullException(nameof(other));
}

Init(other.Id, other.Name, other.ShortDescription, other.FullDescription, other.MessageStrings, other.RichMessageStrings, other.Configuration, other.HelpUri, other.Help, other.Properties);
Init(other.Id, other.DeprecatedIds, other.Name, other.ShortDescription, other.FullDescription, other.MessageStrings, other.RichMessageStrings, other.Configuration, other.HelpUri, other.Help, other.Properties);
}

ISarifNode ISarifNode.DeepClone()
Expand All @@ -176,9 +185,20 @@ private ISarifNode DeepCloneCore()
return new Rule(this);
}

private void Init(string id, Message name, Message shortDescription, Message fullDescription, IDictionary<string, string> messageStrings, IDictionary<string, string> richMessageStrings, RuleConfiguration configuration, Uri helpUri, Message help, IDictionary<string, SerializedPropertyInfo> properties)
private void Init(string id, IEnumerable<string> deprecatedIds, Message name, Message shortDescription, Message fullDescription, IDictionary<string, string> messageStrings, IDictionary<string, string> richMessageStrings, RuleConfiguration configuration, Uri helpUri, Message help, IDictionary<string, SerializedPropertyInfo> properties)
{
Id = id;
if (deprecatedIds != null)
{
var destination_0 = new List<string>();
foreach (var value_0 in deprecatedIds)
{
destination_0.Add(value_0);
}

DeprecatedIds = destination_0;
}

if (name != null)
{
Name = new Message(name);
Expand Down
57 changes: 45 additions & 12 deletions src/Sarif/Autogenerated/RuleEqualityComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,27 @@ public bool Equals(Rule left, Rule right)
return false;
}

if (!object.ReferenceEquals(left.DeprecatedIds, right.DeprecatedIds))
{
if (left.DeprecatedIds == null || right.DeprecatedIds == null)
{
return false;
}

if (left.DeprecatedIds.Count != right.DeprecatedIds.Count)
{
return false;
}

for (int index_0 = 0; index_0 < left.DeprecatedIds.Count; ++index_0)
{
if (left.DeprecatedIds[index_0] != right.DeprecatedIds[index_0])
{
return false;
}
}
}

if (!Message.ValueComparer.Equals(left.Name, right.Name))
{
return false;
Expand Down Expand Up @@ -147,6 +168,18 @@ public int GetHashCode(Rule obj)
result = (result * 31) + obj.Id.GetHashCode();
}

if (obj.DeprecatedIds != null)
{
foreach (var value_6 in obj.DeprecatedIds)
{
result = result * 31;
if (value_6 != null)
{
result = (result * 31) + value_6.GetHashCode();
}
}
}

if (obj.Name != null)
{
result = (result * 31) + obj.Name.ValueGetHashCode();
Expand All @@ -166,12 +199,12 @@ public int GetHashCode(Rule obj)
{
// Use xor for dictionaries to be order-independent.
int xor_0 = 0;
foreach (var value_6 in obj.MessageStrings)
foreach (var value_7 in obj.MessageStrings)
{
xor_0 ^= value_6.Key.GetHashCode();
if (value_6.Value != null)
xor_0 ^= value_7.Key.GetHashCode();
if (value_7.Value != null)
{
xor_0 ^= value_6.Value.GetHashCode();
xor_0 ^= value_7.Value.GetHashCode();
}
}

Expand All @@ -182,12 +215,12 @@ public int GetHashCode(Rule obj)
{
// Use xor for dictionaries to be order-independent.
int xor_1 = 0;
foreach (var value_7 in obj.RichMessageStrings)
foreach (var value_8 in obj.RichMessageStrings)
{
xor_1 ^= value_7.Key.GetHashCode();
if (value_7.Value != null)
xor_1 ^= value_8.Key.GetHashCode();
if (value_8.Value != null)
{
xor_1 ^= value_7.Value.GetHashCode();
xor_1 ^= value_8.Value.GetHashCode();
}
}

Expand All @@ -213,12 +246,12 @@ public int GetHashCode(Rule obj)
{
// Use xor for dictionaries to be order-independent.
int xor_2 = 0;
foreach (var value_8 in obj.Properties)
foreach (var value_9 in obj.Properties)
{
xor_2 ^= value_8.Key.GetHashCode();
if (value_8.Value != null)
xor_2 ^= value_9.Key.GetHashCode();
if (value_9.Value != null)
{
xor_2 ^= value_8.Value.GetHashCode();
xor_2 ^= value_9.Value.GetHashCode();
}
}

Expand Down
1 change: 1 addition & 0 deletions src/Sarif/Autogenerated/SarifRewritingVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,7 @@ public virtual VersionControlDetails VisitVersionControlDetails(VersionControlDe
{
if (node != null)
{
node.MappedTo = VisitNullChecked(node.MappedTo);
Copy link
Author

@ghost ghost Jan 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MappedTo [](start = 21, length = 8)

Note that rule.deprecatedIds does not induce a change in the visitor because it's an array of a primitive type. #ByDesign

}

return node;
Expand Down
Loading