Skip to content

Commit

Permalink
Fixing AnalyzeCommandBase and MultithreadedAnalyzeCommandBase art…
Browse files Browse the repository at this point in the history 
…ifacts generation (#2433)

* Fixing multithreaded artifacts generation

* Fixing tests and flags

* Loosing precision.

* Applying fix for AnalyzeCommandBase

* Enabling tests

* Updating test case and release history

* Creating const to prevent magical numbers everywhere

* Rebaselining tests

* Creating Artifacts flag to keep previous behavior

* Addressing PR feedback.

* Rollback changes

* Update SARIF2012.ProvideRuleProperties_Invalid.sarif

* updating back

* Ordering deprecated names
  • Loading branch information
@eddynaka @yongyan-gh
eddynaka authored and yongyan-gh committed Feb 8, 2022
1 parent cfebb85 commit 351ef43
Show file tree
Hide file tree
Showing 11 changed files with 162 additions and 106 deletions.
1 change: 1 addition & 0 deletions src/ReleaseHistory.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

* BREAKING: `AnalyzeCommandBase` previously persisted all scan target artifacts to SARIF logs rather than only persisting artifacts referenced by an analysis result, when an option to persist hashes, text file or binary information was set. `MultithreadedAnalyzeCommandBase` previously persisted all scan targets artifacts to SARIF logs in cases when hash insertion was eenabled rather than only persisting artifacts referenced by an analysis result. [#2433](https://github.com/microsoft/sarif-sdk/pull/2433)
* BUGFIX: Adjust Json Serialization field order for ReportingDescriptor and skip emit empty AutomationDetails node. [#2420](https://github.com/microsoft/sarif-sdk/pull/2420)
* BREAKING: Fix `InvalidOperationException` when using PropertiesDictionary in a multithreaded application, and remove `[Serializable]` from it. Now use of BinaryFormatter on it will result in `SerializationException`: Type `PropertiesDictionary` is not marked as serializable. [#2415](https://github.com/microsoft/sarif-sdk/pull/2415)

Expand Down
9 changes: 5 additions & 4 deletions src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ private void Analyze(TOptions options, AggregatingLogger logger)
targets = ValidateTargetsExist(_rootContext, targets);

// 5. Initialize report file, if configured.
InitializeOutputFile(options, _rootContext, targets);
InitializeOutputFile(options, _rootContext);

// 6. Instantiate skimmers.
ISet<Skimmer<TContext>> skimmers = CreateSkimmers(options, _rootContext);
Expand Down Expand Up @@ -294,6 +294,7 @@ protected virtual TContext CreateContext(
else
{
context.Hashes = HashUtilities.ComputeHashes(filePath);
_pathToHashDataMap?.Add(filePath, context.Hashes);
}
}
}
Expand Down Expand Up @@ -352,7 +353,7 @@ protected virtual void InitializeConfiguration(TOptions options, TContext contex
}
}

private void InitializeOutputFile(TOptions analyzeOptions, TContext context, ISet<string> targets)
private void InitializeOutputFile(TOptions analyzeOptions, TContext context)
{
string filePath = analyzeOptions.OutputFilePath;
AggregatingLogger aggregatingLogger = (AggregatingLogger)context.Logger;
Expand Down Expand Up @@ -388,7 +389,7 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context, ISe
dataToRemove,
tool: _tool,
run: _run,
analysisTargets: targets,
analysisTargets: null,
quiet: analyzeOptions.Quiet,
invocationTokensToRedact: GenerateSensitiveTokensList(),
invocationPropertiesToLog: analyzeOptions.InvocationPropertiesToLog,
Expand All @@ -404,7 +405,7 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context, ISe
dataToRemove,
tool: _tool,
run: _run,
analysisTargets: targets,
analysisTargets: null,
invocationTokensToRedact: GenerateSensitiveTokensList(),
invocationPropertiesToLog: analyzeOptions.InvocationPropertiesToLog,
levels: analyzeOptions.Level,
Expand Down
7 changes: 3 additions & 4 deletions src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public abstract class MultithreadedAnalyzeCommandBase<TContext, TOptions> : Plug
private Channel<int> _resultsWritingChannel;
private Channel<int> _fileEnumerationChannel;
private Dictionary<string, List<string>> _hashToFilesMap;
private IDictionary<string, HashData> _pathToHashDataMap;
private ConcurrentDictionary<int, TContext> _fileContexts;

public Exception ExecutionException { get; set; }
Expand Down Expand Up @@ -485,12 +486,9 @@ private async Task<bool> HashAsync()
_hashToFilesMap[hashData.Sha256] = paths;
}

_run?.GetFileIndex(new ArtifactLocation { Uri = context.TargetUri },
dataToInsert: _dataToInsert,
hashData: hashData);

paths.Add(localPath);
context.Hashes = hashData;
_pathToHashDataMap?.Add(localPath, hashData);
}

await _fileEnumerationChannel.Writer.WriteAsync(index);
Expand Down Expand Up @@ -705,6 +703,7 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context)
kinds: analyzeOptions.Kind,
insertProperties: analyzeOptions.InsertProperties);
}
_pathToHashDataMap = sarifLogger.AnalysisTargetToHashDataMap;
sarifLogger.AnalysisStarted();
aggregatingLogger.Loggers.Add(sarifLogger);
},
Expand Down
26 changes: 13 additions & 13 deletions src/Sarif/Autogenerated/ReportingDescriptor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,31 +49,31 @@ public SarifNodeKind SarifNodeKind
/// <summary>
/// An array of stable, opaque identifiers by which this report was known in some previous version of the analysis tool.
/// </summary>
[DataMember(Name = "deprecatedIds", IsRequired = false, EmitDefaultValue = false, Order = 8)]
[DataMember(Name = "deprecatedIds", IsRequired = false, EmitDefaultValue = false, Order = 9)]
public virtual IList<string> DeprecatedIds { get; set; }

