From 0461983d6964362ae59a254798dad9c13c6230d6 Mon Sep 17 00:00:00 2001 From: fadwen <110697945+fadwen@users.noreply.github.com> Date: Tue, 6 Oct 2026 00:29:00 -0700 Subject: [PATCH] perf(analysis): one walk of the syntax tree per script, indexed by node type and command name Every rule walked the syntax tree on its own through Find-IslAstNode and Find-IslCommand, and the rules that look for several command names walked it once per name: about forty walks for a script, at about 11 ms each for a 95-line detection. That was most of what an analysis cost. Get-IslAstIndex walks a tree once, on first use, and keeps the nodes grouped by type name and the commands by name for the tree's lifetime, in a ConditionalWeakTable keyed on the AST object. Find-IslAstNode and Find-IslCommand read the index; their parameters, results and result order are unchanged, so no rule changes. The walk and the grouping stay in .NET: FindAll takes a BlockingCollection.TryAdd delegate as its always-true predicate, and Enumerable.ToLookup groups the nodes with Object.GetType as the key selector, because a PowerShell script block called once per node cost more than the walk itself. Measured on a 95-line detection, warm: 91 ms a script against 233 ms, 60 scripts in 5.4 s against 14.0 s, the same 360 findings. The index itself takes 5 ms. --- CHANGELOG.md | 4 + Private/Find-IslAstNode.ps1 | 94 +++++++++++++++++--- Tests/Unit/Private/Find-IslAstNode.Tests.ps1 | 26 +++++- 3 files changed, 110 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e409aca..056e373 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,10 @@ release notes. The tags counted are the ones the Gallery lists, the manifest's plus `PSModule`, the editions and two per exported command, so a new command costs about twice its name. The module contract test holds the same three limits, so a pull request fails before a release does. +- `Test-IntuneScript` runs about two and a half times faster: 91 ms a script against 233 ms for a 95-line + detection, 60 scripts in 5.4 s against 14.0 s. Every rule walked the syntax tree itself, some once per + command name they look for; the tree is now walked once per script and the nodes indexed by type and + the commands by name, and the rules read the index. Findings are unchanged. ## [0.27.0] - 2026-10-05 Fixes to the runtime harness and to four rules, each rule change backed by a tenth validation diff --git a/Private/Find-IslAstNode.ps1 b/Private/Find-IslAstNode.ps1 index 301a173..ee253bd 100644 --- a/Private/Find-IslAstNode.ps1 +++ b/Private/Find-IslAstNode.ps1 @@ -1,14 +1,79 @@ # AST helpers shared by the rules. Type names are compared as strings so the module loads on # Windows PowerShell 5.1, where the PowerShell 7 node types (ternary, pipeline chain) don't exist. +# One walk per tree, shared by every rule that asks about it: the nodes grouped by type name and +# the commands grouped by name. Fifteen rules each walking the tree, some of them once per command +# name they look for, was most of the time an analysis took. The table is keyed on the AST object +# itself, so a sub-tree a caller passes gets an index of its own and a tree that goes out of scope +# takes its index with it. +$script:IslAstIndex = [System.Runtime.CompilerServices.ConditionalWeakTable[object, object]]::new() + +# The walk and the grouping stay in .NET: FindAll calls its predicate once per node, and a script +# block there costs more than the walk itself; BlockingCollection.TryAdd is a Func that +# is always true and keeps the nodes in the order they were visited. Object.GetType as an open +# delegate is the key selector for Enumerable.ToLookup, which keeps each group in source order. +$script:IslAstNodeType = [System.Management.Automation.Language.Ast] +$script:IslAstCollectorType = + [System.Collections.Concurrent.BlockingCollection[System.Management.Automation.Language.Ast]] +$script:IslAstGetType = [System.Delegate]::CreateDelegate( + [System.Func[System.Management.Automation.Language.Ast, type]], [object].GetMethod('GetType')) +$script:IslAstToLookup = ([System.Linq.Enumerable].GetMethods() | + Where-Object { $_.Name -eq 'ToLookup' -and $_.GetParameters().Count -eq 2 } | + Select-Object -First 1).MakeGenericMethod($script:IslAstNodeType, [type]) + +function Get-IslAstIndex { + <# + .SYNOPSIS + The node and command index of an AST, built on first use and kept for the tree's lifetime. + #> + [CmdletBinding()] + [OutputType('IntuneScriptLab.AstIndex')] + param( + [Parameter(Mandatory)] + [System.Management.Automation.Language.Ast]$Ast + ) + $index = $null + if ($script:IslAstIndex.TryGetValue($Ast, [ref]$index)) { return $index } + + # FindAll visits the tree in document order, the root first, and that order is what the groups keep + $collector = $script:IslAstCollectorType::new() + $collect = [System.Delegate]::CreateDelegate([System.Func[System.Management.Automation.Language.Ast, bool]], + $collector, $script:IslAstCollectorType.GetMethod('TryAdd', [type[]]@($script:IslAstNodeType))) + $null = $Ast.FindAll($collect, $true) + $lookup = $script:IslAstToLookup.Invoke($null, @([object]$collector.ToArray(), $script:IslAstGetType)) + + # Keyed by the type's short name, the way the rules ask: a 7-only node type is a key that is + # never there on 5.1, not a type that fails to resolve + $byType = [System.Collections.Generic.Dictionary[string, object]]::new([System.StringComparer]::Ordinal) + foreach ($group in $lookup) { $byType[$group.Key.Name] = $group } + $byCommand = [System.Collections.Generic.Dictionary[string, System.Collections.Generic.List[object]]]::new( + [System.StringComparer]::OrdinalIgnoreCase) + if ($byType.ContainsKey('CommandAst')) { + foreach ($command in $byType['CommandAst']) { + # A command whose name is not a constant (& $tool, "$prefix-Item") has no name to index + $commandName = $command.GetCommandName() + if (-not $commandName) { continue } + if (-not $byCommand.ContainsKey($commandName)) { + $byCommand[$commandName] = [System.Collections.Generic.List[object]]::new() + } + $byCommand[$commandName].Add($command) + } + } + $index = [pscustomobject]@{ + PSTypeName = 'IntuneScriptLab.AstIndex' + ByType = $byType + ByCommand = $byCommand + } + $script:IslAstIndex.Add($Ast, $index) + $index +} + function Find-IslAstNode { <# .SYNOPSIS Finds AST nodes by type name, optionally filtered by a predicate. #> [CmdletBinding()] - # The parameters are used inside the FindAll predicate; the analyzer can't see through it - [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', '')] param( [Parameter(Mandatory)] [System.Management.Automation.Language.Ast]$Ast, @@ -19,12 +84,15 @@ function Find-IslAstNode { [scriptblock]$Where ) - $Ast.FindAll({ - param($node) - if ($node.GetType().Name -notin $TypeName) { return $false } - if ($Where) { return [bool](& $Where $node) } - $true - }, $true) + $index = Get-IslAstIndex -Ast $Ast + $nodes = foreach ($name in $TypeName) { + if ($index.ByType.ContainsKey($name)) { $index.ByType[$name] } + } + # Each type's list is in document order; several types are merged back into it + if ($TypeName.Count -gt 1) { $nodes = $nodes | Sort-Object -Property { $_.Extent.StartOffset } } + foreach ($node in $nodes) { + if (-not $Where -or [bool](& $Where $node)) { $node } + } } function Find-IslCommand { @@ -33,7 +101,6 @@ function Find-IslCommand { Finds command invocations by name (case-insensitive), including aliases the caller lists. #> [CmdletBinding()] - [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', '')] param( [Parameter(Mandatory)] [System.Management.Automation.Language.Ast]$Ast, @@ -41,11 +108,12 @@ function Find-IslCommand { [Parameter(Mandatory)] [string[]]$Name ) - Find-IslAstNode -Ast $Ast -TypeName CommandAst -Where { - param($node) - $commandName = $node.GetCommandName() - $commandName -and $commandName -in $Name + $index = Get-IslAstIndex -Ast $Ast + $commands = foreach ($commandName in $Name) { + if ($index.ByCommand.ContainsKey($commandName)) { $index.ByCommand[$commandName] } } + if ($Name.Count -gt 1) { $commands = $commands | Sort-Object -Property { $_.Extent.StartOffset } } + $commands } function Test-IslCommandParameter { diff --git a/Tests/Unit/Private/Find-IslAstNode.Tests.ps1 b/Tests/Unit/Private/Find-IslAstNode.Tests.ps1 index 9f7e53b..18c8bf9 100644 --- a/Tests/Unit/Private/Find-IslAstNode.Tests.ps1 +++ b/Tests/Unit/Private/Find-IslAstNode.Tests.ps1 @@ -36,6 +36,26 @@ Describe 'Find-IslAstNode' -Tag 'Unit', 'Private' { } } + It 'returns nodes of several types in document order, and the same tree is indexed once' { + InModuleScope IntuneScriptLab -Parameters @{ Ast = $script:Ast } { + $nodes = @(Find-IslAstNode -Ast $Ast -TypeName ExitStatementAst, FunctionDefinitionAst) + $nodes.Count | Should-Be 4 + $nodes[0].GetType().Name | Should-Be 'FunctionDefinitionAst' + $offsets = @($nodes | ForEach-Object { $_.Extent.StartOffset }) + $offsets | Should-BeCollection @($offsets | Sort-Object) + $first = Get-IslAstIndex -Ast $Ast + $second = Get-IslAstIndex -Ast $Ast + [object]::ReferenceEquals($first, $second) | Should-BeTrue + # The index lists what FindAll finds, in the order FindAll finds it + $walked = @($Ast.FindAll({ param($node) $node.GetType().Name -eq 'CommandAst' }, $true)) + $indexed = @($first.ByType['CommandAst']) + $indexed.Count | Should-Be $walked.Count + for ($i = 0; $i -lt $walked.Count; $i++) { + [object]::ReferenceEquals($walked[$i], $indexed[$i]) | Should-BeTrue + } + } + } + It 'returns nothing for a type that is not in the tree' { InModuleScope IntuneScriptLab -Parameters @{ Ast = $script:Ast } { @(Find-IslAstNode -Ast $Ast -TypeName TernaryExpressionAst).Count | Should-Be 0 @@ -48,7 +68,11 @@ Describe 'Find-IslCommand' -Tag 'Unit', 'Private' { It 'matches command names case-insensitively, any of several' { InModuleScope IntuneScriptLab -Parameters @{ Ast = $script:Ast } { @(Find-IslCommand -Ast $Ast -Name 'get-item').Count | Should-Be 2 - @(Find-IslCommand -Ast $Ast -Name 'Get-Item', 'Write-Output').Count | Should-Be 3 + $several = @(Find-IslCommand -Ast $Ast -Name 'Write-Output', 'Get-Item') + $several.Count | Should-Be 3 + # Document order whatever the order of the names asked for + $names = @($several | ForEach-Object { $_.GetCommandName() }) + $names | Should-BeCollection @('Get-Item', 'Get-Item', 'Write-Output') @(Find-IslCommand -Ast $Ast -Name 'Remove-Item').Count | Should-Be 0 } }