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

Eliminate need to specify the SARIF contract resolver. All serializat… #1103

Merged
merged 8 commits into from
Sep 28, 2018
9 changes: 2 additions & 7 deletions src/Sarif.Converters.UnitTests/ConverterTestsBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,8 @@ public SarifLog RunTestCase(string inputData, string expectedResult, bool pretty
// provided an ability to apply additional attributes to emitted code,
// we wouldn't be required to do this. Filed an issue on this.
// https://github.com/Microsoft/jschema/issues/6

var settings = new JsonSerializerSettings()
{
ContractResolver = SarifContractResolver.Instance
};

var log = JsonConvert.DeserializeObject<SarifLog>(actualJson, settings);

var log = JsonConvert.DeserializeObject<SarifLog>(actualJson);

// Pretty-printed JSON literals can consume significant space in test code,
// so we'll provide an option to collapse into minimal form
Expand Down
2 changes: 1 addition & 1 deletion src/Sarif.Converters/FxCopConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public override void Convert(Stream input, IResultLogWriter output, OptionallyEm

if (rules.Count > 0)
{
var rulesDictionary = new Dictionary<string, IRule>();
IDictionary<string, IRule> rulesDictionary = new Dictionary<string, IRule>();

foreach (Rule rule in rules)
{
Expand Down
7 changes: 1 addition & 6 deletions src/Sarif.Driver.UnitTests/Sdk/AnalyzeCommandBaseTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -517,12 +517,7 @@ public void AnalyzeCommand_ReportsWarningOnUnsupportedPlatformForRuleAndNoRulesL

command.RuntimeErrors.Should().Be(runtimeConditions);

JsonSerializerSettings settings = new JsonSerializerSettings()
{
ContractResolver = SarifContractResolver.Instance
};

SarifLog log = JsonConvert.DeserializeObject<SarifLog>(File.ReadAllText(path), settings);
SarifLog log = JsonConvert.DeserializeObject<SarifLog>(File.ReadAllText(path));
Assert.NotNull(log);
Assert.Equal<int>(1, log.Runs.Count);

Expand Down
4 changes: 1 addition & 3 deletions src/Sarif.Driver.UnitTests/TestRuleBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,11 @@ public RuleConfiguration Configuration
}
}

public string RichDescription => throw new NotImplementedException();

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

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

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

public abstract void Analyze(TestAnalysisContext context);

Expand Down
18 changes: 7 additions & 11 deletions src/Sarif.Driver/FileHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,24 @@ namespace Microsoft.CodeAnalysis.Sarif.Driver
/// </summary>
static class FileHelpers
{
public static T ReadSarifFile<T>(string filePath)
public static T ReadSarifFile<T>(string filePath, IContractResolver contractResolver = null)
{
return ReadSarifFile<T>(filePath, SarifContractResolver.Instance);
}
string logText = File.ReadAllText(filePath);

public static T ReadSarifFile<T>(string filePath, IContractResolver contractResolver)
{
JsonSerializerSettings settings = new JsonSerializerSettings()
var settings = new JsonSerializerSettings
{
ContractResolver = contractResolver
ContractResolver = contractResolver,
};

string logText = File.ReadAllText(filePath);

return JsonConvert.DeserializeObject<T>(logText, settings);
return JsonConvert.DeserializeObject<T>(logText);
}

public static void WriteSarifFile<T>(T sarifFile, string outputName, Formatting formatting)
public static void WriteSarifFile<T>(T sarifFile, string outputName, Formatting formatting = Formatting.None, IContractResolver contractResolver = null)
{
var settings = new JsonSerializerSettings
{
ContractResolver = SarifContractResolver.Instance,
ContractResolver = contractResolver,
Formatting = formatting
};

Expand Down
1 change: 0 additions & 1 deletion src/Sarif.Driver/Sdk/ExportRulesMetadataCommandBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ private void OutputSarifRulesMetada(string outputFilePath, ImmutableArray<IRule>

var settings = new JsonSerializerSettings()
{
ContractResolver = SarifContractResolver.Instance,
Formatting = Newtonsoft.Json.Formatting.Indented,
};

Expand Down
11 changes: 3 additions & 8 deletions src/Sarif.FunctionalTests/PropertyBagConverterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,16 @@ public void PropertyBagConverter_RoundTripsStringPropertyWithEscapedCharacters()
}
]
}";
var serializerSettings = new JsonSerializerSettings
{
ContractResolver = SarifContractResolver.Instance,
Formatting = Formatting.Indented
};

SarifLog deserializedLog = JsonConvert.DeserializeObject<SarifLog>(originalLog, serializerSettings);
var settings = new JsonSerializerSettings { Formatting = Formatting.Indented };
Copy link

@ghost ghost Sep 28, 2018

Choose a reason for hiding this comment

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

settings [](start = 16, length = 8)

BUG: You created the settings object but then didn't use it. #Resolved

SarifLog deserializedLog = JsonConvert.DeserializeObject<SarifLog>(originalLog);
Run run = deserializedLog.Runs[0];

int integerProperty = run.GetProperty<int>("int");
integerProperty.Should().Be(42);
string stringProperty = run.GetProperty<string>("string");
stringProperty.Should().Be("'\"\\'");

string reserializedLog = JsonConvert.SerializeObject(deserializedLog, serializerSettings);
string reserializedLog = JsonConvert.SerializeObject(deserializedLog, settings);

reserializedLog.Should().Be(originalLog);
}
Expand Down
1 change: 0 additions & 1 deletion src/Sarif.FunctionalTests/SarifConverterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ private static void EnrichSarifLog(string actualFilePath)
JsonSerializerSettings settings = new JsonSerializerSettings()
{
Formatting = Formatting.Indented,
ContractResolver = SarifContractResolver.Instance
};

string logText = File.ReadAllText(actualFilePath);
Expand Down
11 changes: 3 additions & 8 deletions src/Sarif.FunctionalTests/SarifLogEqualityComparerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,10 @@ public void ValueEquals_ReturnsTrueForTwoIdenticalLogObjects()
const string ComprehensiveTestSamplePath = @"v2\SpecExamples\Comprehensive.sarif";
string comprehensiveTestSampleContents = File.ReadAllText(ComprehensiveTestSamplePath);

JsonSerializerSettings settings = new JsonSerializerSettings
{
ContractResolver = SarifContractResolver.Instance
};
SarifLog expectedLog = JsonConvert.DeserializeObject<SarifLog>(comprehensiveTestSampleContents);
SarifLog actualLog = JsonConvert.DeserializeObject<SarifLog>(comprehensiveTestSampleContents);

SarifLog expected = JsonConvert.DeserializeObject<SarifLog>(comprehensiveTestSampleContents, settings);
SarifLog actual = JsonConvert.DeserializeObject<SarifLog>(comprehensiveTestSampleContents, settings);

expected.ValueEquals(actual).Should().BeTrue();
expectedLog.ValueEquals(actualLog).Should().BeTrue();
}
}
}
28 changes: 24 additions & 4 deletions src/Sarif.FunctionalTests/v2/SpecExamples/Comprehensive.sarif
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@
"copyright": "Copyright (c) 2016 by Example Corporation. All rights reserved."
}
},
"versionControlProvenance": [
{
"uri": "https://github.com/contoso/example",
Copy link

@ghost ghost Sep 28, 2018

Choose a reason for hiding this comment

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

github [](start = 26, length = 6)

example.com #WontFix

Copy link
Member Author

Choose a reason for hiding this comment

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

microsoft has created a contoso organization on github.com, do you see an issue using it?


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

Copy link

Choose a reason for hiding this comment

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

I didn't know that.


In reply to: 221400882 [](ancestors = 221400882,221319843)

"branch": "master",
"tag": "beta1",
"revisionId": "abcdabcdabcdabcdabcdabcdabcdabcdabcdabcd",
"timestamp": "2016-07-16T00:00:00Z"
}
],
"originalUriBaseIds": { "TOOLS_ROOT": "file:///bin/tools/" },
"invocations": [
{
Expand Down Expand Up @@ -150,7 +159,8 @@
"#TOOLS_ROOT#codescanner/config/collections.rsp": {
"contents": {
"text": "\"-input src/collections/*.cpp -log out/collections.sarif -rules all -disable C9999\"."
}
},
"lastModifiedTime": "2016-07-16T05:37:57Z"
}
},
"logicalLocations": {
Expand Down Expand Up @@ -253,7 +263,12 @@
"text": "Variable \"ptr\" declared."
}
},
"module": "platform"
"module": "platform",
"executionOrder": 1,
"timestamp": "2016-07-16T08:01:00Z",
"kind": "declaration",
"nestingLevel": 0,
"state": { "value": "[Not initialized}" }
},
{
"step": 2,
Expand All @@ -272,7 +287,9 @@
},
"fullyQualifiedLogicalName": "collections::list:add"
},
"module": "platform"
"module": "platform",
"executionOrder": 2,
"timestamp": "2016-07-16T08:02:00Z"
},
{
"step": 3,
Expand All @@ -294,7 +311,10 @@
"text": "Uninitialized variable \"ptr\" passed to method \"add_core\"."
}
},
"module": "platform"
"module": "platform",
"executionOrder": 3,
"timestamp": "2016-07-16T08:03:00Z",
"kind": "call"
}
]
}
Expand Down
10 changes: 2 additions & 8 deletions src/Sarif.Multitool.FunctionalTests/Rules/SkimmerTestsBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,12 @@ public abstract class SkimmerTestsBase<TSkimmer> : SarifMultitoolTestBase
where TSkimmer : SkimmerBase<SarifValidationContext>, new()
{
private readonly string _testDirectory;
private readonly JsonSerializerSettings _jsonSerializerSettings;
private const string ExpectedResultsPropertyName = "expectedResults";

public SkimmerTestsBase()
{
string ruleName = typeof(TSkimmer).Name;
_testDirectory = Path.Combine(Environment.CurrentDirectory, TestDataDirectory, ruleName);

_jsonSerializerSettings = new JsonSerializerSettings
{
ContractResolver = SarifContractResolver.Instance
};
}

// For the moment, we support two different test designs.
Expand Down Expand Up @@ -54,7 +48,7 @@ protected void Verify(string testFileName)

string inputLogContents = File.ReadAllText(targetPath);

SarifLog inputLog = JsonConvert.DeserializeObject<SarifLog>(inputLogContents, _jsonSerializerSettings);
SarifLog inputLog = JsonConvert.DeserializeObject<SarifLog>(inputLogContents);

var skimmer = new TSkimmer();

Expand Down Expand Up @@ -106,7 +100,7 @@ protected void Verify(string testFileName)
if (inputLog.Runs[0].TryGetProperty(ExpectedResultsPropertyName, out ExpectedValidationResults expectedResults))
{
// The custom property exists. Use the new, preferred verification method.
SarifLog outputLog = JsonConvert.DeserializeObject<SarifLog>(actualLogContents, _jsonSerializerSettings);
SarifLog outputLog = JsonConvert.DeserializeObject<SarifLog>(actualLogContents);
Verify(outputLog.Runs[0], expectedResults);
resultsWereVerified = true;
}
Expand Down
9 changes: 2 additions & 7 deletions src/Sarif.Multitool.FunctionalTests/SarifMultitoolTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,8 @@ protected static string MakeActualFilePath(string testDirectory, string testFile
// we perform a selective compare of just the elements we care about.
protected static void SelectiveCompare(string actualLogContents, string expectedLogContents)
{
var settings = new JsonSerializerSettings()
{
ContractResolver = SarifContractResolver.Instance
};

SarifLog actualLog = JsonConvert.DeserializeObject<SarifLog>(actualLogContents, settings);
SarifLog expectedLog = JsonConvert.DeserializeObject<SarifLog>(expectedLogContents, settings);
SarifLog actualLog = JsonConvert.DeserializeObject<SarifLog>(actualLogContents);
SarifLog expectedLog = JsonConvert.DeserializeObject<SarifLog>(expectedLogContents);

SelectiveCompare(actualLog, expectedLog);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Sarif.Multitool/Rules/DoNotUseFriendlyNameAsRuleId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class DoNotUseFriendlyNameAsRuleId : SarifValidationSkimmerBase
nameof(RuleResources.SARIF1001_Default)
};

protected override void Analyze(Rule rule, string ruleKey, string rulePointer)
protected override void Analyze(IRule rule, string ruleKey, string rulePointer)
{
if (rule.Id != null &&
rule.Name != null &&
Expand Down
2 changes: 1 addition & 1 deletion src/Sarif.Multitool/Rules/MessagesShouldEndWithPeriod.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ protected override IEnumerable<string> MessageResourceNames
}
}

protected override void Analyze(Rule rule, string ruleKey, string rulePointer)
protected override void Analyze(IRule rule, string ruleKey, string rulePointer)
{
AnalyzeMessageStrings(rule.MessageStrings, rulePointer, SarifPropertyName.MessageStrings);
AnalyzeMessageStrings(rule.RichMessageStrings, rulePointer, SarifPropertyName.RichMessageStrings);
Expand Down
4 changes: 2 additions & 2 deletions src/Sarif.Multitool/Rules/SarifValidationSkimmerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ protected virtual void Analyze(Result result, string resultPointer)
{
}

protected virtual void Analyze(Rule rule, string ruleKey, string rulePointer)
protected virtual void Analyze(IRule rule, string ruleKey, string rulePointer)
{
}
protected virtual void Analyze(Run run, string runPointer)
Expand Down Expand Up @@ -542,7 +542,7 @@ private void Visit(Resources resources, string resourcesPointer)
}
}

private void Visit(Rule rule, string ruleKey, string rulePointer)
private void Visit(IRule rule, string ruleKey, string rulePointer)
{
Analyze(rule, ruleKey, rulePointer);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Sarif.Multitool/Rules/UriMustBeAbsolute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ protected override void Analyze(Result result, string resultPointer)
}
}

