Allow suppression of PSUseSingularNouns for specific function

[fflaten]

PR Summary

Enables suppression of PSUseSingularNouns for specific function.

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Change is not breaking
• Make sure all .cs, .ps1 and .psm1 files have the correct copyright header
• Make sure you've added a new test if existing tests do not effectively test the code changed and/or updated documentation
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.

[fourpastmidnight]

What's the status of this PR?? I need to be able to suppress this rule. Generally, this is a good rule. But, it's flagging the word 'Data' as "plural" even though that word is both Singular and Plural.

[fflaten]

Waiting for review. Same as #1896.

Not sure what's going with the repo really. Only docs updates last 12 months. Would make sense if there was progress on PSSAv2, but IIRC that's also on hold. 😞

[fourpastmidnight]

@fflaten Yup, more of a question directed at the repo owners...your changes LGTM.

[fflaten]

Hopefully @bergmeister or @JamesWTruher can provide an update.

[fourpastmidnight]

@fflaten Quick question on this PR: shouldn't the use of the Target property of the SuppressMessageAttribute be used to suppress a specific function from being flagged by these violations? Isn't that what most of the other rules use?

I ask this, because it's really strange, but, I was able to successfully suppress this rule on two of my functions without the implementation of this PR. But on one function, it will simply not suppress. (And on the two functions where I was able to suppress this rule, I did not need to use the Target property, which is stranger still.....) The biggest question is, why does this sometimes not suppress and sometimes DOES???

[fourpastmidnight]

OK, more information. I don't think this PR is needed to suppress this rule.

Here's my exact situation: I have a PowerShell script that I was linting. There are 3 functions in it that were being flagged by this attribute. After applying the SuppressMessageAttribute, 2 of the 3 were suppressed, but the third was not. Here's the relevant structure of the code that resulted in the behavior I experienced, and afterwards, I'll explain how I finally got the 3rd function suppressed using v1.21.0 and no need for this PR (in my case, anyway).

# SUCCESSFULLY Suppressed
function Get-QueueMessageMetadata {
    [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseSingularNouns', '')]
    [CmdletBinding()]
    Param ( <# Doesn't matter what the parameters are #> )
    Begin { }
}

# FAILED to suppress, as written below
# The below two attributes did not work:
#[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseSingularNouns', '', Target='Get-QueueWithMessages')]
#[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseSingularNouns', '', Target='Get-QueueWithMessages', Scope='function')]
function Get-QueuesWithMessages {
    [CmdletBinding()]
    [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseSingularNouns', '')]
    # The below _also_ did not work, which was moved from outside the function declaration to here
    #[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseSingularNouns', '', Target='Get-QueueWithMessages')]
    Begin { }
}

# SUCCESSFULLY Suppressed
function Start-EmptyQueuesAndStopServices {
    [CmdletBinding()]
    [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseSingularNouns', '')]
    Param ( <# Doesn't matter what the actual params are #> )
    Begin { }
}

Can you spot why two of these were successfully suppressed while the middle one was not? Go ahead, give it another look, and then I'll divulge what I learned........

OK, I'll relieve your suspense if you haven't seen it yet. The middle function has no declared Param ( ) block. Once I added that, the SuppressMessageAttribute worked as expected.

I'm not sure if this is expected behavior. I can't find in any of my resources that the Param ( ) bl ock is required when using the [CmdletBinding()] attribute—the function is parsed and processed by PowerShell just fine. So I would expect that PSScriptAnalyzer would know to apply the attribute to the current context, which is the function scope in which it is enclosed. But that obviously was not happening. More than likely, PSScriptAnalyzer became confused and thought the attribute was being applied to the Begin { } block. Still, I would have expected one of the other attribute declarations I tried to have worked. (Especially the one that was declared at the script scope outside the function but which specified a target containing the function name!)

Unless there's some other use case this PR was addressing, I don't think this PR is actually needed to allow for the suppression of this attribute; it already works.

[fflaten]

@fourpastmidnight There's a difference: inheritance.

function Get-Elements
{
    # This will suppress the rule everywhere inside the targeted function -> both Get-Elements and Get-Containers will be suppressed
    [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseSingularNouns', '', Scope = 'Function', Target = 'Get-Elements')]
    # This will only suppress Get-Elements but still report Get-Containers
    [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseSingularNouns', 'Get-Elements')]
    # All SuppressMessageAttributes must be above a param() block, both at file level or inside a scriptblock.
    param()

    function Get-Containers
    {
        param()
    }
}

Target + Scope is ex. useful when applying the attributes at the top of your script, but it's gready. Using RuleSuppressionId/checkId will allow you exclude that specific occurrence only. Not that it's a realistic scenario but you could even do this:

# Intentionally conflicting functions

# Will be suppressed
function Get-Elements
{
    # This will only suppress Get-Elements
    [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseSingularNouns', 'Get-Elements')]
    param()

    # Will NOT be suppressed
    function Get-Containers
    {
        param()
    }
}

# Will NOT be suppressed
function Get-Elements
{
    # This will only suppress Get-Elements
    [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseSingularNouns', 'Get-Containers')]
    param()

    # Will be suppressed
    function Get-Containers
    {
        param()
    }
}

Multiple rules already supports RuleSuppressionId which is why I provided this PR to be consistent. Ideally though, PSSA would used MessageId for this as RuleSuppressionId\checkId should've been the rule ID just like in C#/.NET. See #276

You already figured most of this out, but some quick comments about the samples in your post:

• First sample: Nothing is suppressed, but any inner functions would be. There's no error in the first place as Get-QueueMessageMetadata is singular.
• Second sample: When using Target you also need Scope (you should get an error detailing this when running Invoke-ScriptAnalyzer). The suppression also needs to occur directly above a param() block as you mentioned. I've requested an alternative to this for PSSAv2 here: Improved rule suppression #1483 (comment)
• Third sample: Works since attribute is above param(), but it will also apply to any child functions etc.

Especially the one that was declared at the script scope outside the function but which specified a target containing the function name!

It should work if you add it above param() at the top of your script.

[fourpastmidnight]

Thanks for your comments. They are helpful to allow me to better understand how PowerShell, and specifically, PSSA is working with attributes.

However, to the contrary; when I did not supply a suppression attribute for Get-QueueMessageMetadata, the function was flagged. I'm running PSSA from a PowerShell Core session, which is using the Pluralization.NET library (instead of EF pluralization service), and data is both singular and plural at the same time and is flagged. In fact, IIRC for this particular rule, there is explicit code to ignore the word data! 😄.

As you pointed out for the second one, I was missing the Param () block. Is this a requirement of PSSA, or, as I suggested, an ambiguity in the PowerShell parser which therefore, makes that a requirement for PSSA? I'm simply curious at this point about the technical underpinning of that requirement, since the advanced function in and of itself does not require a Param() block.

It is interesting to see that the attributes I supplied would suppress the enclosing function and all child functions. That I did not know/understand.

Thanks again for helping me better understand PSSA (and PowerShell). I have many years of PowerShell under my belt, but I continue to learn new things about it every day!

[fourpastmidnight]

Regarding my question on the need for the Param () block, I raised an issue about it via #1930. This is indeed some sort of PowerShell parser ambiguity because of the way the language was designed when incorporating the use of attributes. The addition of the Param () block disambiguates the parsing such that the attributes get applied to the right syntactical block. Other than that, everything else I said above stands.

[fflaten]

It's inherited design/behavior from C#/.NET AFAIK. Attributes in general are placed above the element they should target/associate with, which is why you need param() to basically say "this scriptblock/file". If not it'd keep looking for the first function/class.

Using attributes
Attributes can be placed on almost any declaration, though a specific attribute might restrict the types of declarations on which it's valid. In C#, you specify an attribute by placing the name of the attribute enclosed in square brackets ([]) above the declaration of the entity to which it applies.

Source

In the issue I mentioned in the last post a comment-based suppression method is suggested for PSSAv2 to get more granular control without this limitation, similar to pragma used in other languages.

I can't find in any of my resources that the Param ( ) block is required when using the [CmdletBinding()] attribute—the function is parsed and processed by PowerShell just fine.

It is.

> function t { [CmdletBinding()] }
ParserError:
Line |
   1 |  function t { [CmdletBinding()] }
     |               ~~~~~~~~~~~~~~~~~
     | Unexpected attribute 'CmdletBinding'.

> function t { [CmdletBinding()]param() }
# no error

Thanks again for helping me better understand PSSA (and PowerShell). I have many years of PowerShell under my belt, but I continue to learn new things about it every day!

Likewise. That's what makes it fun 😃

[fourpastmidnight]

Regarding not needing Param () block: Huh! But the function ran just fine without it! I don't like that (it seems) PowerShell sometimes "lets things slide". 😉 When I first came to PowerShell, I was a C# dev. And naturally, I treated PowerShell like C# in the console. Boy was that a mistake! It wasn't long before I got burned and was wondering what the heck was going on! I've come a long way since then, but there are still lots of surprises with PowerShell--to the point where I have a love/hate relationship with it. To be honest, though, I more love it than hate it. It's a deep language.

Thanks again! I'll be more "dogmatic" when writing functions and always supply a Param () block even if no params are needed. In the end, it'll save me a lot of headaches! 😋

[JamesWTruher]

I like this, although we will need additional documentation.

[JamesWTruher]

@sdwheeler - this will need some documentation eventually

[sdwheeler]

@sdwheeler - this will need some documentation eventually

@JamesWTruher I am not sure what needs to be documented here. Can you elaborate? Can't all rules be suppressed (ie. was this a bug) or are there other rules that can't be suppressed? If there are rules that can't be suppressed, then we need a list.

[bergmeister]

@sdwheeler @fflaten Documentation wise we already document here some examples of rules where one can suppress more specifically: https://learn.microsoft.com/en-us/powershell/utility-modules/psscriptanalyzer/using-scriptanalyzer?view=ps-modules#suppressing-rules
My recommendation would be to leave a small remark in above document as well as in the rules own documentation markdown file here. Otherwise LGTM

[bergmeister]

I added a suppression example to the rule info, which I think is all we need to merge this now