PSScriptAnalyzer icon indicating copy to clipboard operation
PSScriptAnalyzer copied to clipboard

Feature request: Mark specific lines or blocks to ignore for specific rules

Open StingyJack opened this issue 7 years ago • 25 comments

Like pragma in c# can disable a warning for a line and then enable it again.

As an example, I've got a few variables that are used to within Invoke-Expression's, and the script analyzer flags them as unused variables but they are actually used. I dont want to ignore the rule for the entire script, just these few variables.

Is there a way to do this?

StingyJack avatar Jan 18 '18 20:01 StingyJack

@StingyJack You could define a local function and suppress it on this local one only (you can define even define functions inside functions to limit its scope). Because of the way scoping works in PowerShell (a child scope inherits the variables from its parent), you do not even need to pass in the variables in theory (although personally I am against this practice for the purpose of clean code).

bergmeister avatar Jan 21 '18 00:01 bergmeister

something like?

#PSSCRIPTANALYZER -PSAvoidUsingCmdletAliases
gci
#PSSCRIPTANALYZER +PSAvoidUsingCmdletAliases

or something more?

JamesWTruher avatar Jan 22 '18 20:01 JamesWTruher

@JamesWTruher Is this a future design proposal because I tried it and it did not work. If so, then yes, I would upvote this. As a simple starter I would suggest something that can just suppress a warning on the next executing line similar to ReSharper

#PSSCRIPTANALYZER SuppressOnce PSAvoidUsingCmdletAliases
gci # gets suppressed
iex 'format c' # does not get suppressed

bergmeister avatar Jan 22 '18 21:01 bergmeister

It's definitely not present, I was just exploring the usability of the suggestion. I do think it would be generally useful and something that would benefit from some sort of RFC. I would like to spark the discussion, for sure.

JamesWTruher avatar Jan 22 '18 22:01 JamesWTruher

@JamesWTruher yes, something like that

StingyJack avatar Jan 23 '18 00:01 StingyJack

You should be able to do it with something like

[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingCmdletAliases", "")]

I just posted an issue where this works for suppressing the error from a single variable (PSUseDeclaredVarsMoreThanAssignments), but it doesn't seem to honor the scope flag (unless I am doing something wrong).

Graham

gmitch64 avatar Feb 06 '18 19:02 gmitch64

I'd like to upvote this - I'd like to see a solution that lets me suppress any of the checks for a section of the code in a file, where a section could be a single line, a couple of lines, or the whole file (I have a couple of ps1 files that purely define variables for our environment, so I get hundreds of variable assigned but never used messages.

Graham

gmitch64 avatar Feb 12 '18 14:02 gmitch64

Does this need an RFC as @JamesWTruher sort-of suggested?

essentialexch avatar Nov 21 '18 18:11 essentialexch

@swngdnz An RFC might help to push it further and alert about its importance. The only downside is that we could end up creating the most beautiful design that is very hard to implement. We should rather strive to go for a minimum viable improvement to suppress e.g. only the next line/statement. So, yes to being loud, I only fear that RFCs sometimes lead to something being discussed to death. At the end of the day we also need a commitment from someone to implement the proposal, otherwise the RFC is pointless.

bergmeister avatar Nov 21 '18 18:11 bergmeister

Wirth taught me 35 years ago that "best" was the death of progress. That's why he kept designing new languages. :-)

That's why I suggested what I thought would be simple (#pragma once/disable/restore). It's similar to what I did in a C compiler I wrote in the 1980's (although I think I used once/push/pop since it was a stack based compiler).

I'll take a look at it. But I'm not sure my C# skills are good enough (I'm guessing it's C#, if it's javascript, I'm completely out of it).

essentialexch avatar Nov 21 '18 18:11 essentialexch

it's C#. Familiarity with the AST (abstract syntax tree) of PowerShell or at least its tokens are essential but I am happy to help with some bits or gluing stuff together at the end.

bergmeister avatar Nov 21 '18 18:11 bergmeister

Late to the party, but I like it too, even though I have no idea if it would ever be worth the effort for you folks to code it.

BTW, I suppress the specific PSUseDeclaredVarsMoreThanAssignments by assigning variables like this to $null. (I have a few legitimate cases where this is intentional, but don't want to suppress the rule since it is frequently a misspelling or logic error.

HerbM avatar Oct 06 '19 17:10 HerbM

I don't want it to vomit an invalid lint to the following code to keep the compatibility between 5- and 6:

if (-not (Test-Path variable:IsWindows)) {
  $IsWindows = Test-Path $env:windir
}

tats-u avatar Dec 23 '19 07:12 tats-u

I'm running into something similar as tats-u. I am getting warnings for Get-CimInstance and Get-WmiObject and would like to suppress them given that I have properly limited the code execution so that they are never run in an incompatible environment.

function Get-OperatingSystemVersion
{
    if (Test-OperatingSystemIsWindows)
    {
        if ((Get-PSMajorVersion) -ge 3)
        {
            # Use Get-CimInstance
            $arrCimInstances = @(Get-CimInstance "Win32_OperatingSystem" -ErrorAction SilentlyContinue)
        } `
        else `
        {
            # Use Get-WmiObject
            $arrCimInstances = @(Get-WmiObject "Win32_OperatingSystem" -ErrorAction SilentlyContinue)
        }
        # ....
}

I would like to be able to use a comment to block certain rules on certain lines/blocks of code. I would greatly prefer not to use the param block for this functionality, as not all my code uses a param block. Also the param block would block the rule for the whole function/code scope, which is not desirable.

franklesniak avatar Jan 06 '20 05:01 franklesniak

@franklesniak The rule says that starting from PowerShell v3, the CIM cmdlets should be preferred, so not sure why you cannot use them. If for whatever reason you want to keep using WMI, you could suppress as follows (see here):

function Get-OperatingSystemVersion
{
    [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWMICmdlet', '', Justification = 'Required for PS 3')]
    Param()

    if (Test-OperatingSystemIsWindows)
    {
        if ((Get-PSMajorVersion) -ge 3)
        {
            # Use Get-CimInstance
            $arrCimInstances = @(Get-CimInstance "Win32_OperatingSystem" -ErrorAction SilentlyContinue)
        } `
            else `

        {
            # Use Get-WmiObject
            $arrCimInstances = @(Get-WmiObject "Win32_OperatingSystem" -ErrorAction SilentlyContinue)
        }
        # ....
    }
}

bergmeister avatar Jan 06 '20 10:01 bergmeister

I'd like to add my vote to this issue. There are definitely use cases where a variable will trigger the "assigned but never used" problem when that's not the case.

I would suggest using a similar functionality to that of DevSkim, which allows for a small comment on the line throwing the error to function as a switch to stop the error. adding '#DevSkim: ignore DS104456' to the end of a line that triggers a warning for using a restricted cmdlet, makes the scanner ignore that specific instance of the violation.

Royalty087 avatar Apr 09 '20 21:04 Royalty087

The current situation is indeed ugly. In order to suppress the relevant warning for a few lines - and only a few lines - of a script, I have to do something with this form:


[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUserDeclaredVarsMoreThanAssignments', '', scope='function')]
$LogEngineHealthEvent					=   $false
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUserDeclaredVarsMoreThanAssignments', '', scope='function')]
$LogEngineLifeCycleEvent				=   $false
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUserDeclaredVarsMoreThanAssignments', '', scope='function')]
$LogProviderHealthEvent					=   $false
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUserDeclaredVarsMoreThanAssignments', '', scope='function')]
$LogProviderLifeCycleEvent				=   $false

LinuxOnTheDesktop avatar May 27 '21 21:05 LinuxOnTheDesktop

I tried using

param(
    [Parameter(Mandatory = $true)]
    [Alias('MP')]
    [String]
    [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', '', scope = 'function')]
    $MyParameter
)

but it doesn't work.

revolter avatar May 30 '21 13:05 revolter

UPVOTE!!!

We have a tool that runs PSScriptAnalyzer as part of our Pull Request process. It will not let anything be checked in that has any Errors or Warnings. Recently, we started seeing a lot of PSUseSingularNouns. In some cases this is intentional. For example: Send-MessageToTeams (As in Microsoft Teams)

We have had to disable that rule so we can keep things the way we want them, but it would be FAR better to be able to disable the rule just in the approved cases.

VWACRansom avatar Sep 30 '21 18:09 VWACRansom

Making a note here in case someone else runs into this issue. I noticed that you cannot place the SuppressMessageAttribute on specific parameters in a Param block, but instead you have to place the attribute on the Param block as a whole.

function Verb-PrefixNoun {
    [CmdletBinding()]
    [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidDefaultValueSwitchParameter', '')]
    Param (
        [Parameter(Mandatory)
        [switch] $MyParameter = $true
    )
}

srbrills avatar Nov 07 '23 18:11 srbrills

Just ran into this when installing Chocolatey on a clean Windows device for the first time. The Choco docs suggest the use of Invoke-Expression. I'd have liked to suppress the PSAvoidUsingInvokeExpression warning with a comment, but it doesn't seem possible.

syedhassaanahmed avatar Feb 15 '24 09:02 syedhassaanahmed

Using comments would be nice. Rubocop does something like:

# rubocop:disable RuleByName
This is a long line 
# rubocop:enable RuleByName

It also supports having multiple rules disabled with a simple string array.

HeyItsGilbert avatar Apr 11 '24 22:04 HeyItsGilbert

Comments should be the way to go IMHO, you do not want to interfere with the script logic itself. eslint also uses the same approach as @HeyItsGilbert mentioned. It also has an disable-next-line comment. Which comes in very handy, so you almost never have to use block with enable and disable, just sometimes.

air2 avatar Apr 25 '24 09:04 air2