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

Correctly map SuggestedCorrection to MarkerCorrection #1749

Merged
merged 3 commits into from
Mar 31, 2022

Conversation

bergmeister
Copy link
Contributor

@bergmeister bergmeister commented Mar 30, 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
Proof here that the message of each correction is associated to it as well (the actual code fix already was)

BEFORE
image

AFTER
image

Copy link
Member

@andyleejordan andyleejordan left a comment

Choose a reason for hiding this comment

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

Everything looks good, just one recurring test value. Some collection is coming through null and something isn't happy about it 😅 Thank you for working on this, excited to get it in!

Co-authored-by: Andy Schwartzmeyer <[email protected]>
@bergmeister
Copy link
Contributor Author

Yes, saw the test failure only after I opened the PR. Somehow, I cannot run the test with VS2022 with test explorer, can you give me some tips how I can run it locally to figure out why the value ends up being null by attaching the debugger and stepping through. Also happy to use VS Code instead alternatives

@andyleejordan
Copy link
Member

I exclusively use the C# extension in VS Code, it adds a "Run Test" and "Debug Test" code lens to each unit test, and it works great!

@bergmeister
Copy link
Contributor Author

Thanks. I suspect it must be a problem of the XUnit VisualStudio NuGet package. It did work in VS-Code out of the box but due to the async nature I couldn't step through the code. I had a suspicion though and noticed a boolean logic in one of my refactoring was not done correctly, so I adjusted that, which fixed the test.

@andyleejordan
Copy link
Member

Good catch!

@andyleejordan andyleejordan changed the title Fix: Associate the message of each Suggestion correction to each own MarkerCorrection Correctly map SuggestedCorrection to MarkerCorrection Mar 31, 2022
@andyleejordan andyleejordan merged commit c49fa2a into PowerShell:master Mar 31, 2022
@SydneyhSmith SydneyhSmith mentioned this pull request Jul 7, 2022
@andyleejordan
Copy link
Member

Hey @bergmeister, I've added a PSSA test suite, what do you think a regression test for this would look like? See: https://github.com/PowerShell/PowerShellEditorServices/blob/main/test/PowerShellEditorServices.Test/Services/Symbols/PSScriptAnalyzerTests.cs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants