Take Development to Master for v1.4.0 release

RuleDocumentation/UseToExportFieldsInManifest.md

@@ -0,0 +1,28 @@
+#UseToExportFieldsInManifest 
+**Severity Level: Warning**
+
+
+##Description
+
+In a module manifest, AliasesToExport, CmdletsToExport, FunctionsToExport and VariablesToExport fields should not use wildcards or $null in their entries. During module auto-discovery, if any of these entries are missing or $null or wildcard, PowerShell does some potentially expensive work to analyze the rest of the module.
+
+##How to Fix
+
+Please consider using an explicit list. 
+
+##Example 1
+
+Wrong： 
+ 	FunctionsToExport = $null
+
+Correct: 
+	FunctionToExport = @()
+
+##Example 2
+Suppose there are only two functions in your module, Get-Foo and Set-Foo that you want to export. Then,
+
+Wrong:	
+	FunctionsToExport = '*'
+
+Correct:
+	FunctionToExport = @(Get-Foo, Set-Foo)	

Rules/ScriptAnalyzerBuiltinRules.csproj

@@ -95,6 +95,7 @@
       <DependentUpon>Strings.resx</DependentUpon>
     </Compile>
     <Compile Include="UseBOMForUnicodeEncodedFile.cs" />
+    <Compile Include="UseToExportFieldsInManifest.cs" />
     <Compile Include="UseOutputTypeCorrectly.cs" />
     <Compile Include="MissingModuleManifestField.cs" />
     <Compile Include="PossibleIncorrectComparisonWithNull.cs" />

Rules/Strings.resx

@@ -798,4 +798,16 @@
   <data name="AvoidNullOrEmptyHelpMessageAttributeName" xml:space="preserve">
     <value>AvoidNullOrEmptyHelpMessageAttribute</value>
   </data>
+  <data name="UseToExportFieldsInManifestCommonName" xml:space="preserve">
+    <value>Use the *ToExport module manifest fields.</value>
+  </data>
+  <data name="UseToExportFieldsInManifestDescription" xml:space="preserve">
+    <value>In a module manifest, AliasesToExport, CmdletsToExport, FunctionsToExport and VariablesToExport fields should not use wildcards or $null in their entries. During module auto-discovery, if any of these entries are missing or $null or wildcard, PowerShell does some potentially expensive work to analyze the rest of the module.</value>
+  </data>
+  <data name="UseToExportFieldsInManifestError" xml:space="preserve">
+    <value>Do not use wildcard or $null in this field. Explicitly specify a list for {0}.  </value>
+  </data>
+  <data name="UseToExportFieldsInManifestName" xml:space="preserve">
+    <value>UseToExportFieldsInManifest</value>
+  </data>
 </root>

Rules/UseToExportFieldsInManifest.cs

