★ wanayoo — archive 1999 https://github.com/PowerShell/PSScriptAnalyzer/issues/849Nouvelle recherche | Portail wanayoo
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 · 16 comments
Open

Comments

@StingyJack
Copy link

@StingyJack StingyJack commented Jan 18, 2018

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

@JamesWTruher JamesWTruher commented Jan 22, 2018

something like?

#PSSCRIPTANALYZER -PSAvoidUsingCmdletAliases
gci
#PSSCRIPTANALYZER +PSAvoidUsingCmdletAliases

or something more?

@bergmeister
Copy link
Collaborator

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

@JamesWTruher JamesWTruher commented Jan 22, 2018

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

@StingyJack StingyJack commented Jan 23, 2018

@JamesWTruher yes, something like that

@gmitch64
Copy link

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

@gmitch64 gmitch64 commented Feb 12, 2018

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

@essentialexch essentialexch commented Nov 21, 2018

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

@bergmeister
Copy link
Collaborator

@bergmeister 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 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 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 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 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 franklesniak commented Jan 6, 2020

I'm running into something similar as tats-u. 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)
        }
        # ....
}
@bergmeister
Copy link
Collaborator

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

@Royalty087 Royalty087 commented Apr 9, 2020

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.

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

Successfully merging a pull request may close this issue.

None yet
10 participants
You can’t perform that action at this time.