PSScriptAnalyzer icon indicating copy to clipboard operation
PSScriptAnalyzer copied to clipboard

Custom rule is invoked twice for a single script

Open lzybkr opened this issue 9 years ago • 8 comments

I am writing custom rules and I see an error reported twice when there should be just one report.

I invoke script analyzer like this:

#11 PS> Invoke-ScriptAnalyzer -CustomRulePath D:\gh\PesterAnalyzerRules -Path a.ps1

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PesterAnalyzerRules\Measure-ShouldT Information  a.ps1      3     Pipe a script block to 'Should Throw' or 'Should Not Throw'
hrow
PesterAnalyzerRules\Measure-ShouldT Information  a.ps1      3     Pipe a script block to 'Should Throw' or 'Should Not Throw'
hrow


@ D:\xtmp
#12 PS> cat a.ps1

Describe "a" {
    "b" | Should Throw
}

In a debugger, I noticed my rule is getting called on two different threads.

lzybkr avatar Jan 10 '17 03:01 lzybkr

I think this happens because scritpanalyzer finds two scriptblocks and run the rule on those two scriptblocks:

PS> {Describe "a" {
>>     "b" | Should Throw
>> }}.Ast.FindAll({$args[0] -is [System.Management.Automation.Language.ScriptBlockAst]}, $true)


Attributes         : {}
UsingStatements    : {}
ParamBlock         :
BeginBlock         :
ProcessBlock       :
EndBlock           : Describe "a" {
                         "b" | Should Throw
                     }
DynamicParamBlock  :
ScriptRequirements :
Extent             : {Describe "a" {
                         "b" | Should Throw
                     }}
Parent             : {Describe "a" {
                         "b" | Should Throw
                     }}

Attributes         : {}
UsingStatements    : {}
ParamBlock         :
BeginBlock         :
ProcessBlock       :
EndBlock           : "b" | Should Throw
DynamicParamBlock  :
ScriptRequirements :
Extent             : {
                         "b" | Should Throw
                     }
Parent             : {
                         "b" | Should Throw
                     }

Therefore, when your implementation searches for Should command in a nested manner, you get two violations.

kapilmb avatar Jan 10 '17 19:01 kapilmb

I see. It is surprising to me that ScriptAnalyzer walks the root ScriptBlockAst for me, I would have thought the rule decides if it needs to analyze nested script blocks.

lzybkr avatar Jan 10 '17 23:01 lzybkr

If you are writing a rule in C# you need to walk the Ast. But, if you write it in PowerShell then ScriptAnalyzer gives you all the instances of the Ast type that the powershell function takes as a parameter.

kapilmb avatar Jan 11 '17 02:01 kapilmb

Documentation could be improved, this document says nothing on the matter. In fact, the only examples have the same flaw - they accept a ScriptBlockAst and pass $true when calling Findall.

I understand the rationale if a rule accepts types other than ScriptBlockAst, but ScriptBlockAst feels like maybe it should be different.

lzybkr avatar Jan 12 '17 02:01 lzybkr

By the definition of ScriptBlockAst here, it is supposed to be the root node. So, sending all the nested ScriptBlocksAsts to the custom rule probably doesn't make much sense. I think I agree with your point of view. Maybe we should add an exception for ScriptBlockAst.

kapilmb avatar Jan 12 '17 04:01 kapilmb

Are there any plans to implement the exception?

fredleejr avatar Oct 27 '18 02:10 fredleejr

Got to say that this is frustrating. I've put in code to check if you are the top AST, to avoid this. But thats a hack.

simonsabin avatar Nov 06 '20 21:11 simonsabin

I ran into the same issue (even trippel entries). But I don't quiet understand the issue. Isn't it just a pure documentation issue as the solution appears to me just setting the searchNestedScriptBlocks parameter in the concerned lines of the (incorrect?) example

 [System.Management.Automation.Language.Ast[]]$methodAst     = $ScriptBlockAst.FindAll($predicate1, $true)
 [System.Management.Automation.Language.Ast[]]$assignmentAst = $ScriptBlockAst.FindAll($predicate2, $true)

to $False:

 [System.Management.Automation.Language.Ast[]]$methodAst     = $ScriptBlockAst.FindAll($predicate1, $false)
 [System.Management.Automation.Language.Ast[]]$assignmentAst = $ScriptBlockAst.FindAll($predicate2, $false)

?

iRon7 avatar Nov 03 '22 15:11 iRon7