Skip to content

Commit

Permalink
Move code from BinSkim, refactor, add unit tests (#2231)
Browse files Browse the repository at this point in the history
* Handle --hashes and dd warning

* Remove debug exception

* Fix null context

* Correct issue with null _cacheByFileHashLogger

* Correct string

* Formatting, add pragma
  • Loading branch information
jameswinkler committed Jan 12, 2021
1 parent 853787c commit bc5df7b
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 1 deletion.
21 changes: 21 additions & 0 deletions src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<OptionallyEmittedData>()
.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);
Expand Down
3 changes: 2 additions & 1 deletion src/Sarif/RuntimeConditions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down Expand Up @@ -57,6 +57,7 @@ public enum RuntimeConditions : int
TargetNotValidToAnalyze = 0x10000000,
OneOrMoreWarningsFired = 0x20000000,
OneOrMoreErrorsFired = 0x40000000,
ObsoleteOption = 0x80000000,

Nonfatal = 0x7FF00000
}
Expand Down
27 changes: 27 additions & 0 deletions src/Sarif/SdkResources.Designer.cs

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

9 changes: 9 additions & 0 deletions src/Sarif/SdkResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -263,4 +263,13 @@ You can also refer to properties in the result's property bag with 'properties.&
<data name="ErrorInvalidQueryPropertyPrefix" xml:space="preserve">
<value>Property name must start with one of: {0}</value>
</data>
<data name="WRN997_ObsoleteOption" xml:space="preserve">
<value>Option '{0}' is obsolete.</value>
</data>
<data name="WRN997_ObsoleteOptionWithReplacement" xml:space="preserve">
<value>Option '{0}' is obsolete. Use '{1}' instead.</value>
</data>
<data name="ComputeFileHashes_ReplaceInsertHashes" xml:space="preserve">
<value>--insert Hashes</value>
</data>
</root>
44 changes: 44 additions & 0 deletions src/Sarif/Warnings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
}
}
23 changes: 23 additions & 0 deletions src/Test.UnitTests.Sarif/WarningsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

using FluentAssertions;

using Moq;

using Xunit;

namespace Microsoft.CodeAnalysis.Sarif
Expand All @@ -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<IAnalysisContext>();
var testLogger = new Mock<IAnalysisLogger>();

testLogger.Setup(x => x.LogConfigurationNotification(It.IsAny<Notification>()));
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<Notification>()), 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<Notification>()), Times.Exactly(2));
Assert.Equal(RuntimeConditions.ObsoleteOption, testContext.Object.RuntimeErrors);
}
}
}

0 comments on commit bc5df7b

Please sign in to comment.