From 4dc2ce46dec57bfeb4900d18f1b895e1c1d38cfd Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Tue, 9 Jun 2015 15:48:01 -0700 Subject: [PATCH 1/8] Rule to detect the absence of Verbose statements in DSC Resource --- Rules/ScriptAnalyzerBuiltinRules.csproj | 2 +- Rules/Strings.Designer.cs | 83 ++++++++--------- Rules/Strings.resx | 19 ++-- Rules/UseDeclaredVarsMoreThanAssignments.cs | 3 +- ...ge.cs => UseVerboseMessageInDSCResouce.cs} | 92 ++++++++----------- 5 files changed, 87 insertions(+), 112 deletions(-) rename Rules/{ProvideVerboseMessage.cs => UseVerboseMessageInDSCResouce.cs} (54%) diff --git a/Rules/ScriptAnalyzerBuiltinRules.csproj b/Rules/ScriptAnalyzerBuiltinRules.csproj index e9b8a56f6..040413abf 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/UseVerboseMessageInDSCResouce.cs similarity index 54% rename from Rules/ProvideVerboseMessage.cs rename to Rules/UseVerboseMessageInDSCResouce.cs index b3d191b56..470f59b30 100644 --- a/Rules/ProvideVerboseMessage.cs +++ b/Rules/UseVerboseMessageInDSCResouce.cs @@ -22,70 +22,58 @@ 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 UseVerboseMessageInDSCResouce : 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); } - //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))) - { - return AstVisitAction.Continue; - } + List expectedTargetResourceFunctionNames = new List(new string[] { "Set-TargetResource", "Test-TargetResource", "Get-TargetResource" }); - var commandAsts = funcAst.Body.FindAll(testAst => testAst is CommandAst, false); - bool hasVerbose = false; + IEnumerable functionDefinitionAsts = Helper.Instance.DscResourceFunctions(ast); - if (commandAsts != null) + foreach (FunctionDefinitionAst functionDefinitionAst in functionDefinitionAsts) { - foreach (CommandAst commandAst in commandAsts) + var commandAsts = functionDefinitionAst.Body.FindAll(testAst => testAst is CommandAst, false); + bool hasVerbose = false; + + if (null != commandAsts) { - hasVerbose |= String.Equals(commandAst.GetCommandName(), "Write-Verbose", StringComparison.OrdinalIgnoreCase); + foreach (CommandAst commandAst in commandAsts) + { + hasVerbose |= String.Equals(commandAst.GetCommandName(), "Write-Verbose", StringComparison.OrdinalIgnoreCase); + } } - } - if (!hasVerbose) - { - DiagnosticRecords.Add(new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.ProvideVerboseMessageErrorFunction, funcAst.Name), - funcAst.Extent, GetName(), DiagnosticSeverity.Information, fileName)); - } + if (!hasVerbose) + { + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseVerboseMessageInDSCResourceErrorFunction, functionDefinitionAst.Name), + functionDefinitionAst.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 +81,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 +90,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 +99,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 +124,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 From c114056035b758b9753e7afc373f1f77d7b480b0 Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Tue, 9 Jun 2015 15:55:59 -0700 Subject: [PATCH 2/8] Moved Verbose Message test to Disabled Rules Folder --- Tests/{Rules => Disabled Rules}/ProvideVerboseMessage.tests.ps1 | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Tests/{Rules => Disabled Rules}/ProvideVerboseMessage.tests.ps1 (100%) 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 From 12e6a6ae49d4469e79081d7678538fef0a6c8adc Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Tue, 9 Jun 2015 16:02:20 -0700 Subject: [PATCH 3/8] Fixed Typo in Filename --- Rules/ScriptAnalyzerBuiltinRules.csproj | 2 +- ...MessageInDSCResouce.cs => UseVerboseMessageInDSCResource.cs} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename Rules/{UseVerboseMessageInDSCResouce.cs => UseVerboseMessageInDSCResource.cs} (98%) diff --git a/Rules/ScriptAnalyzerBuiltinRules.csproj b/Rules/ScriptAnalyzerBuiltinRules.csproj index 040413abf..32697b07f 100644 --- a/Rules/ScriptAnalyzerBuiltinRules.csproj +++ b/Rules/ScriptAnalyzerBuiltinRules.csproj @@ -75,7 +75,7 @@ - + True True diff --git a/Rules/UseVerboseMessageInDSCResouce.cs b/Rules/UseVerboseMessageInDSCResource.cs similarity index 98% rename from Rules/UseVerboseMessageInDSCResouce.cs rename to Rules/UseVerboseMessageInDSCResource.cs index 470f59b30..ae33b1fa5 100644 --- a/Rules/UseVerboseMessageInDSCResouce.cs +++ b/Rules/UseVerboseMessageInDSCResource.cs @@ -25,7 +25,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules /// ProvideVerboseMessage: Analyzes the ast to check that Write-Verbose is called for DSC Resources /// [Export(typeof(IDSCResourceRule))] - public class UseVerboseMessageInDSCResouce : SkipNamedBlock, IDSCResourceRule + public class UseVerboseMessageInDSCResource : SkipNamedBlock, IDSCResourceRule { /// /// AnalyzeDSCResource: Analyzes the ast to check that Write-Verbose is called for DSC Resources From bd0742b789ed69b9d7870cae6e4789dd6475fb87 Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Tue, 9 Jun 2015 16:26:10 -0700 Subject: [PATCH 4/8] Tests for Verbose DSC Rule --- .../MSFT_WaitForAny/MSFT_WaitForAny.psm1 | 8 ++++++ .../UseVerboseMessageInDSCResource.Tests.ps1 | 26 +++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 Tests/Rules/UseVerboseMessageInDSCResource.Tests.ps1 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 From 23981df1612e885880b23690b0590848db3d5d1d Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Wed, 10 Jun 2015 10:00:12 -0700 Subject: [PATCH 5/8] Fix for failing tests since we moved Verbose message rule only for DSC resources --- Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 4 ++-- Tests/Engine/RuleSuppression.ps1 | 2 +- Tests/Engine/RuleSuppression.tests.ps1 | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) 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 } } From 618b07cc7f9990af6b838426173b16cedf2ac94b Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Wed, 10 Jun 2015 12:15:38 -0700 Subject: [PATCH 6/8] Fix Splatted Variable and Pipeline bug in UseCmdletCorrectly and AvoidPositionalParameter --- Engine/Helper.cs | 36 +++++++++++++++++++++++------- Rules/AvoidPositionalParameters.cs | 19 ++++++++++++++-- Rules/UseCmdletCorrectly.cs | 6 +++++ Tests/Rules/GoodCmdlet.ps1 | 6 +++++ Tests/Rules/UseCmdletCorrectly.ps1 | 2 +- 5 files changed, 58 insertions(+), 11 deletions(-) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 373f4bd58..9716db704 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -219,6 +219,21 @@ item is TypeDefinitionAst return false; } + /// + /// Given a commandast, checks whether it uses splatted variable + /// + /// + /// + public bool HasSplattedVariable(CommandAst cmdAst) + { + if (cmdAst == null || cmdAst.CommandElements == null) + { + return false; + } + + return cmdAst.CommandElements.Any(cmdElem => cmdElem is VariableExpressionAst && (cmdElem as VariableExpressionAst).Splatted); + } + /// /// Given a commandast, checks whether positional parameters are used or not. /// @@ -237,6 +252,11 @@ public bool PositionalParameterUsed(CommandAst cmdAst) IEnumerable scriptBlocks = null; bool hasScriptBlockSet = false; + if (HasSplattedVariable(cmdAst)) + { + return false; + } + if (commandInfo != null && commandInfo.CommandType == System.Management.Automation.CommandTypes.Cmdlet) { try @@ -286,19 +306,19 @@ public bool PositionalParameterUsed(CommandAst cmdAst) } else { - //Skip if splatting "@" is used - if (ceAst is VariableExpressionAst) - { - if ((ceAst as VariableExpressionAst).Splatted) - { - continue; - } - } arguments += 1; } } } + // if not the first element in a pipeline, increase the number of arguments by 1 + PipelineAst parent = cmdAst.Parent as PipelineAst; + + if (parent != null && parent.PipelineElements.Count > 1 && parent.PipelineElements[0] != cmdAst) + { + arguments += 1; + } + return arguments > parameters; } diff --git a/Rules/AvoidPositionalParameters.cs b/Rules/AvoidPositionalParameters.cs index 248dc4f71..a52ac550b 100644 --- a/Rules/AvoidPositionalParameters.cs +++ b/Rules/AvoidPositionalParameters.cs @@ -47,8 +47,23 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (Helper.Instance.GetCommandInfo(cmdAst.GetCommandName()) != null && Helper.Instance.PositionalParameterUsed(cmdAst)) { - yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, cmdAst.GetCommandName()), - cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, cmdAst.GetCommandName()); + PipelineAst parent = cmdAst.Parent as PipelineAst; + + if (parent != null && parent.PipelineElements.Count > 1) + { + // raise if it's the first element in pipeline. otherwise no. + if (parent.PipelineElements[0] == cmdAst) + { + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, cmdAst.GetCommandName()), + cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, cmdAst.GetCommandName()); + } + } + // not in pipeline so just raise it normally + else + { + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, cmdAst.GetCommandName()), + cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, cmdAst.GetCommandName()); + } } } } diff --git a/Rules/UseCmdletCorrectly.cs b/Rules/UseCmdletCorrectly.cs index 430e9a2d4..691f4f743 100644 --- a/Rules/UseCmdletCorrectly.cs +++ b/Rules/UseCmdletCorrectly.cs @@ -92,6 +92,12 @@ private bool IsMandatoryParameterExisted(CommandAst cmdAst) return true; } + // ignores if splatted variable is used + if (Helper.Instance.HasSplattedVariable(cmdAst)) + { + return true; + } + // Gets parameters from command elements. ceAsts = cmdAst.CommandElements.Where(foundParamASTs); diff --git a/Tests/Rules/GoodCmdlet.ps1 b/Tests/Rules/GoodCmdlet.ps1 index 24106a3f5..0fceccd60 100644 --- a/Tests/Rules/GoodCmdlet.ps1 +++ b/Tests/Rules/GoodCmdlet.ps1 @@ -85,6 +85,12 @@ function Get-File $a } + $a | Write-Warning + + $b = @{"hash"="table"} + + Write-Debug @b + return @{"hash"="true"} } End diff --git a/Tests/Rules/UseCmdletCorrectly.ps1 b/Tests/Rules/UseCmdletCorrectly.ps1 index 8fb8392c2..373078af8 100644 --- a/Tests/Rules/UseCmdletCorrectly.ps1 +++ b/Tests/Rules/UseCmdletCorrectly.ps1 @@ -2,4 +2,4 @@ Wrong-Cmd Write-Verbose -Message "Write Verbose" Write-Verbose "Warning" -OutVariable $test -Write-Verbose "Warning" \ No newline at end of file +Write-Verbose "Warning" | PipeLineCmdlet \ No newline at end of file From 909db528054c08d6d3e7bf021cfe3e3853d9908d Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Wed, 10 Jun 2015 13:11:44 -0700 Subject: [PATCH 7/8] Removed reference to unused variable --- Rules/UseVerboseMessageInDSCResource.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Rules/UseVerboseMessageInDSCResource.cs b/Rules/UseVerboseMessageInDSCResource.cs index ae33b1fa5..df556d97c 100644 --- a/Rules/UseVerboseMessageInDSCResource.cs +++ b/Rules/UseVerboseMessageInDSCResource.cs @@ -37,9 +37,7 @@ public IEnumerable AnalyzeDSCResource(Ast ast, string fileName if (ast == null) { throw new ArgumentNullException(Strings.NullAstErrorMessage); - } - - List expectedTargetResourceFunctionNames = new List(new string[] { "Set-TargetResource", "Test-TargetResource", "Get-TargetResource" }); + } IEnumerable functionDefinitionAsts = Helper.Instance.DscResourceFunctions(ast); From 8d5eafe696fa0c481823ce9970360130f061f7b9 Mon Sep 17 00:00:00 2001 From: quoctruong Date: Thu, 11 Jun 2015 22:33:15 -0700 Subject: [PATCH 8/8] Fix matches and psversiontable not recognized by variableanalysis --- Engine/SpecialVars.cs | 10 ++++++++-- .../Rules/AvoidGlobalOrUnitializedVarsNoViolations.ps1 | 5 +++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/Engine/SpecialVars.cs b/Engine/SpecialVars.cs index 6c41f57fb..633657e2d 100644 --- a/Engine/SpecialVars.cs +++ b/Engine/SpecialVars.cs @@ -34,6 +34,8 @@ internal class SpecialVars internal const string PSScriptRoot = "PSScriptRoot"; internal const string PSCommandPath = "PSCommandPath"; internal const string ExecutionContext = "ExecutionContext"; + internal const string Matches = "Matches"; + internal const string PSVersionTable = "PSVersionTable"; internal static readonly string[] InitializedVariables; @@ -63,7 +65,9 @@ static SpecialVars() MyInvocation, PSScriptRoot, PSCommandPath, - ExecutionContext + ExecutionContext, + Matches, + PSVersionTable }; internal static readonly Type[] AutomaticVariableTypes = new Type[] { @@ -76,7 +80,9 @@ static SpecialVars() /* MyInvocation */ typeof(InvocationInfo), /* PSScriptRoot */ typeof(string), /* PSCommandPath */ typeof(string), - /* ExecutionContext */ typeof(EngineIntrinsics), + /* ExecutionContext */ typeof(EngineIntrinsics), + /* Matches */ typeof(System.Collections.Hashtable), + /* PSVersionTable */ typeof(System.Collections.Hashtable) }; diff --git a/Tests/Rules/AvoidGlobalOrUnitializedVarsNoViolations.ps1 b/Tests/Rules/AvoidGlobalOrUnitializedVarsNoViolations.ps1 index 42c137872..b2f61dfb8 100644 --- a/Tests/Rules/AvoidGlobalOrUnitializedVarsNoViolations.ps1 +++ b/Tests/Rules/AvoidGlobalOrUnitializedVarsNoViolations.ps1 @@ -9,6 +9,11 @@ function Test { $a = 3; +"hi there!" -match "hi" | Out-Null; +$matches[0]; + +$PSVersionTable; + if ($true) { $a = 4; $c = 3;