@@ -0,0 +1,183 @@
+//
+// Copyright (c) Microsoft Corporation.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+//
+
+using System;
+using System.Collections.Generic;
+using System.Management.Automation.Language;
+using System.Management.Automation;
+using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
+using System.ComponentModel.Composition;
+using System.Globalization;
+using System.Text.RegularExpressions;
+
+namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
+{
+    /// <summary>
+    /// UseToExportFieldsInManifest: Checks if AliasToExport, CmdletsToExport, FunctionsToExport and VariablesToExport 
+    /// fields do not use wildcards and $null in their entries. 
+    /// </summary>
+    [Export(typeof(IScriptRule))]
+    public class UseToExportFieldsInManifest : IScriptRule
+    {
+        /// <summary>
+        /// AnalyzeScript: Analyzes the AST to check if AliasToExport, CmdletsToExport, FunctionsToExport 
+        /// and VariablesToExport fields do not use wildcards and $null in their entries. 
+        /// </summary>
+        /// <param name="ast">The script's ast</param>
+        /// <param name="fileName">The script's file name</param>
+        /// <returns>A List of diagnostic results of this rule</returns>
+        public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
+            if (ast == null)
+            {
+                throw new ArgumentNullException(Strings.NullAstErrorMessage);
+            }
+
+            if (fileName == null || !fileName.EndsWith(".psd1", StringComparison.OrdinalIgnoreCase))
+            {
+                yield break;
+            }
+
+            if (!IsValidManifest(ast, fileName))
+            {
+                yield break;
+            }
+
+            String[] manifestFields = {"FunctionsToExport", "CmdletsToExport", "VariablesToExport", "AliasesToExport"};
+            var hashtableAst = ast.Find(x => x is HashtableAst, false) as HashtableAst;
+
+            if (hashtableAst == null)
+            {                                
+                yield break;
+            }
+
+            foreach(String field in manifestFields)
+            {
+                IScriptExtent extent;
+                if (!HasAcceptableExportField(field, hashtableAst, ast.Extent.Text, out extent) && extent != null)
+                {
+                    yield return new DiagnosticRecord(GetError(field), extent, GetName(), DiagnosticSeverity.Warning, fileName);
+                }
+            }                               
+
+        }
+
+        /// <summary>
+        /// Checks if the manifest file is valid. 
+        /// </summary>
+        /// <param name="ast"></param>
+        /// <param name="fileName"></param>
+        /// <returns>A boolean value indicating the validity of the manifest file.</returns>
+        private bool IsValidManifest(Ast ast, string fileName)
+        {
+            var missingManifestRule = new MissingModuleManifestField();
+            return !missingManifestRule.AnalyzeScript(ast, fileName).GetEnumerator().MoveNext();
+
+        }
+
+        /// <summary>
+        /// Checks if the *ToExport fields are explicitly set to arrays, eg. @(...), and the array entries do not contain any wildcard.
+        /// </summary>
+        /// <param name="key"></param>
+        /// <param name="hast"></param>
+        /// <param name="scriptText"></param>
+        /// <param name="extent"></param>
+        /// <returns>A boolean value indicating if the the ToExport fields are explicitly set to arrays or not.</returns>
+        private bool HasAcceptableExportField(string key, HashtableAst hast, string scriptText, out IScriptExtent extent)
+        {
+            extent = null;
+            foreach (var pair in hast.KeyValuePairs)
+            {
+                if (key.Equals(pair.Item1.Extent.Text.Trim(), StringComparison.OrdinalIgnoreCase))
+                {
+                    // checks if the right hand side of the assignment is an array.
+                    var arrayAst = pair.Item2.Find(x => x is ArrayLiteralAst || x is ArrayExpressionAst, true);
+                    if (arrayAst == null)
+                    {         
+                        extent = pair.Item2.Extent;
+                        return false;
+                    }                        
+                    else
+                    {
+                        //checks if any entry within the array has a wildcard.
+                        var elementWithWildcard = arrayAst.Find(x => x is StringConstantExpressionAst 
+                                                                    && x.Extent.Text.Contains("*"), false);
+                        if (elementWithWildcard != null)
+                        {
+                            extent = elementWithWildcard.Extent;
+                            return false;
+                        }                            
+                        return true;
+                    }
+                }
+            }
+            return true;
+        }       
+
+        public string GetError(string field)
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.UseToExportFieldsInManifestError, field);
+        }
+
+        /// <summary>
+        /// GetName: Retrieves the name of this rule.
+        /// </summary>
+        /// <returns>The name of this rule</returns>
+        public string GetName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseToExportFieldsInManifestName);
+        }
+
+        /// <summary>
+        /// GetCommonName: Retrieves the common name of this rule.
+        /// </summary>
+        /// <returns>The common name of this rule</returns>
+        public string GetCommonName()
+        {
+            return String.Format(CultureInfo.CurrentCulture, Strings.UseToExportFieldsInManifestCommonName);
+        }
+
+        /// <summary>
+        /// GetDescription: Retrieves the description of this rule.
+        /// </summary>
+        /// <returns>The description of this rule</returns>
+        public string GetDescription()
+        {
+            return String.Format(CultureInfo.CurrentCulture, Strings.UseToExportFieldsInManifestDescription);
+        }
+
+        /// <summary>
+        /// Method: Retrieves the type of the rule: builtin, managed or module.
+        /// </summary>
+        public SourceType GetSourceType()
+        {
+            return SourceType.Builtin;
+        }
+
+        /// <summary>
+        /// GetSeverity: Retrieves the severity of the rule: error, warning of information.
+        /// </summary>
+        /// <returns></returns>
+        public RuleSeverity GetSeverity()
+        {
+            return RuleSeverity.Warning;
+        }
+
+        /// <summary>
+        /// Method: Retrieves the module/assembly name the rule is from.
+        /// </summary>
+        public string GetSourceName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
+        }
+    }
+}

Tests/Engine/GetScriptAnalyzerRule.tests.ps1

@@ -56,7 +56,7 @@ Describe "Test Name parameters" {
 
         It "Get Rules with no parameters supplied" {
 			$defaultRules = Get-ScriptAnalyzerRule
-			$defaultRules.Count | Should be 40
+			$defaultRules.Count | Should be 41
 		}
     }
 

Tests/Rules/UseToExportFieldsInManifest.tests.ps1

@@ -0,0 +1,91 @@
+Import-Module PSScriptAnalyzer
+$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
+$testManifestPath = Join-Path $directory "TestManifest"
+$testManifestBadFunctionsWildcardPath = "ManifestBadFunctionsWildcard.psd1"
+$testManifestBadFunctionsWildcardInArrayPath = "ManifestBadFunctionsWildcardInArray.psd1"
+$testManifestBadFunctionsNullPath = "ManifestBadFunctionsNull.psd1"
+$testManifestBadCmdletsWildcardPath = "ManifestBadCmdletsWildcard.psd1"
+$testManifestBadAliasesWildcardPath = "ManifestBadAliasesWildcard.psd1"
+$testManifestBadVariablesWildcardPath = "ManifestBadVariablesWildcard.psd1"
+$testManifestBadAllPath = "ManifestBadAll.psd1"
+$testManifestGoodPath = "ManifestGood.psd1"
+$testManifestInvalidPath = "ManifestInvalid.psd1"
+
+Function Run-PSScriptAnalyzerRule 
+{
+    Param(
+        [Parameter(Mandatory)]
+        [String] $ManifestPath
+    )
+
+    Invoke-ScriptAnalyzer -Path (Resolve-Path (Join-Path $testManifestPath $ManifestPath))`
+                            -IncludeRule PSUseToExportFieldsInManifest
+}
+
+Describe "UseManifestExportFields" {
+
+    Context "Invalid manifest file" {
+        It "does not process the manifest" {
+            $results = Run-PSScriptAnalyzerRule $testManifestInvalidPath
+            $results | Should BeNullOrEmpty            
+        }        
+    }
+
+    Context "Manifest contains violations" {
+
+        It "detects FunctionsToExport with wildcard" {
+            $results = Run-PSScriptAnalyzerRule $testManifestBadFunctionsWildcardPath
+            $results.Count | Should be 1
+            $results[0].Extent.Text | Should be "'*'"
+        }
+
+        It "detects FunctionsToExport with null" {
+            $results = Run-PSScriptAnalyzerRule $testManifestBadFunctionsNullPath
+            $results.Count | Should be 1
+            $results[0].Extent.Text | Should be '$null'
+        }
+
+        It "detects array element containing wildcard" {
+            $results = Run-PSScriptAnalyzerRule $testManifestBadFunctionsWildcardInArrayPath
+            $results.Count | Should be 3
+            $results.Where({$_.Message -match "FunctionsToExport"}).Extent.Text | Should be "'Get-*'"
+            $results.Where({$_.Message -match "CmdletsToExport"}).Extent.Text | Should be "'Update-*'"
+
+            # if more than two elements contain wildcard we can show only the first one as of now.
+            $results.Where({$_.Message -match "VariablesToExport"}).Extent.Text | Should be "'foo*'"
+        }
+
+
+        It "detects CmdletsToExport with wildcard" {
+            $results = Run-PSScriptAnalyzerRule $testManifestBadCmdletsWildcardPath
+            $results.Count | Should be 1
+            $results[0].Extent.Text | Should be "'*'"
+        }
+
+        It "detects AliasesToExport with wildcard" {
+            $results = Run-PSScriptAnalyzerRule $testManifestBadAliasesWildcardPath
+            $results.Count | Should be 1
+            $results[0].Extent.Text | Should be "'*'"
+        }
+
+        It "detects VariablesToExport with wildcard" {
+            $results = Run-PSScriptAnalyzerRule $testManifestBadVariablesWildcardPath
+            $results.Count | Should be 1
+            $results[0].Extent.Text | Should be "'*'"
+        }
+
+        It "detects all the *ToExport violations" {
+            $results = Run-PSScriptAnalyzerRule $testManifestBadAllPath
+            $results.Count | Should be 4
+        }
+    }
+
+    Context "Manifest contains no violations" {
+        It "detects all the *ToExport fields explicitly stating lists" {
+            $results = Run-PSScriptAnalyzerRule $testManifestGoodPath
+            $results.Count | Should be 0
+        }        
+    }
+}
+
+