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

Enable export rules in multitool #2052

Merged
9 commits merged into from
Sep 1, 2020
Merged
Show file tree
Hide file tree
Changes from 3 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
65 changes: 65 additions & 0 deletions src/Sarif.Multitool/ExportCommand.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// 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.Linq;
using System.Reflection;
using System.Text;

using Microsoft.CodeAnalysis.Sarif.Driver;
using Microsoft.CodeAnalysis.Sarif.Multitool.Rules;

namespace Microsoft.CodeAnalysis.Sarif.Multitool
{
public class ExportCommand : CommandBase
Copy link

@ghost ghost Sep 1, 2020

Choose a reason for hiding this comment

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

ExportCommand [](start = 17, length = 13)

Let's call this ExportRuleDocumentation. "Export" is such a generic term, you might be exporting anything. You could imagine an ExportRules command that creates a SARIF file containing the rule metadata. #Closed

{
private readonly IFileSystem _fileSystem;

public ExportCommand()
{
_fileSystem = new FileSystem();
Copy link

@ghost ghost Sep 1, 2020

Choose a reason for hiding this comment

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

_fileSystem = new FileSystem(); [](start = 12, length = 31)

Inject the file system into the constructor:

public ExportRuleDocumentation(IFileSystem fileSystem)
{
    _fileSystem = fileSystem ?? new FileSystem();
}

This makes it easier to unit test. #Closed

}

public int Run(ExportOptions options)
{
try
{
var list = CompositionUtilities.GetExports<SarifValidationSkimmerBase>(
Copy link

@ghost ghost Sep 1, 2020

Choose a reason for hiding this comment

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

list [](start = 20, length = 4)

Let's call this rules since "list" doesn't tell the reader what's in the list. #Closed

Copy link

@ghost ghost Sep 1, 2020

Choose a reason for hiding this comment

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

CompositionUtilities [](start = 27, length = 20)

Good job discovering CompositionUtilities! I didn't know about that class. #Closed

new Assembly[] { Assembly.GetExecutingAssembly() }).ToList();

var sb = new StringBuilder();
sb.AppendLine($"# Rules{Environment.NewLine}");

foreach (SarifValidationSkimmerBase rule in list)
{
BuildRule(rule, sb);
}

_fileSystem.WriteAllText($"{options.OutputDirectoryPath}\\rules.md", sb.ToString());
Copy link

@ghost ghost Sep 1, 2020

Choose a reason for hiding this comment

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

$"{options.OutputDirectoryPath}\rules.md" [](start = 41, length = 42)

Just options.OutputFilePath. #Closed

}
catch (Exception ex)
{
Console.WriteLine(ex);
return FAILURE;
}

return SUCCESS;
}

private void BuildRule(SarifValidationSkimmerBase rule, StringBuilder sb)
{
sb.AppendLine($"## Rule `{rule.Id}.{rule.Name}`{Environment.NewLine}");
Copy link

@ghost ghost Sep 1, 2020

Choose a reason for hiding this comment

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

{rule.Id}.{rule.Name} [](start = 37, length = 21)

There's a new property rule.Moniker that expands to id.name. #Closed

sb.AppendLine($"### Description{Environment.NewLine}");
sb.AppendLine($"{rule.FullDescription.Text}{Environment.NewLine}");
sb.AppendLine($"### Messages{Environment.NewLine}");

foreach (System.Collections.Generic.KeyValuePair<string, MultiformatMessageString> message in rule.MessageStrings)
Copy link

@ghost ghost Sep 1, 2020

Choose a reason for hiding this comment

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

System.Collections.Generic [](start = 21, length = 26)

using System.Collections.Generic; #Closed

{
sb.AppendLine($"#### `{message.Key.Split('_').Last()}`: {rule.DefaultLevel}{Environment.NewLine}");
sb.AppendLine($"{message.Value.Text}{Environment.NewLine}");
}

sb.AppendLine($"---{Environment.NewLine}");
}
}
}
17 changes: 17 additions & 0 deletions src/Sarif.Multitool/ExportOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using CommandLine;

