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.

Sign up for GitHub

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

Jump to bottom

Reduced files array build #1191

Merged
merged 13 commits into from
Dec 31, 2018
Merged

Reduced files array build #1191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
e56f46e
Sarif and Sarif.Converters now building
michaelcfanning Dec 29, 2018
24fc6be
Files array (#1188)
michaelcfanning Dec 29, 2018
6de4aee
DRY out converters to isolate shared code.
michaelcfanning Dec 29, 2018
c296771
Merge from files array branch
michaelcfanning Dec 29, 2018
9b7a6c4
Merge branch 'reduced-files-array-build' of https://github.com/micros…
michaelcfanning Dec 29, 2018
b9b5a15
Restore essential change in schema that converts files dictionary to …
michaelcfanning Dec 29, 2018
7ec751b
Simplify ShouldSerialize logic
michaelcfanning Dec 29, 2018
346bc1d
Remove unused namespaces
michaelcfanning Dec 29, 2018
0c3e250
Respond to PR feedback.
michaelcfanning Dec 31, 2018
5208c43
Respond to PR feedback
michaelcfanning Dec 31, 2018
5a29dd7
Merge branch 'files-array' into reduced-files-array-build
michaelcfanning Dec 31, 2018
43480b0
Further PR feedback.
michaelcfanning Dec 31, 2018
8d505f0
Lost PR feedback recovered.
michaelcfanning Dec 31, 2018
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
38 changes: 8 additions & 30 deletions src/Sarif.Converters/AndroidStudioConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Collections.Immutable;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Text;
using System.Xml;

Expand Down Expand Up @@ -59,41 +58,20 @@ public override void Convert(Stream input, IResultLogWriter output, OptionallyEm
XmlResolver = null
};

ISet<Result> results;
IList<Result> results;
using (XmlReader xmlReader = XmlReader.Create(input, settings))
{
results = ProcessAndroidStudioLog(xmlReader);
}

var tool = new Tool
{
Name = "AndroidStudio"
};

var fileInfoFactory = new FileInfoFactory(null, dataToInsert);
Dictionary<string, FileData> fileDictionary = fileInfoFactory.Create(results);


var run = new Run()
{
Tool = tool,
ColumnKind = ColumnKind.Utf16CodeUnits
Tool = new Tool { Name = ToolFormat.AndroidStudio },
Copy link

@ghost ghost Dec 29, 2018 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

Tool [](start = 27, length = 4)

Did you also mean to remove the local variable tool? #Closed

Copy link

Choose a reason for hiding this comment

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

This is marked "Resolved" but the local variable tool on Line 67 is still there.

In reply to: 244519901 [](ancestors = 244519901)

ColumnKind = ColumnKind.Utf16CodeUnits,
LogicalLocations = this.LogicalLocations,
};

output.Initialize(run);

if (fileDictionary != null && fileDictionary.Any())
{
output.WriteFiles(fileDictionary);
}

if (LogicalLocations != null && LogicalLocations.Any())
{
output.WriteLogicalLocations(LogicalLocations);
}

output.OpenResults();
output.WriteResults(results);
output.CloseResults();
PersistResults(output, results, run);
}

/// <summary>Processes an Android Studio log and writes issues therein to an instance of
Expand All @@ -102,9 +80,9 @@ public override void Convert(Stream input, IResultLogWriter output, OptionallyEm
/// <returns>
/// A list of the <see cref="Result"/> objects translated from the AndroidStudio format.
/// </returns>
private ISet<Result> ProcessAndroidStudioLog(XmlReader xmlReader)
private IList<Result> ProcessAndroidStudioLog(XmlReader xmlReader)
{
var results = new HashSet<Result>(Result.ValueComparer);
var results = new List<Result>();

int problemsDepth = xmlReader.Depth;
xmlReader.ReadStartElement(_strings.Problems);
Expand Down
25 changes: 2 additions & 23 deletions src/Sarif.Converters/ClangAnalyzerConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Collections.Generic;
using System.IO;
using System.Xml;
using Microsoft.CodeAnalysis.Sarif.Writers;

namespace Microsoft.CodeAnalysis.Sarif.Converters
{
Expand Down Expand Up @@ -58,27 +57,7 @@ public override void Convert(Stream input, IResultLogWriter output, OptionallyEm
}
}

var tool = new Tool
{
Name = "Clang"
};

var fileInfoFactory = new FileInfoFactory(MimeType.DetermineFromFileExtension, dataToInsert);
Dictionary<string, FileData> fileDictionary = fileInfoFactory.Create(results);


var run = new Run()
{
Tool = tool
};

output.Initialize(run);

if (fileDictionary != null && fileDictionary.Count > 0) { output.WriteFiles(fileDictionary); }

output.OpenResults();
output.WriteResults(results);
output.CloseResults();
PersistResults(output, results, "Clang Analyzer");
}
finally
{
Expand Down Expand Up @@ -155,7 +134,7 @@ private Result CreateResult(IDictionary<string, object> issueData)
int fileNumber = FindInt(location, "file");
if (_files != null && fileNumber < _files.Count)
{
fileName = _files[fileNumber] as string;
fileName = (string)_files[fileNumber];
}
}

Expand Down
21 changes: 2 additions & 19 deletions src/Sarif.Converters/CppCheckConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
using System.IO;
using System.Xml;

using Microsoft.CodeAnalysis.Sarif.Writers;

namespace Microsoft.CodeAnalysis.Sarif.Converters
{
public class CppCheckConverter : ToolFileConverterBase
Expand Down Expand Up @@ -114,27 +112,12 @@ private void ProcessCppCheckLog(XmlReader reader, IResultLogWriter output, Optio

reader.ReadEndElement(); // </results>

var tool = new Tool
{
Name = "CppCheck",
Version = version,
};

var fileInfoFactory = new FileInfoFactory(uri => MimeType.Cpp, dataToInsert);
Dictionary<string, FileData> fileDictionary = fileInfoFactory.Create(results);

var run = new Run()
{
Tool = tool
Tool = new Tool { Name = "CppCheck", Version = version }
};

output.Initialize(run);

if (fileDictionary != null && fileDictionary.Count > 0) { output.WriteFiles(fileDictionary); }

output.OpenResults();
output.WriteResults(results);
output.CloseResults();
PersistResults(output, results, run);
}
}
}
19 changes: 2 additions & 17 deletions src/Sarif.Converters/FortifyConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.IO;
using System.Linq;
using System.Xml;
using Microsoft.CodeAnalysis.Sarif.Writers;

namespace Microsoft.CodeAnalysis.Sarif.Converters
{
Expand Down Expand Up @@ -85,14 +84,6 @@ public override void Convert(Stream input, IResultLogWriter output, OptionallyEm
}
}

var tool = new Tool
{
Name = "Fortify"
};

var fileInfoFactory = new FileInfoFactory(MimeType.DetermineFromFileExtension, dataToInsert);
Dictionary<string, FileData> fileDictionary = fileInfoFactory.Create(results);

var run = new Run()
{
Id = new RunAutomationDetails
Expand All @@ -102,16 +93,10 @@ public override void Convert(Stream input, IResultLogWriter output, OptionallyEm
Text = runDescription
}
},
Tool = tool
Tool = new Tool { Name = "Fortify" }
Copy link

@ghost ghost Dec 30, 2018 •
edited by michaelcfanning
Loading

Choose a reason for hiding this comment

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

Fortify [](start = 43, length = 7)

Right, here's a case where you can't use the format for the tool name, because there are two formats. Since you can't do it always, I think you should never do it; it's more or less an accident if the format name happens to match the tool name. Besides, the tool's preferred name might not even be a valid identifier (e.g., "Clang Analyzer").

I'll stop harping on this now. #Resolved

};

output.Initialize(run);

if (fileDictionary != null && fileDictionary.Count > 0) { output.WriteFiles(fileDictionary); }

output.OpenResults();
output.WriteResults(results);
output.CloseResults();
PersistResults(output, results, run);
}

/// <summary>Converts a Fortify result to a static analysis results interchange format result.</summary>
Expand Down
38 changes: 8 additions & 30 deletions src/Sarif.Converters/FortifyFprConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,8 @@ internal class FortifyFprConverter : ToolFileConverterBase
private string _automationId;
private string _originalUriBasePath;
private List<Result> _results = new List<Result>();
private List<Notification> _toolNotifications;
private Dictionary<string, FileData> _fileDictionary;
private Dictionary<string, IRule> _ruleDictionary;
private Dictionary<string, Rule> _ruleDictionary;
private Dictionary<ThreadFlowLocation, string> _tflToNodeIdDictionary;
private Dictionary<ThreadFlowLocation, string> _tflToSnippetIdDictionary;
private Dictionary<Location, string> _locationToSnippetIdDictionary;
Expand All @@ -50,9 +49,8 @@ public FortifyFprConverter()
_strings = new FortifyFprStrings(_nameTable);

_results = new List<Result>();
_toolNotifications = new List<Notification>();
_fileDictionary = new Dictionary<string, FileData>();
_ruleDictionary = new Dictionary<string, IRule>();
_ruleDictionary = new Dictionary<string, Rule>();
_tflToNodeIdDictionary = new Dictionary<ThreadFlowLocation, string>();
_tflToSnippetIdDictionary = new Dictionary<ThreadFlowLocation, string>();
_locationToSnippetIdDictionary = new Dictionary<Location, string>();
Expand Down Expand Up @@ -89,8 +87,8 @@ public override void Convert(Stream input, IResultLogWriter output, OptionallyEm
};

