Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upBetter way to implement rules in PS script #1551
Comments
|
I am really looking forward on this,
Few doubts,
Would it be nice if there is a way to register all (the new design allows me to specify more than one namespace) my custom rule namespace in PSSA, so that it will internally handle all the rules from that space ? Or something like below ?
Feel free to ignore if this sounds bad |
Currently you can't construct the attribute without a name, so you're forced to provide it. However perhaps it makes sense to use a default like:
Yeah, suppression would look pretty much as you lay out.
That's actually a really good consideration. The current settings proposal includes specifying a rule collection by path and then adding settings for each rule. You would then need to specify each rule you want to enable from that collection. Thinking on that now, that's cumbersome (if nicely explicit), but the problem is that any rules that require configuration settings must have those settings provided to be able to run. This makes me think that:
|
PSScriptAnalyzer today supports external implementation of rules in PowerShell script.
Alongside the proposal to allow custom binary rules (#1543), PSScriptAnalyzer should also look at how rules written in PowerShell are implemented to ensure that PS script rules:
To address these goals, some proposals are:
Standard assumed arguments
PS script functions today have a very loose parameter contract that expects parameters that either end with the
Astsuffix or theTokensuffix, this means:Instead script rules should be required to accept a clear and concrete set of parameters, similar to the way
TabExpansion2does:Additional points:
[CmdletBinding()], positional and pipeline arguments don't matter to PSSA, as it will simply invoke rules withnew PSCommand().AddCommand(<script rule>).AddParameter("Ast", ast).AddParameter("Tokens", tokens).AddParameter("ScriptFilePath", scriptFilePath)$Tokensparameter should be immutable and ideally is aSystem.Collections.Generic.IReadOnlyList<Token>. But this may not work well with PowerShell, so theToken[]type may be preferable, with an understanding that mutating the token array is against the contractAttribute based rule definition
Today, PS script rules are:
To be consistent with #1543 and addressing #1545, rules instead should be declared with attributes:
We could also provide a type accelerator for this attribute to reduce it to
[Rule(...)].This attribute would provide the possibility of namespace override:
[Rule(..., Namespace = "MyRules")].Write-Diagnostic instead of needing to construct diagnostic objects
Today script rules must construct and emit diagnostic objects themselves. This means:
New-ObjectcallsInstead, a PowerShell-native solution would be for PSSA to introduce a new cmdlet that hooks back into the hosting script analyzer:
This command can:
Constrain and document the assumptions script rules can make about their executing context
Finally, today it's not clear what assumptions a script rule should make about the context in which it's run, in particular whether it shares context with the environment running
Invoke-ScriptAnalyzer.So that PSSA can be as efficient as possible, the only assumptions script rules should make about their running environment are:
Invoke-ScriptAnalyzer, since it may be hosted)In particular, some assumptions we don't want are:
Invoke-ScriptAnalyzerExample
So an example of a rule defined with the new proposed API would be something like this:
This rule can be referenced with
MyRules/UseApprovedVerbsin settings.Further considerations
IReadOnlyList<Token>?processblocks and have all inputs piped in withValueFromPipelineByPropertyName? (This might improve performance)Write-Diagnosticthe right implementation? In particular, how do we ensure every diagnostic has aRuleInfoobject associated with it that links back to the providing rule (and ideally is referentially equal to allRuleInfoobjects referring to the same rule)?Write-Diagnosticactually emit an object, or should it add objects to a list within PSSA?object, since PowerShell will handle any type there?