Updated AvoidGlobalFunctions rule to only warn on global function when in a module

Author: David Reynolds

Rules/AvoidGlobalFunctions.cs

@@ -1,12 +1,9 @@
 using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
 using System;
 using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
-using System.Management.Automation.Language;
-using System.Globalization;
 using System.ComponentModel.Composition;
+using System.Globalization;
+using System.Management.Automation.Language;
 
 namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
 {
@@ -41,7 +38,7 @@ public override AstVisitAction VisitCommand(CommandAst commandAst)
 
         public override AstVisitAction VisitCommandParameter(CommandParameterAst commandParameterAst)
         {
-            if(isScopeParameterForNewAliasCmdlet(commandParameterAst))
+            if(IsScopeParameterForNewAliasCmdlet(commandParameterAst))
             {
                 if ((commandParameterAst.Argument != null)   // if the cmdlet looks like -Scope:Global check Parameter.Argument
                     && (commandParameterAst.Argument.ToString().Equals("Global", StringComparison.OrdinalIgnoreCase)))
@@ -77,7 +74,7 @@ public override AstVisitAction VisitCommandParameter(CommandParameterAst command
 
         public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst functionDefinitionAst)
         {
-            if (functionDefinitionAst.Name.StartsWith("Global:", StringComparison.OrdinalIgnoreCase))
+            if (functionDefinitionAst.Name.StartsWith("Global:", StringComparison.OrdinalIgnoreCase) && IsModule())
             {
                 records.Add(new DiagnosticRecord(
                                 string.Format(CultureInfo.CurrentCulture, Strings.AvoidGlobalFunctionsError),
@@ -92,6 +89,11 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun
         }
         #endregion
 
+        private bool IsModule()
+        {
+            return fileName.EndsWith(".psm1");
+        }
+
         private Ast FindNextAst(Ast ast)
         {
             IEnumerable<Ast> matchingLevelAsts = ast.Parent.FindAll(item => item is Ast, true );
@@ -101,14 +103,14 @@ private Ast FindNextAst(Ast ast)
             {
                 if (currentClosest == null)
                 {
-                    if (isAstAfter(ast, matchingLevelAst)) 
+                    if (IsAstAfter(ast, matchingLevelAst)) 
                     {
                         currentClosest = matchingLevelAst;
                     }
                 }
                 else
                 {
-                    if((isAstAfter(ast, matchingLevelAst)) && (isAstAfter(matchingLevelAst, currentClosest)))
+                    if((IsAstAfter(ast, matchingLevelAst)) && (IsAstAfter(matchingLevelAst, currentClosest)))
                     {
                         currentClosest = matchingLevelAst;
                     }
@@ -118,7 +120,7 @@ private Ast FindNextAst(Ast ast)
             return currentClosest;
         }
 
-        private bool isAstAfter(Ast ast1, Ast ast2)
+        private bool IsAstAfter(Ast ast1, Ast ast2)
         {
             if(ast1.Extent.EndLineNumber > ast2.Extent.StartLineNumber)  // ast1 ends on a line after ast2 starts
             {
@@ -141,7 +143,7 @@ private bool isAstAfter(Ast ast1, Ast ast2)
             }
         }
 
-        private bool isScopeParameterForNewAliasCmdlet(CommandParameterAst commandParameterAst)
+        private bool IsScopeParameterForNewAliasCmdlet(CommandParameterAst commandParameterAst)
         {
             if (commandParameterAst == null || commandParameterAst.ParameterName == null)
                 return false;

Tests/Rules/AvoidGlobalFunctions.ps1 renamed to Tests/Rules/AvoidGlobalFunctions.psm1

@@ -5,17 +5,17 @@ $aliasErrorMessage = "Avoid creating aliases with a Global scope."
 $violationName = "AvoidGlobalFunctions"
 
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
-$violations = Invoke-ScriptAnalyzer $directory\AvoidGlobalFunctions.ps1 | Where-Object {$_.RuleName -eq $violationName}
+$violations = Invoke-ScriptAnalyzer $directory\AvoidGlobalFunctions.psm1 | Where-Object {$_.RuleName -eq $violationName}
 $noViolations = Invoke-ScriptAnalyzer $directory\AvoidGlobalFunctionsNoViolations.ps1 | Where-Object {$_.RuleName -eq $violationName}
 
 
 Describe "$violationName " {
     Context "When there are violations" {
-        It "has 5 avoid using empty Catch block violations" {
+        It "Has 5 avoid using empty Catch block violations" {
             $violations.Count | Should Be 5
         }
 
-        It "has the correct description message" {
+        It "Has the correct description message" {
             $violations[0].Message | Should Match $functionErroMessage
             $violations[1].Message | Should Match $aliasErrorMessage
             $violations[2].Message | Should Match $aliasErrorMessage
@@ -26,7 +26,7 @@ Describe "$violationName " {
     }
 
     Context "When there are no violations" {
-        It "returns no violations" {
+        It "Returns no violations" {
             $noViolations.Count | Should Be 0
         }
     }

Tests/Rules/AvoidGlobalFunctions.tests.ps1

@@ -1,5 +1,5 @@
 
-function functionName {} 
+function global:functionName {} 
 {
 	New-Alias -Name Name -Scope `
 	Script -Value Value