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

Adds DependsOnTags field to Rule #533

Merged
merged 13 commits into from
Mar 7, 2023
8 changes: 4 additions & 4 deletions AppInspector.Benchmarks/DistinctBenchmarks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public List<string> OldCode()
List<string> outList = new();
foreach (var r in ruleSet)
//builds a list of unique tags
foreach (var t in r?.Tags ?? Array.Empty<string>())
foreach (var t in (IList<string>?)r?.Tags ?? Array.Empty<string>())
if (uniqueTags.ContainsKey(t))
{
continue;
Expand All @@ -41,7 +41,7 @@ public List<string> HashSet()
HashSet<string> hashSet = new();
foreach (var r in ruleSet)
//builds a list of unique tags
foreach (var t in r?.Tags ?? Array.Empty<string>())
foreach (var t in (IList<string>?)r?.Tags ?? Array.Empty<string>())
hashSet.Add(t);

var theList = hashSet.ToList();
Expand All @@ -52,14 +52,14 @@ public List<string> HashSet()
[Benchmark]
public List<string> WithLinq()
{
return ruleSet.SelectMany(x => x.Tags ?? Array.Empty<string>()).Distinct().OrderBy(x => x)
return ruleSet.SelectMany(x => (IList<string>?)x.Tags ?? Array.Empty<string>()).Distinct().OrderBy(x => x)
.ToList();
}

[Benchmark]
public List<string> WithLinqAndHashSet()
{
var theList = ruleSet.SelectMany(x => x.Tags ?? Array.Empty<string>()).ToHashSet().ToList();
var theList = ruleSet.SelectMany(x => (IList<string>?)x.Tags ?? Array.Empty<string>()).ToHashSet().ToList();
theList.Sort();
return theList;
}
Expand Down
3 changes: 2 additions & 1 deletion AppInspector.Common/MsgHelp.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ public enum ID
BROWSER_START_SUCCESS,
PACK_MISSING_OUTPUT_ARG,
PACK_RULES_NO_CLI_DEFAULT,
PACK_RULES_NO_DEFAULT
PACK_RULES_NO_DEFAULT,
VERIFY_RULES_DEPENDS_ON_TAG_MISSING
}

public static string GetString(ID id)
Expand Down
3 changes: 2 additions & 1 deletion AppInspector.Common/Properties/Resources.Designer.cs

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

3 changes: 3 additions & 0 deletions AppInspector.Common/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,9 @@
</data>
<data name="VERIFY_RULES_DUPLICATEID_FAIL" xml:space="preserve">
<value>Rule {0} failed from dupicate rule id specified</value>
</data>
<data name="VERIFY_RULES_DUPLICATEID_FAIL" xml:space="preserve">
<value>Rule {0} failed verification. depends_on_tags is set but the specified tags are not set by any rules in the set.</value>
</data>
<data name="VERIFY_RULES_LANGUAGE_FAIL" xml:space="preserve">
<value>Rule {0} failed from unrecognized language {1} specified</value>
Expand Down
4 changes: 2 additions & 2 deletions AppInspector.RulesEngine/AbstractRuleSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ public IEnumerable<ConvertedOatRule> ByFilename(string input)
public IEnumerable<ConvertedOatRule> GetUniversalRules()
{
return _oatRules.Where(x =>
(x.AppInspectorRule.FileRegexes is null || x.AppInspectorRule.FileRegexes.Length == 0) &&
(x.AppInspectorRule.AppliesTo is null || x.AppInspectorRule.AppliesTo.Length == 0));
(x.AppInspectorRule.FileRegexes is null || x.AppInspectorRule.FileRegexes.Count == 0) &&
(x.AppInspectorRule.AppliesTo is null || x.AppInspectorRule.AppliesTo.Count == 0));
}

/// <summary>
Expand Down
3 changes: 2 additions & 1 deletion AppInspector.RulesEngine/MatchRecord.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License. See LICENSE.txt in the project root for license information.

using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Text.Json.Serialization;

