Skip to content

Commit 2df55e0

Browse files
committed
Merge pull request #452 from PowerShell/development
Take Development to Master for v1.4.0 release
2 parents f0f5243 + 38b9e99 commit 2df55e0

17 files changed

+352
-1
lines changed

RuleDocumentation/UseManifestExportFields.md

Whitespace-only changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
#UseToExportFieldsInManifest
2+
**Severity Level: Warning**
3+
4+
5+
##Description
6+
7+
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.
8+
9+
##How to Fix
10+
11+
Please consider using an explicit list.
12+
13+
##Example 1
14+
15+
Wrong:
16+
FunctionsToExport = $null
17+
18+
Correct:
19+
FunctionToExport = @()
20+
21+
##Example 2
22+
Suppose there are only two functions in your module, Get-Foo and Set-Foo that you want to export. Then,
23+
24+
Wrong:
25+
FunctionsToExport = '*'
26+
27+
Correct:
28+
FunctionToExport = @(Get-Foo, Set-Foo)

Rules/ScriptAnalyzerBuiltinRules.csproj

+1
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@
9595
<DependentUpon>Strings.resx</DependentUpon>
9696
</Compile>
9797
<Compile Include="UseBOMForUnicodeEncodedFile.cs" />
98+
<Compile Include="UseToExportFieldsInManifest.cs" />
9899
<Compile Include="UseOutputTypeCorrectly.cs" />
99100
<Compile Include="MissingModuleManifestField.cs" />
100101
<Compile Include="PossibleIncorrectComparisonWithNull.cs" />

Rules/Strings.Designer.cs

+36
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Rules/Strings.resx

+12
Original file line numberDiff line numberDiff line change
@@ -798,4 +798,16 @@
798798
<data name="AvoidNullOrEmptyHelpMessageAttributeName" xml:space="preserve">
799799
<value>AvoidNullOrEmptyHelpMessageAttribute</value>
800800
</data>
801+
<data name="UseToExportFieldsInManifestCommonName" xml:space="preserve">
802+
<value>Use the *ToExport module manifest fields.</value>
803+
</data>
804+
<data name="UseToExportFieldsInManifestDescription" xml:space="preserve">
805+
<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>
806+
</data>
807+
<data name="UseToExportFieldsInManifestError" xml:space="preserve">
808+
<value>Do not use wildcard or $null in this field. Explicitly specify a list for {0}. </value>
809+
</data>
810+
<data name="UseToExportFieldsInManifestName" xml:space="preserve">
811+
<value>UseToExportFieldsInManifest</value>
812+
</data>
801813
</root>

Rules/UseToExportFieldsInManifest.cs

