A new hosting model which is easier for Editor Services and other script analyzer hosting - #1361
A new hosting model which is easier for Editor Services and other script analyzer hosting#1361James Truher (JamesWTruher) wants to merge 62 commits into
Conversation
Sorry, something went wrong.
Had to make the initialization of the analyzer optional as it was clobbering the settings that had been created in the formatter
Hosting seems to work, starting to work on nuget packaging
…nalyzer, CommandCache, and Helper
Had to make the initialization of the analyzer optional as it was clobbering the settings that had been created in the formatter
Hosting seems to work, starting to work on nuget packaging
…nalyzer, CommandCache, and Helper
There really isn't a good way to do this as the underlying apis aren't really thread safe, but we can do _something_ to the hosted analyzer If someone is using both the hosted analyzer and the module there may be some unexpected behaviors but doing something is probably better than doing nothing.
Disposing the helper seems to cause downstream problems. Add async calls to the analyzer
Add a simple tostring method to the HostedAnalyzer
This will check to be sure that we can actually using the nupgk in a project update the version to 1.18.4
Add tests for the api and in the reference assembly
Change the apis which allow adding settings to return the settings object so they can be chained
Change the order of parameters to make them the same. remote extraneous documentation xml in Get-ScriptAnalyzerRule cmdlet Fix test to match new parameter order
| @@ -0,0 +1,5 @@ | |||
| { | |||
| "sdk": { | |||
| "version": "2.2.402" | |||
There was a problem hiding this comment.
.net core 2.2 has reached EOL in December already and PSSA has already moved on to 3.1.101 in the root directory. I think we can simply remove the file
Sorry, something went wrong.
| Helper.Instance = new Helper(runspace.SessionStateProxy.InvokeCommand, writer); | ||
| Helper.Instance.Initialize(); | ||
|
|
||
| var ruleOrder = new string[] |
There was a problem hiding this comment.
note sure if .net optimises that anyway but we could use a constant for this
Sorry, something went wrong.
|
|
||
| Range updatedRange; | ||
| bool fixesWereApplied; | ||
| text = ScriptAnalyzer.Instance.Fix(text, range, out updatedRange, out fixesWereApplied); |
There was a problem hiding this comment.
the above bool fixesWereApplied line can be inlined just by using out bool fixesWereApplied here. Same for the Range parameter
Sorry, something went wrong.
| /// PSUseConsistentWhitespace | ||
| /// PSUseConsistentIndentation | ||
| /// PSAlignAssignmentStatement | ||
| /// PSUseCorrectCasing |
There was a problem hiding this comment.
the exact list of those rules could change, I'd take them out of the comment therefore. Same for other comments
Sorry, something went wrong.
| /// <summary> | ||
| /// The encapsulated results of the analyzer | ||
| /// </summary> | ||
| public class AnalyzerResult |
There was a problem hiding this comment.
please move each class/enum into its own file if it is public
Sorry, something went wrong.
| public IEnumerable<string> ExcludeRules => excludeRules; | ||
| public IEnumerable<string> Severities => severities; | ||
| public IEnumerable<string> CustomRulePath => customRulePath; | ||
| public List<string> IncludeRules => includeRules; |
There was a problem hiding this comment.
I don't think this change is needed?
Sorry, something went wrong.
Christoph Bergmeister (bergmeister)
left a comment
There was a problem hiding this comment.
A lot of code and quite complex but if it makes it better on the PSES side of things or performance wise, then go for it. Most comments are minor but would be nice to be be addressed.
Sorry, something went wrong.
| { | ||
| // the rule does not exist, that's a problem, first create it and then add the key/value pair | ||
| Dictionary<string, object> settingDictionary; | ||
| if ( ! RuleArguments.TryGetValue(rule, out settingDictionary)) |
There was a problem hiding this comment.
variable declaration can be inlined: out Dictionary<string, object> settingDictionary, also in a few lines below
Sorry, something went wrong.
| <owners>Microsoft,PowerShellTeam</owners> | ||
| <projectUrl>https://github.com/PowerShell/PSScriptAnalyzer</projectUrl> | ||
| <iconUrl>https://raw.githubusercontent.com/powershell/psscriptanalyzer/master/logo.png</iconUrl> | ||
| <licenseUrl>https://github.com/PowerShell/PowerShell/blob/master/LICENSE.txt</licenseUrl> |
There was a problem hiding this comment.
| <licenseUrl>https://github.com/PowerShell/PowerShell/blob/master/LICENSE.txt</licenseUrl> | |
| <licenseUrl>https://github.com/PowerShell/PSScriptAnalyzer/blob/master/LICENSE</licenseUrl> |
Sorry, something went wrong.
|
Close and re-open to re-trigger CI |
Sorry, something went wrong.
|
I'm going to close this as it's not needed at the moment. |
Sorry, something went wrong.
PR Summary
This implements a new approach for hosting script analyzer for C# developers. It does not require the developer to manage the powershell engine or runspaces, and handles that internally. It does not need the developer to call the analyzer via pipeline.invoke. It provides a number of APIs which will make it easier for EditorServices to use ScriptAnalyzer.
PR Checklist