namespace Microsoft.ApplicationInspector.RulesEngine;
Expand All @@ -21,7 +22,7 @@ public MatchRecord(Rule rule)
RuleId = rule.Id;
RuleName = rule.Name;
RuleDescription = rule.Description;
Tags = rule.Tags;
Tags = rule.Tags?.ToArray();
Severity = rule.Severity;
}

Expand Down
25 changes: 15 additions & 10 deletions AppInspector.RulesEngine/Rule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ namespace Microsoft.ApplicationInspector.RulesEngine;
/// </summary>
public class Rule
{
private IEnumerable<Regex> _compiled = Array.Empty<Regex>();
private IList<Regex> _compiled = Array.Empty<Regex>();

private string[]? _fileRegexes;
private IList<string>? _fileRegexes;
private bool _updateCompiledFileRegex;

/// <summary>
Expand All @@ -38,6 +38,11 @@ public class Rule
[JsonIgnore]
public bool Disabled { get; set; }

/// <summary>
/// Tags that are required to be present from other rules - regardless of file - for this match to be valid
/// </summary>
[JsonPropertyName("depends_on_tags")] public IList<string>? DependsOnTags { get; set; }

[JsonPropertyName("name")] public string Name { get; set; } = "";

[JsonPropertyName("id")] public string Id { get; set; } = "";
Expand All @@ -46,13 +51,13 @@ public class Rule
public string? Description { get; set; } = "";

[JsonPropertyName("does_not_apply_to")]
public List<string>? DoesNotApplyTo { get; set; }
public IList<string>? DoesNotApplyTo { get; set; }

[JsonPropertyName("applies_to")]
public string[]? AppliesTo { get; set; }
public IList<string>? AppliesTo { get; set; }

[JsonPropertyName("applies_to_file_regex")]
public string[]? FileRegexes
public IList<string>? FileRegexes
{
get => _fileRegexes;
set
Expand All @@ -69,22 +74,22 @@ public IEnumerable<Regex> CompiledFileRegexes
{
if (_updateCompiledFileRegex)
{
_compiled = FileRegexes?.Select(x => new Regex(x, RegexOptions.Compiled)) ?? Array.Empty<Regex>();
_compiled = (IList<Regex>?)FileRegexes?.Select(x => new Regex(x, RegexOptions.Compiled)).ToList() ?? Array.Empty<Regex>();
_updateCompiledFileRegex = false;
}

return _compiled;
}
}

[JsonPropertyName("tags")] public string[]? Tags { get; set; }
[JsonPropertyName("tags")] public IList<string>? Tags { get; set; }

[JsonPropertyName("severity")]
[JsonConverter(typeof(JsonStringEnumConverter))]
public Severity Severity { get; set; } = Severity.Moderate;

[JsonPropertyName("overrides")]
public string[]? Overrides { get; set; }
public IList<string>? Overrides { get; set; }

[JsonPropertyName("patterns")]
public SearchPattern[] Patterns { get; set; } = Array.Empty<SearchPattern>();
Expand All @@ -93,8 +98,8 @@ public IEnumerable<Regex> CompiledFileRegexes
public SearchCondition[]? Conditions { get; set; }

[JsonPropertyName("must-match")]
public string[]? MustMatch { get; set; }
public IList<string>? MustMatch { get; set; }

[JsonPropertyName("must-not-match")]
public string[]? MustNotMatch { get; set; }
public IList<string>? MustNotMatch { get; set; }
}
8 changes: 4 additions & 4 deletions AppInspector.RulesEngine/RuleProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,8 @@ public List<MatchRecord> AnalyzeFile(TextContainer textContainer, FileEntry file

List<MatchRecord> removes = new();

foreach (var m in resultsList.Where(x => x.Rule?.Overrides?.Length > 0))
foreach (var idsToOverride in m.Rule?.Overrides ?? Array.Empty<string>())
foreach (var m in resultsList.Where(x => x.Rule?.Overrides?.Count > 0))
foreach (var idsToOverride in (IList<string>)m.Rule?.Overrides ?? Array.Empty<string>())
// Find all overriden rules and mark them for removal from issues list
foreach (var om in resultsList.FindAll(x => x.Rule?.Id == idsToOverride))
// If the overridden match is a subset of the overriding match
Expand Down Expand Up @@ -416,14 +416,14 @@ List<MatchRecord> ProcessBoundary(ClauseCapture cap)

List<MatchRecord> removes = new();

foreach (var m in resultsList.Where(x => x.Rule?.Overrides?.Length > 0))
foreach (var m in resultsList.Where(x => x.Rule?.Overrides?.Count > 0))
{
if (cancellationToken?.IsCancellationRequested is true)
{
return resultsList;
}

foreach (var ovrd in m.Rule?.Overrides ?? Array.Empty<string>())
foreach (var ovrd in (IList<string>?)m.Rule?.Overrides ?? Array.Empty<string>())
// Find all overriden rules and mark them for removal from issues list
foreach (var om in resultsList.FindAll(x => x.Rule?.Id == ovrd))
if (om.Boundary.Index >= m.Boundary.Index &&
Expand Down
22 changes: 16 additions & 6 deletions AppInspector.RulesEngine/RulesVerifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,16 @@ public List<RuleStatus> CheckIntegrity(AbstractRuleSet ruleSet)
}
}

var allTags = ruleSet.GetAppInspectorRules().SelectMany(x => x.Tags ?? Array.Empty<string>()).ToList();
var rulesWithDependsOnWithNoMatchingTags = ruleSet.GetAppInspectorRules().Where(x => !x.DependsOnTags?.All(tag => allTags.Contains(tag)) ?? false);
foreach(var dependslessRule in rulesWithDependsOnWithNoMatchingTags)
{
_logger.LogError(MsgHelp.GetString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING), dependslessRule.Id);
foreach(var status in ruleStatuses.Where(x => x.RulesId == dependslessRule.Id))
{
status.Errors = status.Errors.Append(MsgHelp.FormatString(MsgHelp.ID.VERIFY_RULES_DEPENDS_ON_TAG_MISSING, dependslessRule.Id));
}
}
return ruleStatuses;
}

