Skip to content

Commit

Permalink
Rename SARIF2012 and add check for friendly name (#2031)
Browse files Browse the repository at this point in the history
Rename `SARIF2012` from `ProvideHelpUris` to `ProvideRuleProperties`. In addition to checking for the `helpUri` property, it now also checks for the `name` property, and checks whether `name` is in the form of a single Pascal case identifier.
  • Loading branch information
eddynaka committed Aug 31, 2020
1 parent 5519b61 commit dd96790
Show file tree
Hide file tree
Showing 12 changed files with 311 additions and 167 deletions.
2 changes: 1 addition & 1 deletion src/Sarif.Multitool/Rules/RuleId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public static class RuleId
public const string ProvideCodeSnippets = "SARIF2010";

public const string ProvideContextRegion = "SARIF2011";
public const string ProvideHelpUris = "SARIF2012";
public const string ProvideRuleProperties = "SARIF2012";
public const string ProvideEmbeddedFileContent = "SARIF2013";
public const string ProvideDynamicMessageContent = "SARIF2014";
public const string EnquoteDynamicMessageContent = "SARIF2015";
Expand Down
32 changes: 27 additions & 5 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.

16 changes: 13 additions & 3 deletions src/Sarif.Multitool/Rules/RuleResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -354,10 +354,14 @@ This is part of a set of authoring practices that make your rule messages more r
<data name="SARIF2015_EnquoteDynamicMessageContent_Note_Default_Text" xml:space="preserve">
<value>{0}: In rule '{1}', the message with id '{2}' includes dynamic content that is not enclosed in single quotes. Enquoting dynamic content makes it easier to spot, and single quotes give a less cluttered appearance.</value>
</data>
<data name="SARIF2012_ProvideHelpUris_FullDescription_Text" xml:space="preserve">
<value>For each rule, provide a URI where users can find detailed information about the rule. This information should include a detailed description of the invalid pattern, an explanation of why the pattern is poor practice (particularly in contexts such as security or accessibility where driving considerations might not be readily apparent), guidance for resolving the problem (including describing circumstances in which ignoring the problem altogether might be appropriate), examples of invalid and valid patterns, and special considerations (such as noting when a violation should never be ignored or suppressed, noting when a violation could cause downstream tool noise, and noting when a rule can be configured in some way to refine or alter the analysis).</value>
<data name="SARIF2012_ProvideRuleProperties_FullDescription_Text" xml:space="preserve">
<value>Rule metadata should provide information that makes it easy to understand and fix the problem.

Provide the 'name' property, which contains a "friendly name" that helps users see at a glance the purpose of the rule. For uniformity of experience across all tools that produce SARIF, the friendly name should be a single Pascal identifier, for example, 'ProvideRuleFriendlyName'.

Provide the 'helpUri' property, which contains a URI where users can find detailed information about the rule. This information should include a detailed description of the invalid pattern, an explanation of why the pattern is poor practice (particularly in contexts such as security or accessibility where driving considerations might not be readily apparent), guidance for resolving the problem (including describing circumstances in which ignoring the problem altogether might be appropriate), examples of invalid and valid patterns, and special considerations (such as noting when a violation should never be ignored or suppressed, noting when a violation could cause downstream tool noise, and noting when a rule can be configured in some way to refine or alter the analysis).</value>
</data>
<data name="SARIF2012_ProvideHelpUris_Note_Default_Text" xml:space="preserve">
<data name="SARIF2012_ProvideRuleProperties_Note_ProvideHelpUri_Text" xml:space="preserve">
<value>{0}: The rule '{1}' does not provide a help URI. Providing a URI where users can find detailed information about the rule helps users to understand the result and how they can best address it.</value>
</data>
<data name="SARIF2013_ProvideEmbeddedFileContent_FullDescription_Text" xml:space="preserve">
Expand Down Expand Up @@ -451,4 +455,10 @@ Semantics: Assuming the reader of the log file (an end user or another tool) has
<data name="SARIF2005_ProvideToolProperties_Warning_ProvideToolnformationUri_Text" xml:space="preserve">
<value>{0}: The tool '{1}' does not provide 'informationUri'. This property helps the developer responsible for addessing a result by providing a way to learn more about the tool.</value>
</data>
<data name="SARIF2012_ProvideRuleProperties_Note_ProvideFriendlyName_Text" xml:space="preserve">
<value>{0}: The rule '{1}' does not provide a "friendly name" in its 'name' property. The friendly name should be a single Pascal identifier, for example, 'ProvideRuleFriendlyName', that helps users see at a glance the purpose of the analysis rule.</value>
</data>
<data name="SARIF2012_ProvideRuleProperties_Note_FriendlyNameNotAPascalIdentifier_Text" xml:space="preserve">
<value>{0}: '{1}' is not a Pascal identifier. For uniformity of experience across all tools that produce SARIF, the friendly name should be a single Pascal identifier, for example, 'ProvideRuleFriendlyName'.</value>
</data>
</root>
8 changes: 8 additions & 0 deletions src/Sarif.Multitool/Rules/SARIF2010.ProvideCodeSnippets.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.Linq;
using Microsoft.CodeAnalysis.Sarif.Writers;
using Microsoft.Json.Pointer;

namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules
Expand Down Expand Up @@ -100,6 +101,13 @@ private bool AnalyzeArtifactLocation(ArtifactLocation artifactLocation)
continue;
}


