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

New AdvSec & GHAS - Enable using config file #2775

Merged
merged 1 commit into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Sarif.Driver/Sdk/AnalyzeOptionsBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ public IEnumerable<RuleKind> RuleKindOption
set => this.ruleKindOption = value?.Count() > 0 ? value : null;
}

public HashSet<RuleKind> RuleKinds => RuleKindOption != null ?
new HashSet<RuleKind>(RuleKindOption) :
new HashSet<RuleKind>(new[] { RuleKind.Sarif });
public RuleKindSet RuleKinds => RuleKindOption != null ?
new RuleKindSet(RuleKindOption) :
new RuleKindSet(new List<RuleKind>(new[] { RuleKind.Sarif }));
}
}
2 changes: 1 addition & 1 deletion src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ public virtual TContext InitializeGlobalContextFromOptions(TOptions options, ref
context.TargetFileSpecifiers = options.TargetFileSpecifiers?.Any() == true ? InitializeStringSet(options.TargetFileSpecifiers) : context.TargetFileSpecifiers;
context.InvocationPropertiesToLog = options.InvocationPropertiesToLog?.Any() == true ? InitializeStringSet(options.InvocationPropertiesToLog) : context.InvocationPropertiesToLog;
context.Traces = options.Trace.Any() ? InitializeStringSet(options.Trace) : context.Traces;
context.RuleKinds = options.RuleKinds;
context.RuleKinds = options.RuleKindOption != null ? options.RuleKinds : context.RuleKinds;
Copy link
Collaborator

@EasyRhinoMSFT EasyRhinoMSFT Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options.RuleKindOption != null ? options.RuleKinds : context.RuleKinds

Can we use this simpler syntax?

options.RuleKindOption ?? context.RuleKinds #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no we can not,
options.RuleKindOption is not the same type as options.RuleKinds

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you see lines above
context.ResultKinds = options.Kind != null ? options.ResultKinds : context.ResultKinds;
Some tricky here, the options.Kind is not the same type as options.ResultKinds.

p.s. I think the naming before is not good for Kind for ResultKinds , it is better to name using xxxOption, or some pattern. People will not know Kind is the command line option for ResultKinds

}

// Less-obviously throw-safe. We don't do these in the finally block because we'd prefer not to mask an earlier Exception during logger initialization.
Expand Down
14 changes: 13 additions & 1 deletion src/Sarif/AnalyzeContextBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public virtual IEnumerable<IOption> GetOptions()
PostUriProperty,
RecurseProperty,
ResultKindsProperty,
RuleKindsProperty,
RichReturnCodeProperty,
TargetFileSpecifiersProperty,
ThreadsProperty,
Expand All @@ -68,7 +69,6 @@ public virtual IEnumerable<IOption> GetOptions()
public virtual IList<Exception> RuntimeExceptions { get; set; }
public virtual bool IsValidAnalysisTarget { get; set; }
public virtual ReportingDescriptor Rule { get; set; }
public HashSet<RuleKind> RuleKinds { get; set; } = new HashSet<RuleKind>();
public PropertiesDictionary Policy { get; set; }
public virtual IAnalysisLogger Logger { get; set; }
public virtual RuntimeConditions RuntimeErrors { get; set; }
Expand Down Expand Up @@ -202,6 +202,12 @@ public ResultKindSet ResultKinds
set => this.Policy.SetProperty(ResultKindsProperty, value);
}

public RuleKindSet RuleKinds
{
get => this.Policy.GetProperty(RuleKindsProperty);
set => this.Policy.SetProperty(RuleKindsProperty, value);
}

public virtual bool RichReturnCode
{
get => this.Policy.GetProperty(RichReturnCodeProperty);
Expand Down Expand Up @@ -352,6 +358,12 @@ public virtual void Dispose()
"One or more result kinds to persist to loggers. Valid values include None, NotApplicable, Pass, Fail, " +
"Review, Open, Informational. Defaults to 'Fail'.");

public static PerLanguageOption<RuleKindSet> RuleKindsProperty { get; } =
new PerLanguageOption<RuleKindSet>(
"CoreSettings", nameof(RuleKinds), defaultValue: () => new RuleKindSet(new[] { RuleKind.Sarif }),
"One or more rule kinds that should be run. Valid values include Sarif, Ado, Ghas. " +
"Defaults to 'Sarif'.");

public static PerLanguageOption<FilePersistenceOptions> OutputFileOptionsProperty { get; } =
new PerLanguageOption<FilePersistenceOptions>(
"CoreSettings", nameof(OutputFileOptions), defaultValue: () => FilePersistenceOptions.PrettyPrint,
Expand Down
2 changes: 2 additions & 0 deletions src/Sarif/IAnalysisContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public interface IAnalysisContext : IDisposable

ResultKindSet ResultKinds { get; set; }

RuleKindSet RuleKinds { get; set; }

public ISet<string> InsertProperties { get; set; }

OptionallyEmittedData DataToInsert { get; set; }
Expand Down
35 changes: 25 additions & 10 deletions src/Sarif/PropertiesDictionaryExtensionMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,35 +80,42 @@ public static void SavePropertiesToXmlStream(
}
}

StringSet stringSet = property as StringSet;
var stringSet = property as StringSet;
if (stringSet != null)
{
SaveSet(writer, stringSet, key);
continue;
}

