Add beginnings of PSScriptAnalyzer 2.0 by rjmholt · Pull Request #1476 · PowerShell/PSScriptAnalyzer · GitHub

Start of the PSScriptAnalyzer 2.0 project.

I'll open up issues to describe the outstanding work items I have in my mind and label/milestone them appropriately.

Here are some comments, looks ok overall, I don't have time for a full line by line review, which I suggest you get from someone in your team since PSSA 2 is a big undertaking.

Following the learnings from #1099, we should be using ToLowerInvariant. But looking at the usage in the switch statement below, we should rather be using C# pattern matching for a case insensitive comparison inside a switch statement, see here for an example: https://stackoverflow.com/a/44447980

Not a huge fan of the reverse switch. Will replace with if

To me it feels like the special case of 1 rule should be handled inside CompositeRuleProvider from a consumer's perspective.

Yeah, it's done here for efficiency. We can decide what's best when we're further along

Can we please try to keep the number of publicly exposed classes/methods generally minimal and instead use internal?

Very much agree. My intent is to try and keep things behind interfaces where it makes sense to. In this case, I'm intending for the builder to be public for hosts, but certainly we will review the API surface

Can we eliminate the hard-coding here and use something like an attribute on the rule whether to run the rule by default or rely on something like typeof/nameof ? The PS prefix should also be defined only once.

I think some of this will change. The key part though is that I want to improve cold start performance by eliminating reflection and should also have a nice declarative way to list rules.

What about specifying the runspace pool as something like number of rules plus 2 (or whatever makes sense)? Just an idea. At the moment we do hard-code the default.

This particular concept I think I will rework and we can figure out the runspace configuration after that

I haven't looked into details but can we please support JSONC (json with comments)?

I'm hoping we can -- I'll see about writing some tests down the line

Boolean.TryParse?

WIP?

Yeah, this didn't work initially but I'm hoping to keep it to work on to see if we can improve it. Alternatively, it might be easier to convert this to a Channel now

maybe add that it is a formatter as well

Maybe we should use the opportunity to remove some of those commented out things in the psd1 but HelpInfoUri could actually be useful to set

Merging this to the WIP fork for PSSA2. Will continue working on that branch

Add beginnings of PSScriptAnalyzer 2.0

8a55d48

Rob Holt (rjmholt) added the Consider - 2.0 label May 6, 2020

Rob Holt (rjmholt) requested review from James Truher (JamesWTruher), Sydney Smith (SydneyhSmith) and Christoph Bergmeister (bergmeister) May 6, 2020 03:02

Rob Holt (rjmholt) added this to the 2.0 milestone May 6, 2020

Christoph Bergmeister (bergmeister) reviewed May 6, 2020

View reviewed changes

This was referenced Jun 4, 2020

Invoke-ScriptAnalyzer should implement SupportsShouldProcess #1517

Open

Provide updatable help for PSScriptAnalyzer #1518

Open

Address @bergmeister 's feedback

f961951

Rob Holt (rjmholt) merged commit 20f3acf into PowerShell:pssa2 Jun 4, 2020

Rob Holt (rjmholt) deleted the rob-pssa2 branch June 4, 2020 19:59

+                          {
+                              foreach (string rulePath in configuration.RulePaths)
+                              {
+                                  string extension = Path.GetExtension(rulePath).ToLower();

+                      public ScriptAnalyzer Build()
+                      {
+                          IRuleProvider ruleProvider = _ruleProviders.Count == 1

+              namespace Microsoft.PowerShell.ScriptAnalyzer.Builder
+              {
+                  public class ScriptAnalyzerBuilder

+                      public static IReadOnlyDictionary<string, IRuleConfiguration> RuleConfiguration { get; } = new Dictionary<string, IRuleConfiguration>(StringComparer.OrdinalIgnoreCase)
+                      {
+                          { "PS/AvoidUsingEmptyCatchBlock", null },

+                      private static IPowerShellCommandDatabase InstantiatePowerShellCommandDatabase()
+                      {
+                          using (Runspace runspace = RunspaceFactory.CreateRunspace())

+              namespace Microsoft.PowerShell.ScriptAnalyzer.Configuration.Json
+              {
+                  internal class JsonConfigurationConverter : JsonConverter<JsonScriptAnalyzerConfiguration>

+                              case VariableExpressionAst varExprAst:
+                                  // $true and $false are VariableExpressionAsts, so look for them here
+                                  switch (varExprAst.VariablePath.UserPath.ToLowerInvariant())

+                  }
+                  /*
+                  internal class DataflowRuleExecutor : IRuleExecutor

+              Copyright = '(c) Microsoft Corporation'
+              # Description of the functionality provided by this module
+              Description = 'PSScriptAnalyzer provides script analysis and checks for potential code defects in the scripts by applying a group of built-in or customized rules on the scripts being analyzed.'


		CompatiblePSEditions = @('Core', 'Desktop')

		# HelpInfo URI of this module

Add beginnings of PSScriptAnalyzer 2.0 by rjmholt · Pull Request #1476 · PowerShell/PSScriptAnalyzer · GitHub

Uh oh!

Add beginnings of PSScriptAnalyzer 2.0 - #1476

Add beginnings of PSScriptAnalyzer 2.0#1476Rob Holt (rjmholt) merged 2 commits intoPowerShell:pssa2PowerShell/PSScriptAnalyzer:pssa2from rjmholt:rob-pssa2rjmholt/PSScriptAnalyzer:rob-pssa2Copy head branch name to clipboard

Conversation

Rob Holt (rjmholt) commented May 6, 2020

Uh oh!

Christoph Bergmeister (bergmeister) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rob Holt (rjmholt) commented Jun 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Add beginnings of PSScriptAnalyzer 2.0#1476
Rob Holt (rjmholt) merged 2 commits into
PowerShell:pssa2from
rjmholt:rob-pssa2