// if mimetype exists and is binary, continue to next
if (!string.IsNullOrEmpty(artifact.MimeType) && MimeType.IsBinaryMimeType(artifact.MimeType))
{
continue;
}

// Checking if we can reconstruct uri from artifact
// If we can't, we still need to validate, since originalUriBaseIds aren't
// required nether artifactLocation.UriBaseId.
Expand Down
78 changes: 0 additions & 78 deletions src/Sarif.Multitool/Rules/SARIF2012.ProvideHelpUris.cs

This file was deleted.

117 changes: 117 additions & 0 deletions src/Sarif.Multitool/Rules/SARIF2012.ProvideRuleProperties.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Collections.Generic;
using System.Text.RegularExpressions;

using Microsoft.Json.Pointer;

namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules
{
public class ProvideRuleProperties : SarifValidationSkimmerBase
{
/// <summary>
/// SARIF2012
/// </summary>
public override string Id => RuleId.ProvideRuleProperties;

/// <summary>
/// Rule metadata should provide information that makes it easy to understand and fix the problem.
///
/// Provide the 'name' property, which contains a "friendly name" that helps users see at a glance
/// the purpose of the rule.For uniformity of experience across all tools that produce SARIF, the
/// friendly name should be a single Pascal identifier, for example, 'ProvideRuleFriendlyName'.
///
/// Provide the 'helpUri' property, which contains a URI where users can find detailed information
/// about the rule.This information should include a detailed description of the invalid pattern,
/// an explanation of why the pattern is poor practice (particularly in contexts such as security
/// or accessibility where driving considerations might not be readily apparent), guidance for
/// resolving the problem(including describing circumstances in which ignoring the problem
/// altogether might be appropriate), examples of invalid and valid patterns, and special considerations
/// (such as noting when a violation should never be ignored or suppressed, noting when a violation
/// could cause downstream tool noise, and noting when a rule can be configured in some way to refine
/// or alter the analysis).
/// </summary>
public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.SARIF2012_ProvideRuleProperties_FullDescription_Text };

protected override IEnumerable<string> MessageResourceNames => new string[] {
nameof(RuleResources.SARIF2012_ProvideRuleProperties_Note_ProvideFriendlyName_Text),
nameof(RuleResources.SARIF2012_ProvideRuleProperties_Note_FriendlyNameNotAPascalIdentifier_Text),
nameof(RuleResources.SARIF2012_ProvideRuleProperties_Note_ProvideHelpUri_Text)
};

public override FailureLevel DefaultLevel => FailureLevel.Note;

protected override void Analyze(Run run, string runPointer)
{
AnalyzeTool(run.Tool, runPointer.AtProperty(SarifPropertyName.Tool));
}

private static readonly Regex s_pascalCaseRegex = new Regex(@"^(\p{Lu}[\p{Ll}\p{Nd}]+)*$", RegexOptions.Compiled | RegexOptions.CultureInvariant);

protected override void Analyze(ReportingDescriptor reportingDescriptor, string reportingDescriptorPointer)
{
if (string.IsNullOrWhiteSpace(reportingDescriptor.Name))
{
// {0}: The rule '{1}' does not provide a "friendly name" in its 'name'
// property. The friendly name should be a single Pascal identifier, for
// example, 'ProvideRuleFriendlyName', that helps users see at a glance
// the purpose of the rule.
LogResult(
reportingDescriptorPointer,
nameof(RuleResources.SARIF2012_ProvideRuleProperties_Note_ProvideFriendlyName_Text),
reportingDescriptor.Id);
return;
}

if (!s_pascalCaseRegex.IsMatch(reportingDescriptor.Name))
{
// {0}: '{1}' is not a Pascal identifier. For uniformity of experience across all tools that
// produce SARIF, the friendly name should be a single Pascal identifier, for example,
// 'ProvideRuleFriendlyName'.
LogResult(
reportingDescriptorPointer.AtProperty(SarifPropertyName.Name),
nameof(RuleResources.SARIF2012_ProvideRuleProperties_Note_FriendlyNameNotAPascalIdentifier_Text),
reportingDescriptor.Name);
return;
}
}

private void AnalyzeTool(Tool tool, string toolPointer)
{
AnalyzeToolDriver(tool.Driver, toolPointer.AtProperty(SarifPropertyName.Driver));
}

private void AnalyzeToolDriver(ToolComponent toolComponent, string toolDriverPointer)
{
if (toolComponent.Rules != null)
{
string rulesPointer = toolDriverPointer.AtProperty(SarifPropertyName.Rules);
for (int i = 0; i < toolComponent.Rules.Count; i++)
{
AnalyzeReportingDescriptor(toolComponent.Rules[i], rulesPointer.AtIndex(i));
}
}
}

private void AnalyzeReportingDescriptor(ReportingDescriptor reportingDescriptor, string reportingDescriptorPointer)
{
if (reportingDescriptor.HelpUri == null)
{
string ruleMoniker = reportingDescriptor.Id;
if (!string.IsNullOrWhiteSpace(reportingDescriptor.Name))
{
ruleMoniker += $".{reportingDescriptor.Name}";
}

// {0}: The rule '{1}' does not provide a help URI. Providing a URI where users can
// find detailed information about the rule helps users to understand the result and
// how they can best address it.
LogResult(
reportingDescriptorPointer,
nameof(RuleResources.SARIF2012_ProvideRuleProperties_Note_ProvideHelpUri_Text),
ruleMoniker);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,12 @@ public void SARIF2011_ProvideContextRegion_Invalid()
=> RunInvalidTestForRule(RuleId.ProvideContextRegion);

[Fact]
public void SARIF2012_ProvideHelpUris_Valid()
=> RunValidTestForRule(RuleId.ProvideHelpUris);
public void SARIF2012_ProvideRuleProperties_Valid()
=> RunValidTestForRule(RuleId.ProvideRuleProperties);

[Fact]
public void SARIF2012_ProvideHelpUris_Invalid()
=> RunInvalidTestForRule(RuleId.ProvideHelpUris);
public void SARIF2012_ProvideRuleProperties_Invalid()
=> RunInvalidTestForRule(RuleId.ProvideRuleProperties);

[Fact]
public void SARIF2013_ProvideEmbeddedFileContent_Valid()
Expand Down
Loading

0 comments on commit dd96790

Please sign in to comment.