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

Snippet Issues: Suppress PSScriptAnalyzer Rules malformed (scope, function, param) #5108

Closed
6 tasks done
TMA-2 opened this issue Dec 1, 2024 · 4 comments · Fixed by #5110
Closed
6 tasks done

Snippet Issues: Suppress PSScriptAnalyzer Rules malformed (scope, function, param) #5108

TMA-2 opened this issue Dec 1, 2024 · 4 comments · Fixed by #5110
Labels
Area-Snippets Issue-Bug A bug to squash. Up for Grabs Will shepherd PRs.

Comments

@TMA-2
Copy link
Contributor

TMA-2 commented Dec 1, 2024

Prerequisites

  • I have written a descriptive issue title.
  • I have searched all open and closed issues to ensure it has not already been reported.
  • I have read the troubleshooting guide.
  • I am sure this issue is with the extension itself and does not reproduce in a standalone PowerShell instance.
  • I have verified that I am using the latest version of Visual Studio Code and the PowerShell extension.
  • If this is a security issue, I have read the security issue reporting guidance.

Summary

The snippets for Function: Suppress PSScriptAnalyzer Rule, Scope, and Parameter are all malformed in various ways, from breaking issues to style suggestions. After noticing more than four, I've decided to just put them in a list with screenshots in the appropriate section. Apologies for the verbosity. Fixed proposals are in Steps to Reproduce.

Just FYI, Issue #4911 addresses a similar issue that has already been resolved. These are still issues as of 2024.5.1. I skimmed current and past issues, but couldn't find any relating to this other than the aforementioned.

Scope: Suppress PSScriptAnalyzer Rule

  • There is a missing comma after the Target= line.
  • Both the <#Category#> and Target tab stops are using 1, clobbering each other's values. Propose separating by changing one of them to 2.
    • I.e.: <#Category#>'${1:PSUseDeclaredVarsMoreThanAssignments}' and Target='${1:${TM_SELECTED_TEXT:RegexOrGlobPatternToMatchName}}'
  • There's an extraneous close brace after the Justification parameter tab stop.
    • I.e.: '${0:Reason for suppressing}}'
  • [SuppressMessageAttribute] is missing from the snippet's prefixes.