protected override void Analyze(Rule rule, string ruleKey, string rulePointer)
protected override void Analyze(IRule rule, string ruleKey, string rulePointer)
{
AnalyzeUri(rule.HelpUri, rulePointer.AtProperty(SarifPropertyName.HelpUri));
}
Expand Down
2 changes: 1 addition & 1 deletion src/Sarif.Multitool/Rules/UrisMustBeValid.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ protected override void Analyze(Result result, string resultPointer)
}
}

protected override void Analyze(Rule rule, string ruleKey, string rulePointer)
protected override void Analyze(IRule rule, string ruleKey, string rulePointer)
{
AnalyzeUri(rule.HelpUri, rulePointer.AtProperty(SarifPropertyName.HelpUri));
}
Expand Down
7 changes: 1 addition & 6 deletions src/Sarif.Multitool/ValidateCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,10 @@ protected override SarifValidationContext CreateContext(ValidateOptions options,

private static SarifLog Deserialize(string logContents)
{
JsonSerializerSettings settings = new JsonSerializerSettings
{
ContractResolver = SarifContractResolver.Instance
};

SarifLog log = null;
try
{
return JsonConvert.DeserializeObject<SarifLog>(logContents, settings);
return JsonConvert.DeserializeObject<SarifLog>(logContents);
}
catch (JsonSerializationException)
{
Expand Down
1 change: 0 additions & 1 deletion src/Sarif.TestUtilities/FileDiffingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ protected static bool AreEquivalentSarifLogs(string actualSarif, string expected

JsonSerializerSettings settings = new JsonSerializerSettings()
{
ContractResolver = SarifContractResolver.Instance,
Formatting = Formatting.Indented
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,18 @@ public void ResultMatchingBaseliner_BaselinesTwoSimpleSarifLogs()
AbsentResultProperties.Should().ContainKey("Run");
AbsentResultProperties["Run"].Should().BeEquivalentTo(baselineLog.Runs[0].InstanceGuid);

calculatedNextBaseline.Runs[0].Results.Where(r => r.BaselineState == BaselineState.Existing).Should().HaveCount(currentLog.Runs[0].Results.Count - 1);

calculatedNextBaseline.Runs[0].Results.Where(r => r.BaselineState == BaselineState.Existing).First().TryGetProperty(SarifLogResultMatcher.ResultMatchingResultPropertyName, out Dictionary<string, string> CurrentResultProperties).Should().BeTrue();
CurrentResultProperties.Should().ContainKey("Run");
CurrentResultProperties["Run"].Should().BeEquivalentTo(currentLog.Runs[0].InstanceGuid);
int existingCount = currentLog.Runs[0].Results.Count - 1;
calculatedNextBaseline.Runs[0].Results.Where(r => r.BaselineState == BaselineState.Existing).Count().Should().Be(existingCount);

if (existingCount > 0)
Copy link
Member Author

@michaelcfanning michaelcfanning Sep 28, 2018

Choose a reason for hiding this comment

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

existingCount [](start = 20, length = 13)

This is a fix for a bug in a test that produces random content (that happened to fail intermittently in one of my validation runs) #Resolved

{
// In the event that we generated a SARIF run of only a single result, we will not have an 'existing' match
// since we adjusted the sole result value by adding a property to it.
calculatedNextBaseline.Runs[0].Results.Where(r => r.BaselineState == BaselineState.Existing).First().TryGetProperty(SarifLogResultMatcher.ResultMatchingResultPropertyName, out Dictionary<string, string> CurrentResultProperties).Should().BeTrue();
CurrentResultProperties.Should().ContainKey("Run");
CurrentResultProperties["Run"].Should().BeEquivalentTo(currentLog.Runs[0].InstanceGuid);
}

calculatedNextBaseline.Runs[0].Results.Where(r => r.BaselineState == BaselineState.New).Should().HaveCount(1);

Expand Down
Loading