Support queries against properties in the result's and the rule's property bags

src/ReleaseHistory.md

@@ -1,10 +1,10 @@
 # SARIF Package Release History (SDK, Driver, Converters, and Multitool)
 
-## **v2.3.7** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.3.7) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.3.7) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.3.7) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.3.7) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.3.7)
 * DEPENDENCY BREAKING: SARIF now requires Newtonsoft.JSON 11.0.2 (rather than 10.0.3)
 * DEPENDENCY: SARIF TypeScript package now requires minimist 1.2.3 or later (rather than >=1.2.0)
 * FEATURE: Add a converter for FlawFinder's CSV output format. [#2092](https://github.com/microsoft/sarif-sdk/issues/2092)
 * FEATURE: Multitool SARIF output is now pretty-printed by default. To remove white space, specify `--minify`. [#2098](https://github.com/microsoft/sarif-sdk/issues/2098)
+* FEATURE: The Multitool `query` command can now evaluate properties in the result and rule property bags, for example `sarif query "properties.confidence:f > 0.95 AND rule.properties.category == 'security'"`
 * FEATURE: The validation rule `SARIF1004.ExpressUriBaseIdsCorrectly` now verifies that if an `artifactLocation.uri` is a relative reference, it does not begin with a slash. [#2090](https://github.com/microsoft/sarif-sdk/issues/2090)
 * FEATURE: Add a setter to `GitHelper.GitExePath`. [#2105](https://github.com/microsoft/sarif-sdk/issues/2105)
 * FEATURE: `GitHelper` will search in %PATH% variable if `git.exe` isn't located in ProgramFiles folder [#2106](https://github.com/microsoft/sarif-sdk/issues/2106)

src/Sarif/Query/Evaluators/PropertyBagPropertyEvaluator.cs

@@ -0,0 +1,108 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.Globalization;
+using System.Linq;
+using System.Text.RegularExpressions;
+using Newtonsoft.Json;
+
+namespace Microsoft.CodeAnalysis.Sarif.Query.Evaluators
+{
+    public class PropertyBagPropertyEvaluator : IExpressionEvaluator<Result>
+    {
+        internal const string ResultPropertyPrefix = "properties.";
+        internal const string RulePropertyPrefix = "rule.properties.";
+
+        private readonly string _propertyName;
+        private readonly bool _propertyBelongsToRule;
+        private readonly IExpressionEvaluator<Result> _evaluator;
+
+        private static readonly Regex s_propertyNameRegex = new Regex(
+            @$"^
+                (?<prefix>properties\.|rule\.properties\.)
+                (?<name>.+?)
+                $",
+            RegexOptions.CultureInvariant | RegexOptions.Compiled | RegexOptions.ExplicitCapture | RegexOptions.IgnorePatternWhitespace);
+
+        public PropertyBagPropertyEvaluator(TermExpression term)
+        {
+            Match match = s_propertyNameRegex.Match(term.PropertyName);
+            _propertyName = match.Groups["name"].Value;
+
+            string prefix = match.Groups["prefix"].Value;
+            if (prefix.Equals(RulePropertyPrefix))
+            {
+                _propertyBelongsToRule = true;
+            }
+            else if (!prefix.Equals(ResultPropertyPrefix))
+            {
+                throw new ArgumentException(
+                    string.Format(
+                        CultureInfo.CurrentCulture,
+                        SdkResources.ErrorInvalidQueryPropertyPrefix,
+                        "'" + string.Join("', '", ResultPropertyPrefix, RulePropertyPrefix)) + "'",
+                    nameof(term));
+            }
+
+            _evaluator = CreateEvaluator(term);
+        }
+
+        // Create an appropriate evaluator object for the specified term. For operators that apply
+        // only to strings (":" (contains), "|>" (startswith), or ">|" (endswith), always create
+        // a string evaluator.  All other operators (==, !=, >, <, etc.) can apply to both strings
+        // and numbers. If the value being compared parses as a number, assume that a numeric
+        // comparison was intended, and create a numeric evaluator. Otherwise, create a string evaluator.
+        // This could cause problems if the comparand is string that happens to look like a number.
+        private IExpressionEvaluator<Result> CreateEvaluator(TermExpression term) =>
+           IsStringComparison(term)
+            ? new StringEvaluator<Result>(GetProperty<string>, term, StringComparison.OrdinalIgnoreCase) as IExpressionEvaluator<Result>
+            : new DoubleEvaluator<Result>(GetProperty<double>, term);
+
+        private static readonly ReadOnlyCollection<CompareOperator> s_stringSpecificOperators =
+            new ReadOnlyCollection<CompareOperator>(
+                new CompareOperator[]
+                {
+                    CompareOperator.Contains,
+                    CompareOperator.EndsWith,
+                    CompareOperator.StartsWith
+                });
+
+        private bool IsStringComparison(TermExpression term)
+            => s_stringSpecificOperators.Contains(term.Operator) || !double.TryParse(term.Value, out _);
+
+        private T GetProperty<T>(Result result)
+        {
+            PropertyBagHolder holder = GetPropertyBagHolder(result);
+            return GetPropertyFromHolder<T>(holder);
+        }
+
+        private PropertyBagHolder GetPropertyBagHolder(Result result) =>
+            _propertyBelongsToRule
+            ? result.GetRule() as PropertyBagHolder
+            : result;
+
+        private T GetPropertyFromHolder<T>(PropertyBagHolder holder)
+        {
+            try
+            {
+                return holder.TryGetProperty(_propertyName, out T value) ? value : default;
+            }
+            catch (JsonReaderException)
+            {
+                // Catch exceptions due to trying to perform a numeric comparison on a
+                // property that turns out to have a string value that can't be parsed
+                // as a number. The result will be that in such a case, the property
+                // will be treated as if its value were numeric zero.
+            }
+
+            return default;
+        }
+
+        public void Evaluate(ICollection<Result> results, BitArray matches)
+            => _evaluator.Evaluate(results, matches);
+    }
+}

src/Sarif/Query/Evaluators/ResultEvaluators.cs → src/Sarif/Query/Evaluators/SarifEvaluators.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Globalization;
 using System.Linq;
 
 namespace Microsoft.CodeAnalysis.Sarif.Query.Evaluators
@@ -10,7 +11,8 @@ public static class SarifEvaluators
     {
         public static IExpressionEvaluator<Result> ResultEvaluator(TermExpression term)
         {
-            switch (term.PropertyName.ToLowerInvariant())
+            string propertyNameLower = term.PropertyName.ToLowerInvariant();
+            switch (propertyNameLower)
             {
                 case "baselinestate":
                     return new EnumEvaluator<Result, BaselineState>(r => r.BaselineState, term);
@@ -43,7 +45,17 @@ public static IExpressionEvaluator<Result> ResultEvaluator(TermExpression term)
                     }, term);
 
                 default:
-                    throw new QueryParseException($"Property Name {term.PropertyName} unrecognized. Known Names: baselineState, correlationGuid, guid, hostedViewerUri, kind, level, message.text, occurrenceCount, rank, ruleId");
+                    if (propertyNameLower.StartsWith(PropertyBagPropertyEvaluator.ResultPropertyPrefix) ||
+                        propertyNameLower.StartsWith(PropertyBagPropertyEvaluator.RulePropertyPrefix))
+                    {
+                        return new PropertyBagPropertyEvaluator(term);
+                    }
+
+                    throw new QueryParseException(
+                        string.Format(
+                            CultureInfo.CurrentCulture,
+                            SdkResources.ErrorInvalidQueryPropertyName,
+                            term.PropertyName));
             }
         }
     }

src/Sarif/SdkResources.resx

@@ -255,4 +255,12 @@
   <data name="GitHelperDefaultInstanceDoesNotPermitCaching" xml:space="preserve">
     <value>When using the static 'Default' instance of GitHelper, you must pass the argument useCache: false to GetRepositoryRoot, which degrades performance. If the performance is not acceptable, create a separate GitHelper instance. You need not pass useCache: true because that is the default.</value>
   </data>
+  <data name="ErrorInvalidQueryPropertyName" xml:space="preserve">
+    <value>The property name '{0}' is unrecognized.
+Known property names: baselineState, correlationGuid, guid, hostedViewerUri, kind, level, message.text, occurrenceCount, rank, ruleId
+You can also refer to properties in the result's property bag with 'properties.&lt;propertyName&gt;', or to properties in the associated rule's property bag with 'properties.rule.&lt;propertyName&gt;'.</value>
+  </data>
+  <data name="ErrorInvalidQueryPropertyPrefix" xml:space="preserve">
+    <value>Property name must start with one of: {0}</value>
+  </data>
 </root>

src/Test.UnitTests.Sarif.Multitool/QueryCommandPropertyBagTests.cs

@@ -0,0 +1,104 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.IO;
+using FluentAssertions;
+using Microsoft.CodeAnalysis.Sarif.Query;
+using Microsoft.CodeAnalysis.Sarif.Query.Evaluators;
+using Newtonsoft.Json;
+using Xunit;
+
+namespace Microsoft.CodeAnalysis.Sarif.Multitool
+{
+    public class QueryCommandPropertyBagTests : IClassFixture<QueryCommandPropertyBagTests.TestFixture>
+    {
+        private const string FilePath = "property-bag-queries.sarif";
+
+        public class TestFixture
+        {
+            public TestFixture()
+            {
+                ResourceExtractor extractor = new ResourceExtractor(typeof(QueryCommandPropertyBagTests));
+                File.WriteAllText(FilePath, extractor.GetResourceText($"QueryCommand.{FilePath}"));
+            }
+        }
+
+        [Fact]
+        public void QueryCommand_CanAccessResultAndRulePropertyBags()
+        {
+            RunAndVerifyCount(2, "properties.name == 'Terisa'");
+            RunAndVerifyCount(2, "rule.properties.Category == 'security'");
+        }
+
+        [Fact]
+        public void QueryCommand_ComparesPropertyBagPropertyNamesCaseSensitively()
+        {
+            RunAndVerifyCount(0, "properties.nAmE == 'Terisa'");
+            RunAndVerifyCount(0, "rule.properties.CaTegORy == 'security'");
+        }
+
+        [Fact]
+        public void QueryCommand_ReadsIntegerProperty()
+        {
+            RunAndVerifyCount(1, "properties.count == 42");
+        }
+
+        [Fact]
+        public void QueryCommand_ReadsFloatProperty()
+        {
+            RunAndVerifyCount(2, "properties.confidence >= 0.95");
+        }
+
+        [Fact]
+        public void QueryCommand_PerformsStringSpecificComparisons()
+        {
+            RunAndVerifyCount(1, "properties.confidence : 95");     // contains
+            RunAndVerifyCount(3, "properties.confidence |> 0.");    // startswith
+            RunAndVerifyCount(1, "properties.confidence >| 9");     // endswith
+        }
+
+        [Fact]
+        public void QueryCommand_TreatsUnparseableValueAsHavingTheDefaultValue()
+        {
+            // In this test, all the results will match, so we need to know how many there are.
+            SarifLog sarifLog = JsonConvert.DeserializeObject<SarifLog>(File.ReadAllText(FilePath));
+            int numResults = sarifLog.Runs[0].Results.Count;
+
+            // 'name' is a string-valued property that doesn't parse to an integer. The
+            // PropertyBagPropertyEvaluator sees a number on the right-hand side of the comparison
+            // and decides that a numeric comparison is intended. When it encounters a property
+            // value that can't be parsed as a number, it treats it as numeric 0 rather than
+            // throwing.
+            RunAndVerifyCount(numResults, "properties.name == 0");
+        }
+
+        // The above tests cover all but one code block in the underlying PropertyBagPropertyEvaluator.
+        // It doesn't cover the case of an invalid "prefix" (that is, a property name that doesn't
+        // start with "properties." or "rule.properties"), because QueryCommand never creates a
+        // PropertyBagPropertyEvaluator for a term with such a property name. So we cover that
+        // case here:
+        [Fact]
+        public void PropertyBagPropertyEvaluator_RejectsPropertyNameWithInvalidPrefix()
+        {
+            var expression = new TermExpression(propertyName: "invalid.prefix", op: CompareOperator.Equals, value: string.Empty);
+
+            Action action = () => new PropertyBagPropertyEvaluator(expression);
+
+            action.Should().Throw<ArgumentException>();
+        }
+
+        private void RunAndVerifyCount(int expectedCount, string expression)
+        {
+            var options = new QueryOptions
+            {
+                Expression = expression,
+                InputFilePath = FilePath,
+                ReturnCount = true
+            };
+
+            int exitCode = new QueryCommand().RunWithoutCatch(options);
+            exitCode.Should().Be(expectedCount);
+        }
+    }
+}

src/Test.UnitTests.Sarif.Multitool/Test.UnitTests.Sarif.Multitool.csproj

@@ -13,16 +13,25 @@
     <IsTestProject>true</IsTestProject>
   </PropertyGroup>
 
+  <ItemGroup>
+    <None Remove="TestData\QueryCommand\property-bag-queries.sarif" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <EmbeddedResource Include="TestData\QueryCommand\property-bag-queries.sarif" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="FluentAssertions" Version="5.10.2" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0" />
-    <PackageReference Include="xunit" Version="2.4.0" />
+    <PackageReference Include="xunit" Version="2.4.1" />
     <PackageReference Include="xunit.runner.visualstudio" Version="2.4.0" />
     <PackageReference Include="coverlet.collector" Version="1.2.0" />
   </ItemGroup>
 
   <ItemGroup>
     <ProjectReference Include="..\Sarif.Multitool\Sarif.Multitool.csproj" />
+    <ProjectReference Include="..\Test.Utilities.Sarif\Test.Utilities.Sarif.csproj" />
   </ItemGroup>
 
 </Project>