From e8bfe7183bc6a231a6b481efc796edbeb74ce80b Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 8 Feb 2016 20:01:27 -0800 Subject: [PATCH 1/7] Adds a new rule to check if HelpMessage parameter attribute is non-null and non-empty. --- .../AvoidNullOrEmptyHelpMessageAttribute.md | Bin 0 -> 1254 bytes Rules/AvoidNullOrEmptyHelpMessageAttribute.cs | 148 ++++++++++++++++++ Rules/ScriptAnalyzerBuiltinRules.csproj | 1 + Rules/Strings.Designer.cs | 36 +++++ Rules/Strings.resx | 12 ++ .../AvoidNullOrEmptyHelpMessageAttribute.ps1 | 70 +++++++++ ...dNullOrEmptyHelpMessageAttribute.tests.ps1 | 24 +++ ...rEmptyHelpMessageAttributeNoViolations.ps1 | 36 +++++ 8 files changed, 327 insertions(+) create mode 100644 RuleDocumentation/AvoidNullOrEmptyHelpMessageAttribute.md create mode 100644 Rules/AvoidNullOrEmptyHelpMessageAttribute.cs create mode 100644 Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.ps1 create mode 100644 Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.tests.ps1 create mode 100644 Tests/Rules/AvoidNullOrEmptyHelpMessageAttributeNoViolations.ps1 diff --git a/RuleDocumentation/AvoidNullOrEmptyHelpMessageAttribute.md b/RuleDocumentation/AvoidNullOrEmptyHelpMessageAttribute.md new file mode 100644 index 0000000000000000000000000000000000000000..fa23285c3bf91e6ca7bd84ad24c72875c737196d GIT binary patch literal 1254 zcmc(e%}&Bl5QWbc6JNm9jkr*wJ^+aeRFs8(2_!BIiwc5GDm0~_#`x;uH@Dnc|KiG+ zo6_mboSAda46h5-)zn-Q#Tw{bGbNl0WoqeOxl%o;ty_Gl&UjBbkw#ceqB(i7Zb0RD zR<)vSP6vC=W3e2()9KQasKHl@NJhuZXB#R!b=Vw{J5}GKQ}8jF<1)i)Ti+Pph+4Vs z@VFm1I=FsjX_yI!hzbtuJvS>@PI(*WqO$RG*o7jF8%3xv^WFN`Ggul^r%UQRU}xOi z*qDOO*D+Nx>Sw$QJ9IbRrojW5fk{xs;k^T&fHH=UJbkZm;1+J(r-Gr3M~XdWh9PGN zVhk=QxL0auDz}%t3oVX8J(lq$|!1)O20(hAD6Uu#fv%=WMB0L3dCkY+Vm^HY)Y3ca8jTzuK4ROBu7A lLo_V*!RFp5W8tpv(>oz^@{cNX%e^VKr}*Eb+WQNA-T)Sd%g+D+ literal 0 HcmV?d00001 diff --git a/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs b/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs new file mode 100644 index 000000000..39a85e484 --- /dev/null +++ b/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs @@ -0,0 +1,148 @@ +// +// 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.Linq; +using System.Management.Automation.Language; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System.ComponentModel.Composition; +using System.Globalization; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ + /// + /// AvoidUsingNullOrEmptyHelpMessageParameter: Check if the HelpMessage parameter is set to a non-empty string. + /// + [Export(typeof(IScriptRule))] + public class AvoidNullOrEmptyHelpMessageAttribute : IScriptRule + { + /// + /// AvoidUsingNullOrEmptyHelpMessageParameter: Check if the HelpMessage parameter is set to a non-empty string. + /// + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + // Finds all functionAst + IEnumerable functionAsts = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true); + + foreach (FunctionDefinitionAst funcAst in functionAsts) + { + if (funcAst.Body == null || funcAst.Body.ParamBlock == null + || funcAst.Body.ParamBlock.Attributes == null || funcAst.Body.ParamBlock.Parameters == null) + continue; + + foreach (var paramAst in funcAst.Body.ParamBlock.Parameters) + { + foreach (var paramAstAttribute in paramAst.Attributes) + { + if (!(paramAstAttribute is AttributeAst)) + continue; + + var namedArguments = (paramAstAttribute as AttributeAst).NamedArguments; + + if (namedArguments == null) + continue; + + foreach (NamedAttributeArgumentAst namedArgument in namedArguments) + { + if (!(String.Equals(namedArgument.ArgumentName, "HelpMessage", StringComparison.OrdinalIgnoreCase)) + || namedArgument.ExpressionOmitted) + continue; + + string errCondition; + if (namedArgument.Argument.Extent.Text.Equals("\"\"")) + { + errCondition = "empty"; + } + else if (namedArgument.Argument.Extent.Text.Equals("$null", StringComparison.OrdinalIgnoreCase)) + { + errCondition = "null"; + } + else + { + errCondition = null; + } + + if (!String.IsNullOrEmpty(errCondition)) + { + string message = string.Format(CultureInfo.CurrentCulture, + Strings.AvoidNullOrEmptyHelpMessageAttributeError, + paramAst.Name.VariablePath.UserPath); + yield return new DiagnosticRecord(message, + paramAst.Extent, + GetName(), + DiagnosticSeverity.Error, + fileName, + paramAst.Name.VariablePath.UserPath); + } + } + } + } + + } + } + + /// + /// GetName: Retrieves the name of this rule. + /// + /// The name of this rule + public string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.AvoidNullOrEmptyHelpMessageAttributeName); + } + + /// + /// GetCommonName: Retrieves the common name of this rule. + /// + /// The common name of this rule + public string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidNullOrEmptyHelpMessageAttributeCommonName); + } + + /// + /// GetDescription: Retrieves the description of this rule. + /// + /// The description of this rule + public string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidNullOrEmptyHelpMessageAttributeDescription); + } + + /// + /// GetSourceType: Retrieves the type of the rule, builtin, managed or module. + /// + public SourceType GetSourceType() + { + return SourceType.Builtin; + } + + /// + /// GetSeverity: Retrieves the severity of the rule: error, warning of information. + /// + /// + public RuleSeverity GetSeverity() + { + return RuleSeverity.Error; + } + + /// + /// GetSourceName: Retrieves the module/assembly name the rule is from. + /// + public string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + } +} diff --git a/Rules/ScriptAnalyzerBuiltinRules.csproj b/Rules/ScriptAnalyzerBuiltinRules.csproj index d5828643a..3862c8b8f 100644 --- a/Rules/ScriptAnalyzerBuiltinRules.csproj +++ b/Rules/ScriptAnalyzerBuiltinRules.csproj @@ -72,6 +72,7 @@ + diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index fe00b561c..f9679f1b5 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -258,6 +258,42 @@ internal static string AvoidInvokingEmptyMembersName { } } + /// + /// Looks up a localized string similar to Avoid using null or empty HelpMessage parameter attribute.. + /// + internal static string AvoidNullOrEmptyHelpMessageAttributeCommonName { + get { + return ResourceManager.GetString("AvoidNullOrEmptyHelpMessageAttributeCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Setting the HelpMessage attribute to an empty string or null value causes PowerShell interpreter to throw an error while executing the corresponding function.. + /// + internal static string AvoidNullOrEmptyHelpMessageAttributeDescription { + get { + return ResourceManager.GetString("AvoidNullOrEmptyHelpMessageAttributeDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to HelpMessage parameter attribute should not be null or empty. To fix a violation of this rule, please set its value to a non-empty string.. + /// + internal static string AvoidNullOrEmptyHelpMessageAttributeError { + get { + return ResourceManager.GetString("AvoidNullOrEmptyHelpMessageAttributeError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to AvoidNullOrEmptyHelpMessageAttribute. + /// + internal static string AvoidNullOrEmptyHelpMessageAttributeName { + get { + return ResourceManager.GetString("AvoidNullOrEmptyHelpMessageAttributeName", resourceCulture); + } + } + /// /// Looks up a localized string similar to Avoid Using ShouldContinue Without Boolean Force Parameter. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 9d92c1ee4..0b2fad980 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -786,4 +786,16 @@ This line has a backtick at the end trailed by a whitespace character. Did you mean for this to be a line continuation? + + Avoid using null or empty HelpMessage parameter attribute. + + + Setting the HelpMessage attribute to an empty string or null value causes PowerShell interpreter to throw an error while executing the corresponding function. + + + HelpMessage parameter attribute should not be null or empty. To fix a violation of this rule, please set its value to a non-empty string. + + + AvoidNullOrEmptyHelpMessageAttribute + \ No newline at end of file diff --git a/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.ps1 b/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.ps1 new file mode 100644 index 000000000..40c15c8d0 --- /dev/null +++ b/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.ps1 @@ -0,0 +1,70 @@ +function BadFuncNullHelpMessage +{ + [CmdletBinding()] + [OutputType([String])] + param( + # this one null value + [Parameter(HelpMessage=$null)] + [string] $Param1="String", + + # this parameter has no help message + [Parameter(HelpMessage="This is helpful.")] + [string] $Param2 + ) + $Param1 + $Param2 = "test" +} + +function BadFuncEmptyHelpMessage +{ + [CmdletBinding()] + [OutputType([String])] + param( + # this has an empty string + [Parameter(HelpMessage="")] + [string] $Param1="String", + + # this parameter has no default value + [Parameter(HelpMessage="This is helpful.")] + [string] $Param2 + ) + $Param1 + $Param2 = "test" +} + +function GoodFunc1($Param1) +{ + $Param1 +} + +# same as BadFunc but this one has no cmdletbinding +function BadFuncNullHelpMessageNoCmdletBinding +{ + param( + # this one null value + [Parameter(HelpMessage=$null)] + [string] $Param1="String", + + # this parameter has no help message + [Parameter(HelpMessage="This is helpful.")] + [string] $Param2 + ) + $Param1 + $Param2 = "test" +} + +# same as BadFunc but this one has no cmdletbinding +function BadFuncEmptyHelpMessageNoCmdletBinding +{ + param( + # this has an empty string + [Parameter(HelpMessage="")] + [string] $Param1="String", + + # this parameter has no default value + [Parameter(HelpMessage="This is helpful.")] + [string] $Param2 + ) + $Param1 + $Param2 = "test" +} \ No newline at end of file diff --git a/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.tests.ps1 b/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.tests.ps1 new file mode 100644 index 000000000..8396caae3 --- /dev/null +++ b/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.tests.ps1 @@ -0,0 +1,24 @@ +Import-Module PSScriptAnalyzer +$violationName = "PSAvoidNullOrEmptyHelpMessageAttribute" +$violationMessage = "HelpMessage parameter attribute should not be null or empty. To fix a violation of this rule, please set its value to a non-empty string." +$directory = Split-Path -Parent $MyInvocation.MyCommand.Path +$violations = Invoke-ScriptAnalyzer "$directory\AvoidNullOrEmptyHelpMessageAttribute.ps1" -IncludeRule PSAvoidNullOrEmptyHelpMessageAttribute +$noViolations = Invoke-ScriptAnalyzer "$directory\AvoidNullOrEmptyHelpMessageAttributeNoViolations.ps1" -IncludeRule PSAvoidNullOrEmptyHelpMessageAttribute + +Describe "AvoidNullOrEmptyHelpMessageAttribute" { + Context "When there are violations" { + It "has 1 provide default value for mandatory parameter violation" { + $violations.Count | Should Be 4 + } + + It "has the correct description message" { + $violations[0].Message | Should Match $violationMessage + } + } + + Context "When there are no violations" { + It "returns no violations" { + $noViolations.Count | Should Be 0 + } + } +} \ No newline at end of file diff --git a/Tests/Rules/AvoidNullOrEmptyHelpMessageAttributeNoViolations.ps1 b/Tests/Rules/AvoidNullOrEmptyHelpMessageAttributeNoViolations.ps1 new file mode 100644 index 000000000..060224c2a --- /dev/null +++ b/Tests/Rules/AvoidNullOrEmptyHelpMessageAttributeNoViolations.ps1 @@ -0,0 +1,36 @@ +function GoodFuncCmdletBinding +{ + [CmdletBinding()] + param( + # this one null value + [Parameter(HelpMessage="This is helpful.")] + [string] $Param1="String", + + # this parameter has no help message + [Parameter(HelpMessage="This is helpful too.")] + [string] $Param2 + ) + $Param1 + $Param2 = "test" +} + +function GoodFunc1($Param1) +{ + $Param1 +} + +# same as BadFunc but this one has no cmdletbinding +function GoodFuncNoCmdletBinding +{ + param( + # this one null value + [Parameter(HelpMessage="This is helpful.")] + [string] $Param1="String", + + # this parameter has no help message + [Parameter(HelpMessage="This is helpful too.")] + [string] $Param2 + ) + $Param1 + $Param2 = "test" +} From 2aa00867b779b53cc760d5863a6c17da838d237f Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 8 Feb 2016 20:08:42 -0800 Subject: [PATCH 2/7] minor change to a avoidnulloremptyhelpmessage test case. --- Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.tests.ps1 b/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.tests.ps1 index 8396caae3..8bd0afe4f 100644 --- a/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.tests.ps1 +++ b/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.tests.ps1 @@ -7,7 +7,7 @@ $noViolations = Invoke-ScriptAnalyzer "$directory\AvoidNullOrEmptyHelpMessageAtt Describe "AvoidNullOrEmptyHelpMessageAttribute" { Context "When there are violations" { - It "has 1 provide default value for mandatory parameter violation" { + It "detects the violations" { $violations.Count | Should Be 4 } From c2a5e4f7690ab4f40d811df00ef7e13dbec61580 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 8 Feb 2016 20:13:28 -0800 Subject: [PATCH 3/7] changes the encoding of the HelpMessage rule documentation markdown. --- .../AvoidNullOrEmptyHelpMessageAttribute.md | Bin 1254 -> 628 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/RuleDocumentation/AvoidNullOrEmptyHelpMessageAttribute.md b/RuleDocumentation/AvoidNullOrEmptyHelpMessageAttribute.md index fa23285c3bf91e6ca7bd84ad24c72875c737196d..f6d4a7ac61e06c400009a82f249081bcc11e35d1 100644 GIT binary patch literal 628 zcmbtROHRWu5M4J??l5Xuv=Q_Gl~@#L>4HzCBC$YSm~`4$ay-`9NmC&%!dbWzSHKfm zR9GRQSk{~I%$qkay>U%?bypd4&yzB^-3*PpMeo%H$04wOt^%e>mMySG)?o)X5ttE7 zn2ED2P2}tKuF&VKUC?ADvA__t-9RW1ju=55C?OCi3s|~H>EoPOObj*(fm)dgAy<`0 z4|CcgFA9u#Z3D7njJLFhf@#~5h!ofs+8|(qdG$RyCua1H>`M68mCZ%q@JsS6c1N12 zZX|vnSnCE9)S8TvGN5&wp*?VA@D!XuuR$YeJk z?{9*qdlhh@R)T-Ko+dA8(wQq(Wj}(>QwwG3{*jaC-R@x7J}+AHmT=P6#neEibPnOG dL}_v@PI(*WqO$RG*o7jF8%3xv^WFN`Ggul^r%UQRU}xOi z*qDOO*D+Nx>Sw$QJ9IbRrojW5fk{xs;k^T&fHH=UJbkZm;1+J(r-Gr3M~XdWh9PGN zVhk=QxL0auDz}%t3oVX8J(lq$|!1)O20(hAD6Uu#fv%=WMB0L3dCkY+Vm^HY)Y3ca8jTzuK4ROBu7A lLo_V*!RFp5W8tpv(>oz^@{cNX%e^VKr}*Eb+WQNA-T)Sd%g+D+ From f37c38316af4f7e95e32aa226c420dda54a469f0 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 8 Feb 2016 20:33:35 -0800 Subject: [PATCH 4/7] fixes the failing tests. --- Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index 7acf5743b..09741b453 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/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 39 + $defaultRules.Count | Should be 40 } } @@ -126,17 +126,17 @@ Describe "Test RuleExtension" { Describe "TestSeverity" { It "filters rules based on the specified rule severity" { $rules = Get-ScriptAnalyzerRule -Severity Error - $rules.Count | Should be 6 + $rules.Count | Should be 7 } It "filters rules based on multiple severity inputs"{ $rules = Get-ScriptAnalyzerRule -Severity Error,Information - $rules.Count | Should be 13 + $rules.Count | Should be 14 } It "takes lower case inputs" { $rules = Get-ScriptAnalyzerRule -Severity error - $rules.Count | Should be 6 + $rules.Count | Should be 7 } } From 710d5abd222648e057ad97fc64bf82cbb6f82300 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 9 Feb 2016 12:26:15 -0800 Subject: [PATCH 5/7] changes severity level, added a voilation condition and other minor changes. --- .../AvoidNullOrEmptyHelpMessageAttribute.md | 30 ++++++++++++++--- Rules/AvoidNullOrEmptyHelpMessageAttribute.cs | 25 +++++++++----- .../AvoidNullOrEmptyHelpMessageAttribute.ps1 | 33 +++++++++++++++++++ ...dNullOrEmptyHelpMessageAttribute.tests.ps1 | 2 +- 4 files changed, 76 insertions(+), 14 deletions(-) diff --git a/RuleDocumentation/AvoidNullOrEmptyHelpMessageAttribute.md b/RuleDocumentation/AvoidNullOrEmptyHelpMessageAttribute.md index f6d4a7ac6..934bb0c43 100644 --- a/RuleDocumentation/AvoidNullOrEmptyHelpMessageAttribute.md +++ b/RuleDocumentation/AvoidNullOrEmptyHelpMessageAttribute.md @@ -1,5 +1,5 @@ #AvoidNullOrEmtpyHelpMessageAttribute -**Severity Level: Error** +**Severity Level: Warning** ##Description @@ -12,12 +12,32 @@ To fix a violation of this rule, please set its value to a non-empty string. ##Example -Wrong： +Wrong: -Function BadFuncEmtpyHelpMessage +Function BadFuncEmtpyHelpMessageEmpty { Param( - [Parameter(HelpMessage="")] + [Parameter(HelpMessage='')] + [String] $Param + ) + + $Param +} + +Function BadFuncEmtpyHelpMessageNull +{ + Param( + [Parameter(HelpMessage=$null)] + [String] $Param + ) + + $Param +} + +Function BadFuncEmtpyHelpMessageNoAssignment +{ + Param( + [Parameter(HelpMessage)] [String] $Param ) @@ -30,7 +50,7 @@ Correct: Function GoodFuncEmtpyHelpMessage { Param( - [Parameter(HelpMessage="This is help.")] + [Parameter(HelpMessage='This is helpful')] [String] $Param ) diff --git a/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs b/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs index 39a85e484..aac56c99d 100644 --- a/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs +++ b/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs @@ -21,13 +21,13 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { /// - /// AvoidUsingNullOrEmptyHelpMessageParameter: Check if the HelpMessage parameter is set to a non-empty string. + /// AvoidUsingNullOrEmptyHelpMessageAttribute: Check if the HelpMessage parameter is set to a non-empty string. /// [Export(typeof(IScriptRule))] public class AvoidNullOrEmptyHelpMessageAttribute : IScriptRule { /// - /// AvoidUsingNullOrEmptyHelpMessageParameter: Check if the HelpMessage parameter is set to a non-empty string. + /// AvoidUsingNullOrEmptyHelpMessageAttribute: Check if the HelpMessage parameter is set to a non-empty string. /// public IEnumerable AnalyzeScript(Ast ast, string fileName) { @@ -39,32 +39,41 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) foreach (FunctionDefinitionAst funcAst in functionAsts) { if (funcAst.Body == null || funcAst.Body.ParamBlock == null - || funcAst.Body.ParamBlock.Attributes == null || funcAst.Body.ParamBlock.Parameters == null) + || funcAst.Body.ParamBlock.Attributes == null || funcAst.Body.ParamBlock.Parameters == null) + { continue; + } foreach (var paramAst in funcAst.Body.ParamBlock.Parameters) { foreach (var paramAstAttribute in paramAst.Attributes) { if (!(paramAstAttribute is AttributeAst)) + { continue; + } var namedArguments = (paramAstAttribute as AttributeAst).NamedArguments; if (namedArguments == null) + { continue; + } foreach (NamedAttributeArgumentAst namedArgument in namedArguments) { - if (!(String.Equals(namedArgument.ArgumentName, "HelpMessage", StringComparison.OrdinalIgnoreCase)) - || namedArgument.ExpressionOmitted) + if (!(namedArgument.ArgumentName.Equals("HelpMessage", StringComparison.OrdinalIgnoreCase))) + { continue; + } string errCondition; - if (namedArgument.Argument.Extent.Text.Equals("\"\"")) + if (namedArgument.ExpressionOmitted + || namedArgument.Argument.Extent.Text.Equals("\"\"") + || namedArgument.Argument.Extent.Text.Equals("\'\'")) { errCondition = "empty"; - } + } else if (namedArgument.Argument.Extent.Text.Equals("$null", StringComparison.OrdinalIgnoreCase)) { errCondition = "null"; @@ -134,7 +143,7 @@ public SourceType GetSourceType() /// public RuleSeverity GetSeverity() { - return RuleSeverity.Error; + return RuleSeverity.Warning; } /// diff --git a/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.ps1 b/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.ps1 index 40c15c8d0..d02329a3d 100644 --- a/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.ps1 +++ b/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.ps1 @@ -67,4 +67,37 @@ function BadFuncEmptyHelpMessageNoCmdletBinding ) $Param1 $Param2 = "test" +} + + +# same as BadFunc but this one has no cmdletbinding +function BadFuncEmptyHelpMessageNoCmdletBindingSingleQoutes +{ + param( + # this has an empty string + [Parameter(HelpMessage='')] + [string] $Param1="String", + + # this parameter has no default value + [Parameter(HelpMessage="This is helpful.")] + [string] $Param2 + ) + $Param1 + $Param2 = "test" +} + +# same as BadFunc but this one has no cmdletbinding +function BadFuncEmptyHelpMessageNoCmdletBindingNoAssignment +{ + param( + # this has an empty string + [Parameter(HelpMessage)] + [string] $Param1="String", + + # this parameter has no default value + [Parameter(HelpMessage="This is helpful.")] + [string] $Param2 + ) + $Param1 + $Param2 = "test" } \ No newline at end of file diff --git a/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.tests.ps1 b/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.tests.ps1 index 8bd0afe4f..a6920361f 100644 --- a/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.tests.ps1 +++ b/Tests/Rules/AvoidNullOrEmptyHelpMessageAttribute.tests.ps1 @@ -8,7 +8,7 @@ $noViolations = Invoke-ScriptAnalyzer "$directory\AvoidNullOrEmptyHelpMessageAtt Describe "AvoidNullOrEmptyHelpMessageAttribute" { Context "When there are violations" { It "detects the violations" { - $violations.Count | Should Be 4 + $violations.Count | Should Be 6 } It "has the correct description message" { From efacf6d5bfa863bc0330902f6611496f0eb4f64c Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 9 Feb 2016 12:29:12 -0800 Subject: [PATCH 6/7] updates severity level. --- Rules/AvoidNullOrEmptyHelpMessageAttribute.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs b/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs index aac56c99d..78311388b 100644 --- a/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs +++ b/Rules/AvoidNullOrEmptyHelpMessageAttribute.cs @@ -91,7 +91,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) yield return new DiagnosticRecord(message, paramAst.Extent, GetName(), - DiagnosticSeverity.Error, + DiagnosticSeverity.Warning, fileName, paramAst.Name.VariablePath.UserPath); } From ce9b6d72ab9dc17eea63f0026554c9f9ec33d205 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Tue, 9 Feb 2016 12:35:52 -0800 Subject: [PATCH 7/7] fixes failing tests. --- Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index 09741b453..e9bb657f5 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -126,17 +126,17 @@ Describe "Test RuleExtension" { Describe "TestSeverity" { It "filters rules based on the specified rule severity" { $rules = Get-ScriptAnalyzerRule -Severity Error - $rules.Count | Should be 7 + $rules.Count | Should be 6 } It "filters rules based on multiple severity inputs"{ $rules = Get-ScriptAnalyzerRule -Severity Error,Information - $rules.Count | Should be 14 + $rules.Count | Should be 13 } It "takes lower case inputs" { $rules = Get-ScriptAnalyzerRule -Severity error - $rules.Count | Should be 7 + $rules.Count | Should be 6 } }