+183
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
//
2+
// Copyright (c) Microsoft Corporation.
3+
//
4+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
5+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
6+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
7+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
8+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
9+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
10+
// THE SOFTWARE.
11+
//
12+
13+
using System;
14+
using System.Collections.Generic;
15+
using System.Management.Automation.Language;
16+
using System.Management.Automation;
17+
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
18+
using System.ComponentModel.Composition;
19+
using System.Globalization;
20+
using System.Text.RegularExpressions;
21+
22+
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
23+
{
24+
/// <summary>
25+
/// UseToExportFieldsInManifest: Checks if AliasToExport, CmdletsToExport, FunctionsToExport and VariablesToExport
26+
/// fields do not use wildcards and $null in their entries.
27+
/// </summary>
28+
[Export(typeof(IScriptRule))]
29+
public class UseToExportFieldsInManifest : IScriptRule
30+
{
31+
/// <summary>
32+
/// AnalyzeScript: Analyzes the AST to check if AliasToExport, CmdletsToExport, FunctionsToExport
33+
/// and VariablesToExport fields do not use wildcards and $null in their entries.
34+
/// </summary>
35+
/// <param name="ast">The script's ast</param>
36+
/// <param name="fileName">The script's file name</param>
37+
/// <returns>A List of diagnostic results of this rule</returns>
38+
public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
39+
{
40+
if (ast == null)
41+
{
42+
throw new ArgumentNullException(Strings.NullAstErrorMessage);
43+
}
44+
45+
if (fileName == null || !fileName.EndsWith(".psd1", StringComparison.OrdinalIgnoreCase))
46+
{
47+
yield break;
48+
}
49+
50+
if (!IsValidManifest(ast, fileName))
51+
{
52+
yield break;
53+
}
54+
55+
String[] manifestFields = {"FunctionsToExport", "CmdletsToExport", "VariablesToExport", "AliasesToExport"};
56+
var hashtableAst = ast.Find(x => x is HashtableAst, false) as HashtableAst;
57+
58+
if (hashtableAst == null)
59+
{
60+
yield break;
61+
}
62+
63+
foreach(String field in manifestFields)
64+
{
65+
IScriptExtent extent;
66+
if (!HasAcceptableExportField(field, hashtableAst, ast.Extent.Text, out extent) && extent != null)
67+
{
68+
yield return new DiagnosticRecord(GetError(field), extent, GetName(), DiagnosticSeverity.Warning, fileName);
69+
}
70+
}
71+
72+
}
73+
74+
/// <summary>
75+
/// Checks if the manifest file is valid.
76+
/// </summary>
77+
/// <param name="ast"></param>
78+
/// <param name="fileName"></param>
79+
/// <returns>A boolean value indicating the validity of the manifest file.</returns>
80+
private bool IsValidManifest(Ast ast, string fileName)
81+
{
82+
var missingManifestRule = new MissingModuleManifestField();
83+
return !missingManifestRule.AnalyzeScript(ast, fileName).GetEnumerator().MoveNext();
84+
85+
}
86+
87+
/// <summary>
88+
/// Checks if the *ToExport fields are explicitly set to arrays, eg. @(...), and the array entries do not contain any wildcard.
89+
/// </summary>
90+
/// <param name="key"></param>
91+
/// <param name="hast"></param>
92+
/// <param name="scriptText"></param>
93+
/// <param name="extent"></param>
94+
/// <returns>A boolean value indicating if the the ToExport fields are explicitly set to arrays or not.</returns>
95+
private bool HasAcceptableExportField(string key, HashtableAst hast, string scriptText, out IScriptExtent extent)
96+
{
97+
extent = null;
98+
foreach (var pair in hast.KeyValuePairs)
99+
{
100+
if (key.Equals(pair.Item1.Extent.Text.Trim(), StringComparison.OrdinalIgnoreCase))
101+
{
102+
// checks if the right hand side of the assignment is an array.
103+
var arrayAst = pair.Item2.Find(x => x is ArrayLiteralAst || x is ArrayExpressionAst, true);
104+
if (arrayAst == null)
105+
{
106+
extent = pair.Item2.Extent;
107+
return false;
108+
}
109+
else
110+
{
111+
//checks if any entry within the array has a wildcard.
112+
var elementWithWildcard = arrayAst.Find(x => x is StringConstantExpressionAst
113+
&& x.Extent.Text.Contains("*"), false);
114+
if (elementWithWildcard != null)
115+
{
116+
extent = elementWithWildcard.Extent;
117+
return false;
118+
}
119+
return true;
120+
}
121+
}
122+
}
123+
return true;
124+
}
125+
126+
public string GetError(string field)
127+
{
128+
return string.Format(CultureInfo.CurrentCulture, Strings.UseToExportFieldsInManifestError, field);
129+
}
130+
131+
/// <summary>
132+
/// GetName: Retrieves the name of this rule.
133+
/// </summary>
134+
/// <returns>The name of this rule</returns>
135+
public string GetName()
136+
{
137+
return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseToExportFieldsInManifestName);
138+
}
139+
140+
/// <summary>
141+
/// GetCommonName: Retrieves the common name of this rule.
142+
/// </summary>
143+
/// <returns>The common name of this rule</returns>
144+
public string GetCommonName()
145+
{
146+
return String.Format(CultureInfo.CurrentCulture, Strings.UseToExportFieldsInManifestCommonName);
147+
}
148+
149+
/// <summary>
150+
/// GetDescription: Retrieves the description of this rule.
151+
/// </summary>
152+
/// <returns>The description of this rule</returns>
153+
public string GetDescription()
154+
{
155+
return String.Format(CultureInfo.CurrentCulture, Strings.UseToExportFieldsInManifestDescription);
156+
}
157+
158+
/// <summary>
159+
/// Method: Retrieves the type of the rule: builtin, managed or module.
160+
/// </summary>
161+
public SourceType GetSourceType()
162+
{
163+
return SourceType.Builtin;
164+
}
165+
166+
/// <summary>
167+
/// GetSeverity: Retrieves the severity of the rule: error, warning of information.
168+
/// </summary>
169+
/// <returns></returns>
170+
public RuleSeverity GetSeverity()
171+
{
172+
return RuleSeverity.Warning;
173+
}
174+
175+
/// <summary>
176+
/// Method: Retrieves the module/assembly name the rule is from.
177+
/// </summary>
178+
public string GetSourceName()
179+
{
180+
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
181+
}
182+
}
183+
}

