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

'PROCESS' is implicitly aliasing 'Get-PROCESS' #1402

Closed
RichMath opened this issue Feb 1, 2020 · 16 comments · Fixed by #1638
Closed

'PROCESS' is implicitly aliasing 'Get-PROCESS' #1402

RichMath opened this issue Feb 1, 2020 · 16 comments · Fixed by #1638
Assignees

Comments

@RichMath
Copy link

RichMath commented Feb 1, 2020

Using version 1.18.3:
In a .psm1 file using a "process" block, this diagnostic is produced:

'PROCESS' is implicitly aliasing 'Get-PROCESS' because it
is missing the 'Get-' prefix. This can introduce possible
problems and make scripts hard to maintain. Please consider
changing command to its full name.

@ghost ghost added the Needs: Triage 🔍 label Feb 1, 2020
@RobertKlohr
Copy link

I'm running into the same issue. I am working on a project where I am coding multiple advanced functions as separate .ps1 files. I have not isolated the triggering event but I most often notice it after I save a the file I'll see the number of errors listed go from zero to some number greater than zero. I now first go look at the process{} block of the function and I will find that it has been renamed to Get-Process.

Sometimes when I go to fix the change it will revert back as I am typing. In these cases I need to close the editor window and re-open it and then make the correction.

PSScriptAnalyzer 1.18.3
VSCODE 1.41.1
PowerShell Preview 2020.1.0

I experienced the same behavior when I switched to PowerShell stable version and disabled all my extensions.

@SydneyhSmith
Copy link
Collaborator

@RichMath thanks for opening this issue! The AST version comes from PowerShell so it would be helpful if you and @rjkgithub could please provide $PSVersionTable

A repro script where this issue is observed would also be very helpful. Thanks!

@bergmeister
Copy link
Collaborator

I cannot reproduce using PowerShell 5.1 or 7 on Windows using the following example. Please specify the example that triggers it for you and supply more info about your info such as PS and OS version

function Get-Noun {

    process {
        
    }

}

@RichMath
Copy link
Author

RichMath commented Feb 4, 2020

Import-Cut.txt

@RichMath thanks for opening this issue! The AST version comes from PowerShell so it would be helpful if you and @rjkgithub could please provide $PSVersionTable
A repro script where this issue is observed would also be very helpful. Thanks!

A script that exhibits this problem is attached to this reply.

Windows version:
Edition: Windows 10 Pro
Version: 1903
OS Build: 18362.592

PS C:\Users\richm> $PSVersionTable

Name Value


PSVersion 5.1.18362.145
PSEdition Desktop
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
BuildVersion 10.0.18362.145
CLRVersion 4.0.30319.42000
WSManStackVersion 3.0
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1

@RichMath
Copy link
Author

RichMath commented Feb 4, 2020

I cannot reproduce using PowerShell 5.1 or 7 on Windows using the following example. Please specify the example that triggers it for you and supply more info about your info such as PS and OS version
function Get-Noun {

process {
    
}

}

I included all the above in my reply to SydneyhSmith.

@bergmeister
Copy link
Collaborator

bergmeister commented Feb 4, 2020

@RichMath Using your supplied Import-Cut.txt in your comment (and renaming it to .psm1), I still cannot reproduce using either PowerShell 5.1 or 7. My OS is Windows 10 Pro 1909, which can be considered equivalent to your PowerShell version as far as I am aware. Since Windows 1903, the version of .Net (on which PowerShell depends on), is 4.8, therefore there should not be a difference between Windows 10 1903 and 1909 given that the PowerShell team only distributes security relates patches to PowerShell 5.1. My version number is 5.1.18362.628 and the difference in the last number should only be because it came from a different Windows build since Windows PowerShell ships as part of Windows

Can you please run Invoke-ScriptAnalyzer Path $PathToFile -Verbose using both a normal PowerShell console and using the PowerShell integrated console in VS-Code please and supply the output of it?
The same for the output of (get-command invoke-scriptanalyzer).module.path as well please.
image

@RichMath
Copy link
Author

RichMath commented Feb 4, 2020

@RichMath Using your supplied Import-Cut.txt in your comment (and renaming it to .psm1), I still cannot reproduce using either PowerShell 5.1 or 7. My OS is Windows 10 Pro 1909, which can be considered equivalent to your PowerShell version as far as I am aware. Since Windows 1903, the version of .Net (on which PowerShell depends on), is 4.8, therefore there should not be a difference between Windows 10 1903 and 1909 given that the PowerShell team only distributes security relates patches to PowerShell 5.1. My version number is 5.1.18362.628 and the difference in the last number should only be because it came from a different Windows build since Windows PowerShell ships as part of Windows
Can you please run Invoke-ScriptAnalyzer Path $PathToFile -Verbose using both a normal PowerShell console and using the PowerShell integrated console in VS-Code please and supply the output of it?
The same for the output of (get-command invoke-scriptanalyzer).module.path as well please.

I think I found the problem. I wasn't able to repro the problem today . . . but I'd made a change to the code last night. Before that change the two "enums" in the script were placed before the "begin{" block. PSScriptAnalyzer did NOT flag that as an error, but Powershell did. The change I made was to move the "begin {" to include those two enums, and that made the "Process" problem disappear.
I've attached the original PSM1 file (that produced the error) and the output of Invoke-ScriptAnalyzer with the -Verbose switch.
If you need more information, please let me know.

Import-Cut.psm1.TXT
PsScriptAnalyzerOutput.txt

@bergmeister
Copy link
Collaborator

bergmeister commented Feb 5, 2020

Thanks, I can reproduce now with your new example (using PowerShell Core, not a PowerShell version issue). A much simpler repro is (and the cause seems to be the enum)

function Import-Cut{
	[CmdletBinding()]
	param()

    enum foo {
        bar
        baz
    }

	process {

	}
}

Although the enum is the cause of it, the implicit Get- aliasing applies to all commands, except to Get-Process, because of process being a keyword, therefore I think just excluding Get-Process in the rule would be a good solution but I am still interested to find out why the parser sees the process block as a CommandAst.
I definitely recommend you to move the enum definition outside of the function definition or into the begin block as you did. It does not seem to cause an error when parsing the script but the parsing result is definitely corrupt, a deficiency of the parser to not spot the invalid syntax I think.

To conclude, the red herring caused by Invoke-scriptanalyzer -scriptdefinition 'process' should be fixed but the issue in the presented script was invalid syntax but the parser did throw an error at parse time unfortunately (at least at runtime as you said)

@RobertKlohr
Copy link

RobertKlohr commented Feb 7, 2020

Sorry I was away for 2 days. I read through the comments and I have some additional information to what you requested.

$PSVersionTable
Name Value


PSVersion 5.1.18362.145
PSEdition Desktop
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
BuildVersion 10.0.18362.145
CLRVersion 4.0.30319.42000
WSManStackVersion 3.0
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1

My example does not have an enum definition in the code so while it may be a trigger it is not the root cause.

I have attached one file where this happens for me but I have about 20 of these files that exhibit this behavior. Each file (.ps1) contains a single advanced function with CmdletBinding that are packaged in a module that serves as an API wrapper.

The basic format of all the files is:

function Verb-Noun {

    [CmdletBinding()]

    param ()

    begin {}

    process {}

    end {}
}

@RobertKlohr
Copy link

RobertKlohr commented Feb 7, 2020

As I stated my problem seemed to be random and I was able to narrow the scope of the triggers. The automatic change to replace what is seen as an alias (I know it's a setting I can disable in VSCode) happens whenever I save the file I am working on and there is a very specific type of error in the block above the process block. The below code has such an error (missing assignment operator in a hashtable) and when I would save this file it would do the replace.

function Test-Script{

    [CmdletBinding()]
    [OutputType([hashtable])]

    param ()

    begin {
        $foobar = @{
           foo foo
        }
    }

    process {
        $foobar}

    end {}
}

This sample also has an error and triggers the rule:

function Test-Script{

    [CmdletBinding()]
    [OutputType([hashtable])]

    param ()

    begin {
        $foobar = @{
           foo = foo{
        }
    }

    process {
        $foobar}

    end {}
}

In fact I tried several tests and any configuration that resulted in triggering "MissingEndCurlyBrace" just before the process block also triggered the "PSAvoidUsingCmdletAliases" rule.

It looks like there is some pattern matching on this rule that is tripping up on the open and close curly brackets that expects no only a matched pair but that there is an operator (maybe more characters) in between them as well. Which is why the enum before the process block triggers but also any hashtable that is missing an operator. That is why the example below, while still containing an error (missing right side of operator in hashtable, does not trigger the "PSAvoidUsingCmdletAliases" rule.

function Test-Script{

    [CmdletBinding()]
    [OutputType([hashtable])]

    param ()

    begin {
        $foobar = @{
           foo =
        }
    }

    process {
        $foobar}

    end {}
}

Currently this is tagged as "Area-Rules" but I think the rule is OK and it is the code that enumerates the commands in the script that has a bug.

@KUTlime
Copy link

KUTlime commented Aug 17, 2020

I've been also experiencing this bug. Enum was also related. My syntax looked fine but the keyword process was actually translated into Get-Process in both major version of PS.

@rjmholt
Copy link
Contributor

rjmholt commented Oct 6, 2020

I've been trying to find the issue where I commented on this earlier, but I just want to point out that the constraints of this bug are narrower than I think the original issue and #1402 (comment) imply.

PSScriptAnalyzer uses the AST that the PowerShell parser returns when a script is parsed, and the parser defines the structure and therefore the meaning of tokens in the script according to its rules. There's no pattern matching or anything like that involved; PSScriptAnalyzer just processes the structure provided to it by the parser, based on whatever the script input was.

There are a couple of scenarios here, so I'll start with the most explicit:

function Test
{
    process { "Hi" }

    enum T { One; Two }
}

Trying to execute this in PowerShell (i.e. I'm not running PSScriptAnalyzer) results in a parser error:

ParserError:
Line |
   5 |      enum T { One; Two }
     |      ~~~~
     | unexpected token 'enum', expected 'begin', 'process', 'end', or 'dynamicparam'.

This is because a PowerShell function or filter either implicitly has a single block (the end block in a function, or the process block in a filter), or explicitly has blocks declared. When blocks have been explicitly declared, tokens other than the block name keywords (and braces) aren't allowed directly within the function/filter definition. Otherwise, if it's a default block, any further tokens will be interpreted as command names. That is, whether process refers to a block name (as a keyword) or Get-Process (as a command name) depends on whether we are already in a named block or not.

So how does the parser know whether we're in a named block? Well, if we're in a function/filter definition and no statements have been seen yet, we check the next token to see if it's a block name keyword. If it is, we have explicit blocks. If it's not we don't.

Once the parser has identified that explicit blocks are being used, no other statements are allowed directly within the function/filter definition.

So if you put process first, it's parsed as a keyword and the statement after it is invalid, causing the syntax error in the example above. But if we reverse the statements:

function Test
{
    enum T { One; Two }
    process { "Hello" }
}

Now executing this function definition in PowerShell succeeds, but let's try running it:

> Test
Get-Process:
Line |
   4 |      process { "Hello" }
     |              ~~~~~~~~~~~
     | Cannot evaluate parameter 'Name' because its argument is specified as a script block and there is no input. A script block cannot be evaluated without input.

This has given us a cryptic error (because this can be a valid usage in PowerShell), but really exposes a bug in our code. The tokens inside the function body comprise a statement (one defining an enum), meaning the parser sees an implicit end block. That means that other block name keyword tokens (begin, process, end, dynamicparam) will not be parsed as such, but instead as command names.

So, then the remaining issue is that when there's an existing syntax error in the script, the parser doesn't correctly repair the structure of the error and incorrectly classifies the process as a command (although "incorrectly" is a strained usage, since it's trying to infer information from bad input). I'd argue in that case that:

  • Fixing such an issue is complex and error prone at PSScriptAnalyzer's level, since it must second guess the AST it is given
  • The parser is really responsible for producing well-structured error output, so the fix belongs in PowerShell itself
  • Given that this is an error state, and a relatively uncommon one, the return on investing on improvements to the parser here is not as high as other changes might be
  • In general, it's hard to write consistent, well-behaved code to deal with malformed input. Running the formatter on scripts with errors on them is inherently risky, since what the user thinks they're looking at and what the parser sees are possibly quite different in those scenarios.

I won't close this issue just yet, since I've seen this case crop up a few times and perhaps there's something we can do about the specific case of process, but I'm not quite sure what yet.

@bergmeister
Copy link
Collaborator

bergmeister commented Jan 24, 2021

Thinking about this again. I agree that the parser is at 'fault' (not saying we should do anything there). But having said that, I think we can also agree that there is no case where one can actually run process as a command as-is in PowerShell without encountering a ParserError of Missing statement block after 'process'., please correct me if one can actually run process as-is.
Technically one can do this function process { 'hello' }; & (Get-Command process) but this is a runtime trick, PSSA only ever sees the AST and therefore I think we can say with certainty that if we encounter a CommandAst with process there is something fishy.

I suggest:

  • check if the command name is process in the rule and do not return a DiagnosticRecord in this case
  • (optional) Return a DiagnosticRecord to warn the user that process is likely a process block and the syntax around it is likely going to throw a runtime error. Alternatively we should maybe have a rule that validates what is within a ScriptBlockAst to catch invalid syntax that the Parser cannot catch for which we have issue PSSA should have a new rule to check for properly used Process blocks #1571

@KUTlime
Copy link

KUTlime commented Feb 7, 2021

This bug/feature comes back and forth to me. Over here, when no begin block is present, I receive false positive. If I hit Format document, the statement process is replaced by Get-Process. 🤦🏿‍♀️ Can somebody please finally do something with this? 🤷🏿‍♀️

obrazek

@bergmeister
Copy link
Collaborator

bergmeister commented Feb 7, 2021

@KUTlime As per above discussion, your syntax around process block is invalid, because when using a process block, only a begin/end block is allowed but other inlined code like in your example because PowerShell will actually try to execute Get-Process and PSSA is actually rightly pointing it out in this case. See #1571

@KUTlime
Copy link

KUTlime commented Feb 7, 2021

@bergmeister I see, thx for clarification.

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