Parameter: Suppress PSScriptAnalyzer Rule

  • The first argument <#Category#> is on the same line as the type declaration, different to the other three snippets. I'd propose moving it to a new line to match.
    • I.e.: [Diagnostics.CodeAnalysis.SuppressMessageAttribute(<#Category#>(...)
  • Like Scope, there are duplicate tab stop IDs on ParameterName and Justification, and both are using $TM_SELECTED_TEXT, so they clobber each other. Propose changing ParameterName to 1 and removing $TM_SELECTED_TEXT from the same line to match the others.
    • I.e.: <#ParameterName#>'${0:${TM_SELECTED_TEXT:ParamName}}' and Justification = '${0:${TM_SELECTED_TEXT:Reason for suppressing}}'

Function: Suppress PSScriptAnalyzer Rule

  • The closing comment on CheckId is missing, breaking the snippet.
    • I.e.: <#CheckId>

General Proposals

  • Remove spaces around assignment operators (mostly Justification) since it looks like most other snippets leave them out, especially the other PSScriptAnalyzer rule snippets.
  • Use a pre-defined list for the Scope attributes since it takes a limited number of values, e.g. ${2|Function,Parameter,<...>|}.

PowerShell Version

Name                           Value
----                           -----
PSVersion                      7.4.6
PSEdition                      Core
GitCommitId                    7.4.6
OS                             Microsoft Windows 10.0.22631
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0


Name             : Visual Studio Code Host
Version          : 2024.5.1
InstanceId       : f5c6b980-e262-4571-8314-cc264fad345d
UI               : System.Management.Automation.Internal.Host.InternalHostUserInterface
CurrentCulture   : en-US
CurrentUICulture : en-US
PrivateData      : Microsoft.PowerShell.ConsoleHost+ConsoleColorProxy
DebuggerEnabled  : True
IsRunspacePushed : False
Runspace         : System.Management.Automation.Runspaces.LocalRunspace

Visual Studio Code Version

1.95.3
f1a4fb101478ce6ec82fe9627c43efbf9e98c813
x64

Extension Version

ironmansoftware.powershellprotools@2024.7.6
martinfliegner.dark-powershell-theme@1.0.0
ms-vscode.powershell@2024.5.1
tylerleonhardt.vscode-inline-values-powershell@0.0.7

Steps to Reproduce

  1. Select the area before a Param block in a PowerShell script or function (or other appropriate area for the given rule).
  2. Press Ctrl+Alt+J or select Snippets: Insert Snippet to bring up snippets
  3. Select suppress-message-rule-scope, suppress-message-rule-function, and suppress-message-rule-parameter in turn.
  4. See description for issues found

Proposed Fixes

Scope: Suppress PSScriptAnalyzer Rule

"Scope: Suppress PSScriptAnalyzer Rule": {
  "prefix": [
    "suppress-message-rule-scope",
    "[SuppressMessageAttribute]"
  ],
  "description": "Suppress a PSScriptAnalyzer rule based on a function/parameter/class/variable/object's name by setting the SuppressMessageAttribute's Target property to a regular expression or a glob pattern. More: https://docs.microsoft.com/en-us/powershell/utility-modules/psscriptanalyzer/overview?view=ps-modules#suppressing-rules",
  "body": [
    "[Diagnostics.CodeAnalysis.SuppressMessageAttribute(",
    "\t<#Category#>'${1:PSUseDeclaredVarsMoreThanAssignments}', <#CheckId#>\\$null,",
    "\tScope='${2|Function,Parameter,Class,Variable,Object|}',",
    "\tTarget='${3:${TM_SELECTED_TEXT:RegexOrGlobPatternToMatchName}}',",
    "\tJustification='${0:Reason for suppressing}'",
    ")]"
  ]
}

Parameter: Suppress PSScriptAnalyzer Rule

"Parameter: Suppress PSScriptAnalyzer Rule": {
  "prefix": [
    "suppress-message-rule-parameter",
    "[SuppressMessageAttribute]"
  ],
  "description": "Suppress a PSScriptAnalyzer rule on a parameter. More: https://docs.microsoft.com/en-us/powershell/utility-modules/psscriptanalyzer/overview?view=ps-modules#suppressing-rules",
  "body": [
    "[Diagnostics.CodeAnalysis.SuppressMessageAttribute(",
    "\t<#Category#>'${1:PSUseDeclaredVarsMoreThanAssignments}',",
    "\t<#ParameterName#>'${2:${TM_SELECTED_TEXT:ParamName}}',",
    "\tJustification = '${0:Reason for suppressing}'",
    ")]"
  ]
},

Function: Suppress PSScriptAnalyzer Rule

"Function: Suppress PSScriptAnalyzer Rule": {
  "prefix": [
    "suppress-message-rule-function",
    "[SuppressMessageAttribute]"
  ],
  "description": "Suppress a PSScriptAnalyzer rule for a function. More: https://docs.microsoft.com/en-us/powershell/utility-modules/psscriptanalyzer/overview?view=ps-modules#suppressing-rules",
  "body": [
    "[Diagnostics.CodeAnalysis.SuppressMessageAttribute(",
    "\t<#Category#>'${1:PSProvideDefaultParameterValue}', <#CheckId#>\\$null,",
    "\tScope='${2|Function,Parameter,Class,Variable,Object|}',",
    "\tJustification='${0:${TM_SELECTED_TEXT:Reason for suppressing}}'",
    ")]"
  ]
},

Visuals

Examples
Function: Suppress PSScriptAnalyzer Rule
Parameter: Suppress PSScriptAnalyzer Rule
Scope: Suppress PSScriptAnalyzer Rule

Logs

N/A. This is solely limited to snippets included with the extension as opposed to anything that would be found in a log,

@TMA-2 TMA-2 added Issue-Bug A bug to squash. Needs: Triage Maintainer attention needed! labels Dec 1, 2024
@JustinGrote
Copy link
Collaborator

@TMA-2 thanks for your submissions! The snippets are just JSON files with a particular syntax, so these are very easy to submit fixes as a pull request, and we'll happily shepherd any pull requests you have for these fixes. Marking this as "up for grabs"

@JustinGrote JustinGrote added Up for Grabs Will shepherd PRs. Area-Snippets and removed Needs: Triage Maintainer attention needed! labels Dec 1, 2024
@TMA-2
Copy link
Contributor Author

TMA-2 commented Dec 2, 2024

@TMA-2 thanks for your submissions! The snippets are just JSON files with a particular syntax, so these are very easy to submit fixes as a pull request, and we'll happily shepherd any pull requests you have for these fixes. Marking this as "up for grabs"

Thanks very much! I'm familiar enough with snippets (unlike JS or TS) that I just might give that a shot.

TMA-2 added a commit to TMA-2/vscode-powershell that referenced this issue Dec 2, 2024
Fix for issue PowerShell#5108

- Corrects a missing close comment in the Function rule snippet
- Corrects the tab stop numbering in the Parameter rule snippet as well as duplicate use of `$TM_SELECTED_TEXT`
- Adds a missing comma in The Scope rule snippet
- Minor formatting
@TMA-2
Copy link
Contributor Author

TMA-2 commented Dec 2, 2024

Well, I've created the PR, hopefully correctly!

@andyleejordan
Copy link
Member

Thanks so much @TMA-2! Changes to the snippets are fairly low risk. I looked over your PR and think it's good, about to get it merged and into the pre-release. We really appreciate it.

andyleejordan pushed a commit that referenced this issue Dec 3, 2024
Fix for issue #5108

- Corrects a missing close comment in the Function rule snippet
- Corrects the tab stop numbering in the Parameter rule snippet as well as duplicate use of `$TM_SELECTED_TEXT`
- Adds a missing comma in The Scope rule snippet
- Minor formatting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Snippets Issue-Bug A bug to squash. Up for Grabs Will shepherd PRs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants