diff --git a/.github/workflows/performance.yml b/.github/workflows/performance.yml new file mode 100644 index 000000000..1887b7ac8 --- /dev/null +++ b/.github/workflows/performance.yml @@ -0,0 +1,436 @@ +name: Compare ScriptAnalyzer performance + +on: + workflow_dispatch: + inputs: + repetitions: + description: Number of fresh-process cold/warm pairs per build + type: choice + options: ['3', '5', '10'] + default: '3' + +permissions: + contents: read + +jobs: + workload: + name: Prepare shared benchmark inputs + runs-on: windows-latest + timeout-minutes: 10 + outputs: + powershell: ${{ steps.revision.outputs.sha }} + semver: ${{ steps.semver-revision.outputs.sha }} + upstream: ${{ steps.analyzer-revisions.outputs.upstream }} + fork: ${{ steps.analyzer-revisions.outputs.fork }} + steps: + - name: Pin all analyzer revisions once + id: analyzer-revisions + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + foreach ($source in @( + @{ Name = 'upstream'; Repository = 'PowerShell/PSScriptAnalyzer'; Ref = 'main' }, + @{ Name = 'fork'; Repository = 'jessehouwing/PSScriptAnalyzer'; Ref = 'main' } + )) { + $revision = git ls-remote "https://github.com/$($source.Repository).git" "refs/heads/$($source.Ref)" + if ($LASTEXITCODE -ne 0 -or $revision -notmatch '^([0-9a-f]{40})\s') { + throw "Cannot resolve $($source.Repository)@$($source.Ref)." + } + "$($source.Name)=$($Matches[1])" >> $env:GITHUB_OUTPUT + } + + - name: Checkout PowerShell source tree once for both builds + uses: actions/checkout@v4 + with: + repository: PowerShell/PowerShell + ref: master + path: workload + persist-credentials: false + + - name: Archive PowerShell source tree and record revision + id: revision + shell: pwsh + run: | + $revision = git -C "$env:GITHUB_WORKSPACE/workload" rev-parse HEAD + if ($LASTEXITCODE -ne 0) { throw 'Cannot resolve workload revision.' } + "sha=$revision" >> $env:GITHUB_OUTPUT + git -C "$env:GITHUB_WORKSPACE/workload" archive --format=zip --output="$env:GITHUB_WORKSPACE/powershell.zip" HEAD + if ($LASTEXITCODE -ne 0) { throw 'Cannot archive PowerShell workload.' } + + - name: Share the exact PowerShell source tree + uses: actions/upload-artifact@v4 + with: + name: performance-workload-powershell + path: powershell.zip + if-no-files-found: error + + - name: Checkout actions-semver-checker + uses: actions/checkout@v4 + with: + repository: jessehouwing/actions-semver-checker + ref: main + path: semver + persist-credentials: false + + - name: Archive actions-semver-checker and record revision + id: semver-revision + shell: pwsh + run: | + $revision = git -C "$env:GITHUB_WORKSPACE/semver" rev-parse HEAD + if ($LASTEXITCODE -ne 0) { throw 'Cannot resolve semver workload revision.' } + "sha=$revision" >> $env:GITHUB_OUTPUT + git -C "$env:GITHUB_WORKSPACE/semver" archive --format=zip --output="$env:GITHUB_WORKSPACE/semver.zip" HEAD + if ($LASTEXITCODE -ne 0) { throw 'Cannot archive semver workload.' } + + - name: Share the exact semver source tree + uses: actions/upload-artifact@v4 + with: + name: performance-workload-semver + path: semver.zip + if-no-files-found: error + + benchmark: + name: Benchmark ${{ matrix.source }} / ${{ matrix.workload }} / ${{ matrix.os }} + needs: workload + runs-on: ${{ matrix.os }} + timeout-minutes: 45 + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + workload: [powershell, semver] + source: [upstream, fork] + include: + - source: upstream + repository: PowerShell/PSScriptAnalyzer + - source: fork + repository: jessehouwing/PSScriptAnalyzer + defaults: + run: + shell: pwsh + env: + DOTNET_NOLOGO: true + DOTNET_GENERATE_ASPNET_CERTIFICATE: false + BENCHMARK_SOURCE: ${{ matrix.source }} + BENCHMARK_REPOSITORY: ${{ matrix.repository }} + BENCHMARK_WORKLOAD: ${{ matrix.workload }} + WORKLOAD_REVISION: ${{ needs.workload.outputs[matrix.workload] }} + ANALYZER_REVISION: ${{ needs.workload.outputs[matrix.source] }} + BENCHMARK_REPETITIONS: ${{ inputs.repetitions }} + steps: + - name: Checkout benchmark harness + uses: actions/checkout@v4 + with: + path: harness + persist-credentials: false + + - name: Checkout pinned analyzer revision + uses: actions/checkout@v4 + with: + repository: ${{ matrix.repository }} + ref: ${{ needs.workload.outputs[matrix.source] }} + path: analyzer + persist-credentials: false + + - name: Download shared input + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 + with: + name: performance-workload-${{ matrix.workload }} + path: workload + + - name: Extract workload source tree + run: Expand-Archive -LiteralPath "$env:GITHUB_WORKSPACE/workload/$env:BENCHMARK_WORKLOAD.zip" -DestinationPath "$env:GITHUB_WORKSPACE/workload/source" + + - name: Install SDK + uses: actions/setup-dotnet@v4 + with: + global-json-file: analyzer/global.json + + - name: Install build resources + run: '& "$env:GITHUB_WORKSPACE/harness/tools/installPSResources.ps1"' + + - name: Build (Release, PowerShell 7) + working-directory: analyzer + run: '& "$env:GITHUB_WORKSPACE/analyzer/build.ps1" -Configuration Release -PSVersion 7 -Verbose' + + - name: Measure repeated cold and warm analysis in fresh shells + run: | + $ErrorActionPreference = 'Stop' + $workspace = $env:GITHUB_WORKSPACE + $resultDirectory = Join-Path $workspace 'results' + $null = New-Item -ItemType Directory -Path $resultDirectory -Force + $pwsh = (Get-Process -Id $PID).Path + $scriptPath = Join-Path $workspace 'workload/source/build.psm1' + $workloadRepository = 'PowerShell/PowerShell' + $analysisArguments = @() + if ($env:BENCHMARK_WORKLOAD -eq 'semver') { + $scriptPath = Join-Path $workspace 'workload/source' + $workloadRepository = 'jessehouwing/actions-semver-checker' + $analysisArguments = @('-Recurse', '-SettingsPath', (Join-Path $scriptPath 'PSScriptAnalyzerSettings.psd1')) + } + $measureScript = Join-Path $workspace 'harness/tools/Measure-ScriptAnalyzerPerformance.ps1' + $sourcePath = Join-Path $workspace 'analyzer' + $manifests = @(Get-ChildItem -Path "$sourcePath/out/PSScriptAnalyzer" -Filter PSScriptAnalyzer.psd1 -Recurse) + if ($manifests.Count -ne 1) { + throw "Expected exactly one built manifest, found $($manifests.Count)." + } + $revision = git -C $sourcePath rev-parse HEAD + if ($LASTEXITCODE -ne 0 -or $revision -ne $env:ANALYZER_REVISION) { + throw 'Analyzer checkout does not match the pinned revision.' + } + $metadata = @{ + Source = $env:BENCHMARK_SOURCE + Repository = $env:BENCHMARK_REPOSITORY + Revision = $revision + Workload = $env:BENCHMARK_WORKLOAD + WorkloadRepository = $workloadRepository + WorkloadRevision = $env:WORKLOAD_REVISION + RunnerOS = $env:RUNNER_OS + RunnerImage = $env:ImageOS + RunnerImageVersion = $env:ImageVersion + ProcessorCount = [Environment]::ProcessorCount + } + for ($sample = 1; $sample -le [int]$env:BENCHMARK_REPETITIONS; $sample++) { + $resultPath = Join-Path $resultDirectory "$env:BENCHMARK_SOURCE-$sample.json" + $attempt = 0 + # Failed upstream runs are retried in entirely fresh processes. + # The job timeout bounds retries if upstream never succeeds. + $PSNativeCommandUseErrorActionPreference = $false + do { + $attempt++ + $attemptLog = Join-Path $resultDirectory "$env:BENCHMARK_SOURCE-$sample-attempt-$attempt.log" + & $pwsh -NoLogo -NoProfile -NonInteractive -File $measureScript ` + -ModulePath $manifests[0].FullName -ScriptPath $scriptPath -ResultPath $resultPath @analysisArguments 2>&1 | + Tee-Object -FilePath $attemptLog + $attemptExitCode = $LASTEXITCODE + if ($attemptExitCode -ne 0) { + if ($env:BENCHMARK_SOURCE -ne 'upstream') { + throw "Benchmark sample $sample failed (exit code $attemptExitCode)." + } + Write-Warning "Upstream sample $sample attempt $attempt failed (exit code $attemptExitCode); retrying in a fresh process." + } + } while ($attemptExitCode -ne 0) + $result = Get-Content -LiteralPath $resultPath -Raw | ConvertFrom-Json + $result | Add-Member -NotePropertyMembers $metadata + $result | Add-Member -NotePropertyName Sample -NotePropertyValue $sample + $result | Add-Member -NotePropertyName Attempts -NotePropertyValue $attempt + $result | Add-Member -NotePropertyName Inconsistent -NotePropertyValue ($attempt -gt 1) + $result | ConvertTo-Json -Depth 20 | Set-Content -LiteralPath $resultPath -Encoding utf8 + $result | Format-List + } + + - name: Upload raw benchmark results + if: always() + uses: actions/upload-artifact@v4 + with: + name: scriptanalyzer-performance-${{ matrix.os }}-${{ matrix.workload }}-${{ matrix.source }} + path: | + results/*.json + results/*.log + if-no-files-found: warn + + compare: + name: Compare ${{ matrix.workload }} / ${{ matrix.os }} + needs: benchmark + if: ${{ !cancelled() }} + runs-on: ${{ matrix.os }} + timeout-minutes: 10 + env: + BENCHMARK_REPETITIONS: ${{ inputs.repetitions }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + workload: [powershell, semver] + steps: + - name: Download benchmark results + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 + with: + pattern: scriptanalyzer-performance-${{ matrix.os }}-${{ matrix.workload }}-* + path: results + merge-multiple: true + + - name: Validate and summarize + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + $resultDirectory = Join-Path $env:GITHUB_WORKSPACE 'results' + $measurements = @{} + foreach ($source in 'upstream', 'fork') { + $measurements[$source] = @(for ($sample = 1; $sample -le [int]$env:BENCHMARK_REPETITIONS; $sample++) { + Get-Content -LiteralPath "$resultDirectory/$source-$sample.json" -Raw | ConvertFrom-Json + }) + } + $baseline = $measurements.upstream[0] + $findingsMatch = $baseline.ColdDiagnosticCount -gt 0 + foreach ($source in 'upstream', 'fork') { + foreach ($candidate in $measurements[$source]) { + foreach ($property in 'InputSHA256', 'InputFileCount', 'SettingsSHA256', 'Recurse', + 'WorkloadRevision', 'PowerShellVersion', 'RunnerOS', 'Culture', 'UICulture') { + if ($baseline.$property -cne $candidate.$property) { + throw "Benchmark property '$property' differs; results are not comparable." + } + } + if ($candidate.Revision -ne $measurements[$source][0].Revision) { + throw "Analyzer revision changed between $source samples." + } + foreach ($run in 'Cold', 'Warm') { + if ($candidate."${run}DiagnosticCount" -ne $baseline.ColdDiagnosticCount -or + $candidate."${run}DiagnosticsSHA256" -cne $baseline.ColdDiagnosticsSHA256) { + $findingsMatch = $false + } + } + } + } + $report = [ordered]@{ + WorkloadRepository = $baseline.WorkloadRepository + WorkloadRevision = $baseline.WorkloadRevision + ExpectedDiagnosticCount = $baseline.ColdDiagnosticCount + FindingsVerified = $findingsMatch + UpstreamInconsistent = @($measurements.upstream | Where-Object Inconsistent).Count -gt 0 + Measurements = @($measurements.upstream) + @($measurements.fork) + } + $report | ConvertTo-Json -Depth 20 | Set-Content -LiteralPath "$resultDirectory/comparison.json" -Encoding utf8 + + function Get-Median($values) { + $sorted = @($values | Sort-Object) + $middle = [int][Math]::Floor($sorted.Count / 2) + if ($sorted.Count % 2) { return $sorted[$middle] } + return ($sorted[$middle - 1] + $sorted[$middle]) / 2 + } + $medians = @{} + foreach ($source in 'upstream', 'fork') { + $medians[$source] = @{ + Cold = Get-Median $measurements[$source].ColdSeconds + Warm = Get-Median $measurements[$source].WarmSeconds + } + } + + $summary = @( + "## Invoke-ScriptAnalyzer performance: $($baseline.Workload) / $($baseline.RunnerOS)" + '' + 'Release builds run in separate jobs with the same OS and PowerShell version and identical input bytes. No analyzer modules or sessions are shared between jobs.' + 'The PowerShell workload analyzes only build.psm1 with default rules. The semver workload analyzes the repository recursively using its PSScriptAnalyzerSettings.psd1, without filtering findings.' + "Each build gets $env:BENCHMARK_REPETITIONS fresh pwsh -NoProfile processes: cold is the first analysis, warm is the immediately following analysis in each process." + 'Shell startup, module import, checkout and build time are excluded. Cold does not mean an empty OS filesystem cache.' + '' + "| Source | Commit | Cold median (s) | Warm median (s) | Cold range (s) | Warm range (s) |" + "| --- | --- | ---: | ---: | --- | --- |" + ) + foreach ($source in 'upstream', 'fork') { + $samples = $measurements[$source] + $reference = 'main' + $cold = $samples.ColdSeconds | Measure-Object -Minimum -Maximum + $warm = $samples.WarmSeconds | Measure-Object -Minimum -Maximum + $marker = if (@($samples | Where-Object Inconsistent).Count) { ' ⚠️' } else { '' } + $summary += "| $($samples[0].Repository)@$reference$marker | $($samples[0].Revision) | $($medians[$source].Cold.ToString('F3')) | $($medians[$source].Warm.ToString('F3')) | $($cold.Minimum.ToString('F3'))–$($cold.Maximum.ToString('F3')) | $($warm.Minimum.ToString('F3'))–$($warm.Maximum.ToString('F3')) |" + } + $summary += '' + if ($report.UpstreamInconsistent) { + $failedAttempts = ($measurements.upstream | ForEach-Object { $_.Attempts - 1 } | Measure-Object -Sum).Sum + $summary += "⚠️ Upstream results were inconsistent: $failedAttempts failed attempts were retried. Timings include successful attempts only and may be biased; failed-attempt logs are retained in the raw artifacts." + $summary += '' + } + foreach ($run in 'Cold', 'Warm') { + foreach ($pair in @(, @('fork', 'upstream'))) { + if ($medians[$pair[1]][$run] -gt 0) { + $change = 100 * ($medians[$pair[0]][$run] / $medians[$pair[1]][$run] - 1) + $summary += "$run median time change ($($pair[0]) vs $($pair[1])): $($change.ToString('F2'))% (positive is slower)." + } + } + } + $summary += @( + '' + "Input: $($baseline.WorkloadRepository) at $($baseline.WorkloadRevision)" + "Input SHA256: $($baseline.InputSHA256); PowerShell files: $($baseline.InputFileCount); recurse: $($baseline.Recurse)" + "Settings SHA256 (empty for default rules): $($baseline.SettingsSHA256)" + "PowerShell: $($baseline.PowerShellVersion)" + '' + 'Samples run on separate hosts per build, not a controlled statistical regression test. Raw artifacts retain each measurement and full normalized diagnostics.' + ) + foreach ($source in 'upstream', 'fork') { + $sample = $measurements[$source][0] + $summary += "$source runner: $($sample.RunnerImage) $($sample.RunnerImageVersion); logical processors: $($sample.ProcessorCount)" + $summary += "Verified $source module: $($sample.ModuleManifest); assembly: $($sample.AnalyzerAssemblyPath)" + } + if (-not $findingsMatch) { + $summary += '**FAILED:** Expected nonzero upstream findings and identical full diagnostic fingerprints across all samples and builds. Timing results must not be treated as a valid comparison.' + } else { + $summary += "Diagnostics verified: every sample returned the same $($baseline.ColdDiagnosticCount) findings, including locations, messages, suppression state, and corrections." + } + $summary | Add-Content -LiteralPath $env:GITHUB_STEP_SUMMARY + $summary | Write-Output + if (-not $findingsMatch) { + throw 'Diagnostic equivalence validation failed. See the job summary and raw benchmark results.' + } + + - name: Upload comparison + if: always() + uses: actions/upload-artifact@v4 + with: + name: scriptanalyzer-performance-${{ matrix.os }}-${{ matrix.workload }}-comparison + path: results/comparison.json + if-no-files-found: warn + + summary: + name: All performance comparisons + needs: compare + if: ${{ !cancelled() }} + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Download comparisons + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 + with: + pattern: scriptanalyzer-performance-*-comparison + path: comparisons + + - name: Summarize all combinations + if: ${{ !cancelled() }} + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + function Get-Median($values) { + $sorted = @($values | Sort-Object) + $middle = [int][Math]::Floor($sorted.Count / 2) + if ($sorted.Count % 2) { return $sorted[$middle] } + return ($sorted[$middle - 1] + $sorted[$middle]) / 2 + } + $summary = @( + '## Performance across all combinations' + '' + 'Times are medians of fresh-process cold/warm samples; lower is better.' + '' + '| OS | Workload | Source | Cold median (s) | Warm median (s) | Status |' + '| --- | --- | --- | ---: | ---: | --- |' + ) + foreach ($os in 'ubuntu-latest', 'windows-latest') { + foreach ($workload in 'powershell', 'semver') { + $path = Join-Path $env:GITHUB_WORKSPACE "comparisons/scriptanalyzer-performance-$os-$workload-comparison/comparison.json" + $report = if (Test-Path -LiteralPath $path) { + Get-Content -LiteralPath $path -Raw | ConvertFrom-Json + } else { $null } + foreach ($source in 'upstream', 'fork') { + $samples = @($report.Measurements | Where-Object Source -EQ $source) + if ($samples.Count -eq 0) { + $summary += "| $os | $workload | $source | — | — | Missing results |" + continue + } + $cold = (Get-Median $samples.ColdSeconds).ToString('F3', [Globalization.CultureInfo]::InvariantCulture) + $warm = (Get-Median $samples.WarmSeconds).ToString('F3', [Globalization.CultureInfo]::InvariantCulture) + $status = if ($report.FindingsVerified) { 'Verified' } else { 'INVALID: diagnostics differ' } + if (@($samples | Where-Object Inconsistent).Count) { + $status += '; ⚠️ retried samples' + } + $summary += "| $os | $workload | $source | $cold | $warm | $status |" + } + } + } + $summary += @( + '' + 'Sources: upstream = PowerShell/PSScriptAnalyzer@main; fork = jessehouwing/PSScriptAnalyzer@main.' + 'Compare timings only within the same OS/workload. Invalid results must not be used for performance comparisons; retried samples may be biased.' + 'Builds run on separate hosts, not a controlled statistical regression test. Per-combination summaries and artifacts retain revisions, ranges, diagnostics, and retry details.' + ) + $summary | Add-Content -LiteralPath $env:GITHUB_STEP_SUMMARY + $summary | Write-Output diff --git a/Engine/CommandInfoCache.cs b/Engine/CommandInfoCache.cs index aa9d725f3..56d7585df 100644 --- a/Engine/CommandInfoCache.cs +++ b/Engine/CommandInfoCache.cs @@ -3,6 +3,8 @@ using System; using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Management.Automation; using System.Linq; using System.Management.Automation.Runspaces; @@ -14,9 +16,25 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer /// internal class CommandInfoCache : IDisposable { + private const string GetCommandName = "Microsoft.PowerShell.Core\\Get-Command"; + private readonly ConcurrentDictionary> _commandInfoCache; - private readonly RunspacePool _runspacePool; - private bool disposed = false; + private readonly ConcurrentDictionary> _parameterSnapshots + = new ConcurrentDictionary>(); + private readonly ConcurrentDictionary> _mandatoryParameters + = new ConcurrentDictionary>(); + + /// + /// Guards all access to so that only one thread at a time drives the + /// PowerShell engine. The engine is not thread safe, so concurrent lookups can fail transiently, + /// see https://github.com/PowerShell/PowerShell/issues/4003. + /// A monitor is used rather than a semaphore because it is re-entrant, which avoids a deadlock + /// should a lookup ever end up calling back into the cache on the same thread. + /// + private readonly object _runspaceLock = new object(); + + private readonly Runspace _runspace; + private volatile bool disposed = false; /// /// Create a fresh command info cache instance. @@ -24,11 +42,13 @@ internal class CommandInfoCache : IDisposable public CommandInfoCache() { _commandInfoCache = new ConcurrentDictionary>(); - _runspacePool = RunspaceFactory.CreateRunspacePool(1, 10); - _runspacePool.Open(); + // A single runspace rather than a pool: all lookups are serialized on it, so that the + // PowerShell engine is never driven concurrently. + _runspace = RunspaceFactory.CreateRunspace(); + _runspace.Open(); } - /// Dispose the runspace pool + /// Dispose the runspace public void Dispose() { Dispose(true); @@ -37,17 +57,23 @@ public void Dispose() protected virtual void Dispose(bool disposing) { - if ( disposed ) + // Always take the lock, also on the finalizer path, so that 'disposed' is never + // published without the runspace being disposed along with it and so that the runspace + // cannot be disposed while a lookup is in flight. + lock (_runspaceLock) { - return; - } + if ( disposed ) + { + return; + } - if ( disposing ) - { - _runspacePool.Dispose(); - } + disposed = true; - disposed = true; + if ( disposing ) + { + _runspace.Dispose(); + } + } } /// @@ -58,6 +84,19 @@ protected virtual void Dispose(bool disposing) /// When needed due to runspace affinity problems of some PowerShell objects. /// public CommandInfo GetCommandInfo(string commandName, CommandTypes? commandTypes = null, bool bypassCache = false) + { + try + { + return GetCachedCommandInfo(commandName, commandTypes, bypassCache); + } + catch (Exception exception) when (IsGetCommandResolutionException(exception)) + { + // Failed Lazy lookups have already been evicted; never cache a lookup failure as a miss. + return null; + } + } + + private CommandInfo GetCachedCommandInfo(string commandName, CommandTypes? commandTypes, bool bypassCache) { if (string.IsNullOrWhiteSpace(commandName)) { @@ -70,9 +109,35 @@ public CommandInfo GetCommandInfo(string commandName, CommandTypes? commandTypes return GetCommandInfoInternal(commandName, commandTypes); } // Atomically either use PowerShell to query a command info object, or fetch it from the cache - return _commandInfoCache.GetOrAdd(key, new Lazy(() => GetCommandInfoInternal(commandName, commandTypes))).Value; + if (!_commandInfoCache.TryGetValue(key, out var lazyCommandInfo)) + { + lazyCommandInfo = _commandInfoCache.GetOrAdd(key, CreateLookup(commandName, commandTypes)); + } + try + { + return lazyCommandInfo.Value; + } + catch + { + // Lazy caches exceptions forever, which would make every subsequent lookup of this + // command fail for the lifetime of the process. Evict the entry so that the next lookup + // can try again. Only remove the faulted instance so that a replacement that another + // thread may already have added is left alone. + RemoveLookup(key, lazyCommandInfo); + throw; + } } + private void RemoveLookup(CommandLookupKey key, Lazy lookup) + { + ((ICollection>>)_commandInfoCache) + .Remove(new KeyValuePair>(key, lookup)); + } + + private Lazy CreateLookup(string commandName, CommandTypes? commandTypes) + { + return new Lazy(() => GetCommandInfoInternal(commandName, commandTypes)); + } /// /// Get a CommandInfo object of the given command name @@ -99,29 +164,216 @@ private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? command // For more details see https://github.com/PowerShell/PowerShell/issues/9308 actualCmdName = WildcardPattern.Escape(actualCmdName); - using (var ps = System.Management.Automation.PowerShell.Create()) + // Serialize all use of the PowerShell engine. Only cache misses reach this point; + // lookups that are already cached are served without taking the lock. + lock (_runspaceLock) { - ps.RunspacePool = _runspacePool; + if (disposed) + { + return null; + } + + using (var ps = System.Management.Automation.PowerShell.Create()) + { + ps.Runspace = _runspace; + + ps.AddCommand(GetCommandName) + .AddParameter("Name", actualCmdName) + .AddParameter("ErrorAction", "SilentlyContinue"); + + if (commandType != null) + { + ps.AddParameter("CommandType", commandType); + } + + if (!string.IsNullOrEmpty(moduleName)) + { + ps.AddParameter("Module", moduleName); + } - ps.AddCommand("Get-Command") - .AddParameter("Name", actualCmdName) - .AddParameter("ErrorAction", "SilentlyContinue"); + Collection result = ps.Invoke(); - if (commandType != null) + // 'Get-Command' is invoked with 'SilentlyContinue', so a resolution error can only + // mean that the engine failed to resolve 'Get-Command' itself in the runspace. + // That happened intermittently when lookups ran concurrently because the PowerShell engine + // is not thread safe, see https://github.com/PowerShell/PowerShell/issues/4003 and + // https://github.com/PowerShell/PSScriptAnalyzer/issues/2205 + // Lookups are serialized now, so this should no longer occur; if it does, the cache + // entry is evicted and the lookup surfaces as a null command info. + // SilentlyContinue can set HadErrors without populating the stream for an unknown name. + if (ps.HadErrors && ps.Streams.Error.Count > 0 && ps.Streams.Error.All(IsGetCommandResolutionError)) + { + throw ps.Streams.Error[0].Exception; + } + return result.FirstOrDefault(); + } + } + } + + private static bool IsGetCommandResolutionError(ErrorRecord errorRecord) + { + return IsGetCommandResolutionException(errorRecord?.Exception); + } + + /// + /// Retrieves parameter metadata without allowing other threads to drive the command's runspace. + /// + public Dictionary GetCommandParameters( + string commandName, CommandTypes? commandTypes = null, bool bypassCache = false) + { + return GetCommandMetadata(commandName, commandTypes, bypassCache, command => + { + lock (_runspaceLock) { - ps.AddParameter("CommandType", commandType); + // Dynamic parameter getters execute PowerShell code and mutate runspace state, + // even though they look like ordinary property reads. + if (disposed) return null; + return command.Parameters; } + }); + } - if (!string.IsNullOrEmpty(moduleName)) + /// + /// Retrieves parameter sets under the same lock as command lookups and dynamic parameter queries. + /// + public ReadOnlyCollection GetCommandParameterSets(string commandName) + { + return GetCommandMetadata(commandName, null, false, command => + { + lock (_runspaceLock) { - ps.AddParameter("Module", moduleName); + if (disposed) return null; + return command.ParameterSets; } + }); + } - return ps.Invoke() - .FirstOrDefault(); + /// + /// Contains PowerShell metadata failures: the failed command object is evicted so subsequent + /// calls can recover, and null is returned. Unavailable metadata is never cached. + /// Unexpected exceptions still propagate. + /// + private T GetCommandMetadata( + string commandName, CommandTypes? commandTypes, bool bypassCache, Func readMetadata) + where T : class + { + // Resolve Lazy values outside the runspace lock: another thread's factory may need it. + var command = GetCommandInfo(commandName, commandTypes, bypassCache); + if (disposed || command == null || command.CommandType == CommandTypes.Application) return null; + try + { + return readMetadata(command); + } + catch (Exception exception) when (IsMetadataException(exception)) + { + var key = new CommandLookupKey(commandName, commandTypes); + if (_commandInfoCache.TryGetValue(key, out var lookup) + && lookup.IsValueCreated && ReferenceEquals(lookup.Value, command)) + { + RemoveLookup(key, lookup); + } + return null; } } + private static bool IsRunspaceAffinityException(Exception exception) + { + // PowerShell objects can have runspace affinity, see PowerShell issue 4003 and PSSA issue 1708. + return exception is InvalidOperationException || exception is NullReferenceException; + } + + private static bool IsMetadataException(Exception exception) + { + return IsRunspaceAffinityException(exception) + || exception is RuntimeException || exception is PSNotSupportedException; + } + + private static CmdletInfo GetStaticCmdlet(CommandInfo command) + { + // Aliases, functions, subclasses and provider/dynamic cmdlets retain the locked, + // uncached path. IDynamicParameters includes implementations inherited from base types. + if (command == null || command.GetType() != typeof(CmdletInfo)) return null; + var cmdlet = (CmdletInfo)command; + return cmdlet.ImplementingType != null + && !typeof(IDynamicParameters).IsAssignableFrom(cmdlet.ImplementingType) ? cmdlet : null; + } + + public IReadOnlyDictionary GetParameterSnapshot( + string commandName, CommandTypes? commandTypes = null, bool bypassCache = false) + { + return GetCommandMetadata(commandName, commandTypes, bypassCache, + command => GetParameterSnapshot(command, bypassCache)); + } + + private IReadOnlyDictionary GetParameterSnapshot(CommandInfo command, bool bypassCache) + { + var staticCmdlet = bypassCache ? null : GetStaticCmdlet(command); + if (disposed || command == null) return null; + if (staticCmdlet != null && _parameterSnapshots.TryGetValue(staticCmdlet, out var cached)) return cached; + + lock (_runspaceLock) + { + if (disposed) return null; + if (staticCmdlet != null && _parameterSnapshots.TryGetValue(staticCmdlet, out cached)) return cached; + var parameters = command.Parameters; + if (parameters == null) return null; + var snapshot = new ReadOnlyDictionary( + parameters.ToDictionary(p => p.Key, p => new CommandParameterSnapshot(p.Value), parameters.Comparer)); + if (staticCmdlet != null) _parameterSnapshots[staticCmdlet] = snapshot; + return snapshot; + } + } + + public IReadOnlyList GetMandatoryParameterNames(string commandName) + { + return GetCommandMetadata(commandName, null, false, + command => GetMandatoryParameterNames(command, bypassCache: false)); + } + + private IReadOnlyList GetMandatoryParameterNames(CommandInfo command, bool bypassCache) + { + var staticCmdlet = bypassCache ? null : GetStaticCmdlet(command); + if (disposed || command == null) return null; + if (staticCmdlet != null && _mandatoryParameters.TryGetValue(staticCmdlet, out var cached)) return cached; + + lock (_runspaceLock) + { + if (disposed) return null; + if (staticCmdlet != null && _mandatoryParameters.TryGetValue(staticCmdlet, out cached)) return cached; + var parameterSets = command.ParameterSets; + var parameters = command.Parameters; + if (parameterSets == null || parameters == null) return null; + int setCount = parameterSets.Count; + var mandatory = new List(); + foreach (var parameter in parameters.Values) + { + if (parameter.Attributes.Count >= setCount + && parameter.Attributes.OfType().Count(a => a.Mandatory) >= setCount) + { + mandatory.Add(parameter.Name); + } + } + var snapshot = mandatory.AsReadOnly(); + if (staticCmdlet != null) _mandatoryParameters[staticCmdlet] = snapshot; + return snapshot; + } + } + + private static bool IsGetCommandResolutionException(Exception exception) + { + if (exception is CommandNotFoundException) + { + return true; + } + + if (exception is ParentContainsErrorRecordException parentContainsErrorRecordException) + { + return IsGetCommandResolutionException(parentContainsErrorRecordException.InnerException); + } + + return false; + } + private struct CommandLookupKey : IEquatable { private readonly string Name; @@ -146,7 +398,7 @@ public override int GetHashCode() unchecked { int hash = 17; - hash = hash * 31 + Name.ToUpperInvariant().GetHashCode(); + hash = hash * 31 + StringComparer.OrdinalIgnoreCase.GetHashCode(Name); hash = hash * 31 + CommandTypes.GetHashCode(); return hash; } diff --git a/Engine/CommandParameterSnapshot.cs b/Engine/CommandParameterSnapshot.cs new file mode 100644 index 000000000..75ccc3650 --- /dev/null +++ b/Engine/CommandParameterSnapshot.cs @@ -0,0 +1,24 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Collections.ObjectModel; +using System.Linq; +using System.Management.Automation; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer +{ + /// Detached parameter facts, safe to consume without driving a PowerShell runspace. + public sealed class CommandParameterSnapshot + { + public string Name { get; } + public bool SwitchParameter { get; } + public ReadOnlyCollection Aliases { get; } + + internal CommandParameterSnapshot(ParameterMetadata parameter) + { + Name = parameter.Name; + SwitchParameter = parameter.SwitchParameter; + Aliases = new ReadOnlyCollection(parameter.Aliases.ToArray()); + } + } +} diff --git a/Engine/Helper.cs b/Engine/Helper.cs index f36d17433..a9633156f 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -330,6 +330,15 @@ public PSModuleInfo GetModuleManifest(string filePath, out IEnumerableShares validation between built-in rules during one syntax-tree analysis only. + public PSModuleInfo GetModuleManifestForAnalysis(string filePath, out IEnumerable errorRecords) + { + var cache = ModuleManifestAnalysisCache.Current; + return cache == null + ? GetModuleManifest(filePath, out errorRecords) + : cache.Get(this, filePath, out errorRecords); + } + /// /// Checks if the error record is MissingMemberException /// @@ -399,18 +408,22 @@ public HashSet GetExportedFunction(Ast ast) List exportFunctionsCmdlet = Helper.Instance.CmdletNameAndAliases("export-modulemember"); // find functions exported - IEnumerable cmdAsts = ast.FindAll(item => item is CommandAst - && exportFunctionsCmdlet.Contains((item as CommandAst).GetCommandName(), StringComparer.OrdinalIgnoreCase), true); - - CommandInfo exportMM = Helper.Instance.GetCommandInfo("export-modulemember", CommandTypes.Cmdlet); - - // switch parameters - IEnumerable switchParams = (exportMM != null) ? exportMM.Parameters.Values.Where(pm => pm.SwitchParameter) : Enumerable.Empty(); + var cmdAsts = ast.FindAll(item => item is CommandAst + && exportFunctionsCmdlet.Contains((item as CommandAst).GetCommandName(), StringComparer.OrdinalIgnoreCase), true).ToArray(); + if (cmdAsts.Length == 0) + { + return exportedFunctions; + } - if (exportMM == null) + // Export-ModuleMember has no dynamic parameters. Resolve names from its static + // metadata instead of ResolveParameter(), which re-enters the cached command's + // runspace and races with command lookups and metadata queries on other rule threads. + var parameters = GetCommandParameterSnapshot("export-modulemember", CommandTypes.Cmdlet); + if (parameters == null) { return exportedFunctions; } + var switchParams = parameters.Values.Where(pm => pm.SwitchParameter); foreach (CommandAst cmdAst in cmdAsts) { @@ -429,7 +442,20 @@ public HashSet GetExportedFunction(Ast ast) if (ceAst is CommandParameterAst) { var paramAst = ceAst as CommandParameterAst; - var param = exportMM.ResolveParameter(paramAst.ParameterName); + CommandParameterSnapshot param; + if (!parameters.TryGetValue(paramAst.ParameterName, out param)) + { + param = parameters.Values.FirstOrDefault(pm => + pm.Aliases.Contains(paramAst.ParameterName, StringComparer.OrdinalIgnoreCase)); + if (param == null) + { + var matches = parameters.Values.Where(pm => + pm.Name.StartsWith(paramAst.ParameterName, StringComparison.OrdinalIgnoreCase) + || pm.Aliases.Any(alias => alias.StartsWith(paramAst.ParameterName, StringComparison.OrdinalIgnoreCase))) + .Take(2).ToArray(); + param = matches.Length == 1 ? matches[0] : null; + } + } if (param == null) { @@ -671,6 +697,36 @@ public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = null, return CommandInfoCache.GetCommandInfo(name, commandTypes: commandType, bypassCache: bypassCache); } + /// + /// Retrieves command parameters with centralized recovery; returns null when metadata is unavailable. + /// + public Dictionary GetCommandParameters( + string name, CommandTypes? commandType = null, bool bypassCache = false) + { + return CommandInfoCache.GetCommandParameters(name, commandType, bypassCache); + } + + /// + /// Retrieves command parameter sets with centralized recovery; returns null when metadata is unavailable. + /// + public ReadOnlyCollection GetCommandParameterSets(string name) + { + return CommandInfoCache.GetCommandParameterSets(name); + } + + /// Gets detached parameter facts or null when unavailable; only static cmdlet metadata is cached. + public IReadOnlyDictionary GetCommandParameterSnapshot( + string name, CommandTypes? commandType = null, bool bypassCache = false) + { + return CommandInfoCache.GetParameterSnapshot(name, commandType, bypassCache); + } + + /// Gets the mandatory parameter summary under a single runspace lock, or null when unavailable. + public IReadOnlyList GetMandatoryParameterNames(string name) + { + return CommandInfoCache.GetMandatoryParameterNames(name); + } + /// /// Returns the get, set and test targetresource dsc function /// diff --git a/Engine/ModuleManifestAnalysisCache.cs b/Engine/ModuleManifestAnalysisCache.cs new file mode 100644 index 000000000..4564f88e7 --- /dev/null +++ b/Engine/ModuleManifestAnalysisCache.cs @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Management.Automation; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer +{ + // Owned by one AnalyzeSyntaxTree call and explicitly installed on its rule workers. + // It cannot outlive that analysis or affect unrelated public GetModuleManifest callers. + internal sealed class ModuleManifestAnalysisCache + { + [ThreadStatic] + internal static ModuleManifestAnalysisCache Current; + + private readonly Dictionary results = new Dictionary(StringComparer.Ordinal); + + internal PSModuleInfo Get(Helper helper, string path, out IEnumerable errors) + { + lock (results) + { + if (!results.TryGetValue(path, out var result)) + { + var module = helper.GetModuleManifest(path, out var moduleErrors); + result = new Result { Module = module, Errors = moduleErrors }; + results.Add(path, result); + } + errors = result.Errors; + return result.Module; + } + } + + internal Scope Enter() => new Scope(this); + + internal struct Scope : IDisposable + { + private readonly ModuleManifestAnalysisCache previous; + + internal Scope(ModuleManifestAnalysisCache cache) + { + previous = Current; + Current = cache; + } + + public void Dispose() => Current = previous; + } + + private sealed class Result + { + internal PSModuleInfo Module; + internal IEnumerable Errors; + } + } +} diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index adc81f2d3..9fed504ec 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -2128,6 +2128,7 @@ public IEnumerable AnalyzeSyntaxTree( string fileName = filePathIsNullOrWhiteSpace ? String.Empty : System.IO.Path.GetFileName(filePath); if (this.ScriptRules != null) { + var manifestCache = new ModuleManifestAnalysisCache(); var allowedRules = this.ScriptRules.Where(IsRuleAllowed); if (allowedRules.Any()) { @@ -2139,6 +2140,7 @@ public IEnumerable AnalyzeSyntaxTree( // Ensure that any unhandled errors from Rules are converted to non-terminating errors // We want the Engine to continue functioning even if one or more Rules throws an exception + using (manifestCache.Enter()) try { if (helpRule && helpFile) diff --git a/README.md b/README.md index 1d2252082..bbf3750f6 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,12 @@ To install **PSScriptAnalyzer** from source code: Import-Module .\out\PSScriptAnalyzer\[version]\PSScriptAnalyzer.psd1 ``` +- Command metadata handling lives in `CommandInfoCache`, not individual rules. Its parameter, + parameter-set and snapshot APIs return null when PowerShell metadata is unavailable, while + unexpected exceptions still propagate. Failed lookups and command objects with failed + metadata are evicted so subsequent calls can recover; genuine missing commands retain + negative caching. + To confirm installation: run `Get-ScriptAnalyzerRule` in the PowerShell console to obtain the built-in rules. diff --git a/Rules/MissingModuleManifestField.cs b/Rules/MissingModuleManifestField.cs index 180767766..8b7b40f7e 100644 --- a/Rules/MissingModuleManifestField.cs +++ b/Rules/MissingModuleManifestField.cs @@ -40,7 +40,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (Helper.IsModuleManifest(fileName)) { IEnumerable errorRecords; - var psModuleInfo = Helper.Instance.GetModuleManifest(fileName, out errorRecords); + var psModuleInfo = Helper.Instance.GetModuleManifestForAnalysis(fileName, out errorRecords); if (errorRecords != null) { foreach (var errorRecord in errorRecords) diff --git a/Rules/UseCmdletCorrectly.cs b/Rules/UseCmdletCorrectly.cs index ccec27e0b..fda126a21 100644 --- a/Rules/UseCmdletCorrectly.cs +++ b/Rules/UseCmdletCorrectly.cs @@ -145,41 +145,9 @@ private bool MandatoryParameterExists(CommandAst cmdAst) // Gets mandatory parameters from cmdlet. // If cannot find any mandatory parameter, it's not necessary to do a further check for current cmdlet. - var mandatoryParameters = new List(); - try - { - int noOfParamSets = cmdInfo.ParameterSets.Count; - foreach (ParameterMetadata pm in cmdInfo.Parameters.Values) - { - int count = 0; - - if (pm.Attributes.Count < noOfParamSets) - { - continue; - } + var mandatoryParameters = Helper.Instance.GetMandatoryParameterNames(cmdAst.GetCommandName()); - foreach (Attribute attr in pm.Attributes) - { - if (!(attr is ParameterAttribute)) continue; - if (((ParameterAttribute)attr).Mandatory) - { - count += 1; - } - } - - if (count >= noOfParamSets) - { - mandatoryParameters.Add(pm); - } - } - } - catch (Exception) - { - // For cases like cmd.exe. Also for runtime exception - return true; - } - - if (mandatoryParameters.Count == 0) + if (mandatoryParameters == null || mandatoryParameters.Count == 0) { return true; } @@ -188,8 +156,8 @@ private bool MandatoryParameterExists(CommandAst cmdAst) foreach (CommandElementAst commandElementAst in cmdAst.CommandElements.OfType()) { CommandParameterAst cpAst = (CommandParameterAst)commandElementAst; - if (mandatoryParameters.Count(item => - item.Name.Equals(cpAst.ParameterName, StringComparison.OrdinalIgnoreCase)) > 0) + if (mandatoryParameters.Any(item => + item.Equals(cpAst.ParameterName, StringComparison.OrdinalIgnoreCase))) { return true; } @@ -254,6 +222,3 @@ public string GetSourceName() } } - - - diff --git a/Rules/UseCorrectCasing.cs b/Rules/UseCorrectCasing.cs index f4f2c40b7..8a5ce9e12 100644 --- a/Rules/UseCorrectCasing.cs +++ b/Rules/UseCorrectCasing.cs @@ -119,24 +119,22 @@ public override IEnumerable AnalyzeScript(Ast ast, string file } var commandParameterAsts = commandAst.FindAll( - testAst => testAst is CommandParameterAst, true).Cast(); - Dictionary availableParameters; - try + testAst => testAst is CommandParameterAst, true).Cast().ToArray(); + if (commandParameterAsts.Length == 0) { - availableParameters = commandInfo.Parameters; + continue; } - // It's a known issue that objects from PowerShell can have a runspace affinity, - // therefore if that happens, we query a fresh object instead of using the cache. - // https://github.com/PowerShell/PowerShell/issues/4003 - catch (InvalidOperationException) + var availableParameters = Helper.Instance.GetCommandParameterSnapshot(commandName); + if (availableParameters is null) { - commandInfo = Helper.Instance.GetCommandInfo(commandName, bypassCache: true); - availableParameters = commandInfo.Parameters; + // The parameters of this command cannot be determined reliably, + // so skip the parameter casing check instead of failing the analysis. + continue; } foreach (var commandParameterAst in commandParameterAsts) { var parameterName = commandParameterAst.ParameterName; - if (availableParameters.TryGetValue(parameterName, out ParameterMetadata parameterMetaData)) + if (availableParameters.TryGetValue(parameterName, out CommandParameterSnapshot parameterMetaData)) { var correctlyCasedParameterName = parameterMetaData.Name; if (!parameterName.Equals(correctlyCasedParameterName, StringComparison.Ordinal)) diff --git a/Rules/UseToExportFieldsInManifest.cs b/Rules/UseToExportFieldsInManifest.cs index 9bf612f83..2dcaabe31 100644 --- a/Rules/UseToExportFieldsInManifest.cs +++ b/Rules/UseToExportFieldsInManifest.cs @@ -51,7 +51,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) // check if valid module manifest IEnumerable errorRecord = null; - PSModuleInfo psModuleInfo = Helper.Instance.GetModuleManifest(fileName, out errorRecord); + PSModuleInfo psModuleInfo = Helper.Instance.GetModuleManifestForAnalysis(fileName, out errorRecord); if ((errorRecord != null && errorRecord.Count() > 0) || psModuleInfo == null) { yield break; diff --git a/Tests/Build/MeasureScriptAnalyzerPerformance.tests.ps1 b/Tests/Build/MeasureScriptAnalyzerPerformance.tests.ps1 new file mode 100644 index 000000000..03f512895 --- /dev/null +++ b/Tests/Build/MeasureScriptAnalyzerPerformance.tests.ps1 @@ -0,0 +1,229 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +Describe 'Performance benchmark diagnostic normalization' -Skip:($PSVersionTable.PSVersion.Major -lt 7) { + BeforeAll { + $harnessPath = Join-Path $PSScriptRoot '../../tools/Measure-ScriptAnalyzerPerformance.ps1' + $tokens = $null + $parseErrors = $null + $harness = [System.Management.Automation.Language.Parser]::ParseFile( + $harnessPath, [ref]$tokens, [ref]$parseErrors) + if ($parseErrors.Count) { throw $parseErrors[0] } + foreach ($name in 'ConvertTo-WorkloadPath', 'ConvertTo-DiagnosticJson') { + $function = $harness.Find({ + param($node) + $node -is [System.Management.Automation.Language.FunctionDefinitionAst] -and $node.Name -eq $name + }, $false) + . ([scriptblock]::Create($function.Extent.Text)) + } + + function Get-TestDiagnostic { + param([string]$Root) + $file = Join-Path $Root 'example.ps1' + [pscustomobject]@{ + RuleName = 'PSUseCorrectCasing' + Severity = 'Information' + Message = "Incorrect casing in $Root" + ScriptName = 'example.ps1' + ScriptPath = $file + RuleSuppressionID = 'Get-Item' + IsSuppressed = $false + Extent = [pscustomobject]@{ + File = $file + StartLineNumber = 1 + StartColumnNumber = 1 + EndLineNumber = 1 + EndColumnNumber = 9 + StartOffset = 0 + EndOffset = 8 + Text = 'get-item' + } + SuggestedCorrections = @([pscustomobject]@{ + File = $file + StartLineNumber = 1 + StartColumnNumber = 1 + EndLineNumber = 1 + EndColumnNumber = 9 + Text = 'Get-Item' + Description = 'Correct command casing' + }) + } + } + } + + It 'ignores checkout-root differences while preserving diagnostic content' { + $inputRoot = Join-Path $TestDrive 'first' + $first = ConvertTo-DiagnosticJson (Get-TestDiagnostic $inputRoot) + $inputRoot = Join-Path $TestDrive 'second' + $second = ConvertTo-DiagnosticJson (Get-TestDiagnostic $inputRoot) + $first | Should -BeExactly $second + ($first | ConvertFrom-Json).ScriptPath | Should -BeExactly 'example.ps1' + } + + It 'detects changes in even when diagnostic counts match' -ForEach @( + @{ Field = 'Message' } + @{ Field = 'RuleName' } + @{ Field = 'RuleSuppressionID' } + ) { + $inputRoot = $TestDrive + $diagnostic = Get-TestDiagnostic $inputRoot + $before = ConvertTo-DiagnosticJson $diagnostic + $diagnostic.$Field = 'changed' + ConvertTo-DiagnosticJson $diagnostic | Should -Not -BeExactly $before + } + + It 'preserves locations, suppression state and suggested corrections' { + $inputRoot = $TestDrive + $diagnostic = Get-TestDiagnostic $inputRoot + $before = ConvertTo-DiagnosticJson $diagnostic + $diagnostic.Extent.StartOffset = 1 + ConvertTo-DiagnosticJson $diagnostic | Should -Not -BeExactly $before + $diagnostic.Extent.StartOffset = 0 + $diagnostic.IsSuppressed = $true + ConvertTo-DiagnosticJson $diagnostic | Should -Not -BeExactly $before + $diagnostic.IsSuppressed = $false + $diagnostic.SuggestedCorrections[0].Text = 'Different-Command' + ConvertTo-DiagnosticJson $diagnostic | Should -Not -BeExactly $before + } + + It 'handles missing extents and corrections' { + $inputRoot = $TestDrive + $diagnostic = Get-TestDiagnostic $inputRoot + $diagnostic.Extent = $null + $diagnostic.SuggestedCorrections = $null + $result = ConvertTo-DiagnosticJson $diagnostic | ConvertFrom-Json + $result.Extent | Should -BeNullOrEmpty + $result.SuggestedCorrections.Count | Should -Be 0 + } +} + +Describe 'Performance comparison validation' -Skip:($PSVersionTable.PSVersion.Major -lt 7) { + BeforeAll { + $workflow = Get-Content -LiteralPath (Join-Path $PSScriptRoot '../../.github/workflows/performance.yml') -Raw + $comparison = [regex]::Match($workflow, '(?ms) - name: Validate and summarize\r?\n.*? run: \|\r?\n(?