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

rules 1011 2008 (split from original) #1921

Merged
Show file tree
Hide file tree
Changes from 5 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
3 changes: 2 additions & 1 deletion src/Sarif.Multitool/Rules/RuleId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public static class RuleId
public const string IndexPropertiesMustBeConsistentWithArrays = "SARIF1009";
public const string InvalidUriInOriginalUriBaseIds = "SARIF1018";
public const string RuleIdMustBeConsistent = "SARIF1010";
public const string ReferToFinalSchema = "SARIF1020";
public const string ReferenceFinalSchema = "SARIF1011";
public const string ProvideSchema = "SARIF2008";
}
}
51 changes: 30 additions & 21 deletions src/Sarif.Multitool/Rules/RuleResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 9 additions & 6 deletions src/Sarif.Multitool/Rules/RuleResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -195,16 +195,19 @@
<data name="SARIF1010_RuleIdMustBeConsistent_FullDescription_Text" xml:space="preserve">
<value>In every result, at least one of the properties result.ruleId and result.rule.id must be present. If both are present, they must be equal.</value>
</data>
<data name="SARIF1020_ReferenceToOldSchemaVersion" xml:space="preserve">
<data name="SARIF1011_ReferenceFinalSchema_Error_Default_Text" xml:space="preserve">
<value>{0}: The $schema property value '{1}' does not refer to the final version of the SARIF 2.1.0 schema. If you are using an earlier version of the SARIF format, consider upgrading your analysis tool to produce the final version. If this file does in fact conform to the final version of the schema, upgrade the tool to populate the $schema property with a URL that refers to the final version of the schema.</value>
</data>
<data name="SARIF1020_ReferToFinalSchema" xml:space="preserve">
<value>The $schema property should be present, and must refer to the final version of the SARIF 2.1.0 schema. This enables IDEs to provide Intellisense for SARIF log files.</value>
</data>
<data name="SARIF1020_SchemaReferenceMissing" xml:space="preserve">
<value>{0}: The SARIF log file does not contain a $schema property. Add a $schema property that refers to the final version of the SARIF 2.1.0 schema. This enables IDEs to provide Intellisense for SARIF log files.</value>
<data name="SARIF1011_ReferenceFinalSchema_FullDescription_Text" xml:space="preserve">
<value>The $schema property must refer to the final version of the SARIF 2.1.0 schema. This enables IDEs to provide Intellisense for SARIF log files.</value>
</data>
<data name="SARIF1009_IndexPropertiesMustBeConsistentWithArrays_Error_TargetArrayMustExist_Text" xml:space="preserve">
<value>{0}: This "{1}" object contains a property "{2}" with value {3}, but "{4}" is absent.</value>
</data>
<data name="SARIF2008_ProvideSchema_FullDescription_Text" xml:space="preserve">
<value>The $schema property should be present. This enables IDEs to provide Intellisense for SARIF log files.</value>
</data>
<data name="SARIF2008_ProvideSchema_Warning_Default_Text" xml:space="preserve">
<value>{0}: The SARIF log file does not contain a $schema property. Add a $schema property that refers to the final version of the SARIF 2.1.0 schema. This enables IDEs to provide Intellisense for SARIF log files.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,20 @@

namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules
{
public class ReferToFinalSchema : SarifValidationSkimmerBase
public class ReferenceFinalSchema : SarifValidationSkimmerBase
{
public override MultiformatMessageString FullDescription => new MultiformatMessageString
{
Text = RuleResources.SARIF1020_ReferToFinalSchema
Text = RuleResources.SARIF1011_ReferenceFinalSchema_FullDescription_Text
};

public override FailureLevel DefaultLevel => FailureLevel.Error;

public override string Id => RuleId.ReferToFinalSchema;
public override string Id => RuleId.ReferenceFinalSchema;

protected override IEnumerable<string> MessageResourceNames => new string[]
{
nameof(RuleResources.SARIF1020_ReferenceToOldSchemaVersion),
nameof(RuleResources.SARIF1020_SchemaReferenceMissing)
nameof(RuleResources.SARIF1011_ReferenceFinalSchema_Error_Default_Text)
};

protected override void Analyze(SarifLog log, string logPointer)
Expand All @@ -33,7 +32,7 @@ private void AnalyzeSchema(Uri schemaUri, string pointer)
{
if (schemaUri == null)
{
LogResult(pointer, nameof(RuleResources.SARIF1020_SchemaReferenceMissing));
// If SchemaUri is not present, it will be caught by SARIF2008 rule.
return;
}

Expand All @@ -42,7 +41,7 @@ private void AnalyzeSchema(Uri schemaUri, string pointer)
&& !schemaUri.OriginalString.EndsWith(VersionConstants.SchemaVersionAsPublishedToSchemaStoreOrg)
&& !schemaUri.OriginalString.EndsWith($"{VersionConstants.SchemaVersionAsPublishedToSchemaStoreOrg}.json"))
{
LogResult(pointer.AtProperty(SarifPropertyName.Schema), nameof(RuleResources.SARIF1020_ReferenceToOldSchemaVersion), schemaUri.OriginalString);
LogResult(pointer.AtProperty(SarifPropertyName.Schema), nameof(RuleResources.SARIF1011_ReferenceFinalSchema_Error_Default_Text), schemaUri.OriginalString);
return;
}
}
Expand Down
40 changes: 40 additions & 0 deletions src/Sarif.Multitool/Rules/SARIF2008.ProvideSchema.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.Json.Pointer;
using System;
using System.Collections.Generic;

namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules
{
public class ProvideSchema : SarifValidationSkimmerBase
{
public override MultiformatMessageString FullDescription => new MultiformatMessageString
{
Text = RuleResources.SARIF2008_ProvideSchema_FullDescription_Text
};

public override FailureLevel DefaultLevel => FailureLevel.Error;
Copy link

@ghost ghost Jun 19, 2020

Choose a reason for hiding this comment

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

Error [](start = 66, length = 5)

Warning (so rename resource strings accordingly). #Closed

Copy link

Choose a reason for hiding this comment

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

run.$schema is optional, so not error-level.


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

Copy link

Choose a reason for hiding this comment

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

Oh, I see that the resources do say "warning".


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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch!!!


In reply to: 443047522 [](ancestors = 443047522,443047241)


public override string Id => RuleId.ProvideSchema;

protected override IEnumerable<string> MessageResourceNames => new string[]
{
nameof(RuleResources.SARIF2008_ProvideSchema_Warning_Default_Text)
};

protected override void Analyze(SarifLog log, string logPointer)
{
AnalyzeSchema(log.SchemaUri, logPointer);
}

private void AnalyzeSchema(Uri schemaUri, string pointer)
{
if (schemaUri == null)
{
LogResult(pointer, nameof(RuleResources.SARIF2008_ProvideSchema_Warning_Default_Text));
return;
}
}
}
}
2 changes: 2 additions & 0 deletions src/Sarif.Multitool/ValidateCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ protected override void AnalyzeTarget(

if (!string.IsNullOrEmpty(sarifText))
{
// DISCUSS IN PR: Deserializing the object here injects "$Schema" property, corrupting our test for
// SARIF2008_ProvideSchema
Copy link
Contributor Author

@harleenkohli harleenkohli Jun 19, 2020

Choose a reason for hiding this comment

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

Larry - need to discuss how to solve this. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is also a possible bug in our Validate command. Isnt it? If you confirm, i can log a bug, mark the test cases as ignore and move on for now.


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

Copy link

Choose a reason for hiding this comment

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

Yes, change to SARIF2008 is the right fix.


In reply to: 443046418 [](ancestors = 443046418,443040878)

context.InputLogContents = sarifText;
context.InputLog = context.InputLogContents != null ? Deserialize(context.InputLogContents) : null;

Expand Down
31 changes: 22 additions & 9 deletions src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using FluentAssertions;
using Microsoft.CodeAnalysis.Sarif.Multitool;
using Microsoft.CodeAnalysis.Sarif.Multitool.Rules;
Expand Down Expand Up @@ -133,12 +134,20 @@ public void SARIF1010_RuleIdMustBeConsistent_Invalid()
=> RunTest(MakeInvalidTestFileName(RuleId.RuleIdMustBeConsistent, nameof(RuleId.RuleIdMustBeConsistent)));

[Fact]
public void SARIF1020_SchemaMustBePresentAndConsistent_Valid()
=> RunTest(MakeValidTestFileName(RuleId.ReferToFinalSchema, nameof(RuleId.ReferToFinalSchema)));
public void SARIF1011_ReferenceFinalSchema_Valid()
=> RunTest(MakeValidTestFileName(RuleId.ReferenceFinalSchema, nameof(RuleId.ReferenceFinalSchema)));

[Fact]
public void SARIF1020_SchemaMustBePresentAndConsistent_Invalid()
=> RunTest(MakeInvalidTestFileName(RuleId.ReferToFinalSchema, nameof(RuleId.ReferToFinalSchema)));
public void SARIF1011_ReferenceFinalSchema_Invalid()
=> RunTest(MakeInvalidTestFileName(RuleId.ReferenceFinalSchema, nameof(RuleId.ReferenceFinalSchema)));

[Fact]
public void SARIF2008_ProvideSchema_Valid()
=> RunTest(MakeValidTestFileName(RuleId.ProvideSchema, nameof(RuleId.ProvideSchema)));

[Fact]
public void SARIF2008_ProvideSchema_Invalid()
=> RunTest(MakeInvalidTestFileName(RuleId.ProvideSchema, nameof(RuleId.ProvideSchema)));

private const string ValidTestFileNameSuffix = "_Valid.sarif";
private const string InvalidTestFileNameSuffix = "_Invalid.sarif";
Expand All @@ -163,11 +172,14 @@ protected override string ConstructTestOutputFromInputResource(string inputResou

// All SARIF rule prefixes require update to current release.
// All rules with JSON prefix are low level syntax/deserialization checks.
// We can't transform these test inputs as that operation fixes up erros in the file.
// Also, don't transform the tests for SARIF1020, because that rule examines the actual contents of the $schema
// property, so we can't change it.
// We can't transform these test inputs as that operation fixes up errors in the file.
// Also, don't transform the tests for SARIF1011 or SARIF2008, because these rules
// examine the actual contents of the $schema property.

string[] shouldNotTransform = { "SARIF1011", "SARIF2008" };

bool updateInputsToCurrentSarif = ruleUnderTest.StartsWith("SARIF")
&& ruleUnderTest != "SARIF1020" ? true : false;
&& !shouldNotTransform.Contains(ruleUnderTest);

var validateOptions = new ValidateOptions
{
Expand Down Expand Up @@ -200,7 +212,8 @@ protected override string ConstructTestOutputFromInputResource(string inputResou

returnCode.Should().Be(0);

SarifLog actualLog = JsonConvert.DeserializeObject<SarifLog>(File.ReadAllText(actualLogFilePath));
string actualLogFileContents = File.ReadAllText(actualLogFilePath);
SarifLog actualLog = JsonConvert.DeserializeObject<SarifLog>(actualLogFileContents);

// First, we'll strip any validation results that don't originate with the rule under test
var newResults = new List<Result>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,17 @@
"helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html"
},
{
"id": "SARIF1020",
"name": "ReferToFinalSchema",
"id": "SARIF1011",
"name": "ReferenceFinalSchema",
"shortDescription": {
"text": "The $schema property should be present, and must refer to the final version of the SARIF 2.1.0 schema."
"text": "The $schema property must refer to the final version of the SARIF 2.1.0 schema."
},
"fullDescription": {
"text": "The $schema property should be present, and must refer to the final version of the SARIF 2.1.0 schema. This enables IDEs to provide Intellisense for SARIF log files."
"text": "The $schema property must refer to the final version of the SARIF 2.1.0 schema. This enables IDEs to provide Intellisense for SARIF log files."
},
"messageStrings": {
"ReferenceToOldSchemaVersion": {
"Error_Default": {
"text": "{0}: The $schema property value '{1}' does not refer to the final version of the SARIF 2.1.0 schema. If you are using an earlier version of the SARIF format, consider upgrading your analysis tool to produce the final version. If this file does in fact conform to the final version of the schema, upgrade the tool to populate the $schema property with a URL that refers to the final version of the schema."
},
"SchemaReferenceMissing": {
"text": "{0}: The SARIF log file does not contain a $schema property. Add a $schema property that refers to the final version of the SARIF 2.1.0 schema. This enables IDEs to provide Intellisense for SARIF log files."
}
},
"helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html"
Expand Down Expand Up @@ -310,11 +307,11 @@
]
},
{
"ruleId": "SARIF1020",
"ruleId": "SARIF1011",
"ruleIndex": 1,
"level": "error",
"message": {
"id": "ReferenceToOldSchemaVersion",
"id": "Error_Default",
"arguments": [
"$schema",
"ht%tp://json.schemastore.org/sarif-2.0.0"
Expand Down
Loading