PowerShellEditorServices icon indicating copy to clipboard operation
PowerShellEditorServices copied to clipboard

What the hosted PSSA could look like

Open TylerLeonhardt opened this issue 4 years ago • 1 comments

This is DRAFT. CI will fail because there's no nuget package on nuget.org!

This is the bare minimum work that I would do to adopt the hosted analyzer. It already cuts the code in AnalysisService.cs by half!

I recommend just looking at my version of the AnalysisService.cs and looking around for HostedAnalyzer, Analyze(, Format( - so you can see how the hosted analyzer is used there.

Some additional work that could be done:

  • Replace all instances of Hashtable (when pertaining to PSSA) with the proper Settings type
  • ~Remove the ScriptFileMarker type which seems to be some intermediate type between a DiagnosticRecord from PSSA and the outgoing (aka language server protocol) type of Diagnostic. I tried it, it's totally possible to switch over to using Diagnostic everywhere ScriptFileMarker is used. That would remove another type and a good chunk of code.~
  • Now that ScriptFileMarker is gone, we can continue to remove random types like MarkerCorrection could be removed but this involves understanding CodeAction request more

cc @JamesWTruher Some notes that I took on the Hosted Analyzer:

  • [x] Analyze() - Invoke-ScriptAnalyzer has a -Severity param. Can we have a severity param?
  • [x] Fix() - Invoke-Formatter Has a -Range parameter that we use. Can we have a range parameter?
  • [x] Settings.*RuleArgument() - should return Settings objects so we can chain them like the builder pattern
  • [x] Format() - Invoke-Formatter Has a -Range parameter that we use. Can we have a range parameter?
  • [x] Can it be possible to create a Settings object from a list of strings (which are the rules we care about)?
  • [x] ~Fix() & Format() - should those also have the ability to pass the AST in addition to the script?~ No they should not.
  • [x] ~Cancellation support... this might be too much work... but it'd be nice. I've kinda worked around it in this PR by "cancelling" anything else after we get a response back from Analyze()~ deferred.

TylerLeonhardt avatar Oct 31 '19 11:10 TylerLeonhardt

Codacy Here is an overview of what got changed by this pull request:


Issues
======
+ Solved 18
- Added 1
           

Complexity increasing per file
==============================
- src/PowerShellEditorServices/Services/Analysis/AnalysisService.cs  5
- src/PowerShellEditorServices/Services/Analysis/MarkerCorrection.cs  1
- src/PowerShellEditorServices/Services/Analysis/DiagnosticCreationHelper.cs  4
         

Complexity decreasing per file
==============================
+ src/PowerShellEditorServices/Services/TextDocument/Handlers/FormattingHandlers.cs  -1
+ src/PowerShellEditorServices/Services/PowerShellContext/Handlers/GetCommentHelpHandler.cs  -4
         

See the complete overview on Codacy

TylerLeonhardt avatar Dec 09 '19 21:12 TylerLeonhardt

@TylerLeonhardt do you think there's any value in looking at this PR in particular 5 years later, or should I just close it and keep the idea in the back of minds of hosting PSSA? (We all know it would be nice to do, especially combined with a PSSA rewrite.)

andyleejordan avatar Feb 12 '24 23:02 andyleejordan