AlignAssignmentStatement overhaul to fix issues and include handing of Enums. #2132
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Summary
Overhauls the AlignAssignmentStatement rule to include handling of enums as well as fix several issues with Hashtable alignment.
I am very open to feedback and suggestions 😀.
Setting defaults have changed. Previously the rule could be enabled, but not do anything, as
CheckHashtable
(previously the only setting and gating all functionality) defaulted to$false
. I've changed this (as well as the new settings) to default to$true
. If you enable the rule - it will do something now. You can then optionally disable any parts you don't want. The rule is opt-in and there are already breaking changes in this PR (see the baby and the bath water below). Docs update shows and explains new settings.Hashtable alignment issues resolved include:
Erroring when the key of a key-value pair is an expression containing an equals sign
Invoke-ScriptAnalyzer: Start position cannot be before End position.
This is because of the way the current implementation finds
Equals
tokens. It find the equals token within the expression.It results in a correction extent that has an end column which is before the start column.
Erasing inline comments when they appear between the key of the key-value pair and the
Equals
token.results in
Performance scaling poorly. This is due to how
Equals
tokens of Key-Value pairs are located. For each Key-Value pair the list of tokens is scanned (from the beginning) until the relevantEquals
token is found. This means that, as a worst case, performance scales quadratically - proportional to half the number of Key-Value pairs in the file * the number of tokens.We can improve this by scanning the tokens once and keeping track of all the
Equals
tokens and what line they appear on. This reduces complexity to essentially linear time with negligible memory use. Goes fromO(H*T)
->O(H+T)
.I've put together some stress-test files (one saved as a gist here and script to generate them here) and ran only the
AlignAssignmentStatement
rule on them. Results below from my machine, averaged over 5 runs (with the first cold run discarded):The baby is thrown out with the bath water. If a single Key-Value pair is not in the expected format (Key and Equals-sign on same line, no two kvp on same line etc) the whole hashtable is skipped. I don't think this should be the case. The rule should consider and align the keys which are in the expected format and ignore the others. The rule is opt-in and so people using it want alignment - we should do our best to provide it.
I've done my best to expand the test coverage.
Fixes #1739
Fixes #1860
Would address downstream VSCode Extension issue: PowerShell/vscode-powershell#5138
PR Checklist
.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
to the beginning of the title and remove the prefix when the PR is ready.