/// <summary>
/// A unique identifier for the reporting descriptor in the form of a GUID.
/// </summary>
[DataMember(Name = "guid", IsRequired = false, EmitDefaultValue = false, Order = 9)]
[DataMember(Name = "guid", IsRequired = false, EmitDefaultValue = false, Order = 10)]
public virtual string Guid { get; set; }

/// <summary>
/// An array of unique identifies in the form of a GUID by which this report was known in some previous version of the analysis tool.
/// </summary>
[DataMember(Name = "deprecatedGuids", IsRequired = false, EmitDefaultValue = false, Order = 10)]
[DataMember(Name = "deprecatedGuids", IsRequired = false, EmitDefaultValue = false, Order = 11)]
public virtual IList<string> DeprecatedGuids { get; set; }

/// <summary>
/// A report identifier that is understandable to an end user.
/// </summary>
[DataMember(Name = "name", IsRequired = false, EmitDefaultValue = false, Order = 7)]
[DataMember(Name = "name", IsRequired = false, EmitDefaultValue = false, Order = 2)]
public virtual string Name { get; set; }

/// <summary>
/// An array of readable identifiers by which this report was known in some previous version of the analysis tool.
/// </summary>
[DataMember(Name = "deprecatedNames", IsRequired = false, EmitDefaultValue = false, Order = 11)]
[DataMember(Name = "deprecatedNames", IsRequired = false, EmitDefaultValue = false, Order = 3)]
public virtual IList<string> DeprecatedNames { get; set; }

/// <summary>
Expand All @@ -85,46 +85,46 @@ public SarifNodeKind SarifNodeKind
/// <summary>
/// A description of the report. Should, as far as possible, provide details sufficient to enable resolution of any problem indicated by the result.
/// </summary>
[DataMember(Name = "fullDescription", IsRequired = false, EmitDefaultValue = false, Order = 2)]
[DataMember(Name = "fullDescription", IsRequired = false, EmitDefaultValue = false, Order = 4)]
public virtual MultiformatMessageString FullDescription { get; set; }

