Modify UseToExportFieldInManifest rule

* Doesn't check for VariablesToExport
* Adds ability to wrap around correction string

Author: Kapil Borle

Rules/UseToExportFieldsInManifest.cs

@@ -20,6 +20,7 @@
 using System.Globalization;
 using System.Text.RegularExpressions;
 using System.Diagnostics;
+using System.Text;
 
 namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
 {
@@ -33,7 +34,6 @@ public class UseToExportFieldsInManifest : IScriptRule
 
         private const string functionsToExport = "FunctionsToExport";
         private const string cmdletsToExport = "CmdletsToExport";
-        private const string variablesToExport = "VariablesToExport";
         private const string aliasesToExport = "AliasesToExport";
 
         /// <summary>
@@ -70,7 +70,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                 yield break;
             }
 
-            string[] manifestFields = { functionsToExport, cmdletsToExport, variablesToExport, aliasesToExport };
+            string[] manifestFields = { functionsToExport, cmdletsToExport, aliasesToExport };
 
             foreach(string field in manifestFields)
             {
@@ -85,19 +85,43 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                         fileName,
                         suggestedCorrections: GetCorrectionExtent(field, extent, psModuleInfo));
                 }
+                else
+                {
+
+                }
             }                               
 
         }
 
         private string GetListLiteral<T>(Dictionary<string, T> exportedItems)
         {
+            const int lineWidth = 64;
             if (exportedItems == null || exportedItems.Keys == null)
             {
                 return null;
             }
-            var sbuilder = new System.Text.StringBuilder();
+            var sbuilder = new StringBuilder();
             sbuilder.Append("@(");
-            sbuilder.Append(string.Join(", ", exportedItems.Keys.Select(key => string.Format("'{0}'", key))));
+            var sbuilderInner = new StringBuilder();
+            int charLadder = lineWidth;
+            int keyCount = exportedItems.Keys.Count;
+            foreach (var key in exportedItems.Keys)
+            {
+                sbuilderInner.Append("'");
+                sbuilderInner.Append(key);
+                sbuilderInner.Append("'");
+                if (--keyCount > 0)
+                {
+                    sbuilderInner.Append(", ");
+                    if (sbuilderInner.Length > charLadder)
+                    {
+                        charLadder += lineWidth;
+                        sbuilderInner.AppendLine();
+                        sbuilderInner.Append('\t', 2);
+                    }
+                }
+            }
+            sbuilder.Append(sbuilderInner);
             sbuilder.Append(")");
             return sbuilder.ToString();
         }
@@ -118,16 +142,16 @@ private List<CorrectionExtent> GetCorrectionExtent(string field, IScriptExtent e
                 case cmdletsToExport:
                     correctionText = GetListLiteral(psModuleInfo.ExportedCmdlets);
                     break;
-                case variablesToExport:
-                    correctionText = GetListLiteral(psModuleInfo.ExportedVariables);
-                    break;
                 case aliasesToExport:
                     correctionText = GetListLiteral(psModuleInfo.ExportedAliases);
                     break;
                 default:
                     throw new NotImplementedException(string.Format("{0} not implemented", field));
             }
-            string description = string.Format("Replace {0} with {1}", extent.Text, correctionText);
+            string description = string.Format(
+                Strings.UseToExportFieldsInManifestCorrectionDescription,
+                extent.Text,
+                correctionText);
             corrections.Add(new CorrectionExtent(
                 extent.StartLineNumber,
                 extent.EndLineNumber,

Tests/Rules/TestManifest/ManifestBadFunctionsNull.psm1

@@ -1,12 +1,50 @@
-Function Get-Foo
+Function Get-Foo1
 {
 
 }
 
-Function Get-Bar
+Function Get-Foo2
+{
+
+}
+Function Get-Foo3
+{
+
+}
+Function Get-Foo4
+{
+
+}
+Function Get-Foo5
+{
+
+}
+Function Get-Foo6
+{
+
+}
+Function Get-Foo7
+{
+
+}
+Function Get-Foo8
+{
+
+}
+Function Get-Foo9
+{
+
+}
+Function Get-Foo10
+{
+
+}
+Function Get-Foo11
+{
+
+}
+Function Get-Foo12
 {
 
 }
 
-Export-ModuleMember -Function Get-Foo
-Export-ModuleMember -Function Get-Bar

Tests/Rules/UseToExportFieldsInManifest.tests.ps1

@@ -53,20 +53,18 @@ Describe "UseManifestExportFields" {
             $results[0].Extent.Text | Should be '$null'
         }
 
-	It "suggests corrections for FunctionsToExport with null" {
+	It "suggests corrections for FunctionsToExport with null and line wrapping" {
 	    $violations = Run-PSScriptAnalyzerRule $testManifestBadFunctionsNullPath
 	    $violationFilepath = Join-path $testManifestPath $testManifestBadFunctionsNullPath
-	    Test-CorrectionExtent $violationFilepath $violations[0] 1  '$null' "@('Get-Foo', 'Get-Bar')"
+	    Test-CorrectionExtent $violationFilepath $violations[0] 1  '$null' "@('Get-Foo1', 'Get-Foo2', 'Get-Foo3', 'Get-Foo4', 'Get-Foo5', 'Get-Foo6', `r`n`t`t'Get-Foo7', 'Get-Foo8', 'Get-Foo9', 'Get-Foo10', 'Get-Foo11', `r`n`t`t'Get-Foo12')"
 	}
 
         It "detects array element containing wildcard" {
+	    # if more than two elements contain wildcard we can show only the first one as of now.
             $results = Run-PSScriptAnalyzerRule $testManifestBadFunctionsWildcardInArrayPath
-            $results.Count | Should be 3
+            $results.Count | Should be 2
             $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*'"
         }
 
 
@@ -88,15 +86,9 @@ Describe "UseManifestExportFields" {
 	    Test-CorrectionExtent $violationFilepath $violations[0] 1  "'*'" "@('gfoo', 'gbar')"
 	}
 
-        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
+            $results.Count | Should be 3
         }
     }