diff --git a/Engine/Helper.cs b/Engine/Helper.cs
index c39aa2ae8..70bd69f6b 100644
--- a/Engine/Helper.cs
+++ b/Engine/Helper.cs
@@ -241,10 +241,11 @@ public bool PositionalParameterUsed(CommandAst cmdAst)
/// Given a command's name, checks whether it exists
///
///
+ ///
///
- public CommandInfo GetCommandInfo(string name)
+ public CommandInfo GetCommandInfo(string name, CommandTypes commandType=CommandTypes.All)
{
- return Helper.Instance.MyCmdlet.InvokeCommand.GetCommand(name, CommandTypes.All);
+ return Helper.Instance.MyCmdlet.InvokeCommand.GetCommand(name, commandType);
}
///
@@ -259,6 +260,29 @@ public IEnumerable DscResourceFunctions(Ast ast)
&& resourceFunctionNames.Contains((item as FunctionDefinitionAst).Name, StringComparer.OrdinalIgnoreCase), true);
}
+ ///
+ /// Gets all the strings contained in an array literal ast
+ ///
+ ///
+ ///
+ public List GetStringsFromArrayLiteral(ArrayLiteralAst alAst)
+ {
+ List result = new List();
+
+ if (alAst != null && alAst.Elements != null)
+ {
+ foreach (ExpressionAst eAst in alAst.Elements)
+ {
+ if (eAst is StringConstantExpressionAst)
+ {
+ result.Add((eAst as StringConstantExpressionAst).Value);
+ }
+ }
+ }
+
+ return result;
+ }
+
///
/// Returns true if the block should be skipped as it has a name
/// that matches keyword
diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs
index 1e8eec481..b6f0e4c02 100644
--- a/Rules/ProvideCommentHelp.cs
+++ b/Rules/ProvideCommentHelp.cs
@@ -22,6 +22,7 @@
using System.Globalization;
using System.Threading;
using System.Reflection;
+using System.Management.Automation;
namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules
{
@@ -31,6 +32,8 @@ namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules
[Export(typeof(IScriptRule))]
public class ProvideCommentHelp : SkipTypeDefinition, IScriptRule
{
+ private HashSet exportedFunctions;
+
///
/// AnalyzeScript: Analyzes the ast to check that cmdlets have help.
///
@@ -42,6 +45,131 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) {
DiagnosticRecords.Clear();
this.fileName = fileName;
+ exportedFunctions = new HashSet(StringComparer.OrdinalIgnoreCase);
+ List exportFunctionsCmdlet = Helper.Instance.CmdletNameAndAliases("export-modulemember");
+
+ // find functions exported
+ IEnumerable cmdAsts = ast.FindAll(item => item is CommandAst
+ && exportFunctionsCmdlet.Contains((item as CommandAst).GetCommandName(), StringComparer.OrdinalIgnoreCase), true);
+
+ CommandInfo exportMM = Helper.Instance.GetCommandInfo("export-modulemember", CommandTypes.Cmdlet);
+
+ // switch parameters
+ IEnumerable switchParams = (exportMM != null) ? exportMM.Parameters.Values.Where(pm => pm.SwitchParameter) : Enumerable.Empty();
+
+ if (exportMM == null)
+ {
+ return DiagnosticRecords;
+ }
+
+ foreach (CommandAst cmdAst in cmdAsts)
+ {
+ if (cmdAst.CommandElements == null || cmdAst.CommandElements.Count < 2)
+ {
+ continue;
+ }
+
+ int i = 1;
+
+ while (i < cmdAst.CommandElements.Count)
+ {
+ CommandElementAst ceAst = cmdAst.CommandElements[i];
+ ExpressionAst exprAst = null;
+
+ if (ceAst is CommandParameterAst)
+ {
+ var paramAst = ceAst as CommandParameterAst;
+ var param = exportMM.ResolveParameter(paramAst.ParameterName);
+
+ if (param == null)
+ {
+ i += 1;
+ continue;
+ }
+
+ if (string.Equals(param.Name, "function", StringComparison.OrdinalIgnoreCase))
+ {
+ // checks for the case of -Function:"verb-nouns"
+ if (paramAst.Argument != null)
+ {
+ exprAst = paramAst.Argument;
+ }
+ // checks for the case of -Function "verb-nouns"
+ else if (i < cmdAst.CommandElements.Count - 1)
+ {
+ i += 1;
+ exprAst = cmdAst.CommandElements[i] as ExpressionAst;
+ }
+ }
+ // some other parameter. we just checks whether the one after this is positional
+ else if (i < cmdAst.CommandElements.Count - 1)
+ {
+ // the next element is a parameter like -module so just move to that one
+ if (cmdAst.CommandElements[i + 1] is CommandParameterAst)
+ {
+ i += 1;
+ continue;
+ }
+
+ // not a switch parameter so the next element is definitely the argument to this parameter
+ if (paramAst.Argument == null && !switchParams.Contains(param))
+ {
+ // skips the next element
+ i += 1;
+ }
+
+ i += 1;
+ continue;
+ }
+ }
+ else if (ceAst is ExpressionAst)
+ {
+ exprAst = ceAst as ExpressionAst;
+ }
+
+ if (exprAst != null)
+ {
+ // One string so just add this to the list
+ if (exprAst is StringConstantExpressionAst)
+ {
+ exportedFunctions.Add((exprAst as StringConstantExpressionAst).Value);
+ }
+ // Array of the form "v-n", "v-n1"
+ else if (exprAst is ArrayLiteralAst)
+ {
+ exportedFunctions.UnionWith(Helper.Instance.GetStringsFromArrayLiteral(exprAst as ArrayLiteralAst));
+ }
+ // Array of the form @("v-n", "v-n1")
+ else if (exprAst is ArrayExpressionAst)
+ {
+ ArrayExpressionAst arrExAst = exprAst as ArrayExpressionAst;
+ if (arrExAst.SubExpression != null && arrExAst.SubExpression.Statements != null)
+ {
+ foreach (StatementAst stAst in arrExAst.SubExpression.Statements)
+ {
+ if (stAst is PipelineAst)
+ {
+ PipelineAst pipeAst = stAst as PipelineAst;
+ if (pipeAst.PipelineElements != null)
+ {
+ foreach (CommandBaseAst cmdBaseAst in pipeAst.PipelineElements)
+ {
+ if (cmdBaseAst is CommandExpressionAst)
+ {
+ exportedFunctions.UnionWith(Helper.Instance.GetStringsFromArrayLiteral((cmdBaseAst as CommandExpressionAst).Expression as ArrayLiteralAst));
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
+ i += 1;
+ }
+ }
+
ast.Visit(this);
return DiagnosticRecords;
@@ -59,10 +187,8 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun
return AstVisitAction.SkipChildren;
}
- if (!string.Equals(funcAst.Name, "Get-TargetResource", StringComparison.OrdinalIgnoreCase) && !string.Equals(funcAst.Name, "Set-TargetResource", StringComparison.OrdinalIgnoreCase) &&
- !string.Equals(funcAst.Name, "Test-TargetResource", StringComparison.OrdinalIgnoreCase))
+ if (exportedFunctions.Contains(funcAst.Name))
{
-
if (funcAst.GetHelpContent() == null)
{
DiagnosticRecords.Add(
diff --git a/Tests/Rules/BadCmdlet.ps1 b/Tests/Rules/BadCmdlet.ps1
index b93658d0b..546dc31a8 100644
--- a/Tests/Rules/BadCmdlet.ps1
+++ b/Tests/Rules/BadCmdlet.ps1
@@ -50,4 +50,17 @@
End
{
}
-}
\ No newline at end of file
+}
+
+# Provide comment help should not be raised here because this is not exported
+function NoComment
+{
+ Write-Verbose "No Comment"
+}
+
+function Comment
+{
+ Write-Verbose "Should raise providecommenthelp error"
+}
+
+Export-ModuleMember Verb-Files, Comment
\ No newline at end of file
diff --git a/Tests/Rules/ProvideCommentHelp.tests.ps1 b/Tests/Rules/ProvideCommentHelp.tests.ps1
index 6ea13d0ca..30a183710 100644
--- a/Tests/Rules/ProvideCommentHelp.tests.ps1
+++ b/Tests/Rules/ProvideCommentHelp.tests.ps1
@@ -8,8 +8,8 @@ $noViolations = Invoke-ScriptAnalyzer $directory\GoodCmdlet.ps1 | Where-Object {
Describe "ProvideCommentHelp" {
Context "When there are violations" {
- It "has 1 provide comment help violation" {
- $violations.Count | Should Be 1
+ It "has 2 provide comment help violations" {
+ $violations.Count | Should Be 2
}
It "has the correct description message" {