UseShouldProcessCorrectly: scope local function shadowing to the call site

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: jessehouwing

Rules/UseShouldProcessCorrectly.cs

@@ -292,14 +292,24 @@ private bool DeclaresSupportsShouldProcess(FunctionDefinitionAst ast)
         /// Checks if the given command supports ShouldProcess
         /// </summary>
         /// <returns>False if input is null. If the input command has declares SupportsShouldProcess attribute, returns true</returns>
-        private bool SupportsShouldProcess(string cmdName)
+        private bool SupportsShouldProcess(string cmdName, IEnumerable<Ast> callSites)
         {
             if (String.IsNullOrWhiteSpace(cmdName))
             {
                 return false;
             }
 
-            CommandInfo cmdInfo = Helper.Instance.GetCommandInfo(cmdName);
+            // Credit the name if any invocation of it resolves to a real command rather than a function
+            // that is only in scope elsewhere.
+            CommandInfo cmdInfo = null;
+            foreach (Ast callSite in callSites)
+            {
+                cmdInfo = Helper.Instance.GetCommandInfo(cmdName, callSite: callSite);
+                if (cmdInfo != null)
+                {
+                    break;
+                }
+            }
             if (cmdInfo == null)
             {
                 return false;
@@ -410,7 +420,7 @@ private void CheckForSupportShouldProcess()
                 if (funcDigraph.GetOutDegree(v) == 0)
                 {
                     // Member vertices carry a method name, which PowerShell never resolves as a command.
-                    if (v.IsCommand && SupportsShouldProcess(v.Name))
+                    if (v.IsCommand && SupportsShouldProcess(v.Name, v.CallSites))
                     {
                         commandsWithSupportShouldProcess.Add(v);
                     }
@@ -451,6 +461,9 @@ public Ast Ast
         /// <summary>True when the vertex was reached through an actual command invocation.</summary>
         public bool IsCommand { get; set; }
 
+        /// <summary>Every command invocation merged into this name-keyed vertex.</summary>
+        public List<Ast> CallSites { get; } = new List<Ast>();
+
         private string name;
         private Ast ast;
         private bool isNestedFunctionDefinition;
@@ -572,6 +585,7 @@ public void AddVertex(Vertex vertex)
                     // Vertices are keyed on name alone, so a name used both as a command and as a
                     // member must stay marked as a command however the two are ordered.
                     v.IsCommand |= vertex.IsCommand;
+                    v.CallSites.AddRange(vertex.CallSites);
                     if (vertex.Ast != null
                         && vertex.Ast is FunctionDefinitionAst)
                     {
@@ -635,6 +649,7 @@ public override AstVisitAction VisitCommand(CommandAst ast)
             }
 
             var vertex = new Vertex (cmdName, ast) { IsCommand = true };
+            vertex.CallSites.Add(ast);
             AddVertex(vertex);
             if (IsWithinFunctionDefinition())
             {

Tests/Rules/UseShouldProcessCorrectly.tests.ps1

@@ -280,6 +280,37 @@ function Foo
         }
     }
 
+    Context "Nested function definitions only shadow commands within their own scope" {
+        It "credits a call to the real command when a nested function elsewhere shares its name" {
+            $scriptDef = @'
+function Outer
+{
+    function Remove-Item { param($path) }
+}
+function Invoke-Thing
+{
+    [CmdletBinding(SupportsShouldProcess)]
+    param($path)
+    Remove-Item $path
+}
+'@
+            Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule PSShouldProcess | Should -BeNullOrEmpty
+        }
+
+        It "does not credit a call that resolves to a nested function inside its own scope" {
+            $scriptDef = @'
+function Invoke-Thing
+{
+    [CmdletBinding(SupportsShouldProcess)]
+    param($path)
+    function Remove-Item { param($path) }
+    Remove-Item $path
+}
+'@
+            @(Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule PSShouldProcess).Count | Should -Be 1
+        }
+    }
+
     Context "Method calls are not command calls" {
         # A quoted member name may contain a hyphen, so Remove-Item serves as a member whose name also
         # belongs to a cmdlet that declares SupportsShouldProcess on every platform.