Skip to content
Merged

Perf #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
254 changes: 202 additions & 52 deletions .github/workflows/performance.yml

Large diffs are not rendered by default.

266 changes: 193 additions & 73 deletions Engine/CommandInfoCache.cs

Large diffs are not rendered by default.

24 changes: 24 additions & 0 deletions Engine/CommandParameterSnapshot.cs
Original file line number Diff line number Diff line change
@@ -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
{
/// <summary>Detached parameter facts, safe to consume without driving a PowerShell runspace.</summary>
public sealed class CommandParameterSnapshot
{
public string Name { get; }
public bool SwitchParameter { get; }
public ReadOnlyCollection<string> Aliases { get; }

internal CommandParameterSnapshot(ParameterMetadata parameter)
{
Name = parameter.Name;
SwitchParameter = parameter.SwitchParameter;
Aliases = new ReadOnlyCollection<string>(parameter.Aliases.ToArray());
}
}
}
40 changes: 33 additions & 7 deletions Engine/Helper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,15 @@ public PSModuleInfo GetModuleManifest(string filePath, out IEnumerable<ErrorReco
return psModuleInfo;
}

/// <summary>Shares validation between built-in rules during one syntax-tree analysis only.</summary>
public PSModuleInfo GetModuleManifestForAnalysis(string filePath, out IEnumerable<ErrorRecord> errorRecords)
{
var cache = ModuleManifestAnalysisCache.Current;
return cache == null
? GetModuleManifest(filePath, out errorRecords)
: cache.Get(this, filePath, out errorRecords);
}

/// <summary>
/// Checks if the error record is MissingMemberException
/// </summary>
Expand Down Expand Up @@ -399,18 +408,22 @@ public HashSet<string> GetExportedFunction(Ast ast)
List<string> exportFunctionsCmdlet = Helper.Instance.CmdletNameAndAliases("export-modulemember");

// find functions exported
IEnumerable<Ast> cmdAsts = ast.FindAll(item => item is CommandAst
&& exportFunctionsCmdlet.Contains((item as CommandAst).GetCommandName(), StringComparer.OrdinalIgnoreCase), true);
var cmdAsts = ast.FindAll(item => item is CommandAst
&& exportFunctionsCmdlet.Contains((item as CommandAst).GetCommandName(), StringComparer.OrdinalIgnoreCase), true).ToArray();
if (cmdAsts.Length == 0)
{
return exportedFunctions;
}

// 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 = GetCommandParameters("export-modulemember", CommandTypes.Cmdlet);
var parameters = GetCommandParameterSnapshot("export-modulemember", CommandTypes.Cmdlet);
if (parameters == null)
{
return exportedFunctions;
}
IEnumerable<ParameterMetadata> switchParams = parameters.Values.Where(pm => pm.SwitchParameter);
var switchParams = parameters.Values.Where(pm => pm.SwitchParameter);

foreach (CommandAst cmdAst in cmdAsts)
{
Expand All @@ -429,7 +442,7 @@ public HashSet<string> GetExportedFunction(Ast ast)
if (ceAst is CommandParameterAst)
{
var paramAst = ceAst as CommandParameterAst;
ParameterMetadata param;
CommandParameterSnapshot param;
if (!parameters.TryGetValue(paramAst.ParameterName, out param))
{
param = parameters.Values.FirstOrDefault(pm =>
Expand Down Expand Up @@ -685,7 +698,7 @@ public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = null,
}

/// <summary>
/// Retrieves command parameters while serializing access to the cached command's runspace.
/// Retrieves command parameters with centralized recovery; returns null when metadata is unavailable.
/// </summary>
public Dictionary<string, ParameterMetadata> GetCommandParameters(
string name, CommandTypes? commandType = null, bool bypassCache = false)
Expand All @@ -694,13 +707,26 @@ public Dictionary<string, ParameterMetadata> GetCommandParameters(
}

/// <summary>
/// Retrieves command parameter sets while serializing access to the cached command's runspace.
/// Retrieves command parameter sets with centralized recovery; returns null when metadata is unavailable.
/// </summary>
public ReadOnlyCollection<CommandParameterSetInfo> GetCommandParameterSets(string name)
{
return CommandInfoCache.GetCommandParameterSets(name);
}

/// <summary>Gets detached parameter facts or null when unavailable; only static cmdlet metadata is cached.</summary>
public IReadOnlyDictionary<string, CommandParameterSnapshot> GetCommandParameterSnapshot(
string name, CommandTypes? commandType = null, bool bypassCache = false)
{
return CommandInfoCache.GetParameterSnapshot(name, commandType, bypassCache);
}

/// <summary>Gets the mandatory parameter summary under a single runspace lock, or null when unavailable.</summary>
public IReadOnlyList<string> GetMandatoryParameterNames(string name)
{
return CommandInfoCache.GetMandatoryParameterNames(name);
}

/// <summary>
/// Returns the get, set and test targetresource dsc function
/// </summary>
Expand Down
55 changes: 55 additions & 0 deletions Engine/ModuleManifestAnalysisCache.cs
Original file line number Diff line number Diff line change
@@ -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<string, Result> results = new Dictionary<string, Result>(StringComparer.Ordinal);

internal PSModuleInfo Get(Helper helper, string path, out IEnumerable<ErrorRecord> 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<ErrorRecord> Errors;
}
}
}
2 changes: 2 additions & 0 deletions Engine/ScriptAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2128,6 +2128,7 @@ public IEnumerable<DiagnosticRecord> 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())
{
Expand All @@ -2139,6 +2140,7 @@ public IEnumerable<DiagnosticRecord> 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)
Expand Down
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,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.