_invocation = new Invocation();
_invocation.ToolNotifications = new List<Notification>();
_results.Clear();
_toolNotifications.Clear();
_fileDictionary.Clear();
_ruleDictionary.Clear();
_tflToNodeIdDictionary.Clear();
Expand All @@ -116,7 +114,8 @@ public override void Convert(Stream input, IResultLogWriter output, OptionallyEm
InstanceId = _automationId + "/"
},
Tool = tool,
Invocations = new[] { _invocation }
Invocations = new[] { _invocation },
Resources = new Resources { Rules = _ruleDictionary}
Copy link

@ghost ghost Dec 30, 2018 •
edited by michaelcfanning
Loading

Choose a reason for hiding this comment

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

Rules [](start = 45, length = 5)

Doesn't this mean you'll emit

"resources": {
  "rules": {}
}

if the dictionary is empty, rather than omitting the "rules" object? (In fact, rather than omitting "resources" entirely?) How about:

var run = new Run
{
    ...
};

if (_ruleDictionary.Any())
{
    run.Resources = new Resources { Rules = _ruleDictionary };
}
``` #WontFix

Copy link
Member Author

Choose a reason for hiding this comment

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

our deserialization logic can elide the resources object in cases where its contained strings and rules are empty, is my thought. this keeps the initialization logic in one place. eventually, we should consider having the OM initialize core objects automatically, to make object creation more concise. will require building out the ShouldSerializeXXX helpers exhaustively. All this is one reason I built that helper that automatically hydrates the OM with all possible items. This will help comprehensively test the helpers described above.

In reply to: 244521205 [](ancestors = 244521205)

};

if (!string.IsNullOrWhiteSpace(_originalUriBasePath))
Expand All @@ -128,28 +127,7 @@ public override void Convert(Stream input, IResultLogWriter output, OptionallyEm
};
}

output.Initialize(run);

(output as ResultLogJsonWriter).WriteInvocations(run.Invocations);

if (_fileDictionary.Any())
{
output.WriteFiles(_fileDictionary);
}

output.OpenResults();
output.WriteResults(_results);
output.CloseResults();

if (_ruleDictionary.Any())
{
output.WriteRules(_ruleDictionary);
}

if (_toolNotifications.Any())
{
output.WriteToolNotifications(_toolNotifications);
}
PersistResults(output, _results, run);
}

private void ParseFprFile(Stream input)
Expand Down Expand Up @@ -871,7 +849,7 @@ private void ParseErrors()
string errorCode = _reader.GetAttribute(_strings.CodeAttribute);
string message = _reader.ReadElementContentAsString();

_toolNotifications.Add(new Notification
_invocation.ToolNotifications.Add(new Notification
{
Id = errorCode,
Level = NotificationLevel.Error,
Expand Down Expand Up @@ -930,7 +908,7 @@ private void AddMessagesToResults()
{
foreach (Result result in _results)
{
IRule rule;
Rule rule;
if (_ruleDictionary.TryGetValue(result.RuleId, out rule))
{
Message message = rule.ShortDescription ?? rule.FullDescription;
Expand Down
36 changes: 8 additions & 28 deletions src/Sarif.Converters/FxCopConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using System.Reflection;
using System.Xml;
using System.Xml.Schema;
using Microsoft.CodeAnalysis.Sarif.Writers;

namespace Microsoft.CodeAnalysis.Sarif.Converters
{
Expand Down Expand Up @@ -53,46 +52,27 @@ public override void Convert(Stream input, IResultLogWriter output, OptionallyEm
reader.ResultRead += (FxCopLogReader.Context current) => { results.Add(CreateResult(current)); };
reader.Read(context, input);

Tool tool = new Tool
{
Name = "FxCop"
};

var fileInfoFactory = new FileInfoFactory(MimeType.DetermineFromFileExtension, dataToInsert);
Dictionary<string, FileData> fileDictionary = fileInfoFactory.Create(results);

var run = new Run()
{
Tool = tool
Tool = new Tool { Name = "FxCop"}
};

output.Initialize(run);

if (fileDictionary != null && fileDictionary.Any())
{
output.WriteFiles(fileDictionary);
}

if (LogicalLocations != null && LogicalLocations.Any())
{
output.WriteLogicalLocations(LogicalLocations);
}

output.OpenResults();
output.WriteResults(results);
output.CloseResults();

if (rules.Count > 0)
{
IDictionary<string, IRule> rulesDictionary = new Dictionary<string, IRule>();
IDictionary<string, Rule> rulesDictionary = new Dictionary<string, Rule>();
Copy link

@ghost ghost Dec 30, 2018 •
edited by michaelcfanning
Loading

Choose a reason for hiding this comment

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

Rule [](start = 36, length = 4)

Yes! Step by step... #ByDesign


foreach (Rule rule in rules)
{
rulesDictionary[rule.Id] = rule;
}

output.WriteRules(rulesDictionary);
run.Resources = new Resources
{
Rules = rulesDictionary
};
}

PersistResults(output, results, run);
}

internal Rule CreateRule(FxCopLogReader.Context context)
Expand Down
Loading