Tests/Engine/GetScriptAnalyzerRule.tests.ps1

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ Describe "Test Name parameters" {
5656

5757
It "Get Rules with no parameters supplied" {
5858
$defaultRules = Get-ScriptAnalyzerRule
59-
$defaultRules.Count | Should be 40
59+
$defaultRules.Count | Should be 41
6060
}
6161
}
6262

Binary file not shown.
6.41 KB
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
6.41 KB
Binary file not shown.
6.42 KB
Binary file not shown.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
Import-Module PSScriptAnalyzer
2+
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
3+
$testManifestPath = Join-Path $directory "TestManifest"
4+
$testManifestBadFunctionsWildcardPath = "ManifestBadFunctionsWildcard.psd1"
5+
$testManifestBadFunctionsWildcardInArrayPath = "ManifestBadFunctionsWildcardInArray.psd1"
6+
$testManifestBadFunctionsNullPath = "ManifestBadFunctionsNull.psd1"
7+
$testManifestBadCmdletsWildcardPath = "ManifestBadCmdletsWildcard.psd1"
8+
$testManifestBadAliasesWildcardPath = "ManifestBadAliasesWildcard.psd1"
9+
$testManifestBadVariablesWildcardPath = "ManifestBadVariablesWildcard.psd1"
10+
$testManifestBadAllPath = "ManifestBadAll.psd1"
11+
$testManifestGoodPath = "ManifestGood.psd1"
12+
$testManifestInvalidPath = "ManifestInvalid.psd1"
13+
14+
Function Run-PSScriptAnalyzerRule
15+
{
16+
Param(
17+
[Parameter(Mandatory)]
18+
[String] $ManifestPath
19+
)
20+
21+
Invoke-ScriptAnalyzer -Path (Resolve-Path (Join-Path $testManifestPath $ManifestPath))`
22+
-IncludeRule PSUseToExportFieldsInManifest
23+
}
24+
25+
Describe "UseManifestExportFields" {
26+
27+
Context "Invalid manifest file" {
28+
It "does not process the manifest" {
29+
$results = Run-PSScriptAnalyzerRule $testManifestInvalidPath
30+
$results | Should BeNullOrEmpty
31+
}
32+
}
33+
34+
Context "Manifest contains violations" {
35+
36+
It "detects FunctionsToExport with wildcard" {
37+
$results = Run-PSScriptAnalyzerRule $testManifestBadFunctionsWildcardPath
38+
$results.Count | Should be 1
39+
$results[0].Extent.Text | Should be "'*'"
40+
}
41+
42+
It "detects FunctionsToExport with null" {
43+
$results = Run-PSScriptAnalyzerRule $testManifestBadFunctionsNullPath
44+
$results.Count | Should be 1
45+
$results[0].Extent.Text | Should be '$null'
46+
}
47+
48+
It "detects array element containing wildcard" {
49+
$results = Run-PSScriptAnalyzerRule $testManifestBadFunctionsWildcardInArrayPath
50+
$results.Count | Should be 3
51+
$results.Where({$_.Message -match "FunctionsToExport"}).Extent.Text | Should be "'Get-*'"
52+
$results.Where({$_.Message -match "CmdletsToExport"}).Extent.Text | Should be "'Update-*'"
53+
54+
# if more than two elements contain wildcard we can show only the first one as of now.
55+
$results.Where({$_.Message -match "VariablesToExport"}).Extent.Text | Should be "'foo*'"
56+
}
57+
58+
59+
It "detects CmdletsToExport with wildcard" {
60+
$results = Run-PSScriptAnalyzerRule $testManifestBadCmdletsWildcardPath
61+
$results.Count | Should be 1
62+
$results[0].Extent.Text | Should be "'*'"
63+
}
64+
65+
It "detects AliasesToExport with wildcard" {
66+
$results = Run-PSScriptAnalyzerRule $testManifestBadAliasesWildcardPath
67+
$results.Count | Should be 1
68+
$results[0].Extent.Text | Should be "'*'"
69+
}
70+
71+
It "detects VariablesToExport with wildcard" {
72+
$results = Run-PSScriptAnalyzerRule $testManifestBadVariablesWildcardPath
73+
$results.Count | Should be 1
74+
$results[0].Extent.Text | Should be "'*'"
75+
}
76+
77+
It "detects all the *ToExport violations" {
78+
$results = Run-PSScriptAnalyzerRule $testManifestBadAllPath
79+
$results.Count | Should be 4
80+
}
81+
}
82+
83+
Context "Manifest contains no violations" {
84+
It "detects all the *ToExport fields explicitly stating lists" {
85+
$results = Run-PSScriptAnalyzerRule $testManifestGoodPath
86+
$results.Count | Should be 0
87+
}
88+
}
89+
}
90+
91+

0 commit comments

Comments
 (0)