Expand Down
2 changes: 1 addition & 1 deletion Rules/MissingModuleManifestField.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
if (Helper.IsModuleManifest(fileName))
{
IEnumerable<ErrorRecord> 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)
Expand Down
42 changes: 4 additions & 38 deletions Rules/UseCmdletCorrectly.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ParameterMetadata>();
try
{
int noOfParamSets = Helper.Instance.GetCommandParameterSets(cmdAst.GetCommandName()).Count;
foreach (ParameterMetadata pm in Helper.Instance.GetCommandParameters(cmdAst.GetCommandName()).Values)
{
int count = 0;

if (pm.Attributes.Count < noOfParamSets)
{
continue;
}

foreach (Attribute attr in pm.Attributes)
{
if (!(attr is ParameterAttribute)) continue;
if (((ParameterAttribute)attr).Mandatory)
{
count += 1;
}
}
var mandatoryParameters = Helper.Instance.GetMandatoryParameterNames(cmdAst.GetCommandName());

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;
}
Expand All @@ -188,8 +156,8 @@ private bool MandatoryParameterExists(CommandAst cmdAst)
foreach (CommandElementAst commandElementAst in cmdAst.CommandElements.OfType<CommandParameterAst>())
{
CommandParameterAst cpAst = (CommandParameterAst)commandElementAst;
if (mandatoryParameters.Count<ParameterMetadata>(item =>
item.Name.Equals(cpAst.ParameterName, StringComparison.OrdinalIgnoreCase)) > 0)
if (mandatoryParameters.Any(item =>
item.Equals(cpAst.ParameterName, StringComparison.OrdinalIgnoreCase)))
{
return true;
}
Expand Down Expand Up @@ -254,5 +222,3 @@ public string GetSourceName()
}
}



35 changes: 5 additions & 30 deletions Rules/UseCorrectCasing.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,21 +119,12 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
}

var commandParameterAsts = commandAst.FindAll(
testAst => testAst is CommandParameterAst, true).Cast<CommandParameterAst>();
Dictionary<string, ParameterMetadata> availableParameters;
try
testAst => testAst is CommandParameterAst, true).Cast<CommandParameterAst>().ToArray();
if (commandParameterAsts.Length == 0)
{
availableParameters = Helper.Instance.GetCommandParameters(commandName);
}
// 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
// The affinity problem surfaces as an InvalidOperationException or as a
// NullReferenceException, see https://github.com/PowerShell/PSScriptAnalyzer/issues/1708
catch (Exception exception) when (exception is InvalidOperationException || exception is NullReferenceException)
{
availableParameters = GetParametersFromFreshCommandInfo(commandName);
continue;
}
var availableParameters = Helper.Instance.GetCommandParameterSnapshot(commandName);
if (availableParameters is null)
{
// The parameters of this command cannot be determined reliably,
Expand All @@ -143,7 +134,7 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
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))
Expand All @@ -168,22 +159,6 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
}
}

/// <summary>
/// Queries a fresh <see cref="CommandInfo"/> object to work around the runspace affinity problem
/// of the PowerShell engine and returns its parameters, or null if they cannot be determined.
/// </summary>
private Dictionary<string, ParameterMetadata> GetParametersFromFreshCommandInfo(string commandName)
{
try
{
return Helper.Instance.GetCommandParameters(commandName, bypassCache: true);
}
catch (Exception exception) when (exception is InvalidOperationException || exception is NullReferenceException)
{
return null;
}
}

/// <summary>
/// For a command like "gci -path c:", returns the extent of "gci" in the command
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion Rules/UseToExportFieldsInManifest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)

// check if valid module manifest
IEnumerable<ErrorRecord> 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;
Expand Down
Loading
Loading