IntegerSet integerSet = property as IntegerSet;
var integerSet = property as IntegerSet;
if (integerSet != null)
{
SaveSet(writer, integerSet, key);
continue;
}

FailureLevelSet failureLevelSet = property as FailureLevelSet;
var failureLevelSet = property as FailureLevelSet;
if (failureLevelSet != null)
{
SaveSet(writer, failureLevelSet, key);
continue;
}

ResultKindSet resultKindSet = property as ResultKindSet;
var resultKindSet = property as ResultKindSet;
if (resultKindSet != null)
{
SaveSet(writer, resultKindSet, key);
continue;
}

IDictionary pb = property as IDictionary;
var ruleKindSet = property as RuleKindSet;
if (ruleKindSet != null)
{
SaveSet(writer, ruleKindSet, key);
continue;
}

var pb = property as IDictionary;
if (pb != null)
{
pb.SavePropertiesToXmlStream(writer, settings, key, settingNameToDescriptionMap);
Expand Down Expand Up @@ -158,7 +165,7 @@ private static void SaveSet<T>(XmlWriter writer, HashSet<T> items, string key)
writer.WriteAttributeString(KEY_ID, key);
writer.WriteAttributeString(TYPE_ID, items.GetType().Name);

T[] sorted = new T[items.Count];
var sorted = new T[items.Count];
items.CopyTo(sorted, 0);
Array.Sort(sorted);

Expand Down Expand Up @@ -233,29 +240,36 @@ public static void LoadPropertiesFromXmlStream(this IDictionary propertyBag, Xml
if (typeName == STRING_SET_ID ||
typeName == INTEGER_SET_ID ||
typeName == RESULT_KIND_SET_ID ||
typeName == RULE_KIND_SET_ID ||
typeName == FAILURE_LEVEL_SET_ID)
{
if (typeName == STRING_SET_ID)
{
StringSet set = new StringSet();
var set = new StringSet();
propertyBag[key] = set;
LoadSet(set, reader);
}
else if (typeName == INTEGER_SET_ID)
{
IntegerSet set = new IntegerSet();
var set = new IntegerSet();
propertyBag[key] = set;
LoadSet(set, reader);
}
else if (typeName == FAILURE_LEVEL_SET_ID)
{
FailureLevelSet set = new FailureLevelSet();
var set = new FailureLevelSet();
propertyBag[key] = set;
LoadSet(set, reader);
}
else if (typeName == RESULT_KIND_SET_ID)
{
var set = new ResultKindSet();
propertyBag[key] = set;
LoadSet(set, reader);
}
else
{
ResultKindSet set = new ResultKindSet();
var set = new RuleKindSet();
propertyBag[key] = set;
LoadSet(set, reader);
}
Expand Down Expand Up @@ -359,6 +373,7 @@ private static Type GetType(string typeName)
private const string PROPERTY_ID = "Property";
private const string STRING_SET_ID = "StringSet";
private const string INTEGER_SET_ID = "IntegerSet";
private const string RULE_KIND_SET_ID = "RuleKindSet";
private const string RESULT_KIND_SET_ID = "ResultKindSet";
private const string FAILURE_LEVEL_SET_ID = "FailureLevelSet";
internal const string PROPERTIES_ID = "Properties";
Expand Down
25 changes: 25 additions & 0 deletions src/Sarif/RuleKindSet.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// 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.Collections.Generic;
using System.Runtime.Serialization;

using Newtonsoft.Json;

namespace Microsoft.CodeAnalysis.Sarif
{
[Serializable]
[JsonConverter(typeof(TypedPropertiesDictionaryConverter))]
public class RuleKindSet : HashSet<RuleKind>
Copy link
Collaborator

@EasyRhinoMSFT EasyRhinoMSFT Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RuleKindSet

Why couldn't we use a HashSet? #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is following the same pattern as other fields we have, the ResultKind.
The commanline nuget package the options have some limitation so there is some conversions needed

{
public RuleKindSet() { }

public RuleKindSet(IEnumerable<RuleKind> values) : base(values) { }

protected RuleKindSet(SerializationInfo info, StreamingContext context)
: base(info, context)
{
}
}
}
10 changes: 10 additions & 0 deletions src/Sarif/TypedPropertiesDictionaryConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
ja = JArray.Load(reader);
return new IntegerSet(ja.Values().Select(token => int.Parse(token.ToString())));
}
else if (objectType == typeof(RuleKindSet))
{
ja = JArray.Load(reader);
return new IntegerSet(ja.Values().Select(token => int.Parse(token.ToString())));
}
else if (objectType == typeof(Version))
{
return JsonConvert.DeserializeObject<Version>(reader.ReadAsString(), _versionConverter);
Expand Down Expand Up @@ -131,6 +136,11 @@ public override void WriteJson(JsonWriter writer, object value, JsonSerializer s
ja = new JArray(resultKindSet.Select(i => new JValue(i)));
ja.WriteTo(writer);
}
else if (value is RuleKindSet ruleKindSet)
{
ja = new JArray(ruleKindSet.Select(i => new JValue(i)));
ja.WriteTo(writer);
}
else
{
var dictionary = (IDictionary)value;
Expand Down
Loading