/// <summary>
/// A set of name/value pairs with arbitrary names. Each value is a multiformatMessageString object, which holds message strings in plain text and (optionally) Markdown format. The strings can include placeholders, which can be used to construct a message in combination with an arbitrary number of additional string arguments.
/// </summary>
[DataMember(Name = "messageStrings", IsRequired = false, EmitDefaultValue = false, Order = 5)]
[DataMember(Name = "messageStrings", IsRequired = false, EmitDefaultValue = false, Order = 6)]
public virtual IDictionary<string, MultiformatMessageString> MessageStrings { get; set; }

/// <summary>
/// Default reporting configuration information.
/// </summary>
[DataMember(Name = "defaultConfiguration", IsRequired = false, EmitDefaultValue = false, Order = 12)]
[DataMember(Name = "defaultConfiguration", IsRequired = false, EmitDefaultValue = false, Order = 13)]
public virtual ReportingConfiguration DefaultConfiguration { get; set; }

/// <summary>
/// A URI where the primary documentation for the report can be found.
/// </summary>
[DataMember(Name = "helpUri", IsRequired = false, EmitDefaultValue = false, Order = 3)]
[DataMember(Name = "helpUri", IsRequired = false, EmitDefaultValue = false, Order = 14)]
[JsonConverter(typeof(Microsoft.CodeAnalysis.Sarif.Readers.UriConverter))]
public virtual Uri HelpUri { get; set; }

/// <summary>
/// Provides the primary documentation for the report, useful when there is no online documentation.
/// </summary>
[DataMember(Name = "help", IsRequired = false, EmitDefaultValue = false, Order = 4)]
[DataMember(Name = "help", IsRequired = false, EmitDefaultValue = false, Order = 5)]
public virtual MultiformatMessageString Help { get; set; }

/// <summary>
/// An array of objects that describe relationships between this reporting descriptor and others.
/// </summary>
[DataMember(Name = "relationships", IsRequired = false, EmitDefaultValue = false, Order = 13)]
[JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate, Order = 13)]
[DataMember(Name = "relationships", IsRequired = false, EmitDefaultValue = false, Order = 15)]
[JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate, Order = 15)]
public virtual IList<ReportingDescriptorRelationship> Relationships { get; set; }

/// <summary>
/// Key/value pairs that provide additional information about the report.
/// </summary>
[JsonProperty(Order = 14)]
[DataMember(Name = "properties", IsRequired = false, EmitDefaultValue = false, Order = 14)]
[DataMember(Name = "properties", IsRequired = false, EmitDefaultValue = false, Order = 16)]
internal override IDictionary<string, SerializedPropertyInfo> Properties { get; set; }

/// <summary>
Expand Down
26 changes: 13 additions & 13 deletions src/Sarif/NotYetAutoGenerated/ReportingDescriptor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,31 +49,31 @@ public SarifNodeKind SarifNodeKind
/// <summary>
/// An array of stable, opaque identifiers by which this report was known in some previous version of the analysis tool.
/// </summary>
[DataMember(Name = "deprecatedIds", IsRequired = false, EmitDefaultValue = false, Order = 8)]
[DataMember(Name = "deprecatedIds", IsRequired = false, EmitDefaultValue = false, Order = 9)]
public virtual IList<string> DeprecatedIds { get; set; }

/// <summary>
/// A unique identifier for the reporting descriptor in the form of a GUID.
/// </summary>
[DataMember(Name = "guid", IsRequired = false, EmitDefaultValue = false, Order = 9)]
[DataMember(Name = "guid", IsRequired = false, EmitDefaultValue = false, Order = 10)]
public virtual string Guid { get; set; }

/// <summary>
/// An array of unique identifies in the form of a GUID by which this report was known in some previous version of the analysis tool.
/// </summary>
[DataMember(Name = "deprecatedGuids", IsRequired = false, EmitDefaultValue = false, Order = 10)]
[DataMember(Name = "deprecatedGuids", IsRequired = false, EmitDefaultValue = false, Order = 11)]
public virtual IList<string> DeprecatedGuids { get; set; }

