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.

Sign up for GitHub

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

Jump to bottom

Fixing json configuration export #2305

Merged
merged 3 commits into from
Mar 3, 2021
Merged

Fixing json configuration export #2305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
14f9b37
Fixing json configuration export
eddynaka Mar 2, 2021
c150217
Merge branch 'main' into users/ednakamu/fixing-json-configuration-export
eddynaka Mar 2, 2021
9dc7c7e
Merge branch 'main' into users/ednakamu/fixing-json-configuration-export
eddynaka Mar 3, 2021
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
8 changes: 6 additions & 2 deletions src/Sarif/PropertiesDictionary.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.Xml;

using Newtonsoft.Json;
using Newtonsoft.Json.Converters;
using Newtonsoft.Json.Linq;

namespace Microsoft.CodeAnalysis.Sarif
Expand All @@ -20,7 +21,9 @@ public class PropertiesDictionary : TypedPropertiesDictionary<object>
{
internal const string DEFAULT_POLICY_NAME = "default";

public PropertiesDictionary() : this(null) { }
public PropertiesDictionary() : this(null)
{
}

public PropertiesDictionary(PropertiesDictionary initializer) :
this(initializer, null)
Expand Down Expand Up @@ -157,8 +160,9 @@ public void SaveToJson(string filePath, bool prettyPrint = true)

var settings = new JsonSerializerSettings
{
Formatting = formatting
Formatting = formatting,
};
settings.Converters.Add(new StringEnumConverter());

File.WriteAllText(filePath, JsonConvert.SerializeObject(this, settings));
}
Expand Down
2 changes: 2 additions & 0 deletions src/Test.UnitTests.Sarif.Multitool.Library/Test.UnitTests.Sarif.Multitool.Library.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
<None Remove="TestData\PageCommand\elfie-arriba.sarif" />
<None Remove="TestData\RebaseUriCommand\ExpectedOutputs\RunWithArtifacts.sarif" />
<None Remove="TestData\RebaseUriCommand\RunWithArtifacts.sarif" />
<None Remove="TestData\ValidateCommand\Configuration.json" />
<None Remove="TestData\ValidateCommand\Configuration.xml" />
<None Remove="xunit.runner.json" />
</ItemGroup>
Expand Down Expand Up @@ -56,6 +57,7 @@
<EmbeddedResource Include="TestData\PageCommand\elfie-arriba.sarif" />
<EmbeddedResource Include="TestData\RebaseUriCommand\ExpectedOutputs\RunWithArtifacts.sarif" />
<EmbeddedResource Include="TestData\RebaseUriCommand\Inputs\RunWithArtifacts.sarif" />
<EmbeddedResource Include="TestData\ValidateCommand\Configuration.json" />
<EmbeddedResource Include="TestData\ValidateCommand\Configuration.xml" />
<EmbeddedResource Include="TestData\ValidateCommand\ValidateSarif.sarif" />
</ItemGroup>
Expand Down
5 changes: 5 additions & 0 deletions src/Test.UnitTests.Sarif.Multitool.Library/TestData/ValidateCommand/Configuration.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"SARIF2002.ProvideMessageArguments.Options": {
"RuleEnabled": "error"
Copy link
Member

Choose a reason for hiding this comment

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

error [](start = 20, length = 5)

Shouldn't this be cased as 'Error'? Also, why are we fixing the JSON configuration, how did this come up? Note that we can't actually deserialize our own JSON format!! I can explain why offline, the reason is that we can't apply the type information in the JSON (the original XML design uses element attributes for this. To make everything work, we'd need a new JSON serializer/deserializer that persist the type data as special properties at each level.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With this change both Error or error would work.
With this change we would be able to export the configuration as json and import.

The issue was that we were serializing the enum as value instead of string, which causes the issue.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have a few tests locally that i was using a custom xml. Then i was trying a custom config in json and i saw this error

In reply to: 586535491 [](ancestors = 586535491,586522294)

}
}
17 changes: 16 additions & 1 deletion src/Test.UnitTests.Sarif.Multitool.Library/ValidateCommandTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public class ValidateCommandTests
""$schema"": ""http://json-schema.org/draft-04/schema#"",
""type"": ""object""
}";

private const string SchemaFilePath = @"c:\schemas\SimpleSchemaForTest.json";
private const string LogFileDirectory = @"C:\Users\John\logs";
private const string LogFileName = "example.sarif";
Expand Down Expand Up @@ -103,7 +104,7 @@ public void WhenWeDoNotHaveResultsWithoutVerbose()
}

[Fact]
public void WhenWeDoHaveConfigurationChangingFailureLevel()
public void WhenWeDoHaveConfigurationChangingFailureLevelXml()
{
string path = "ValidateSarif.sarif";
string configuration = "Configuration.xml";
Expand All @@ -116,6 +117,20 @@ public void WhenWeDoHaveConfigurationChangingFailureLevel()
sarifLog.Runs[0].Results.Count.Should().Be(1);
}

[Fact]
public void WhenWeDoHaveConfigurationChangingFailureLevelJson()
{
string path = "ValidateSarif.sarif";
string configuration = "Configuration.json";
string outputPath = "ValidateSarifOutput.sarif";
File.WriteAllText(path, Extractor.GetResourceText($"ValidateCommand.{path}"));
File.WriteAllText(configuration, Extractor.GetResourceText($"ValidateCommand.{configuration}"));

SarifLog sarifLog = ExecuteTest(path, outputPath, configuration);
sarifLog.Runs.Count.Should().Be(1);
sarifLog.Runs[0].Results.Count.Should().Be(1);
}

private static SarifLog ExecuteTest(string path, string outputPath, string configuration = null)
{
var options = new ValidateOptions
Expand Down