Expand Down Expand Up @@ -124,7 +134,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule)
}
}

foreach (var pattern in rule.FileRegexes ?? Array.Empty<string>())
foreach (var pattern in (IList<string>?)rule.FileRegexes ?? Array.Empty<string>())
try
{
_ = new Regex(pattern, RegexOptions.Compiled);
Expand Down Expand Up @@ -273,7 +283,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule)
StringComparer.InvariantCultureIgnoreCase) ?? true) ?? "csharp";

// validate all must match samples are matched
foreach (var mustMatchElement in rule.MustMatch ?? Array.Empty<string>())
foreach (var mustMatchElement in (IList<string>?)rule.MustMatch ?? Array.Empty<string>())
{
var tc = new TextContainer(mustMatchElement, language, _options.LanguageSpecs);
if (!_analyzer.Analyze(singleList, tc).Any())
Expand All @@ -285,7 +295,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule)
}

// validate no must not match conditions are matched
foreach (var mustNotMatchElement in rule.MustNotMatch ?? Array.Empty<string>())
foreach (var mustNotMatchElement in (IList<string>?)rule.MustNotMatch ?? Array.Empty<string>())
{
var tc = new TextContainer(mustNotMatchElement, language, _options.LanguageSpecs);
if (_analyzer.Analyze(singleList, tc).Any())
Expand All @@ -296,7 +306,7 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule)
}
}

if (rule.Tags?.Length == 0)
if (rule.Tags?.Count == 0)
{
_logger?.LogError("Rule must specify tags. {0}", rule.Id);
errors.Add($"Rule must specify tags. {rule.Id}");
Expand Down Expand Up @@ -326,8 +336,8 @@ public RuleStatus CheckIntegrity(ConvertedOatRule convertedOatRule)
RulesName = rule.Name,
Errors = errors,
OatIssues = _analyzer.EnumerateRuleIssues(convertedOatRule),
HasPositiveSelfTests = rule.MustMatch?.Length > 0,
HasNegativeSelfTests = rule.MustNotMatch?.Length > 0
HasPositiveSelfTests = rule.MustMatch?.Count > 0,
HasNegativeSelfTests = rule.MustNotMatch?.Count > 0
};
}
}
Loading