Suggested corrections for violations

[kapilmb]

Whenever PSScriptAnalyzer (PSSA) is invoked on a PowerShell (PS) script, it runs a set of rules against the contents of the script. Each rule checks for some sort of violation. As of now, the violations need to be fixed manually by the script author. Many of the violations, however, are simple fixes that can be automated. For example, gci triggers the PSAvoidUsingAliases rule and can be easily rectified by replacing gci with Get-ChildItem.

Whenever there are violations in a script, PSSA invocation emits a list of DiagnosticRecord (DR) instances. Each DR instance corresponds to one violation of a rule. The DR also contains the extent of the violation. We want to provide the correction extent that can be used to remove the violation. Whenever, a rule processes a script syntax tree, it has, in most cases, all the information needed to correct the violation. Hence, it would be efficient for the rule itself to emit the correction extent such that we can tag on a corrections property to each emitted DR. There are three cases that such a correction extent needs to address:

• Replace the violation text with correction text.
• For Example, replace gci with Get-ChildItem
• Add text to the script that corrects the violation.
• For Example, should process related changes.
• Ability to propogate the change within the entire script
• For Example, consider the PSUseSingularNoun rule. It triggers whenever a function definition is found wherein the noun part is plural. To rectify the violation we can suggest the singular version of the noun and then apply the change to all the instances of the function in the script.

The primary motivation behind this feature is to allow editors to take advantage of this property to enable quick fix scenarios for PS scripts.

[KirkMunro]

Love this.

A few thoughts:

• Some changes will span multiple script files. As this feature is implemented, please consider providing support (parameters) to apply a broad change (like in scenario 3) across multiple files. A simple example is identifying a function in a module that has a pluralized noun, and fixing that function name across the entire module -- everywhere it is invoked, in an Export-ModuleMember call, and in the manifest.
• Some changes will be breaking, such as renaming a function in a module that has been published, and PSScriptAnalyzer is about best practices. It would be ideal if PSScriptAnalyzer could identify required changes (for example, when renaming a function/fixing the verb/noun, identify the extent(s) that need to be updated. That might mean a rule emitting a violation with a collection of extents in a DR, maybe identifying dependencies between them with the root extent where the problem was found, and other extents that are impacted via modification of the root extent, either in a list via properties that define dependencies or in a tree structure or some such. This may also include identifying code that could/should optionally be added (e.g. if you are fixing a naming violation of a function in a published script or module, you really should define an alias to map the old name to the new, corrected name), in which case the extent could identify the entry point where the code would be added along with the code that would be added. That would allow for many editors to easily get functionality similar to plugins in Visual Studio, where correcting an auto-correctable violation involves making the base correction and walking through other recommended and optional corrections, approving or skipping each one as you pass through a single file, with an option to continue walking through the entire module that the file belongs to (assuming you have some logic to identify when a function that is defined in a ps1/psm1 file appears to belong to a module by some simple logic that analyzes the file structure where it is stored).

You might not hit these targets on the first go, but I wanted to add them while they were top of mind because I think this is what this feature/functionality should strive towards.

[daviwil]

The model we've designed will enable multi-file corrections in the first release. A single rule will be able to make insertions, modifications, or deletions in an arbitrary set of files. The example you give for renaming a function and finding all of the occurrences and changing them is one that we're actively investigating.

The idea of defining an alias to map the old name to the new one is great! I think we'll be at the mercy of editors when it comes to giving the user optional, additional corrections while performing a primary one (like renaming a cmdlet and then getting the option to add an alias). Right now VS Code doesn't give you the ability to do a multi-step correct, you just select a single correction item and it applies that.

I'm looking forward to this one!

[kapilmb]

@KirkMunro Those are great suggestions. Thanks!

As @daviwil mentioned, the current model that we have can allow multi-file edits. But for the first release of this feature we are planning to roll out support for only script-wide edits. This is due to the way PSSA works as of now. Having the ability to propagate a change within a module and across modules is something that will need a significant redesign of the PSSA engine. Nevertheless, these are great suggestions to have on our TODO list.

[joeyaiello]

I agree with everything said so far in this thread with one caveat: we might want to hold off on implementing any auto-correct for changes which may require edits throughout multiple files.

As @KirkMunro suggests, that design is probably going to a bit trickier, and I don't want to paint ourselves into a corner here.

[kapilmb]

At present we have added this capability (see PR #508) to the following rules

• PSAvoidUsingCmdletAliases
• PSAvoidUsingPlainTextForPassword
• PSMisleadingBacktick
• PSUseToExportFieldsInManifest

Which other rules should be equipped with this functionality so that it can improve authoring/editing PS scripts experience?

[kilasuit]

I would be more suggestive of moving this functionality to its own cmdlet, Invoke-ScriptAnalyzerCorrection

Reason being for me is that it allows me as an author to either use it or not use it especially for the reasons listed by Kirk above.

I like the idea to be able to do this though and think it's great it's being put on the table but I'm a little concerned that doing so as an additional aspect to Invoke-ScriptAnalyzer will likely pollute it's real purpose - To inform the author about what they could be looking to improve in their own writing practices.

[KirkMunro]

I don't think we need another cmdlet. But the idea of applying auto-correct is interesting and makes me think back to tools like chkdsk (am I aging myself?). That would allow users to get the details about what could be improved by default (like it does today, but with an identifier/flag in the default format applied to the output or a "[AutoCorrectible]" prefix/qualifier in the issue description that clearly indicates whether or not a rule is autocorrectable), and then PSScriptAnalyzer could be run again with an optional -AutoCorrect switch to apply the changes. I think that would be a better approach than a separate cmdlet.

[kapilmb]

@kilasuit This feature only emits a property which can be consumed by external utilities (e.g. editors) to provide auto-correct capability. Invoke-ScriptAnalyzer by itself will not modify the scripts that it analyzes.

[KirkMunro]

@kapilmb Thanks for that clarificatin, that makes more sense, so that editors can apply correction to one or multiple issues identified by this feature, leveraging their UI properly to highlight auto-correctible items and to show what the corrections look like before applying them.

[kapilmb]

A PR - #508 - that adds this feature was merged. Hence, I will close this thread and would like to point everyone to the new thread - #516 - where I would like to request community's input on what rules would be most valuable w.r.t the suggested corrections feature.