/// <summary>
/// A report identifier that is understandable to an end user.
/// </summary>
[DataMember(Name = "name", IsRequired = false, EmitDefaultValue = false, Order = 7)]
[DataMember(Name = "name", IsRequired = false, EmitDefaultValue = false, Order = 2)]
public virtual string Name { get; set; }

/// <summary>
/// An array of readable identifiers by which this report was known in some previous version of the analysis tool.
/// </summary>
[DataMember(Name = "deprecatedNames", IsRequired = false, EmitDefaultValue = false, Order = 11)]
[DataMember(Name = "deprecatedNames", IsRequired = false, EmitDefaultValue = false, Order = 3)]
public virtual IList<string> DeprecatedNames { get; set; }

/// <summary>
Expand All @@ -85,46 +85,46 @@ public SarifNodeKind SarifNodeKind
/// <summary>
/// A description of the report. Should, as far as possible, provide details sufficient to enable resolution of any problem indicated by the result.
/// </summary>
[DataMember(Name = "fullDescription", IsRequired = false, EmitDefaultValue = false, Order = 2)]
[DataMember(Name = "fullDescription", IsRequired = false, EmitDefaultValue = false, Order = 4)]
public virtual MultiformatMessageString FullDescription { get; set; }

/// <summary>
/// A set of name/value pairs with arbitrary names. Each value is a multiformatMessageString object, which holds message strings in plain text and (optionally) Markdown format. The strings can include placeholders, which can be used to construct a message in combination with an arbitrary number of additional string arguments.
/// </summary>
[DataMember(Name = "messageStrings", IsRequired = false, EmitDefaultValue = false, Order = 5)]
[DataMember(Name = "messageStrings", IsRequired = false, EmitDefaultValue = false, Order = 6)]
public virtual IDictionary<string, MultiformatMessageString> MessageStrings { get; set; }

/// <summary>
/// Default reporting configuration information.
/// </summary>
[DataMember(Name = "defaultConfiguration", IsRequired = false, EmitDefaultValue = false, Order = 12)]
[DataMember(Name = "defaultConfiguration", IsRequired = false, EmitDefaultValue = false, Order = 13)]
public virtual ReportingConfiguration DefaultConfiguration { get; set; }

/// <summary>
/// A URI where the primary documentation for the report can be found.
/// </summary>
[DataMember(Name = "helpUri", IsRequired = false, EmitDefaultValue = false, Order = 3)]
[DataMember(Name = "helpUri", IsRequired = false, EmitDefaultValue = false, Order = 14)]
[JsonConverter(typeof(Microsoft.CodeAnalysis.Sarif.Readers.UriConverter))]
public virtual Uri HelpUri { get; set; }

/// <summary>
/// Provides the primary documentation for the report, useful when there is no online documentation.
/// </summary>
[DataMember(Name = "help", IsRequired = false, EmitDefaultValue = false, Order = 4)]
[DataMember(Name = "help", IsRequired = false, EmitDefaultValue = false, Order = 5)]
public virtual MultiformatMessageString Help { get; set; }

/// <summary>
/// An array of objects that describe relationships between this reporting descriptor and others.
/// </summary>
[DataMember(Name = "relationships", IsRequired = false, EmitDefaultValue = false, Order = 13)]
[JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate, Order = 13)]
[DataMember(Name = "relationships", IsRequired = false, EmitDefaultValue = false, Order = 15)]
[JsonProperty(DefaultValueHandling = DefaultValueHandling.IgnoreAndPopulate, Order = 15)]
public virtual IList<ReportingDescriptorRelationship> Relationships { get; set; }

/// <summary>
/// Key/value pairs that provide additional information about the report.
/// </summary>
[JsonProperty(Order = 14)]
[DataMember(Name = "properties", IsRequired = false, EmitDefaultValue = false, Order = 14)]
[DataMember(Name = "properties", IsRequired = false, EmitDefaultValue = false, Order = 16)]
internal override IDictionary<string, SerializedPropertyInfo> Properties { get; set; }

/// <summary>
Expand Down
Loading

0 comments on commit 351ef43

Please sign in to comment.