Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return a code action for each diagnostic record #1718

Merged
merged 2 commits into from
Feb 23, 2022

Conversation

andyleejordan
Copy link
Member

@andyleejordan andyleejordan commented Feb 19, 2022

Currently, PSSA's SuggestCorrections property of the
DiagnosticRecord object already is an array, but no built-in rules
return more than one suggested correction, which led to
PowerShellEditorServices not correctly translating this array before
returning it as an code action as it only ever displayed one. This fixes
it by making it generic again so it returns a code action for each
suggest correction. It's basically just performing a foreach loop and
calling ToTextEdit just once.

This is a pre-requisite for an implementation of this, where a built-in
rule will return multiple suggest corrections:
PowerShell/PSScriptAnalyzer#1767

I've manually tested this with a modified version of PSScriptAnalyzer
where I return two suggested corrections. The extension's UI now shows
me the two different suggested code actions and also applies them
correctly.

Replaces #1713.

bergmeister and others added 2 commits February 18, 2022 16:34
Currently, PSSA's `SuggestCorrections` property of the
`DiagnosticRecord` object already is an array, but no built-in rules
return more than one suggested correction, which led to
`PowerShellEditorServices` not correctly translating this array before
returning it as an code action as it only ever displayed one. This fixes
it by making it generic again so it returns a code action for each
suggest correction. It's basically just performing a `foreach` loop and
calling `ToTextEdit` just once.

This is a pre-requisite for an implementation of this, where a built-in
rule will return multiple suggest corrections:
PowerShell/PSScriptAnalyzer#1767

I've manually tested this with a modified version of PSScriptAnalyzer
where I return two suggested corrections. The extension's UI now shows
me the two different suggested code actions and also applies them
correctly.
@andyleejordan andyleejordan changed the title Andschwa/multiple codeactions Return a code action for each diagnostic record Feb 19, 2022
@andyleejordan andyleejordan marked this pull request as ready for review February 19, 2022 00:55
Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andyleejordan andyleejordan merged commit ea5da00 into master Feb 23, 2022
@andyleejordan andyleejordan deleted the andschwa/multiple-codeactions branch February 23, 2022 21:32
@bergmeister
Copy link
Contributor

Sorry, forgot about this one. Since this change is slightly different, will re-test just to confirm

@andyleejordan
Copy link
Member Author

The actual changes should be no different, I cherry-picked the commit: a4c4fdf

@bergmeister
Copy link
Contributor

Ok, just seeing the other changes made me think there is a chance it could affect it. I tested with the latest preview extension since it's already released and can confirm it still works the way I tested it with my own build :-)

@andyleejordan
Copy link
Member Author

Yay! Thanks for testing!!

andyleejordan added a commit that referenced this pull request Mar 31, 2022
With the recent fix in PR #1718, PSES processes now all Correction objects from PSSA but the message specifically was just taken from the last correction here. I did not notice this at first because I thought I had to tweak my rule first to emit two different messages until I realized it was another bug in PSES.

Related: PowerShell/PSScriptAnalyzer#1782

Co-authored-by: Andy Schwartzmeyer <andrew@schwartzmeyer.com>
@SydneyhSmith SydneyhSmith mentioned this pull request Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants