Skip to content

/ PSScriptAnalyzer

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

Update compatibility profiles for PowerShell 7 #1429

Merged
merged 22 commits into from Mar 31, 2020
Merged

Update compatibility profiles for PowerShell 7 #1429

merged 22 commits into from Mar 31, 2020

Conversation

@rjmholt
Copy link
Member

rjmholt commented Mar 27, 2020

PR Summary

  • Adds compatibility profiles for PS 7 and 6.2
  • Moves the 6.1 profiles to the optional folder
  • Port an old fix for alias collection that never made it in, where we get aliases that don't have any declaring module (ones declared in the initial session state)
  • Fix the race condition in query objects causing intermittent test failures (I finally got a local repro) by making query initialisation eager
  • Make compatibility rules supressable by their particular command/parameter, type/member names
  • Add tests for PS 7 profiles

PR Checklist
rjmholt added 9 commits Mar 25, 2020
@rjmholt
Update compatibility profiles for PS7
baae6d8
@rjmholt
Update tests
26b8169
@rjmholt
Fix alias collection
dae0a9d
@rjmholt
Update profiles
9e2e609
@rjmholt
Update documentation
f7c5934
@rjmholt
Make queryable profile initialization eager
95e1a77
@rjmholt
Add supression IDs for compatibility rules
35f4524
@rjmholt
Fix NREs and remove Console.WriteLine
0c1f5d8
@rjmholt
Add supression documentation
Loading status checks…
1080b6a
@rjmholt
Activate tests in CI
Loading status checks…
e94e6d7
@TylerLeonhardt
TylerLeonhardt reviewed Mar 28, 2020
View changes
RuleDocumentation/UseCompatibleTypes.md Outdated
The rule can also be suppressed only for particular types:

```powershell
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSUseCompatibleTypes", "", Target="System.Management.Automation.Security.SystemPolicy")]

This comment has been minimized.

Sign in to view
@TylerLeonhardt

TylerLeonhardt Mar 28, 2020 Member

Shouldn't this be?

Suggested change
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSUseCompatibleTypes", "", Target="System.Management.Automation.Security.SystemPolicy")]
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSUseCompatibleTypes", "System.Management.Automation.Security.SystemPolicy")]
@bergmeister
bergmeister reviewed Mar 28, 2020
View changes
Copy link
Collaborator

bergmeister left a comment •
edited

Looks good but 1 or 2 minor comments probably need to be addressed. Also: I'd rather remove the 6.1 profiles as it's usage is unsupported anyway as it heavily adds to the installation payload of PSSA. By the time PSSA ships, even 6.2 will only have a few months support left.
CI needs to be fixed though of course.
...soft.PowerShell.CrossCompatibility/Collection/PowerShellDataCollector.cs Show resolved Hide resolved
...soft.PowerShell.CrossCompatibility/Query/Platform/OperatingSystemData.cs Outdated Show resolved Hide resolved
@rjmholt
Copy link
Member Author

rjmholt commented Mar 28, 2020

CI needs to be fixed though of course.

Yeah it looks like this reflects a legitimate bug... I'll need to look into it
rjmholt added 10 commits Mar 30, 2020
@rjmholt
Remove 6.1 profiles
Loading status checks…
1e52b7b
@rjmholt
Fix type loading
Loading status checks…
477d010
@rjmholt
Update profiles collected with fixed module
Loading status checks…
230bdcd
@rjmholt
Remove Wait-Debugger
Loading status checks…
8dcd2b2
@rjmholt
Fix other tests
Loading status checks…
f6f4b5a
@rjmholt
Update tests for PS 3
Loading status checks…
a5c081f
@rjmholt
Fix PS 4 tests
Loading status checks…
add491a
@rjmholt
Fix types tests
Loading status checks…
615bbdb
@rjmholt
Fix syntax
Loading status checks…
ebbce23
@rjmholt
Fix the way parse errors are filtered out in tests
Loading status checks…
dec6873
@rjmholt
Copy link
Member Author

rjmholt commented Mar 31, 2020

@bergmeister I've got this working now, if you've got time to take a look
@rjmholt
Correct docs
Loading status checks…
7d8562d
@bergmeister
bergmeister approved these changes Mar 31, 2020
View changes
RuleDocumentation/UseCompatibleCommands.md Outdated Show resolved Hide resolved
@rjmholt
Add more specific Win10 name to docs
Loading status checks…
baa0e7f
@bergmeister
bergmeister approved these changes Mar 31, 2020
View changes
Copy link
Collaborator

bergmeister left a comment

Awesome, thanks
@TylerLeonhardt
TylerLeonhardt reviewed Mar 31, 2020
View changes
RuleDocumentation/UseCompatibleCommands.md
And also suppressed only for parameters:

```powershell
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSUseCompatibleCommands", "Import-Module/FullyQualifiedName")]

This comment has been minimized.

Sign in to view
@TylerLeonhardt

TylerLeonhardt Mar 31, 2020 Member

❤️
@rjmholt rjmholt merged commit b6e0736 into PowerShell:master Mar 31, 2020
12 checks passed
12 checks passed
PSScriptAnalyzer-CI Build #20200331.14 succeeded
Details
PSScriptAnalyzer-CI (Build Full_Build) Build Full_Build succeeded
Details
PSScriptAnalyzer-CI (Test Ubuntu_16_04) Test Ubuntu_16_04 succeeded
Details
PSScriptAnalyzer-CI (Test Ubuntu_18_04) Test Ubuntu_18_04 succeeded
Details
PSScriptAnalyzer-CI (Test Windows_Server2016_PowerShell_5_1) Test Windows_Server2016_PowerShell_5_1 succeeded
Details
PSScriptAnalyzer-CI (Test Windows_Server2016_PowerShell_Core) Test Windows_Server2016_PowerShell_Core succeeded
Details
PSScriptAnalyzer-CI (Test Windows_Server2019_PowerShell_5_1) Test Windows_Server2019_PowerShell_5_1 succeeded
Details
PSScriptAnalyzer-CI (Test Windows_Server2019_PowerShell_Core) Test Windows_Server2019_PowerShell_Core succeeded
Details
PSScriptAnalyzer-CI (Test macOS_10_14_Mojave) Test macOS_10_14_Mojave succeeded
Details
PSScriptAnalyzer-CI (Test macOS_10_15_Catalina) Test macOS_10_15_Catalina succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TylerLeonhardt TylerLeonhardt

@bergmeister bergmeister

@JamesWTruher JamesWTruher

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@rjmholt @TylerLeonhardt @bergmeister
You can’t perform that action at this time.