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

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

Open
StingyJack opened this issue Jan 18, 2018 · 23 comments

Comments

@StingyJack
Copy link

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?

@bergmeister
Copy link
Collaborator

bergmeister commented Jan 21, 2018

@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).

@JamesWTruher
Copy link
Member

something like?

#PSSCRIPTANALYZER -PSAvoidUsingCmdletAliases
gci
#PSSCRIPTANALYZER +PSAvoidUsingCmdletAliases

or something more?

@bergmeister
Copy link
Collaborator

bergmeister commented Jan 22, 2018

@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

@JamesWTruher
Copy link
Member

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.

@StingyJack
Copy link
Author

@JamesWTruher yes, something like that

@gmitch64
Copy link

gmitch64 commented Feb 6, 2018

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
Copy link

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

@essentialexch
Copy link

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

@bergmeister
Copy link
Collaborator

bergmeister commented Nov 21, 2018

@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.

@essentialexch
Copy link

essentialexch commented Nov 21, 2018

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).

@bergmeister
Copy link
Collaborator

bergmeister commented Nov 21, 2018

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.

@HerbM
Copy link

HerbM commented Oct 6, 2019

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.

@tats-u
Copy link

tats-u commented Dec 23, 2019

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
}

@franklesniak
Copy link

franklesniak commented Jan 6, 2020

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.

@bergmeister
Copy link
Collaborator

bergmeister commented Jan 6, 2020

@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)
        }
        # ....
    }
}

@Royalty087
Copy link

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.

@LinuxOnTheDesktop
Copy link

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

@revolter
Copy link

revolter commented May 30, 2021

I tried using

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

but it doesn't work.

@VWACRansom
Copy link

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.

@srbrills
Copy link

srbrills commented Nov 7, 2023

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
    )
}

@syedhassaanahmed
Copy link

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.

@HeyItsGilbert
Copy link

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.

@air2
Copy link

air2 commented Apr 25, 2024

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.

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

No branches or pull requests