diff --git a/Rules/ScriptAnalyzerBuiltinRules.csproj b/Rules/ScriptAnalyzerBuiltinRules.csproj index e9b8a56f6..32697b07f 100644 --- a/Rules/ScriptAnalyzerBuiltinRules.csproj +++ b/Rules/ScriptAnalyzerBuiltinRules.csproj @@ -75,7 +75,7 @@ - + True True diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 54175ef88..8c76e720b 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -1,7 +1,7 @@ //------------------------------------------------------------------------------ // // This code was generated by a tool. -// Runtime Version:4.0.30319.35317 +// Runtime Version:4.0.30319.42000 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. @@ -1167,51 +1167,6 @@ internal static string ProvideDefaultParameterValueName { } } - /// - /// Looks up a localized string similar to Verbose. - /// - internal static string ProvideVerboseMessageCommonName { - get { - return ResourceManager.GetString("ProvideVerboseMessageCommonName", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to Checks that Write-Verbose is called at least once in every cmdlet or script. This is in line with the PowerShell best practices.. - /// - internal static string ProvideVerboseMessageDescription { - get { - return ResourceManager.GetString("ProvideVerboseMessageDescription", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to There is no call to Write-Verbose in the function ‘{0}’.. - /// - internal static string ProvideVerboseMessageErrorFunction { - get { - return ResourceManager.GetString("ProvideVerboseMessageErrorFunction", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to ProvideVerboseMessage. - /// - internal static string ProvideVerboseMessageName { - get { - return ResourceManager.GetString("ProvideVerboseMessageName", resourceCulture); - } - } - - /// - /// Looks up a localized string similar to There is no call to Write-Verbose in the script.. - /// - internal static string ProvideVerboseMessageScript { - get { - return ResourceManager.GetString("ProvideVerboseMessageScript", resourceCulture); - } - } - /// /// Looks up a localized string similar to Reserved Cmdlet Chars. /// @@ -1913,5 +1868,41 @@ internal static string UseTypeAtVariableAssignmentName { return ResourceManager.GetString("UseTypeAtVariableAssignmentName", resourceCulture); } } + + /// + /// Looks up a localized string similar to Use verbose message in DSC resource. + /// + internal static string UseVerboseMessageInDSCResourceCommonName { + get { + return ResourceManager.GetString("UseVerboseMessageInDSCResourceCommonName", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to It is a best practice to emit informative, verbose messages in DSC resource functions. This helps in debugging issues when a DSC configuration is executed.. + /// + internal static string UseVerboseMessageInDSCResourceDescription { + get { + return ResourceManager.GetString("UseVerboseMessageInDSCResourceDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to There is no call to Write-Verbose in DSC function ‘{0}’. If you are using Write-Verbose in a helper function, suppress this rule application.. + /// + internal static string UseVerboseMessageInDSCResourceErrorFunction { + get { + return ResourceManager.GetString("UseVerboseMessageInDSCResourceErrorFunction", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to UseVerboseMessageInDSCResource. + /// + internal static string UseVerboseMessageInDSCResourceName { + get { + return ResourceManager.GetString("UseVerboseMessageInDSCResourceName", resourceCulture); + } + } } } diff --git a/Rules/Strings.resx b/Rules/Strings.resx index c16fa35e3..dd02abedf 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -276,17 +276,14 @@ PS - - Checks that Write-Verbose is called at least once in every cmdlet or script. This is in line with the PowerShell best practices. + + It is a best practice to emit informative, verbose messages in DSC resource functions. This helps in debugging issues when a DSC configuration is executed. - - There is no call to Write-Verbose in the function ‘{0}’. + + There is no call to Write-Verbose in DSC function ‘{0}’. If you are using Write-Verbose in a helper function, suppress this rule application. - - Verbose - - - There is no call to Write-Verbose in the script. + + Use verbose message in DSC resource Some fields of the module manifest (such as ModuleVersion) are required. @@ -459,8 +456,8 @@ MissingModuleManifestField - - ProvideVerboseMessage + + UseVerboseMessageInDSCResource Command Not Found diff --git a/Rules/UseDeclaredVarsMoreThanAssignments.cs b/Rules/UseDeclaredVarsMoreThanAssignments.cs index 1f1ecf089..f90302500 100644 --- a/Rules/UseDeclaredVarsMoreThanAssignments.cs +++ b/Rules/UseDeclaredVarsMoreThanAssignments.cs @@ -35,8 +35,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - IEnumerable assignmentAsts = ast.FindAll(testAst => testAst is AssignmentStatementAst, true); - IEnumerable assingmentVarAsts; + IEnumerable assignmentAsts = ast.FindAll(testAst => testAst is AssignmentStatementAst, true); IEnumerable varAsts = ast.FindAll(testAst => testAst is VariableExpressionAst, true); IEnumerable varsInAssignment; diff --git a/Rules/ProvideVerboseMessage.cs b/Rules/UseVerboseMessageInDSCResource.cs similarity index 53% rename from Rules/ProvideVerboseMessage.cs rename to Rules/UseVerboseMessageInDSCResource.cs index b3d191b56..df556d97c 100644 --- a/Rules/ProvideVerboseMessage.cs +++ b/Rules/UseVerboseMessageInDSCResource.cs @@ -22,70 +22,56 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { /// - /// ProvideVerboseMessage: Analyzes the ast to check that Write-Verbose is called at least once in every cmdlet or script. + /// ProvideVerboseMessage: Analyzes the ast to check that Write-Verbose is called for DSC Resources /// - [Export(typeof(IScriptRule))] - public class ProvideVerboseMessage : SkipNamedBlock, IScriptRule + [Export(typeof(IDSCResourceRule))] + public class UseVerboseMessageInDSCResource : SkipNamedBlock, IDSCResourceRule { /// - /// AnalyzeScript: Analyzes the ast to check that Write-Verbose is called at least once in every cmdlet or script. + /// AnalyzeDSCResource: Analyzes the ast to check that Write-Verbose is called for DSC Resources /// The script's ast /// The script's file name /// - public IEnumerable AnalyzeScript(Ast ast, string fileName) + public IEnumerable AnalyzeDSCResource(Ast ast, string fileName) { - if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - - ClearList(); - this.AddNames(new List() { "Configuration", "Workflow" }); - DiagnosticRecords.Clear(); - - this.fileName = fileName; - //We only check that advanced functions should have Write-Verbose - ast.Visit(this); - - return DiagnosticRecords; - } - - /// - /// Visit function and checks that it has write verbose - /// - /// - /// - public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst funcAst) - { - if (funcAst == null) + if (ast == null) { - return AstVisitAction.SkipChildren; - } + throw new ArgumentNullException(Strings.NullAstErrorMessage); + } + + IEnumerable functionDefinitionAsts = Helper.Instance.DscResourceFunctions(ast); - //Write-Verbose is not required for non-advanced functions - if (funcAst.Body == null || funcAst.Body.ParamBlock == null - || funcAst.Body.ParamBlock.Attributes == null || - funcAst.Body.ParamBlock.Parameters == null || - !funcAst.Body.ParamBlock.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(CmdletBindingAttribute))) + foreach (FunctionDefinitionAst functionDefinitionAst in functionDefinitionAsts) { - return AstVisitAction.Continue; - } + var commandAsts = functionDefinitionAst.Body.FindAll(testAst => testAst is CommandAst, false); + bool hasVerbose = false; - var commandAsts = funcAst.Body.FindAll(testAst => testAst is CommandAst, false); - bool hasVerbose = false; + if (null != commandAsts) + { + foreach (CommandAst commandAst in commandAsts) + { + hasVerbose |= String.Equals(commandAst.GetCommandName(), "Write-Verbose", StringComparison.OrdinalIgnoreCase); + } + } - if (commandAsts != null) - { - foreach (CommandAst commandAst in commandAsts) + if (!hasVerbose) { - hasVerbose |= String.Equals(commandAst.GetCommandName(), "Write-Verbose", StringComparison.OrdinalIgnoreCase); + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseVerboseMessageInDSCResourceErrorFunction, functionDefinitionAst.Name), + functionDefinitionAst.Extent, GetName(), DiagnosticSeverity.Information, fileName); } - } - if (!hasVerbose) - { - DiagnosticRecords.Add(new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.ProvideVerboseMessageErrorFunction, funcAst.Name), - funcAst.Extent, GetName(), DiagnosticSeverity.Information, fileName)); } - - return AstVisitAction.Continue; + } + + /// + /// AnalyzeDSCClass: This function returns nothing in the case of dsc class. + /// + /// + /// + /// + public IEnumerable AnalyzeDSCClass(Ast ast, string fileName) + { + return Enumerable.Empty(); } /// @@ -93,7 +79,7 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun /// public string GetName() { - return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.ProvideVerboseMessageName); + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseVerboseMessageInDSCResourceName); } /// @@ -102,7 +88,7 @@ public string GetName() /// The common name of this rule public string GetCommonName() { - return string.Format(CultureInfo.CurrentCulture, Strings.ProvideVerboseMessageCommonName); + return string.Format(CultureInfo.CurrentCulture, Strings.UseVerboseMessageInDSCResourceCommonName); } /// @@ -111,7 +97,7 @@ public string GetCommonName() /// The description of this rule public string GetDescription() { - return string.Format(CultureInfo.CurrentCulture, Strings.ProvideVerboseMessageDescription); + return string.Format(CultureInfo.CurrentCulture, Strings.UseVerboseMessageInDSCResourceDescription); } /// @@ -136,7 +122,7 @@ public RuleSeverity GetSeverity() /// public string GetSourceName() { - return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + return string.Format(CultureInfo.CurrentCulture, Strings.DSCSourceName); } } -} +} \ No newline at end of file diff --git a/Tests/Rules/ProvideVerboseMessage.tests.ps1 b/Tests/Disabled Rules/ProvideVerboseMessage.tests.ps1 similarity index 100% rename from Tests/Rules/ProvideVerboseMessage.tests.ps1 rename to Tests/Disabled Rules/ProvideVerboseMessage.tests.ps1 diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index fa5346b6a..2606aab63 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -130,11 +130,11 @@ Describe "TestSeverity" { Describe "TestWildCard" { It "filters rules based on the -Name wild card input" { $rules = Get-ScriptAnalyzerRule -Name PSDSC* - $rules.Count | Should be 6 + $rules.Count | Should be 7 } It "filters rules based on wild card input and severity"{ $rules = Get-ScriptAnalyzerRule -Name PSDSC*　-Severity Information - $rules.Count | Should be 3 + $rules.Count | Should be 4 } } diff --git a/Tests/Engine/RuleSuppression.ps1 b/Tests/Engine/RuleSuppression.ps1 index d5cd4e055..b63aef84c 100644 --- a/Tests/Engine/RuleSuppression.ps1 +++ b/Tests/Engine/RuleSuppression.ps1 @@ -5,7 +5,7 @@ Param( function SuppressMe () { - [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSProvideVerboseMessage")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSProvideCommentHelp")] [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSProvideDefaultParameterValue", "unused1")] Param([string]$unUsed1, [int] $unUsed2) { diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index bb6ff4a77..17d8a719d 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -5,7 +5,7 @@ $violations = Invoke-ScriptAnalyzer $directory\RuleSuppression.ps1 Describe "RuleSuppressionWithoutScope" { Context "Function" { It "Does not raise violations" { - $suppression = $violations | Where-Object { $_.RuleName -eq "PSProvideVerboseMessage" } + $suppression = $violations | Where-Object { $_.RuleName -eq "PSProvideCommentHelp" } $suppression.Count | Should Be 0 } } diff --git a/Tests/Rules/DSCResources/MSFT_WaitForAny/MSFT_WaitForAny.psm1 b/Tests/Rules/DSCResources/MSFT_WaitForAny/MSFT_WaitForAny.psm1 index 096cf8b54..5b8ee83d9 100644 --- a/Tests/Rules/DSCResources/MSFT_WaitForAny/MSFT_WaitForAny.psm1 +++ b/Tests/Rules/DSCResources/MSFT_WaitForAny/MSFT_WaitForAny.psm1 @@ -29,6 +29,8 @@ function Get-TargetResource [Uint32] $ThrottleLimit = 32 #Powershell New-CimSession default throttle value ) + Write-Verbose "In Get-TargetResource" + Import-Module $PSScriptRoot\..\..\PSDSCxMachine.psm1 $b = @{"hash" = "table"} @@ -75,10 +77,14 @@ function Set-TargetResource [Uint32] $ThrottleLimit = 32 #Powershell New-CimSession default throttle value ) + Write-Verbose "In Set-TargetResource" + Import-Module $PSScriptRoot\..\..\PSDSCxMachine.psm1 if ($PSBoundParameters["Verbose"]) { + Write-Verbose "Calling xMachine with Verbose parameter" + PSDSCxMachine\Set-_InternalPSDscXMachineTR ` -RemoteResourceId $ResourceName ` -RemoteMachine $NodeName ` @@ -130,6 +136,8 @@ function Test-TargetResource [Uint32] $ThrottleLimit = 32 #Powershell New-CimSession default throttle value ) + Write-Verbose "In Test-TargetResource" + Import-Module $PSScriptRoot\..\..\PSDSCxMachine.psm1 $a = $true diff --git a/Tests/Rules/UseVerboseMessageInDSCResource.Tests.ps1 b/Tests/Rules/UseVerboseMessageInDSCResource.Tests.ps1 new file mode 100644 index 000000000..9b2b3c5b8 --- /dev/null +++ b/Tests/Rules/UseVerboseMessageInDSCResource.Tests.ps1 @@ -0,0 +1,26 @@ +Import-Module PSScriptAnalyzer + +$violationMessage = "There is no call to Write-Verbose in DSC function ‘Set-TargetResource’. If you are using Write-Verbose in a helper function, suppress this rule application." +$violationName = "PSDSCUseVerboseMessageInDSCResource" +$directory = Split-Path -Parent $MyInvocation.MyCommand.Path +$violations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAll\MSFT_WaitForAll.psm1 | Where-Object {$_.RuleName -eq $violationName} +$noViolations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAny\MSFT_WaitForAny.psm1 | Where-Object {$_.RuleName -eq $violationName} +$noClassViolations = Invoke-ScriptAnalyzer -ErrorAction SilentlyContinue $directory\DSCResources\MyDscResource\MyDscResource.psm1 | Where-Object {$_.RuleName -eq $violationName} + +Describe "UseVerboseMessageInDSCResource" { + Context "When there are violations" { + It "has 2 Verbose Message violations" { + $violations.Count | Should Be 2 + } + + 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