namespace Microsoft.CodeAnalysis.Sarif.Multitool
{
[Verb("export", HelpText = "Export rules to md file.")]
Copy link

@ghost ghost Sep 1, 2020

Choose a reason for hiding this comment

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

export [](start = 11, length = 6)

"export-rule-documentation" #Closed

Copy link

@ghost ghost Sep 1, 2020

Choose a reason for hiding this comment

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

Export rules to md file. [](start = 32, length = 24)

`"Export the documentation for the validation rules to a Markdown file." #Closed

public class ExportOptions
{
[Option(
'o',
"output-directory",
HelpText = "A directory to output the exported file.")]
Copy link

@ghost ghost Sep 1, 2020

Choose a reason for hiding this comment

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

directory [](start = 29, length = 9)

Let them specify the file name, not just the directory:

        [Option('o',
            "output-file-path",
            Default: "ValidationRules.md",
            HelpText = "Path to the generated Markdown file.  Default: ValidationRules.md in the current directory.")]
        public string OutputFilePath { get; internal set; }
``` #Closed

public string OutputDirectoryPath { get; internal set; }
}
}
2 changes: 2 additions & 0 deletions src/Sarif.Multitool/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ internal static class Program
public static int Main(string[] args)
{
return Parser.Default.ParseArguments<
ExportOptions,
ValidateOptions,
ConvertOptions,
RewriteOptions,
Expand All @@ -26,6 +27,7 @@ public static int Main(string[] args)
ResultMatchSetOptions,
FileWorkItemsOptions>(args)
.MapResult(
(ExportOptions options) => new ExportCommand().Run(options),
(ValidateOptions validateOptions) => new ValidateCommand().Run(validateOptions),
(ConvertOptions convertOptions) => new ConvertCommand().Run(convertOptions),
(RewriteOptions rewriteOptions) => new RewriteCommand().Run(rewriteOptions),
Expand Down
3 changes: 3 additions & 0 deletions src/Shared/CommonAssemblyInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
[assembly: CLSCompliant(false)]
[assembly: NeutralResourcesLanguage("en-US", UltimateResourceFallbackLocation.MainAssembly)]

// This reference necessary for Multitool.
[assembly: InternalsVisibleTo("Sarif.Multitool, PublicKey=0024000004800000940000000602000000240000525341310004000001000100433fbf156abe9718142bdbd48a440e779a1b708fd21486ee0ae536f4c548edf8a7185c1e3ac89ceef76c15b8cc2497906798779a59402f9b9e27281fb15e7111566cdc9a9f8326301d45320623c5222089cf4d0013f365ae729fb0a9c9d15138042825cd511a0f3d4887a7b92f4c2749f81b410813d297b73244cf64995effb1")]
Copy link

@ghost ghost Sep 1, 2020

Choose a reason for hiding this comment

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

Multitool [](start = 37, length = 9)

Could you explain why you need this? What is the exact error you get if you don't provide it? #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.

since CompositionUtilities is internal, we need this


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

Copy link

Choose a reason for hiding this comment

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

Let's make CompositionUtilities public.

If we kept it internal -- which I don't recommend -- I wouldn't put the InternalsVisibleTo in the shared CommonAssemblyInfo.cs. I would just add a new AssemblyInfo.cs to the driver project so that only the Driver made its internals visible to the Multitool. And I would add a more specific comment like "Multitool requires access to the internal class CompositionUtilities."


In reply to: 481273990 [](ancestors = 481273990,481265173)


// This reference necessary for the MOQ test engine.
[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=0024000004800000940000000602000000240000525341310004000001000100c547cac37abd99c8db225ef2f6c8a3602f3b3606cc9891605d02baa56104f4cfc0734aa39b93bf7852f7d9266654753cc297e7d2edfe0bac1cdcf9f717241550e0a7b191195b7667bb4f64bcb8e2121380fd1d9d46ad2d92d2d15605093924cceaf74c4861eff62abf69b9291ed0a340e113be11e6a7d3113e92484cf7045cc7")]

Expand Down