diff --git a/Rules/AvoidAlias.cs b/Rules/AvoidAlias.cs index c6435833d..2cc8979d9 100644 --- a/Rules/AvoidAlias.cs +++ b/Rules/AvoidAlias.cs @@ -22,6 +22,24 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #endif public class AvoidAlias : IScriptRule { + private sealed class FunctionAnalysisContext + { + public Dictionary>> FunctionNameIndex { get; set; } + + public Dictionary ScriptDefinedFunctionCache { get; } = new Dictionary(); + + public Dictionary CallerScopedScriptBlockCache { get; } = new Dictionary(); + + public Dictionary IsolatedRunspaceScriptBlockCache { get; } = new Dictionary(); + + public void ClearCaches() + { + ScriptDefinedFunctionCache.Clear(); + CallerScopedScriptBlockCache.Clear(); + IsolatedRunspaceScriptBlockCache.Clear(); + } + } + private readonly string allowListArgName = "allowlist"; // keep legacy argument name for next version to allow customers to transition but remove later private readonly string allowListLegacyArgName = "whitelist"; @@ -97,6 +115,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) } // Finds all CommandAsts. IEnumerable foundAsts = ast.FindAll(testAst => testAst is CommandAst, true); + FunctionAnalysisContext functionAnalysis = GetFunctionNameIndex(ast); // Iterates all CommandAsts and check the command name. foreach (CommandAst cmdAst in foundAsts) @@ -119,7 +138,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) } string cmdletNameIfCommandNameWasAlias = Helper.Instance.GetCmdletNameFromAlias(commandName); - bool isDscResourceDeclaration = IsDscResourceDeclaration(cmdAst); + bool isDscResourceDeclaration = IsDscResourceDeclaration(cmdAst, functionAnalysis); if (isDscResourceDeclaration && String.IsNullOrEmpty(cmdletNameIfCommandNameWasAlias)) { continue; @@ -140,7 +159,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) } // If we find match of any kind, do not continue with the Get-{commandname} check - if ( Helper.Instance.GetCommandInfo(commandName) != null ) { + if (Helper.Instance.GetCommandInfo(commandName) != null || IsScriptDefinedFunction(cmdAst, functionAnalysis)) { continue; } @@ -192,7 +211,9 @@ private bool IgnoreCommandast(CommandAst cmdAst) return false; } - private bool IsDscResourceDeclaration(CommandAst cmdAst) + private bool IsDscResourceDeclaration( + CommandAst cmdAst, + FunctionAnalysisContext functionAnalysis) { bool hasConfigurationAncestor = false; for (Ast parent = cmdAst.Parent; parent != null; parent = parent.Parent) @@ -214,7 +235,8 @@ private bool IsDscResourceDeclaration(CommandAst cmdAst) } } - if (!hasConfigurationAncestor || GetDscResourceBody(cmdAst) == null) + if (!hasConfigurationAncestor || GetDscResourceBody(cmdAst) == null + || !IsDscResourceCommand(cmdAst, functionAnalysis)) { return false; } @@ -238,7 +260,7 @@ private bool IsDscResourceDeclaration(CommandAst cmdAst) if (parent is CommandExpressionAst commandExpression && (!(commandExpression.Expression is ScriptBlockExpressionAst body) - || !IsFollowingLineDscResourceBody(body))) + || !IsFollowingLineDscResourceBody(body, functionAnalysis))) { return false; } @@ -249,9 +271,7 @@ private bool IsDscResourceDeclaration(CommandAst cmdAst) if (scriptBlock.Parent is CommandAst enclosingCommand) { if (GetDscResourceBody(enclosingCommand) != scriptBlock - || !String.IsNullOrEmpty(Helper.Instance.GetCmdletNameFromAlias(enclosingCommand.GetCommandName())) - || (!String.Equals(enclosingCommand.GetCommandName(), "Node", StringComparison.OrdinalIgnoreCase) - && Helper.Instance.GetCommandInfo(enclosingCommand.GetCommandName()) != null)) + || !IsDscResourceCommand(enclosingCommand, functionAnalysis)) { return false; } @@ -268,7 +288,9 @@ private bool IsDscResourceDeclaration(CommandAst cmdAst) return false; } - private bool IsFollowingLineDscResourceBody(ScriptBlockExpressionAst body) + private bool IsFollowingLineDscResourceBody( + ScriptBlockExpressionAst body, + FunctionAnalysisContext functionAnalysis) { if (!(body.Parent is CommandExpressionAst bodyExpression) || !(bodyExpression.Parent is PipelineAst bodyPipeline) @@ -297,16 +319,1097 @@ private bool IsFollowingLineDscResourceBody(ScriptBlockExpressionAst body) return false; } - string declarationCommandName = declaration.GetCommandName(); return GetDscResourceBody(declaration) == body - && String.IsNullOrEmpty(Helper.Instance.GetCmdletNameFromAlias(declarationCommandName)) - && (String.Equals(declarationCommandName, "Node", StringComparison.OrdinalIgnoreCase) - || Helper.Instance.GetCommandInfo(declarationCommandName) == null); + && IsDscResourceCommand(declaration, functionAnalysis); + } + + return false; + } + + private bool IsDscResourceCommand( + CommandAst cmdAst, + FunctionAnalysisContext functionAnalysis) + { + string commandName = cmdAst.GetCommandName(); + return !IsScriptDefinedFunction(cmdAst, functionAnalysis) + && String.IsNullOrEmpty(Helper.Instance.GetCmdletNameFromAlias(commandName)) + && (String.Equals(commandName, "Node", StringComparison.OrdinalIgnoreCase) + || Helper.Instance.GetCommandInfo(commandName) == null); + } + + private bool IsScriptDefinedFunction( + CommandAst cmdAst, + FunctionAnalysisContext functionAnalysis) + { + string commandName = cmdAst.GetCommandName(); + if (commandName == null) + { + return false; + } + + if (!String.IsNullOrEmpty(Helper.Instance.GetCmdletNameFromAlias(commandName))) + { + return false; + } + + if (functionAnalysis.ScriptDefinedFunctionCache.TryGetValue(cmdAst, out bool? cachedResult)) + { + return cachedResult.GetValueOrDefault(); + } + + functionAnalysis.ScriptDefinedFunctionCache[cmdAst] = null; + ScriptBlockAst currentScope = null; + for (Ast parent = cmdAst.Parent; parent != null; parent = parent.Parent) + { + if (parent is ScriptBlockAst scope) + { + if (currentScope == null && !IsCallerScopedScriptBlock(scope, functionAnalysis)) + { + currentScope = scope; + } + + if (functionAnalysis.FunctionNameIndex.TryGetValue( + scope, + out Dictionary> functionNames) + && functionNames.TryGetValue(commandName, out List functionDefinitions) + && functionDefinitions.Any(function => + !IsDescendantOf(function, cmdAst) + && (!function.Name.StartsWith("private:", StringComparison.OrdinalIgnoreCase) + || currentScope == scope) + && !IsFunctionDeclaredAfterCommand(function, cmdAst, currentScope, functionAnalysis))) + { + functionAnalysis.ScriptDefinedFunctionCache[cmdAst] = true; + return true; + } + + if (IsIsolatedRunspaceScriptBlock(scope, functionAnalysis)) + { + break; + } + } } + functionAnalysis.ScriptDefinedFunctionCache[cmdAst] = false; return false; } + private bool IsFunctionDeclaredAfterCommand( + FunctionDefinitionAst function, + CommandAst cmdAst, + ScriptBlockAst commandScope, + FunctionAnalysisContext functionAnalysis) + { + if (commandScope == null) + { + return false; + } + + ScriptBlockAst functionScope = GetFunctionScope(function, functionAnalysis); + if (functionScope == null) + { + return false; + } + + CommandAst immediateInvocation = null; + if (commandScope != functionScope + && !IsImmediatelyInvokedChildScope(commandScope, functionScope, out immediateInvocation)) + { + return false; + } + + int functionNamedBlockPhase = GetNamedBlockExecutionPhase(function, out FunctionDefinitionAst functionOwner); + int commandNamedBlockPhase = GetNamedBlockExecutionPhase(cmdAst, out FunctionDefinitionAst commandOwner); + if (functionOwner != null + && functionOwner == commandOwner + && functionNamedBlockPhase >= 0 + && commandNamedBlockPhase >= 0 + && functionNamedBlockPhase != commandNamedBlockPhase) + { + return functionNamedBlockPhase > commandNamedBlockPhase; + } + + int functionPhase = GetForEachObjectExecutionPhase(function, out CommandAst functionCallback); + int commandPhase = GetForEachObjectExecutionPhase(cmdAst, out CommandAst commandCallback); + if (functionCallback == commandCallback + && functionPhase >= 0 + && commandPhase >= 0 + && functionPhase != commandPhase) + { + return functionPhase > commandPhase; + } + + int commandExecutionOffset = immediateInvocation == null + ? cmdAst.Extent.StartOffset + : immediateInvocation.Extent.StartOffset; + return function.Extent.StartOffset > commandExecutionOffset; + } + + private bool IsImmediatelyInvokedChildScope( + ScriptBlockAst commandScope, + ScriptBlockAst functionScope, + out CommandAst invocation) + { + invocation = null; + if (!(commandScope.Parent is ScriptBlockExpressionAst scriptBlock)) + { + return false; + } + + CommandAst command = GetScriptBlockCommand(scriptBlock); + if (command == null || command.InvocationOperator != TokenKind.Ampersand) + { + return false; + } + + ScriptBlockAst parentScope = null; + for (Ast parent = commandScope.Parent; parent != null; parent = parent.Parent) + { + if (parent is ScriptBlockAst scope) + { + parentScope = scope; + break; + } + } + + if (parentScope != functionScope) + { + return false; + } + + invocation = command; + return true; + } + + private int GetNamedBlockExecutionPhase(Ast ast, out FunctionDefinitionAst containingFunction) + { + NamedBlockAst namedBlock = null; + containingFunction = null; + for (Ast current = ast; current != null; current = current.Parent) + { + if (namedBlock == null && current is NamedBlockAst block) + { + namedBlock = block; + } + + if (current != ast && current is FunctionDefinitionAst function) + { + containingFunction = function; + break; + } + } + + if (namedBlock == null || containingFunction == null) + { + return -1; + } + + switch (namedBlock.BlockKind) + { + case TokenKind.Begin: + return 0; + case TokenKind.Process: + return 1; + case TokenKind.End: + return 2; + default: + return -1; + } + } + + private int GetForEachObjectExecutionPhase(Ast ast, out CommandAst callbackCommand) + { + callbackCommand = null; + for (Ast current = ast; current != null; current = current.Parent) + { + if (!(current is ScriptBlockAst scope) + || !(scope.Parent is ScriptBlockExpressionAst scriptBlock)) + { + continue; + } + + CommandAst commandAst = GetScriptBlockCommand(scriptBlock); + if (commandAst == null + || !IsForEachObjectCommand(GetCallbackCommandName(commandAst))) + { + continue; + } + + string parameterName = GetScriptBlockParameterName(commandAst, scriptBlock); + if (IsParameter(parameterName, "Begin")) + { + callbackCommand = commandAst; + return 0; + } + + if (IsParameter(parameterName, "Process")) + { + callbackCommand = commandAst; + return 1; + } + + if (IsParameter(parameterName, "End")) + { + callbackCommand = commandAst; + return 2; + } + } + + return -1; + } + + private FunctionAnalysisContext GetFunctionNameIndex(Ast ast) + { + FunctionDefinitionAst[] functionDefinitions = ast.FindAll( + candidate => candidate is FunctionDefinitionAst, + true).OfType().ToArray(); + + var functionAnalysis = new FunctionAnalysisContext + { + FunctionNameIndex = new Dictionary>>() + }; + + foreach (FunctionDefinitionAst function in functionDefinitions) + { + if (IsInConstantFalseBranch(function)) + { + continue; + } + + ScriptBlockAst scope = GetNearestFunctionScope(function); + if (scope != null) + { + AddFunctionToIndex(functionAnalysis.FunctionNameIndex, scope, function); + } + } + + var previousScopes = new Dictionary(); + foreach (FunctionDefinitionAst function in functionDefinitions) + { + if (!IsInConstantFalseBranch(function)) + { + previousScopes[function] = GetNearestFunctionScope(function); + } + } + + for (int iteration = 0; iteration <= previousScopes.Count; iteration++) + { + functionAnalysis.ClearCaches(); + var functionNameIndex = new Dictionary>>(); + var currentScopes = new Dictionary(); + foreach (FunctionDefinitionAst function in previousScopes.Keys) + { + ScriptBlockAst scope = GetFunctionScope(function, functionAnalysis); + currentScopes[function] = scope; + if (scope != null) + { + AddFunctionToIndex(functionNameIndex, scope, function); + } + } + + bool scopesChanged = previousScopes.Any(pair => currentScopes[pair.Key] != pair.Value); + functionAnalysis.FunctionNameIndex = functionNameIndex; + previousScopes = currentScopes; + if (!scopesChanged) + { + break; + } + } + + functionAnalysis.ClearCaches(); + return functionAnalysis; + } + + private void AddFunctionToIndex( + Dictionary>> functionNameIndex, + ScriptBlockAst scope, + FunctionDefinitionAst function) + { + string functionName = Helper.Instance.FunctionNameWithoutScope(function.Name); + if (String.IsNullOrEmpty(functionName)) + { + return; + } + + if (!functionNameIndex.TryGetValue( + scope, + out Dictionary> functionNames)) + { + functionNames = new Dictionary>(StringComparer.OrdinalIgnoreCase); + functionNameIndex.Add(scope, functionNames); + } + + if (functionNames.TryGetValue(functionName, out List functionDefinitions)) + { + functionDefinitions.Add(function); + } + else + { + functionNames.Add(functionName, new List { function }); + } + } + + private ScriptBlockAst GetNearestFunctionScope(FunctionDefinitionAst function) + { + for (Ast parent = function.Parent; parent != null; parent = parent.Parent) + { + if (parent is ScriptBlockAst scope) + { + return scope; + } + } + + return null; + } + + private ScriptBlockAst GetFunctionScope( + FunctionDefinitionAst function, + FunctionAnalysisContext functionAnalysis) + { + for (Ast parent = function.Parent; parent != null; parent = parent.Parent) + { + if (parent is ScriptBlockAst scope + && !IsCallerScopedScriptBlock(scope, functionAnalysis)) + { + return scope; + } + } + + return null; + } + + private bool IsCallerScopedScriptBlock( + ScriptBlockAst scope, + FunctionAnalysisContext functionAnalysis) + { + if (functionAnalysis.CallerScopedScriptBlockCache.TryGetValue(scope, out bool? cachedResult)) + { + return cachedResult.GetValueOrDefault(); + } + + functionAnalysis.CallerScopedScriptBlockCache[scope] = null; + bool result = false; + if (!(scope.Parent is ScriptBlockExpressionAst scriptBlock)) + { + functionAnalysis.CallerScopedScriptBlockCache[scope] = result; + return result; + } + + CommandAst commandAst = GetScriptBlockCommand(scriptBlock); + if (commandAst == null || IsScriptDefinedFunction(commandAst, functionAnalysis)) + { + functionAnalysis.CallerScopedScriptBlockCache[scope] = result; + return result; + } + + if (commandAst.InvocationOperator == TokenKind.Dot + && commandAst.CommandElements[0] == scriptBlock) + { + result = true; + } + else + { + string commandName = GetCallbackCommandName(commandAst); + if (IsForEachObjectCommand(commandName)) + { + if (HasCommandParameter(commandAst, "MemberName") + || HasPositionalCommandParameter(commandAst, scriptBlock, "MemberName")) + { + functionAnalysis.CallerScopedScriptBlockCache[scope] = result; + return result; + } + + string parameterName = GetScriptBlockParameterName(commandAst, scriptBlock); + result = !IsParameter(parameterName, "Parallel") + && (IsParameter(parameterName, "Begin") + || IsParameter(parameterName, "Process") + || IsParameter(parameterName, "End") + || IsParameter(parameterName, "RemainingScripts")); + } + else if (IsWhereObjectCommand(commandName)) + { + result = IsParameter(GetScriptBlockParameterName(commandAst, scriptBlock), "FilterScript"); + } + else if (IsInvokeCommand(commandName) + && IsParameter(GetScriptBlockParameterName(commandAst, scriptBlock), "ScriptBlock")) + { + result = HasEnabledCommandParameter(commandAst, "NoNewScope") + && !IsRemoteInvokeCommand(commandAst); + } + } + + functionAnalysis.CallerScopedScriptBlockCache[scope] = result; + return result; + } + + private bool IsIsolatedRunspaceScriptBlock( + ScriptBlockAst scope, + FunctionAnalysisContext functionAnalysis) + { + if (functionAnalysis.IsolatedRunspaceScriptBlockCache.TryGetValue(scope, out bool? cachedResult)) + { + return cachedResult.GetValueOrDefault(); + } + + functionAnalysis.IsolatedRunspaceScriptBlockCache[scope] = null; + bool result = false; + if (!(scope.Parent is ScriptBlockExpressionAst scriptBlock)) + { + functionAnalysis.IsolatedRunspaceScriptBlockCache[scope] = result; + return result; + } + + CommandAst commandAst = GetScriptBlockCommand(scriptBlock); + if (commandAst == null) + { + commandAst = GetPrecedingDscScriptPropertyCommand(scriptBlock); + } + + if (commandAst == null || IsScriptDefinedFunction(commandAst, functionAnalysis)) + { + functionAnalysis.IsolatedRunspaceScriptBlockCache[scope] = result; + return result; + } + + string commandName = GetCallbackCommandName(commandAst); + string parameterName = GetScriptBlockParameterName(commandAst, scriptBlock); + if (IsForEachObjectCommand(commandName) && IsParameter(parameterName, "Parallel")) + { + result = true; + } + else if ((String.Equals(commandName, "Start-Job", StringComparison.OrdinalIgnoreCase) + || String.Equals(commandName, "Start-ThreadJob", StringComparison.OrdinalIgnoreCase)) + && (IsParameter(parameterName, "ScriptBlock") + || IsParameter(parameterName, "InitializationScript"))) + { + result = true; + } + else if (String.Equals(commandName, "Invoke-Command", StringComparison.OrdinalIgnoreCase) + && IsParameter(parameterName, "ScriptBlock") + && IsRemoteInvokeCommand(commandAst)) + { + result = true; + } + else if (String.Equals(commandName, "InlineScript", StringComparison.OrdinalIgnoreCase)) + { + result = true; + } + else + { + result = (String.Equals(commandName, "GetScript", StringComparison.OrdinalIgnoreCase) + || String.Equals(commandName, "TestScript", StringComparison.OrdinalIgnoreCase) + || String.Equals(commandName, "SetScript", StringComparison.OrdinalIgnoreCase)) + && commandAst.CommandElements.Count > 1 + && String.Equals(commandAst.CommandElements[1].ToString(), "=", StringComparison.Ordinal); + } + + functionAnalysis.IsolatedRunspaceScriptBlockCache[scope] = result; + return result; + } + + private CommandAst GetPrecedingDscScriptPropertyCommand(ScriptBlockExpressionAst scriptBlock) + { + if (!(scriptBlock.Parent is CommandExpressionAst commandExpression) + || !(commandExpression.Parent is PipelineAst pipeline)) + { + return null; + } + + Ast statement = pipeline; + for (Ast parent = pipeline.Parent; parent != null; parent = parent.Parent) + { + var statements = (parent as NamedBlockAst)?.Statements + ?? (parent as StatementBlockAst)?.Statements; + if (statements == null) + { + statement = parent; + continue; + } + + int index = statements.IndexOf(statement as StatementAst); + if (index <= 0 + || !(statements[index - 1] is PipelineAst propertyPipeline) + || propertyPipeline.PipelineElements.Count != 1 + || !(propertyPipeline.PipelineElements[0] is CommandAst propertyCommand) + || propertyCommand.CommandElements.Count != 2 + || !String.Equals(propertyCommand.CommandElements[1].ToString(), "=", StringComparison.Ordinal)) + { + return null; + } + + string propertyName = propertyCommand.GetCommandName(); + return String.Equals(propertyName, "GetScript", StringComparison.OrdinalIgnoreCase) + || String.Equals(propertyName, "TestScript", StringComparison.OrdinalIgnoreCase) + || String.Equals(propertyName, "SetScript", StringComparison.OrdinalIgnoreCase) + ? propertyCommand + : null; + } + + return null; + } + + private CommandAst GetScriptBlockCommand(ScriptBlockExpressionAst scriptBlock) + { + Ast current = scriptBlock; + while (current.Parent != null) + { + if (current.Parent is CommandAst commandAst) + { + return commandAst; + } + + if (current.Parent is ArrayLiteralAst arrayLiteral) + { + current = arrayLiteral; + continue; + } + + if (current.Parent is ArrayExpressionAst arrayExpression) + { + current = arrayExpression; + continue; + } + + if (current.Parent is ParenExpressionAst + || current.Parent is CommandExpressionAst + || current.Parent is PipelineAst + || current.Parent is StatementBlockAst) + { + current = current.Parent; + continue; + } + + if (current.Parent is CommandParameterAst parameter + && parameter.Argument == current) + { + current = parameter; + continue; + } + + return null; + } + + return null; + } + + private string GetCallbackCommandName(CommandAst commandAst) + { + string commandName = commandAst.GetCommandName(); + if (commandName == null) + { + return null; + } + + const string coreModulePrefix = "Microsoft.PowerShell.Core\\"; + if (commandName.StartsWith(coreModulePrefix, StringComparison.OrdinalIgnoreCase)) + { + commandName = commandName.Substring(coreModulePrefix.Length); + } + else if (commandName.EndsWith("\\Start-ThreadJob", StringComparison.OrdinalIgnoreCase)) + { + commandName = "Start-ThreadJob"; + } + else if (commandName.IndexOf('\\') >= 0) + { + return null; + } + + string cmdletName = Helper.Instance.GetCmdletNameFromAlias(commandName); + return String.IsNullOrEmpty(cmdletName) ? commandName : cmdletName; + } + + private string GetScriptBlockParameterName( + CommandAst commandAst, + ScriptBlockExpressionAst scriptBlock) + { + CommandInfo commandInfo = Helper.Instance.GetCommandInfo(GetCallbackCommandName(commandAst)); + if (commandInfo == null) + { + return null; + } + + Dictionary parameters = GetCommandParameters(commandInfo); + + CommandParameterAst attachedParameter = commandAst.CommandElements + .OfType() + .FirstOrDefault(parameter => parameter.Argument != null + && (parameter.Argument == scriptBlock + || IsDescendantOf(scriptBlock, parameter.Argument))); + if (parameters == null) + { + return IsInvokeCommand(GetCallbackCommandName(commandAst)) + && attachedParameter != null + && IsInvokeCommandScriptBlockParameter(attachedParameter.ParameterName) + ? "ScriptBlock" + : null; + } + + if (attachedParameter != null) + { + return ResolveParameter(parameters, attachedParameter.ParameterName)?.Name; + } + + if (IsWhereObjectCommand(GetCallbackCommandName(commandAst)) + && commandAst.CommandElements + .OfType() + .Any(parameter => IsParameter( + ResolveParameter(parameters, parameter.ParameterName)?.Name, + "Property"))) + { + return null; + } + + CommandElementAst argumentElement = commandAst.CommandElements + .FirstOrDefault(element => element == scriptBlock + || IsDescendantOf(scriptBlock, element)); + int scriptBlockIndex = argumentElement == null + ? -1 + : commandAst.CommandElements.IndexOf(argumentElement); + if (scriptBlockIndex < 0) + { + return null; + } + + int positionalIndex = 0; + for (int i = 1; i <= scriptBlockIndex; i++) + { + CommandElementAst commandElement = commandAst.CommandElements[i]; + if (i == scriptBlockIndex) + { + return GetPositionalScriptBlockParameter(parameters, positionalIndex)?.Name; + } + + if (!(commandElement is CommandParameterAst parameter)) + { + positionalIndex++; + continue; + } + + ParameterMetadata parameterMetadata = ResolveParameter(parameters, parameter.ParameterName); + if (parameter.Argument != null + || parameterMetadata?.ParameterType == typeof(SwitchParameter)) + { + continue; + } + + if (i + 1 == scriptBlockIndex) + { + return parameterMetadata?.Name; + } + + if (i + 1 < scriptBlockIndex + && !(commandAst.CommandElements[i + 1] is CommandParameterAst)) + { + i++; + } + } + + return null; + } + + private bool HasPositionalCommandParameter( + CommandAst commandAst, + ScriptBlockExpressionAst scriptBlock, + string parameterName) + { + CommandInfo commandInfo = Helper.Instance.GetCommandInfo(GetCallbackCommandName(commandAst)); + Dictionary parameters = commandInfo == null + ? null + : GetCommandParameters(commandInfo); + if (parameters == null) + { + return false; + } + + CommandElementAst argumentElement = commandAst.CommandElements + .FirstOrDefault(element => element == scriptBlock + || IsDescendantOf(scriptBlock, element)); + int scriptBlockIndex = argumentElement == null + ? -1 + : commandAst.CommandElements.IndexOf(argumentElement); + if (scriptBlockIndex < 0) + { + return false; + } + + int positionalIndex = 0; + for (int i = 1; i < scriptBlockIndex; i++) + { + CommandElementAst commandElement = commandAst.CommandElements[i]; + if (!(commandElement is CommandParameterAst parameter)) + { + if (IsPositionalParameter(parameters, positionalIndex, parameterName)) + { + return true; + } + + positionalIndex++; + continue; + } + + ParameterMetadata metadata = ResolveParameter(parameters, parameter.ParameterName); + if (parameter.Argument != null || metadata?.ParameterType == typeof(SwitchParameter)) + { + continue; + } + + if (i + 1 < scriptBlockIndex + && !(commandAst.CommandElements[i + 1] is CommandParameterAst)) + { + i++; + } + } + + return false; + } + + private bool IsPositionalParameter( + Dictionary parameters, + int position, + string parameterName) + { + return parameters.Values.Any(parameter => + String.Equals(parameter.Name, parameterName, StringComparison.OrdinalIgnoreCase) + && parameter.Attributes.OfType() + .Any(attribute => attribute.Position == position)); + } + + private ParameterMetadata ResolveParameter( + Dictionary parameters, + string parameterName) + { + if (String.IsNullOrEmpty(parameterName)) + { + return null; + } + + ParameterMetadata exactMatch = null; + var prefixMatches = new List(); + foreach (ParameterMetadata parameter in parameters.Values) + { + if (String.Equals(parameter.Name, parameterName, StringComparison.OrdinalIgnoreCase) + || parameter.Aliases.Contains(parameterName, StringComparer.OrdinalIgnoreCase)) + { + exactMatch = parameter; + break; + } + + if (parameter.Name.StartsWith(parameterName, StringComparison.OrdinalIgnoreCase) + || parameter.Aliases.Any(alias => alias.StartsWith(parameterName, StringComparison.OrdinalIgnoreCase))) + { + prefixMatches.Add(parameter); + } + } + + return exactMatch ?? (prefixMatches.Count == 1 ? prefixMatches[0] : null); + } + + private ParameterMetadata GetPositionalScriptBlockParameter( + Dictionary parameters, + int position) + { + ParameterMetadata match = null; + ParameterMetadata remainingScriptsParameter = null; + foreach (ParameterMetadata parameter in parameters.Values) + { + Type parameterType = parameter.ParameterType; + if (parameterType.IsArray) + { + parameterType = parameterType.GetElementType(); + } + + if (!typeof(ScriptBlock).IsAssignableFrom(parameterType)) + { + continue; + } + + IEnumerable parameterAttributes = parameter.Attributes.OfType(); + if (parameterAttributes.Any(attribute => attribute.Position == position)) + { + if (match != null) + { + return null; + } + + match = parameter; + } + + if (parameterAttributes.Any(attribute => attribute.ValueFromRemainingArguments)) + { + if (remainingScriptsParameter != null) + { + return null; + } + + remainingScriptsParameter = parameter; + } + } + + return match ?? remainingScriptsParameter; + } + + private bool HasCommandParameter(CommandAst commandAst, string parameterName) + { + CommandInfo commandInfo = Helper.Instance.GetCommandInfo(GetCallbackCommandName(commandAst)); + if (commandInfo == null) + { + return false; + } + + Dictionary parameters = GetCommandParameters(commandInfo); + return commandAst.CommandElements + .OfType() + .Any(parameter => + { + string resolvedName = parameters == null + ? null + : ResolveParameter(parameters, parameter.ParameterName)?.Name; + return resolvedName?.Equals(parameterName, StringComparison.OrdinalIgnoreCase) == true + || (resolvedName == null + && IsInvokeCommand(GetCallbackCommandName(commandAst)) + && IsInvokeCommandParameter(parameter.ParameterName, parameterName)); + }); + } + + private bool HasEnabledCommandParameter(CommandAst commandAst, string parameterName) + { + CommandInfo commandInfo = Helper.Instance.GetCommandInfo(GetCallbackCommandName(commandAst)); + Dictionary parameters = commandInfo == null + ? null + : GetCommandParameters(commandInfo); + foreach (CommandParameterAst parameter in commandAst.CommandElements.OfType()) + { + string resolvedName = parameters == null + ? null + : ResolveParameter(parameters, parameter.ParameterName)?.Name; + if (!String.Equals(resolvedName, parameterName, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + if (parameter.Argument == null) + { + return true; + } + + try + { + return LanguagePrimitives.IsTrue(parameter.Argument.SafeGetValue()); + } + catch (InvalidOperationException) + { + return true; + } + } + + return false; + } + + private bool IsRemoteInvokeCommand(CommandAst commandAst) + { + CommandInfo commandInfo = Helper.Instance.GetCommandInfo(GetCallbackCommandName(commandAst)); + if (commandInfo == null) + { + return false; + } + + Dictionary parameters = GetCommandParameters(commandInfo); + if (parameters == null) + { + return false; + } + + string[] remoteTargetParameters = + { + "ComputerName", + "Session", + "ConnectionUri", + "HostName", + "VMId", + "VMName", + "ContainerId" + }; + + foreach (CommandParameterAst parameter in commandAst.CommandElements.OfType()) + { + string parameterName = ResolveParameter(parameters, parameter.ParameterName)?.Name; + if (remoteTargetParameters.Contains(parameterName, StringComparer.OrdinalIgnoreCase)) + { + return true; + } + } + + int positionalIndex = 0; + for (int i = 1; i < commandAst.CommandElements.Count; i++) + { + CommandElementAst element = commandAst.CommandElements[i]; + if (element is CommandParameterAst parameter) + { + ParameterMetadata metadata = ResolveParameter(parameters, parameter.ParameterName); + if (parameter.Argument == null + && metadata?.ParameterType != typeof(SwitchParameter) + && (i + 1 < commandAst.CommandElements.Count + && !(commandAst.CommandElements[i + 1] is CommandParameterAst))) + { + i++; + } + + continue; + } + + bool isScriptBlock = element is ScriptBlockExpressionAst + || element.FindAll(candidate => candidate is ScriptBlockExpressionAst, true).Any(); + if (!isScriptBlock + && remoteTargetParameters.Any(name => + parameters.TryGetValue(name, out ParameterMetadata metadata) + && metadata.Attributes.OfType() + .Any(attribute => attribute.Position == positionalIndex))) + { + return true; + } + + positionalIndex++; + } + + return false; + } + + private Dictionary GetCommandParameters(CommandInfo commandInfo) + { + if ((commandInfo.CommandType & CommandTypes.Application) != 0) + { + return null; + } + + try + { + return commandInfo.Parameters; + } + catch (InvalidOperationException) + { + return GetCommandParametersWithoutCache(commandInfo); + } + catch (NotSupportedException) + { + return GetCommandParametersWithoutCache(commandInfo); + } + } + + private Dictionary GetCommandParametersWithoutCache(CommandInfo commandInfo) + { + try + { + return Helper.Instance.GetCommandInfo(commandInfo.Name, bypassCache: true)?.Parameters; + } + catch (InvalidOperationException) + { + return null; + } + catch (NotSupportedException) + { + return null; + } + } + + private bool IsInvokeCommand(string commandName) + { + return String.Equals(commandName, "Invoke-Command", StringComparison.OrdinalIgnoreCase); + } + + private bool IsInvokeCommandScriptBlockParameter(string parameterName) + { + return String.Equals(parameterName, "ScriptBlock", StringComparison.OrdinalIgnoreCase) + || String.Equals(parameterName, "Command", StringComparison.OrdinalIgnoreCase); + } + + private bool IsInvokeCommandParameter(string parameterName, string expectedName) + { + if (String.Equals(expectedName, "ScriptBlock", StringComparison.OrdinalIgnoreCase)) + { + return IsInvokeCommandScriptBlockParameter(parameterName); + } + + if (String.Equals(expectedName, "ComputerName", StringComparison.OrdinalIgnoreCase)) + { + return String.Equals(parameterName, "ComputerName", StringComparison.OrdinalIgnoreCase) + || String.Equals(parameterName, "Cn", StringComparison.OrdinalIgnoreCase); + } + + if (String.Equals(expectedName, "Session", StringComparison.OrdinalIgnoreCase)) + { + return String.Equals(parameterName, "Session", StringComparison.OrdinalIgnoreCase) + || String.Equals(parameterName, "PSSession", StringComparison.OrdinalIgnoreCase); + } + + return false; + } + + private bool IsParameter(string parameter, string parameterName) + { + return String.Equals(parameter, parameterName, StringComparison.OrdinalIgnoreCase); + } + + private bool IsForEachObjectCommand(string commandName) + { + return String.Equals(commandName, "ForEach-Object", StringComparison.OrdinalIgnoreCase); + } + + private bool IsWhereObjectCommand(string commandName) + { + return String.Equals(commandName, "Where-Object", StringComparison.OrdinalIgnoreCase); + } + + private bool IsInConstantFalseBranch(FunctionDefinitionAst function) + { + for (Ast parent = function.Parent; parent != null; parent = parent.Parent) + { + if (!(parent is IfStatementAst ifStatement)) + { + continue; + } + + foreach (Tuple clause in ifStatement.Clauses) + { + if (IsDescendantOf(function, clause.Item2) && IsConstantFalseCondition(clause.Item1)) + { + return true; + } + } + } + + return false; + } + + private bool IsDescendantOf(Ast ast, Ast ancestor) + { + for (Ast parent = ast.Parent; parent != null; parent = parent.Parent) + { + if (parent == ancestor) + { + return true; + } + } + + return false; + } + + private bool IsConstantFalseCondition(PipelineBaseAst condition) + { + ExpressionAst expression = condition.GetPureExpression(); + if (expression == null) + { + return false; + } + + try + { + return !LanguagePrimitives.IsTrue(expression.SafeGetValue()); + } + catch (InvalidOperationException) + { + return false; + } + } + private ScriptBlockExpressionAst GetDscResourceBody(CommandAst cmdAst) { if (cmdAst.InvocationOperator != TokenKind.Unknown diff --git a/Tests/Rules/AvoidUsingAlias.tests.ps1 b/Tests/Rules/AvoidUsingAlias.tests.ps1 index 814574525..f0822c605 100644 --- a/Tests/Rules/AvoidUsingAlias.tests.ps1 +++ b/Tests/Rules/AvoidUsingAlias.tests.ps1 @@ -63,6 +63,424 @@ gci -Path C:\ $noViolations.Count | Should -Be 0 } + It "does not add the Get prefix to script-defined functions or skip their script block arguments" { + $target = 'function script:vErB {}; Verb Instance { ChildItem . {} }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'ChildItem' + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | + Should -Be ($target.Replace('ChildItem', 'Get-ChildItem')) + } + + It "respects private function visibility" { + $target = 'function private:Verb {}; Verb; . { Verb }; & { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | + Should -Be ($target.Replace('& { Verb }', '& { Get-Verb }')) + } + + It "does not resolve function declarations before their execution" { + $target = 'Verb; function Verb {}' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "respects execution order across caller-scoped blocks" -TestCases @( + @{ Target = 'Verb; . { function Verb {} }' } + @{ Target = '. { Verb }; function Verb {}' } + ) { + param($Target) + + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $Target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "does not resolve declarations after an immediately invoked child block" { + $target = '& { Verb }; function Verb {}' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "respects function begin and process execution order" { + $target = 'function Invoke-Example { process { Verb } begin { function Verb {} } }; Invoke-Example' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + } + + It "respects execution order across ForEach-Object callback phases" -TestCases @( + @{ + Target = '1 | ForEach-Object -Process { Verb } -Begin { function Verb {} }' + ExpectedViolations = 0 + ExpectedText = '1 | ForEach-Object -Process { Verb } -Begin { function Verb {} }' + } + @{ + Target = '1 | ForEach-Object -Begin { Verb } -Process { function Verb {} }' + ExpectedViolations = 1 + ExpectedText = '1 | ForEach-Object -Begin { Get-Verb } -Process { function Verb {} }' + } + ) { + param($Target, $ExpectedViolations, $ExpectedText) + + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $Target -IncludeRule $violationName) + + $violations.Count | Should -Be $ExpectedViolations + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $Target -Settings $settings | Should -Be $ExpectedText + } + + It "does not apply caller scope to script-block arguments of dot-invoked commands" { + $target = 'function private:Verb {}; . Invoke-Command -ScriptBlock { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "indexes declarations in directly dot-sourced blocks in the caller scope" { + $target = '. { function private:Verb {}; Verb }; Verb' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + } + + It "uses caller scope for parameterized dot-sourced blocks" { + $target = 'function private:Verb {}; . { param($value) Verb } 1' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + } + + It "recognizes caller scope for local Invoke-Command NoNewScope callbacks" { + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + $target = 'function private:Verb {}; Invoke-Command -NoNewScope -ScriptBlock { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + + $target = 'Invoke-Command -NoNewScope -ScriptBlock { function private:Verb {}; Verb }; Verb' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + } + + It "does not treat a newline-separated DSC script property as caller-scoped" { + $target = @' +function Verb {}; +Script Test { + SetScript = + { Verb } +} +'@ + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "respects caller scope in callbacks" -TestCases @( + @{ Command = 'ForEach-Object' } + @{ Command = 'Where-Object' } + @{ Command = 'Microsoft.PowerShell.Core\ForEach-Object' } + @{ Command = 'Microsoft.PowerShell.Core\Where-Object' } + ) { + param($Command) + + $target = "function private:Verb {}; 1 | $Command { Verb }" + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + } + + It "preserves caller scope when an alias takes precedence over a same-named function" { + $target = 'function where {}; function private:Verb {}; 1 | where { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'where' + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | + Should -Be ($target.Replace('1 | where', '1 | Where-Object')) + } + + It "respects caller scope for colon-attached callback arguments" { + foreach ($command in @( + 'ForEach-Object -Process:{ Verb }' + 'Where-Object -FilterScript:{ Verb }' + )) { + $target = "function private:Verb {}; 1 | $command" + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + } + } + + It "uses the bound callback parameter after switches and abbreviated parameters" { + $target = 'function private:Verb {}; 1 | ForEach-Object -Verbose { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + + $target = 'function private:Verb {}; 1 | ForEach-Object -Proc { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + $violations.Count | Should -Be 0 + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + } + + It "does not apply caller scope to ForEach-Object parallel callbacks" -skip:($PSVersionTable.PSVersion.Major -lt 7) { + $target = 'function private:Verb {}; 1 | ForEach-Object -Parallel { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "does not find functions outside ForEach-Object parallel callbacks" -skip:($PSVersionTable.PSVersion.Major -lt 7) { + $target = 'function Verb {}; 1 | ForEach-Object -Parallel { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "does not find functions outside isolated execution boundaries" -TestCases @( + @{ Boundary = 'Start-Job { Verb }' } + @{ Boundary = 'Invoke-Command -ComputerName localhost -ScriptBlock { Verb }' } + @{ Boundary = 'Invoke-Command -Session $session -ScriptBlock { Verb }' } + @{ Boundary = 'Invoke-Command localhost { Verb }' } + @{ Boundary = 'Invoke-Command -Verbose localhost -ScriptBlock { Verb }' } + @{ Boundary = 'Invoke-Command -Verbose:$true localhost -ScriptBlock { Verb }' } + @{ Boundary = 'Invoke-Command -ConnectionUri http://server:5985/wsman -ScriptBlock { Verb }' } + @{ Boundary = 'InlineScript { Verb }' } + @{ Boundary = 'Script Test { GetScript = { Verb } }' } + ) { + param($Boundary) + + $target = "function Verb {}; $Boundary" + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "respects visible script-defined commands before callback scope rules" -skip:($PSVersionTable.PSVersion.Major -lt 7) { + $target = 'function ForEach-Object { param([scriptblock]$Parallel) & $Parallel }; function Verb {}; ForEach-Object -Parallel { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + + $target = 'function ForEach-Object { param([scriptblock]$Process) & $Process }; function private:Verb {}; 1 | ForEach-Object -Process { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + Invoke-Formatter -ScriptDefinition $target -Settings $settings | + Should -Be ($target.Replace('{ Verb }', '{ Get-Verb }')) + } + + It "resolves dot-sourced callback commands before classifying nested callbacks" { + $target = '. { function ForEach-Object { param([scriptblock]$Process) & $Process } }; ForEach-Object { function private:Verb {}; Verb }; Verb' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | + Should -Be ($target.Substring(0, $target.LastIndexOf(' Verb')) + ' Get-Verb') + } + + It "does not let callback declarations change command resolution at invocation time" -TestCases @( + @{ + Target = 'function private:Verb {}; 1 | ForEach-Object { function ForEach-Object { param([scriptblock]$Process) & $Process } }; function Unrelated {}; 1 | ForEach-Object -Process { Verb }' + } + @{ + Target = 'function private:Verb {}; 1 | ForEach-Object { function ForEach-Object { param([scriptblock]$Process) & $Process } }; 1 | ForEach-Object -Process { Verb }' + } + ) { + param($Target) + + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $Target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "respects caller scope for script blocks inside array callback arguments" { + $target = 'function private:Verb {}; 1 | ForEach-Object -Process {}, { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + } + + It "respects caller scope for callbacks in array subexpressions" { + $target = 'function private:Verb {}; 1 | ForEach-Object -Process @({ Verb })' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + } + + It "does not index script blocks inside array input data" { + $target = 'ForEach-Object -InputObject { function Verb {} }, { function Other {} } -Process { $_ }; Verb' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "respects caller scope for trailing RemainingScripts callbacks" { + $target = 'function private:Verb {}; 1 | ForEach-Object {}, {}, { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + } + + It "resolves abbreviated and colon-attached parallel callback parameters" -skip:($PSVersionTable.PSVersion.Major -lt 7) { + foreach ($parameter in @('-Pa { Verb }', '-Pa:{ Verb }')) { + $target = "function Verb {}; 1 | ForEach-Object $parameter" + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + } + + It "recognizes functions declared in ForEach-Object parallel callbacks" -skip:($PSVersionTable.PSVersion.Major -lt 7) { + $target = '1 | ForEach-Object -Parallel { function Verb {}; Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + } + + It "does not index script blocks passed as ForEach-Object input data" { + $target = 'ForEach-Object -InputObject { function Verb {} } -Process { $_ }; Verb' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "does not index functions in constant-false branches" { + $target = 'if ($false) { function Verb {} }; Verb' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "does not classify Where-Object comparison values as executable callbacks" { + $target = 'Where-Object -Property Name -EQ { function Verb {} }; Verb' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "does not classify ForEach-Object MemberName arguments as executable callbacks" { + $target = '1 | ForEach-Object -MemberName Equals { function Verb {} }; Verb' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + + $target = '1 | ForEach-Object Equals { function Verb {} }; Verb' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "respects caller scope for parenthesized literal callbacks" { + $target = 'function private:Verb {}; 1 | ForEach-Object -Process ({ Verb })' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + } + + It "does not find functions outside module-qualified thread jobs" -skip:($null -eq (Get-Command Start-ThreadJob -ErrorAction SilentlyContinue)) { + $target = 'function Verb {}; ThreadJob\Start-ThreadJob -ScriptBlock { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "does not treat an explicitly disabled NoNewScope switch as caller-scoped" { + $target = 'function private:Verb {}; Invoke-Command -NoNewScope:$false -ScriptBlock { Verb }' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + $violations[0].SuggestedCorrections[0].Text | Should -Be 'Get-Verb' + } + + It "handles deeply nested caller-scoped callbacks without repeated visibility scans" { + $depth = 12 + $target = ('1 | ForEach-Object { ' * $depth) + 'Verb' + (' }' * $depth) + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 1 + $violations[0].Extent.Text | Should -Be 'Verb' + } + + It "indexes declarations in parameterized dot-sourced blocks in the caller scope" { + $target = '. { param($value) function private:Verb {}; Verb } 1; Verb' + $violations = @(Invoke-ScriptAnalyzer -ScriptDefinition $target -IncludeRule $violationName) + + $violations.Count | Should -Be 0 + $settings = @{ Rules = @{ PSAvoidUsingCmdletAliases = @{ Enable = $true } } } + Invoke-Formatter -ScriptDefinition $target -Settings $settings | Should -Be $target + } + It "should return no violation for assignment statement-like command in dsc configuration" -skip:($IsLinux -or $IsMacOS) { $target = @' Configuration MyDscConfiguration { @@ -97,6 +515,92 @@ Configuration PackageTest { } Context "DSC resource ASTs" { + It "recognizes functions declared in the scope as executable commands" -TestCases @( + @{ Scope = 'script' } + @{ Scope = 'configuration' } + ) { + param($Scope) + + $scriptFunction = if ($Scope -eq 'script') { 'function script:vErB {}' } + $configurationFunction = if ($Scope -eq 'configuration') { 'function vErB {}' } + $target = @" +$scriptFunction +{ + $configurationFunction + Verb SameLine { ChildItem . {} } + Verb FollowingLine + { + ChildItem . {} + } + Package PackageExample {} +} +"@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($target, [ref]$null, [ref]$null) + $body = $ast.EndBlock.Statements[-1].PipelineElements[0].Expression.Copy() + $configuration = [System.Management.Automation.Language.ConfigurationDefinitionAst]::new( + $ast.Extent, $body, 'Resource', + [System.Management.Automation.Language.StringConstantExpressionAst]::new($ast.Extent, 'Test', 'BareWord')) + [System.Management.Automation.Language.StatementAst[]]$statements = if ($Scope -eq 'script') { + @($ast.EndBlock.Statements[0].Copy(), $configuration) + } else { + @($configuration) + } + $scriptAst = [System.Management.Automation.Language.ScriptBlockAst]::new( + $ast.Extent, $null, + [System.Management.Automation.Language.StatementBlockAst]::new($ast.Extent, $statements, $null), $false) + $rule = [Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.AvoidAlias]::new() + + $violations = @($rule.AnalyzeScript($scriptAst, $null)) + + $violations.Count | Should -Be 2 + $violations | ForEach-Object { + $_.Extent.Text | Should -Be 'ChildItem' + $_.SuggestedCorrections[0].Text | Should -Be 'Get-ChildItem' + } + } + + It "does not treat functions declared in unrelated scopes as executable commands" -TestCases @( + @{ FunctionName = 'Verb' } + @{ FunctionName = 'script:Verb' } + @{ FunctionName = 'global:Verb' } + ) { + param($FunctionName) + + $target = @" +{ + function Get-Example { + function $FunctionName {} + } + Verb ResourceExample { ChildItem NestedResource {} } +} +"@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($target, [ref]$null, [ref]$null) + $body = $ast.EndBlock.Statements[0].PipelineElements[0].Expression.Copy() + $configuration = [System.Management.Automation.Language.ConfigurationDefinitionAst]::new( + $ast.Extent, $body, 'Resource', + [System.Management.Automation.Language.StringConstantExpressionAst]::new($ast.Extent, 'Test', 'BareWord')) + $rule = [Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.AvoidAlias]::new() + + @($rule.AnalyzeScript($configuration, $null)).Count | Should -Be 0 + } + + It "does not index functions in constant-false branches when identifying DSC resources" { + $target = @' +{ + if ($false) { function Verb {} } + Verb ResourceExample { ChildItem NestedResource {} } +} +'@ + $ast = [System.Management.Automation.Language.Parser]::ParseInput($target, [ref]$null, [ref]$null) + $body = $ast.EndBlock.Statements[0].PipelineElements[0].Expression.Copy() + $configuration = [System.Management.Automation.Language.ConfigurationDefinitionAst]::new( + $ast.Extent, $body, 'Resource', + [System.Management.Automation.Language.StringConstantExpressionAst]::new($ast.Extent, 'Test', 'BareWord')) + $rule = [Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.AvoidAlias]::new() + + @($rule.AnalyzeScript($configuration, $null)).Count | Should -Be 0 + } + It "does not expand nested resource names in a same-line Node declaration when node is discoverable" -skip:(!(Get-Command node -CommandType Application -ErrorAction Ignore)) { $target = @' { diff --git a/docs/Rules/AvoidUsingCmdletAliases.md b/docs/Rules/AvoidUsingCmdletAliases.md index 2204a2190..b73a7c1ee 100644 --- a/docs/Rules/AvoidUsingCmdletAliases.md +++ b/docs/Rules/AvoidUsingCmdletAliases.md @@ -20,6 +20,10 @@ name, doing so reduces code readability and maintainability. PowerShell also supports implicit aliases. When a cmdlet name isn't found, PowerShell appends `Get-` to the command as a fallback. For example, typing `verb` executes `Get-Verb`. +Functions declared in the script are not implicit aliases. In DSC configurations, resource +declarations are also excluded from implicit alias correction, while executable script blocks +passed to script-defined functions are still analyzed. + Aliases create inconsistency across codebases because different authors learn and use different aliases. Aliases can potentially make code difficult to read and understand in collaborative environments. It can also cause issues with script portability and availability.