From bc5df7bcb85c12a0f19d7fff94759d71374f0738 Mon Sep 17 00:00:00 2001 From: jameswinkler <77122232+jameswinkler@users.noreply.github.com> Date: Tue, 12 Jan 2021 10:58:53 -0800 Subject: [PATCH] Move code from BinSkim, refactor, add unit tests (#2231) * Handle --hashes and dd warning * Remove debug exception * Fix null context * Correct issue with null _cacheByFileHashLogger * Correct string * Formatting, add pragma --- src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs | 21 +++++++++++ src/Sarif/RuntimeConditions.cs | 3 +- src/Sarif/SdkResources.Designer.cs | 27 +++++++++++++ src/Sarif/SdkResources.resx | 9 +++++ src/Sarif/Warnings.cs | 44 ++++++++++++++++++++++ src/Test.UnitTests.Sarif/WarningsTests.cs | 23 +++++++++++ 6 files changed, 126 insertions(+), 1 deletion(-) diff --git a/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs b/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs index 36d1089d1..220f118b9 100644 --- a/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs +++ b/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs @@ -57,10 +57,31 @@ public string DefaultConfigurationPath public override int Run(TOptions options) { + // To correctly initialize the logger, we must first add Hashes to dataToInsert +#pragma warning disable CS0618 // Type or member is obsolete + if (options.ComputeFileHashes) +#pragma warning restore CS0618 + { + OptionallyEmittedData dataToInsert = options.DataToInsert.ToFlags(); + dataToInsert |= OptionallyEmittedData.Hashes; + + options.DataToInsert = Enum.GetValues(typeof(OptionallyEmittedData)).Cast() + .Where(oed => dataToInsert.HasFlag(oed)).ToList(); + } + // 0. Initialize an common logger that drives all outputs. This // object drives logging for console, statistics, etc. using (AggregatingLogger logger = InitializeLogger(options)) { + // Once the logger has been correctly initialized, we can raise a warning + _rootContext = CreateContext(options, logger, RuntimeErrors); +#pragma warning disable CS0618 // Type or member is obsolete + if (options.ComputeFileHashes) +#pragma warning restore CS0618 + { + Warnings.LogObsoleteOption(_rootContext, "--hashes", SdkResources.ComputeFileHashes_ReplaceInsertHashes); + } + try { Analyze(options, logger); diff --git a/src/Sarif/RuntimeConditions.cs b/src/Sarif/RuntimeConditions.cs index 967929d9f..b705f03df 100644 --- a/src/Sarif/RuntimeConditions.cs +++ b/src/Sarif/RuntimeConditions.cs @@ -11,7 +11,7 @@ namespace Microsoft.CodeAnalysis.Sarif // code paths. These conditions are a combination of fatal // and non-fatal circumstances [Flags] - public enum RuntimeConditions : int + public enum RuntimeConditions : uint { None = 0, @@ -57,6 +57,7 @@ public enum RuntimeConditions : int TargetNotValidToAnalyze = 0x10000000, OneOrMoreWarningsFired = 0x20000000, OneOrMoreErrorsFired = 0x40000000, + ObsoleteOption = 0x80000000, Nonfatal = 0x7FF00000 } diff --git a/src/Sarif/SdkResources.Designer.cs b/src/Sarif/SdkResources.Designer.cs index 91cdc1e65..b471ba623 100644 --- a/src/Sarif/SdkResources.Designer.cs +++ b/src/Sarif/SdkResources.Designer.cs @@ -96,6 +96,15 @@ public class SdkResources { } } + /// + /// Looks up a localized string similar to --insert Hashes. + /// + public static string ComputeFileHashes_ReplaceInsertHashes { + get { + return ResourceManager.GetString("ComputeFileHashes_ReplaceInsertHashes", resourceCulture); + } + } + /// /// Looks up a localized string similar to {0}{1}: error {2}: {3}. /// @@ -476,6 +485,24 @@ public class SdkResources { } } + /// + /// Looks up a localized string similar to Option '{0}' is obsolete.. + /// + public static string WRN997_ObsoleteOption { + get { + return ResourceManager.GetString("WRN997_ObsoleteOption", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Option '{0}' is obsolete. Use '{1}' instead.. + /// + public static string WRN997_ObsoleteOptionWithReplacement { + get { + return ResourceManager.GetString("WRN997_ObsoleteOptionWithReplacement", resourceCulture); + } + } + /// /// Looks up a localized string similar to Rule '{0}' was disabled as it cannot run on the current platform '{1}'. It can only run on '{2}'.. /// diff --git a/src/Sarif/SdkResources.resx b/src/Sarif/SdkResources.resx index f5d6add42..9ced6087c 100644 --- a/src/Sarif/SdkResources.resx +++ b/src/Sarif/SdkResources.resx @@ -263,4 +263,13 @@ You can also refer to properties in the result's property bag with 'properties.& Property name must start with one of: {0} + + Option '{0}' is obsolete. + + + Option '{0}' is obsolete. Use '{1}' instead. + + + --insert Hashes + \ No newline at end of file diff --git a/src/Sarif/Warnings.cs b/src/Sarif/Warnings.cs index d0e36baac..8290be47d 100644 --- a/src/Sarif/Warnings.cs +++ b/src/Sarif/Warnings.cs @@ -10,8 +10,15 @@ namespace Microsoft.CodeAnalysis.Sarif { public static class Warnings { + // Configuration warnings: public const string Wrn997_InvalidTarget = "WRN997.InvalidTarget"; + public const string Wrn997_ObsoleteOption = "WRN997.ObsoleteOption"; + public const string Wrn997_ObsoleteOptionWithReplacement = "WRN997.ObsoleteOptionWithReplacement"; + + // Rule disabling tool warnings: public const string Wrn998_UnsupportedPlatform = "WRN998.UnsupportedPlatform"; + + // Analysis halting tool warnings: public const string Wrn999_RuleExplicitlyDisabled = "WRN999.RuleExplicitlyDisabled"; public static void LogExceptionInvalidTarget(IAnalysisContext context) @@ -122,5 +129,42 @@ public static void LogRuleExplicitlyDisabled(IAnalysisContext context, string ru context.RuntimeErrors |= RuntimeConditions.RuleWasExplicitlyDisabled; } + + public static void LogObsoleteOption(IAnalysisContext context, string obsoleteOption, string replacement = null) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + string message; + + if (string.IsNullOrWhiteSpace(replacement)) + { + message = string.Format(CultureInfo.InvariantCulture, + SdkResources.WRN997_ObsoleteOption, + obsoleteOption); + } + else + { + message = string.Format(CultureInfo.InvariantCulture, + SdkResources.WRN997_ObsoleteOptionWithReplacement, + obsoleteOption, + replacement); + } + + context.Logger.LogConfigurationNotification( + new Notification + { + Descriptor = new ReportingDescriptorReference + { + Id = Wrn997_ObsoleteOption + }, + Message = new Message { Text = message }, + Level = FailureLevel.Warning, + }); + + context.RuntimeErrors |= RuntimeConditions.ObsoleteOption; + } } } diff --git a/src/Test.UnitTests.Sarif/WarningsTests.cs b/src/Test.UnitTests.Sarif/WarningsTests.cs index cae981748..544499d0c 100644 --- a/src/Test.UnitTests.Sarif/WarningsTests.cs +++ b/src/Test.UnitTests.Sarif/WarningsTests.cs @@ -4,6 +4,8 @@ using FluentAssertions; +using Moq; + using Xunit; namespace Microsoft.CodeAnalysis.Sarif @@ -26,5 +28,26 @@ public void Warnings_LogRuleWasExplicitlyDisabled() testLogger.ConfigurationNotifications.Count.Should().Equals(1); testLogger.ConfigurationNotifications[0].Descriptor.Id.Should().Equals(ruleId); } + + [Fact] + public void Warnings_ObsoleteOption() + { + var testContext = new Mock(); + var testLogger = new Mock(); + + testLogger.Setup(x => x.LogConfigurationNotification(It.IsAny())); + testContext.SetupProperty(x => x.RuntimeErrors); + testContext.Setup(x => x.Logger).Returns(testLogger.Object); + + Warnings.LogObsoleteOption(testContext.Object, "testObsoleteOption"); + + testLogger.Verify(x => x.LogConfigurationNotification(It.IsAny()), Times.Once); + Assert.Equal(RuntimeConditions.ObsoleteOption, testContext.Object.RuntimeErrors); + // We don't verify the specifics of the output string + + Warnings.LogObsoleteOption(testContext.Object, "testObsoleteOption", "testReplacement"); + testLogger.Verify(x => x.LogConfigurationNotification(It.IsAny()), Times.Exactly(2)); + Assert.Equal(RuntimeConditions.ObsoleteOption, testContext.Object.RuntimeErrors); + } } }