From 4dc2ce46dec57bfeb4900d18f1b895e1c1d38cfd Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Tue, 9 Jun 2015 15:48:01 -0700 Subject: [PATCH 01/25] 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 02/25] 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 03/25] 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 04/25] 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 05/25] 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 06/25] 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 07/25] 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 08/25] 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; From fe4bf82961f883500c64ff6df6b58ebf6c3e9c02 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Fri, 12 Jun 2015 15:42:59 -0700 Subject: [PATCH 09/25] Improve Performance of ScriptAnalyzer by using BackgroundWorkers --- .../Commands/InvokeScriptAnalyzerCommand.cs | 171 +++++++++++++----- 1 file changed, 123 insertions(+), 48 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index 02ef89a75..8d56fb732 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -12,6 +12,7 @@ using System.Text.RegularExpressions; using System; +using System.ComponentModel; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Globalization; @@ -20,6 +21,9 @@ using System.Management.Automation.Language; using System.IO; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System.Threading.Tasks; +using System.Collections.Concurrent; +using System.Threading; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands { @@ -267,6 +271,14 @@ private void ProcessPath(string path) } + ConcurrentBag diagnostics; + ConcurrentBag suppressed; + Dictionary> ruleSuppressions; + List includeRegexList; + List excludeRegexList; + CountdownEvent cde; + ConcurrentDictionary> ruleDictionary; + /// /// Analyzes a single script file. /// @@ -275,15 +287,16 @@ private void AnalyzeFile(string filePath) { Token[] tokens = null; ParseError[] errors = null; - List diagnostics = new List(); - List suppressed = new List(); + diagnostics = new ConcurrentBag(); + suppressed = new ConcurrentBag(); + ruleDictionary = new ConcurrentDictionary>(); // Use a List of KVP rather than dictionary, since for a script containing inline functions with same signature, keys clash List> cmdInfoTable = new List>(); //Check wild card input for the Include/ExcludeRules and create regex match patterns - List includeRegexList = new List(); - List excludeRegexList = new List(); + includeRegexList = new List(); + excludeRegexList = new List(); if (includeRule != null) { foreach (string rule in includeRule) @@ -331,7 +344,7 @@ private void AnalyzeFile(string filePath) return; } - Dictionary> ruleSuppressions = Helper.Instance.GetRuleSuppression(ast); + ruleSuppressions = Helper.Instance.GetRuleSuppression(ast); foreach (List ruleSuppressionsList in ruleSuppressions.Values) { @@ -360,44 +373,24 @@ private void AnalyzeFile(string filePath) if (ScriptAnalyzer.Instance.ScriptRules != null) { - foreach (IScriptRule scriptRule in ScriptAnalyzer.Instance.ScriptRules) + cde = new CountdownEvent(ScriptAnalyzer.Instance.ScriptRules.Count()); + + foreach (var scriptRule in ScriptAnalyzer.Instance.ScriptRules) { - bool includeRegexMatch = false; - bool excludeRegexMatch = false; - foreach (Regex include in includeRegexList) - { - if (include.IsMatch(scriptRule.GetName())) - { - includeRegexMatch = true; - break; - } - } + BackgroundWorker bg = new BackgroundWorker(); + bg.DoWork += bg_DoWork; + bg.RunWorkerAsync(new object[] { scriptRule }); + } - foreach (Regex exclude in excludeRegexList) - { - if (exclude.IsMatch(scriptRule.GetName())) - { - excludeRegexMatch = true; - break; - } - } + cde.Wait(); - if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) + foreach (var rule in ruleDictionary.Keys) + { + List verboseOrErrors = ruleDictionary[rule]; + WriteVerbose(verboseOrErrors[0] as string); + if (verboseOrErrors.Count == 2) { - WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); - - // Ensure that any unhandled errors from Rules are converted to non-terminating errors - // We want the Engine to continue functioning even if one or more Rules throws an exception - try - { - var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, filePath).ToList()); - diagnostics.AddRange(records.Item2); - suppressed.AddRange(records.Item1); - } - catch (Exception scriptRuleException) - { - WriteError(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath)); - } + WriteError(verboseOrErrors[1] as ErrorRecord); } } } @@ -437,8 +430,14 @@ private void AnalyzeFile(string filePath) try { var records = Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, tokenRule.AnalyzeTokens(tokens, filePath).ToList()); - diagnostics.AddRange(records.Item2); - suppressed.AddRange(records.Item1); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } } catch (Exception tokenRuleException) { @@ -489,8 +488,14 @@ private void AnalyzeFile(string filePath) try { var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList()); - diagnostics.AddRange(records.Item2); - suppressed.AddRange(records.Item1); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } } catch (Exception dscResourceRuleException) { @@ -532,8 +537,14 @@ private void AnalyzeFile(string filePath) try { var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList()); - diagnostics.AddRange(records.Item2); - suppressed.AddRange(records.Item1); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } } catch (Exception dscResourceRuleException) { @@ -573,15 +584,20 @@ private void AnalyzeFile(string filePath) } } - diagnostics.AddRange(ScriptAnalyzer.Instance.GetExternalRecord(ast, tokens, exRules.ToArray(), this, fileName)); + foreach (var record in ScriptAnalyzer.Instance.GetExternalRecord(ast, tokens, exRules.ToArray(), this, fileName)) + { + diagnostics.Add(record); + } } #endregion + IEnumerable diagnosticsList = diagnostics; + if (severity != null) { var diagSeverity = severity.Select(item => Enum.Parse(typeof(DiagnosticSeverity), item, true)); - diagnostics = diagnostics.Where(item => diagSeverity.Contains(item.Severity)).ToList(); + diagnosticsList = diagnostics.Where(item => diagSeverity.Contains(item.Severity)); } //Output through loggers @@ -596,7 +612,7 @@ private void AnalyzeFile(string filePath) } else { - foreach (DiagnosticRecord diagnostic in diagnostics) + foreach (DiagnosticRecord diagnostic in diagnosticsList) { logger.LogObject(diagnostic, this); } @@ -604,6 +620,65 @@ private void AnalyzeFile(string filePath) } } + void bg_DoWork(object sender, DoWorkEventArgs e) + { + bool includeRegexMatch = false; + bool excludeRegexMatch = false; + + object[] parameters = e.Argument as object[]; + + IScriptRule scriptRule = parameters[0] as IScriptRule; + + foreach (Regex include in includeRegexList) + { + if (include.IsMatch(scriptRule.GetName())) + { + includeRegexMatch = true; + break; + } + } + + foreach (Regex exclude in excludeRegexList) + { + if (exclude.IsMatch(scriptRule.GetName())) + { + excludeRegexMatch = true; + break; + } + } + + List result = new List(); + + if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) + { + //WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); + result.Add(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); + + // Ensure that any unhandled errors from Rules are converted to non-terminating errors + // We want the Engine to continue functioning even if one or more Rules throws an exception + try + { + var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, ast.Extent.File).ToList()); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } + } + catch (Exception scriptRuleException) + { + result.Add(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, ast.Extent.File)); + } + } + + ruleDictionary[scriptRule.GetName()] = result; + + cde.Signal(); + } + #endregion } } \ No newline at end of file From 967461a50b0b629b04978145f338da955307f1d4 Mon Sep 17 00:00:00 2001 From: quoctruong Date: Mon, 15 Jun 2015 01:16:30 -0700 Subject: [PATCH 10/25] Fix Session State Error --- Rules/AvoidUsingDeprecatedManifestFields.cs | 3 ++- Rules/MissingModuleManifestField.cs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Rules/AvoidUsingDeprecatedManifestFields.cs b/Rules/AvoidUsingDeprecatedManifestFields.cs index 964522490..8ec58ca60 100644 --- a/Rules/AvoidUsingDeprecatedManifestFields.cs +++ b/Rules/AvoidUsingDeprecatedManifestFields.cs @@ -41,7 +41,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (String.Equals(System.IO.Path.GetExtension(fileName), ".psd1", StringComparison.OrdinalIgnoreCase)) { - var ps = System.Management.Automation.PowerShell.Create(RunspaceMode.CurrentRunspace); + var ps = System.Management.Automation.PowerShell.Create(); IEnumerable result = null; try { @@ -73,6 +73,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) } } + ps.Dispose(); } } diff --git a/Rules/MissingModuleManifestField.cs b/Rules/MissingModuleManifestField.cs index 9b0dd5954..6df37980c 100644 --- a/Rules/MissingModuleManifestField.cs +++ b/Rules/MissingModuleManifestField.cs @@ -38,7 +38,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (String.Equals(System.IO.Path.GetExtension(fileName), ".psd1", StringComparison.OrdinalIgnoreCase)) { - var ps = System.Management.Automation.PowerShell.Create(RunspaceMode.CurrentRunspace); + var ps = System.Management.Automation.PowerShell.Create(); try { @@ -68,6 +68,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) } } + ps.Dispose(); } } From 5d1325e55e4b032066514e778a1e5068f71afa03 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Mon, 15 Jun 2015 12:22:11 -0700 Subject: [PATCH 11/25] Ignore PSObject and PSCustomObject for UseOutputTypeCorrectly rule --- Rules/UseOutputTypeCorrectly.cs | 13 +++++++++---- Tests/Rules/GoodCmdlet.ps1 | 8 +++++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/Rules/UseOutputTypeCorrectly.cs b/Rules/UseOutputTypeCorrectly.cs index a41c3a23c..93d8353be 100644 --- a/Rules/UseOutputTypeCorrectly.cs +++ b/Rules/UseOutputTypeCorrectly.cs @@ -102,15 +102,20 @@ public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst fun List> returnTypes = FindPipelineOutput.OutputTypes(funcAst, _classes); + HashSet specialTypes = new HashSet(StringComparer.OrdinalIgnoreCase); + specialTypes.Add(typeof(Unreached).FullName); + specialTypes.Add(typeof(Undetermined).FullName); + specialTypes.Add(typeof(object).FullName); + specialTypes.Add(typeof(void).FullName); + specialTypes.Add(typeof(PSCustomObject).FullName); + specialTypes.Add(typeof(PSObject).FullName); + foreach (Tuple returnType in returnTypes) { string typeName = returnType.Item1; if (String.IsNullOrEmpty(typeName) - || String.Equals(typeof(Unreached).FullName, typeName, StringComparison.OrdinalIgnoreCase) - || String.Equals(typeof(Undetermined).FullName, typeName, StringComparison.OrdinalIgnoreCase) - || String.Equals(typeof(object).FullName, typeName, StringComparison.OrdinalIgnoreCase) - || String.Equals(typeof(void).FullName, typeName, StringComparison.OrdinalIgnoreCase) + || specialTypes.Contains(typeName) || outputTypes.Contains(typeName, StringComparer.OrdinalIgnoreCase)) { continue; diff --git a/Tests/Rules/GoodCmdlet.ps1 b/Tests/Rules/GoodCmdlet.ps1 index 0fceccd60..93958ebf7 100644 --- a/Tests/Rules/GoodCmdlet.ps1 +++ b/Tests/Rules/GoodCmdlet.ps1 @@ -28,7 +28,7 @@ function Get-File HelpUri = 'http://www.microsoft.com/', ConfirmImpact='Medium')] [Alias()] - [OutputType([String], [System.Double], [Hashtable])] + [OutputType([String], [System.Double], [Hashtable], "MyCustom.OutputType")] [OutputType("System.Int32", ParameterSetName="ID")] Param @@ -91,6 +91,12 @@ function Get-File Write-Debug @b + [pscustomobject]@{ + PSTypeName = 'MyCustom.OutputType' + Prop1 = 'SomeValue' + Prop2 = 'OtherValue' + } + return @{"hash"="true"} } End From 83d6d5f886af761d01c035000144b97a07e9b64f Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Tue, 16 Jun 2015 12:42:47 -0700 Subject: [PATCH 12/25] Fix Null pointer exception in avoid default value switch parameter --- Rules/AvoidDefaultTrueValueSwitchParameter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rules/AvoidDefaultTrueValueSwitchParameter.cs b/Rules/AvoidDefaultTrueValueSwitchParameter.cs index a2f892571..932da65e6 100644 --- a/Rules/AvoidDefaultTrueValueSwitchParameter.cs +++ b/Rules/AvoidDefaultTrueValueSwitchParameter.cs @@ -39,7 +39,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) // Iterrates all ParamAsts and check if any are switch. foreach (ParameterAst paramAst in paramAsts) { - if (paramAst.Attributes.Any(attr => string.Equals(attr.TypeName.GetReflectionType().FullName, "system.management.automation.switchparameter", StringComparison.OrdinalIgnoreCase)) + if (paramAst.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(System.Management.Automation.SwitchParameter)) && paramAst.DefaultValue != null && String.Equals(paramAst.DefaultValue.Extent.Text, "$true", StringComparison.OrdinalIgnoreCase)) { yield return new DiagnosticRecord( From 8241b864aa35831a448c03063bc9346f65718477 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Fri, 12 Jun 2015 15:42:59 -0700 Subject: [PATCH 13/25] Improve Performance of ScriptAnalyzer by using BackgroundWorkers --- .../Commands/InvokeScriptAnalyzerCommand.cs | 171 +++++++++++++----- 1 file changed, 123 insertions(+), 48 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index 02ef89a75..8d56fb732 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -12,6 +12,7 @@ using System.Text.RegularExpressions; using System; +using System.ComponentModel; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Globalization; @@ -20,6 +21,9 @@ using System.Management.Automation.Language; using System.IO; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System.Threading.Tasks; +using System.Collections.Concurrent; +using System.Threading; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands { @@ -267,6 +271,14 @@ private void ProcessPath(string path) } + ConcurrentBag diagnostics; + ConcurrentBag suppressed; + Dictionary> ruleSuppressions; + List includeRegexList; + List excludeRegexList; + CountdownEvent cde; + ConcurrentDictionary> ruleDictionary; + /// /// Analyzes a single script file. /// @@ -275,15 +287,16 @@ private void AnalyzeFile(string filePath) { Token[] tokens = null; ParseError[] errors = null; - List diagnostics = new List(); - List suppressed = new List(); + diagnostics = new ConcurrentBag(); + suppressed = new ConcurrentBag(); + ruleDictionary = new ConcurrentDictionary>(); // Use a List of KVP rather than dictionary, since for a script containing inline functions with same signature, keys clash List> cmdInfoTable = new List>(); //Check wild card input for the Include/ExcludeRules and create regex match patterns - List includeRegexList = new List(); - List excludeRegexList = new List(); + includeRegexList = new List(); + excludeRegexList = new List(); if (includeRule != null) { foreach (string rule in includeRule) @@ -331,7 +344,7 @@ private void AnalyzeFile(string filePath) return; } - Dictionary> ruleSuppressions = Helper.Instance.GetRuleSuppression(ast); + ruleSuppressions = Helper.Instance.GetRuleSuppression(ast); foreach (List ruleSuppressionsList in ruleSuppressions.Values) { @@ -360,44 +373,24 @@ private void AnalyzeFile(string filePath) if (ScriptAnalyzer.Instance.ScriptRules != null) { - foreach (IScriptRule scriptRule in ScriptAnalyzer.Instance.ScriptRules) + cde = new CountdownEvent(ScriptAnalyzer.Instance.ScriptRules.Count()); + + foreach (var scriptRule in ScriptAnalyzer.Instance.ScriptRules) { - bool includeRegexMatch = false; - bool excludeRegexMatch = false; - foreach (Regex include in includeRegexList) - { - if (include.IsMatch(scriptRule.GetName())) - { - includeRegexMatch = true; - break; - } - } + BackgroundWorker bg = new BackgroundWorker(); + bg.DoWork += bg_DoWork; + bg.RunWorkerAsync(new object[] { scriptRule }); + } - foreach (Regex exclude in excludeRegexList) - { - if (exclude.IsMatch(scriptRule.GetName())) - { - excludeRegexMatch = true; - break; - } - } + cde.Wait(); - if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) + foreach (var rule in ruleDictionary.Keys) + { + List verboseOrErrors = ruleDictionary[rule]; + WriteVerbose(verboseOrErrors[0] as string); + if (verboseOrErrors.Count == 2) { - WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); - - // Ensure that any unhandled errors from Rules are converted to non-terminating errors - // We want the Engine to continue functioning even if one or more Rules throws an exception - try - { - var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, filePath).ToList()); - diagnostics.AddRange(records.Item2); - suppressed.AddRange(records.Item1); - } - catch (Exception scriptRuleException) - { - WriteError(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath)); - } + WriteError(verboseOrErrors[1] as ErrorRecord); } } } @@ -437,8 +430,14 @@ private void AnalyzeFile(string filePath) try { var records = Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, tokenRule.AnalyzeTokens(tokens, filePath).ToList()); - diagnostics.AddRange(records.Item2); - suppressed.AddRange(records.Item1); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } } catch (Exception tokenRuleException) { @@ -489,8 +488,14 @@ private void AnalyzeFile(string filePath) try { var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList()); - diagnostics.AddRange(records.Item2); - suppressed.AddRange(records.Item1); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } } catch (Exception dscResourceRuleException) { @@ -532,8 +537,14 @@ private void AnalyzeFile(string filePath) try { var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList()); - diagnostics.AddRange(records.Item2); - suppressed.AddRange(records.Item1); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } } catch (Exception dscResourceRuleException) { @@ -573,15 +584,20 @@ private void AnalyzeFile(string filePath) } } - diagnostics.AddRange(ScriptAnalyzer.Instance.GetExternalRecord(ast, tokens, exRules.ToArray(), this, fileName)); + foreach (var record in ScriptAnalyzer.Instance.GetExternalRecord(ast, tokens, exRules.ToArray(), this, fileName)) + { + diagnostics.Add(record); + } } #endregion + IEnumerable diagnosticsList = diagnostics; + if (severity != null) { var diagSeverity = severity.Select(item => Enum.Parse(typeof(DiagnosticSeverity), item, true)); - diagnostics = diagnostics.Where(item => diagSeverity.Contains(item.Severity)).ToList(); + diagnosticsList = diagnostics.Where(item => diagSeverity.Contains(item.Severity)); } //Output through loggers @@ -596,7 +612,7 @@ private void AnalyzeFile(string filePath) } else { - foreach (DiagnosticRecord diagnostic in diagnostics) + foreach (DiagnosticRecord diagnostic in diagnosticsList) { logger.LogObject(diagnostic, this); } @@ -604,6 +620,65 @@ private void AnalyzeFile(string filePath) } } + void bg_DoWork(object sender, DoWorkEventArgs e) + { + bool includeRegexMatch = false; + bool excludeRegexMatch = false; + + object[] parameters = e.Argument as object[]; + + IScriptRule scriptRule = parameters[0] as IScriptRule; + + foreach (Regex include in includeRegexList) + { + if (include.IsMatch(scriptRule.GetName())) + { + includeRegexMatch = true; + break; + } + } + + foreach (Regex exclude in excludeRegexList) + { + if (exclude.IsMatch(scriptRule.GetName())) + { + excludeRegexMatch = true; + break; + } + } + + List result = new List(); + + if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) + { + //WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); + result.Add(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); + + // Ensure that any unhandled errors from Rules are converted to non-terminating errors + // We want the Engine to continue functioning even if one or more Rules throws an exception + try + { + var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, ast.Extent.File).ToList()); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } + } + catch (Exception scriptRuleException) + { + result.Add(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, ast.Extent.File)); + } + } + + ruleDictionary[scriptRule.GetName()] = result; + + cde.Signal(); + } + #endregion } } \ No newline at end of file From bb1cf1d025eb7dac7731acc73c0826599caf368f Mon Sep 17 00:00:00 2001 From: quoctruong Date: Mon, 15 Jun 2015 01:16:30 -0700 Subject: [PATCH 14/25] Fix Session State Error --- Rules/AvoidUsingDeprecatedManifestFields.cs | 3 ++- Rules/MissingModuleManifestField.cs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Rules/AvoidUsingDeprecatedManifestFields.cs b/Rules/AvoidUsingDeprecatedManifestFields.cs index 964522490..8ec58ca60 100644 --- a/Rules/AvoidUsingDeprecatedManifestFields.cs +++ b/Rules/AvoidUsingDeprecatedManifestFields.cs @@ -41,7 +41,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (String.Equals(System.IO.Path.GetExtension(fileName), ".psd1", StringComparison.OrdinalIgnoreCase)) { - var ps = System.Management.Automation.PowerShell.Create(RunspaceMode.CurrentRunspace); + var ps = System.Management.Automation.PowerShell.Create(); IEnumerable result = null; try { @@ -73,6 +73,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) } } + ps.Dispose(); } } diff --git a/Rules/MissingModuleManifestField.cs b/Rules/MissingModuleManifestField.cs index 9b0dd5954..6df37980c 100644 --- a/Rules/MissingModuleManifestField.cs +++ b/Rules/MissingModuleManifestField.cs @@ -38,7 +38,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (String.Equals(System.IO.Path.GetExtension(fileName), ".psd1", StringComparison.OrdinalIgnoreCase)) { - var ps = System.Management.Automation.PowerShell.Create(RunspaceMode.CurrentRunspace); + var ps = System.Management.Automation.PowerShell.Create(); try { @@ -68,6 +68,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) } } + ps.Dispose(); } } From e73107483b87a5c0f8ad79ff081ee3660740cfd4 Mon Sep 17 00:00:00 2001 From: Yuting Chen Date: Tue, 16 Jun 2015 14:45:59 -0700 Subject: [PATCH 15/25] Remove the check for ScritpBlock in PositionalParameter --- Engine/Helper.cs | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 9716db704..da5bbb890 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -249,8 +249,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst) CommandInfo commandInfo = GetCommandInfo(GetCmdletNameFromAlias(cmdAst.GetCommandName())) ?? GetCommandInfo(cmdAst.GetCommandName()); IEnumerable switchParams = null; - IEnumerable scriptBlocks = null; - bool hasScriptBlockSet = false; if (HasSplattedVariable(cmdAst)) { @@ -262,15 +260,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst) try { switchParams = commandInfo.Parameters.Values.Where(pm => pm.SwitchParameter); - scriptBlocks = commandInfo.ParameterSets; - foreach (CommandParameterSetInfo cmdParaset in scriptBlocks) - { - if (String.Equals(cmdParaset.Name, "ScriptBlockSet", StringComparison.OrdinalIgnoreCase)) - { - hasScriptBlockSet = true; - } - } - } catch (Exception) { @@ -284,8 +273,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst) foreach (CommandElementAst ceAst in cmdAst.CommandElements) { - if (!hasScriptBlockSet) - { if (ceAst is CommandParameterAst) { // Skip if it's a switch parameter @@ -308,7 +295,7 @@ public bool PositionalParameterUsed(CommandAst cmdAst) { arguments += 1; } - } + } // if not the first element in a pipeline, increase the number of arguments by 1 From 3e2555cee67bc8f6ef8b64e2ce83fa077c15e728 Mon Sep 17 00:00:00 2001 From: Yuting Chen Date: Tue, 16 Jun 2015 14:52:17 -0700 Subject: [PATCH 16/25] Add tests --- Tests/Rules/AvoidPositionalParametersNoViolations.ps1 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tests/Rules/AvoidPositionalParametersNoViolations.ps1 b/Tests/Rules/AvoidPositionalParametersNoViolations.ps1 index 5ae0bd222..41562ffd8 100644 --- a/Tests/Rules/AvoidPositionalParametersNoViolations.ps1 +++ b/Tests/Rules/AvoidPositionalParametersNoViolations.ps1 @@ -3,4 +3,5 @@ Get-ChildItem -Path "Tests" Clear-Host Split-Path -Path "Random" -leaf Get-Process | Where-Object {$_.handles -gt 200} -get-service-computername localhost | where {($_.status -eq "Running") -and ($_.CanStop -eq $true)} \ No newline at end of file +get-service-computername localhost | where {($_.status -eq "Running") -and ($_.CanStop -eq $true)} +1, 2, $null, 4 | ForEach-Object {"Hello"} \ No newline at end of file From 8ba1409d1beea1f87f45cfef0399c913d5819019 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Wed, 17 Jun 2015 17:21:06 -0700 Subject: [PATCH 17/25] Fix test failures and use tasks instead of backgroundworkers --- .../Commands/InvokeScriptAnalyzerCommand.cs | 158 ++++++++---------- .../AvoidGlobalOrUnitializedVars.tests.ps1 | 6 +- .../Rules/AvoidPositionalParameters.tests.ps1 | 2 +- .../AvoidShouldContinueWithoutForce.tests.ps1 | 2 +- .../AvoidUserNameAndPasswordParams.tests.ps1 | 2 +- Tests/Rules/AvoidUsingAlias.tests.ps1 | 2 +- .../AvoidUsingPlainTextForPassword.tests.ps1 | 2 +- .../AvoidUsingUninitializedVariable.Tests.ps1 | 2 +- Tests/Rules/ProvideCommentHelp.tests.ps1 | 2 +- .../ProvideDefaultParameterValue.tests.ps1 | 2 +- ...eturnCorrectTypesForDSCFunctions.tests.ps1 | 2 +- ...eDeclaredVarsMoreThanAssignments.tests.ps1 | 2 +- Tests/Rules/UseOutputTypeCorrectly.tests.ps1 | 2 +- .../UseVerboseMessageInDSCResource.Tests.ps1 | 2 +- 14 files changed, 87 insertions(+), 101 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index 8d56fb732..1144c8a58 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -271,14 +271,6 @@ private void ProcessPath(string path) } - ConcurrentBag diagnostics; - ConcurrentBag suppressed; - Dictionary> ruleSuppressions; - List includeRegexList; - List excludeRegexList; - CountdownEvent cde; - ConcurrentDictionary> ruleDictionary; - /// /// Analyzes a single script file. /// @@ -287,16 +279,16 @@ private void AnalyzeFile(string filePath) { Token[] tokens = null; ParseError[] errors = null; - diagnostics = new ConcurrentBag(); - suppressed = new ConcurrentBag(); - ruleDictionary = new ConcurrentDictionary>(); + ConcurrentBag diagnostics = new ConcurrentBag(); + ConcurrentBag suppressed = new ConcurrentBag(); + BlockingCollection> verboseOrErrors = new BlockingCollection>(); // Use a List of KVP rather than dictionary, since for a script containing inline functions with same signature, keys clash List> cmdInfoTable = new List>(); //Check wild card input for the Include/ExcludeRules and create regex match patterns - includeRegexList = new List(); - excludeRegexList = new List(); + List includeRegexList = new List(); + List excludeRegexList = new List(); if (includeRule != null) { foreach (string rule in includeRule) @@ -344,7 +336,7 @@ private void AnalyzeFile(string filePath) return; } - ruleSuppressions = Helper.Instance.GetRuleSuppression(ast); + Dictionary> ruleSuppressions = Helper.Instance.GetRuleSuppression(ast); foreach (List ruleSuppressionsList in ruleSuppressions.Values) { @@ -373,24 +365,77 @@ private void AnalyzeFile(string filePath) if (ScriptAnalyzer.Instance.ScriptRules != null) { - cde = new CountdownEvent(ScriptAnalyzer.Instance.ScriptRules.Count()); + var tasks = ScriptAnalyzer.Instance.ScriptRules.Select(scriptRule => Task.Factory.StartNew(() => + { + bool includeRegexMatch = false; + bool excludeRegexMatch = false; - foreach (var scriptRule in ScriptAnalyzer.Instance.ScriptRules) - { - BackgroundWorker bg = new BackgroundWorker(); - bg.DoWork += bg_DoWork; - bg.RunWorkerAsync(new object[] { scriptRule }); - } + foreach (Regex include in includeRegexList) + { + if (include.IsMatch(scriptRule.GetName())) + { + includeRegexMatch = true; + break; + } + } + + foreach (Regex exclude in excludeRegexList) + { + if (exclude.IsMatch(scriptRule.GetName())) + { + excludeRegexMatch = true; + break; + } + } + + if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) + { + List result = new List(); + + //WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); + result.Add(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); + + // Ensure that any unhandled errors from Rules are converted to non-terminating errors + // We want the Engine to continue functioning even if one or more Rules throws an exception + try + { + var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, ast.Extent.File).ToList()); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } + } + catch (Exception scriptRuleException) + { + result.Add(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, ast.Extent.File)); + } - cde.Wait(); + verboseOrErrors.Add(result); + } + })); - foreach (var rule in ruleDictionary.Keys) + Task.Factory.ContinueWhenAll(tasks.ToArray(), t => verboseOrErrors.CompleteAdding()); + + while (!verboseOrErrors.IsCompleted) { - List verboseOrErrors = ruleDictionary[rule]; - WriteVerbose(verboseOrErrors[0] as string); - if (verboseOrErrors.Count == 2) + List data = null; + try + { + data = verboseOrErrors.Take(); + } + catch (InvalidOperationException) { } + + if (data != null) { - WriteError(verboseOrErrors[1] as ErrorRecord); + WriteVerbose(data[0] as string); + if (data.Count == 2) + { + WriteError(data[1] as ErrorRecord); + } } } } @@ -620,65 +665,6 @@ private void AnalyzeFile(string filePath) } } - void bg_DoWork(object sender, DoWorkEventArgs e) - { - bool includeRegexMatch = false; - bool excludeRegexMatch = false; - - object[] parameters = e.Argument as object[]; - - IScriptRule scriptRule = parameters[0] as IScriptRule; - - foreach (Regex include in includeRegexList) - { - if (include.IsMatch(scriptRule.GetName())) - { - includeRegexMatch = true; - break; - } - } - - foreach (Regex exclude in excludeRegexList) - { - if (exclude.IsMatch(scriptRule.GetName())) - { - excludeRegexMatch = true; - break; - } - } - - List result = new List(); - - if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) - { - //WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); - result.Add(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); - - // Ensure that any unhandled errors from Rules are converted to non-terminating errors - // We want the Engine to continue functioning even if one or more Rules throws an exception - try - { - var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, ast.Extent.File).ToList()); - foreach (var record in records.Item2) - { - diagnostics.Add(record); - } - foreach (var suppressedRec in records.Item1) - { - suppressed.Add(suppressedRec); - } - } - catch (Exception scriptRuleException) - { - result.Add(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, ast.Extent.File)); - } - } - - ruleDictionary[scriptRule.GetName()] = result; - - cde.Signal(); - } - #endregion } } \ No newline at end of file diff --git a/Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1 b/Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1 index 0b90d1b71..47a709f27 100644 --- a/Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1 +++ b/Tests/Rules/AvoidGlobalOrUnitializedVars.tests.ps1 @@ -2,14 +2,14 @@ $globalMessage = "Found global variable 'Global:1'." $globalName = "PSAvoidGlobalVars" $nonInitializedName = "PSAvoidUninitializedVariable" -$nonInitializedMessage = "Variable 'a' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables." +$nonInitializedMessage = "Variable 'globalVars' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables." $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidGlobalOrUnitializedVars.ps1 $dscResourceViolations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAny\MSFT_WaitForAny.psm1 | Where-Object {$_.RuleName -eq $nonInitializedName} $globalViolations = $violations | Where-Object {$_.RuleName -eq $globalName} $nonInitializedViolations = $violations | Where-Object {$_.RuleName -eq $nonInitializedName} $noViolations = Invoke-ScriptAnalyzer $directory\AvoidGlobalOrUnitializedVarsNoViolations.ps1 -$noGlobalViolations = $noViolations | Where-Object {$_.RuleName -eq $violationName} +$noGlobalViolations = $noViolations | Where-Object {$_.RuleName -eq $globalName} $noUninitializedViolations = $noViolations | Where-Object {$_.RuleName -eq $nonInitializedName} Describe "AvoidGlobalVars" { @@ -23,7 +23,7 @@ Describe "AvoidGlobalVars" { } It "has the correct description message" { - $violations[0].Message | Should Match $globalMessage + $globalViolations[0].Message | Should Match $globalMessage } } diff --git a/Tests/Rules/AvoidPositionalParameters.tests.ps1 b/Tests/Rules/AvoidPositionalParameters.tests.ps1 index 82be8a726..2e5156edd 100644 --- a/Tests/Rules/AvoidPositionalParameters.tests.ps1 +++ b/Tests/Rules/AvoidPositionalParameters.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "Cmdlet 'Get-Content' has positional parameter. Please use named parameters instead of positional parameters when calling a command." +$violationMessage = "Cmdlet 'Write-Host' has positional parameter. Please use named parameters instead of positional parameters when calling a command." $violationName = "PSAvoidUsingPositionalParameters" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidPositionalParameters.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/AvoidShouldContinueWithoutForce.tests.ps1 b/Tests/Rules/AvoidShouldContinueWithoutForce.tests.ps1 index 4fab2e16b..9daf1cf3e 100644 --- a/Tests/Rules/AvoidShouldContinueWithoutForce.tests.ps1 +++ b/Tests/Rules/AvoidShouldContinueWithoutForce.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "Function 'Verb-Noun' in file 'AvoidShouldContinueWithoutForce.ps1' uses ShouldContinue but does not have a boolean force parameter. The force parameter will allow users of the script to bypass ShouldContinue prompt" +$violationMessage = "Function 'Verb-Noun2' in file 'AvoidShouldContinueWithoutForce.ps1' uses ShouldContinue but does not have a boolean force parameter. The force parameter will allow users of the script to bypass ShouldContinue prompt" $violationName = "PSAvoidShouldContinueWithoutForce" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidShouldContinueWithoutForce.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 b/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 index e50e9fcdc..14f567160 100644 --- a/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 +++ b/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 @@ -1,6 +1,6 @@ Import-Module PSScriptAnalyzer -$violationMessage = "Function 'TestFunction' has both username and password parameters. A credential parameter of type PSCredential should be used." +$violationMessage = "Function 'Verb-Noun' has both username and password parameters. A credential parameter of type PSCredential should be used." $violationName = "PSAvoidUsingUserNameAndPasswordParams" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParams.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/AvoidUsingAlias.tests.ps1 b/Tests/Rules/AvoidUsingAlias.tests.ps1 index 7c9d74db1..63e8f2780 100644 --- a/Tests/Rules/AvoidUsingAlias.tests.ps1 +++ b/Tests/Rules/AvoidUsingAlias.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "'iex' is an alias of 'Invoke-Expression'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content." +$violationMessage = "'cls' is an alias of 'Clear-Host'. Alias can introduce possible problems and make scripts hard to maintain. Please consider changing alias to its full content." $violationName = "PSAvoidUsingCmdletAliases" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidUsingAlias.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1 b/Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1 index 2c298a7cf..3dd6ff7c7 100644 --- a/Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1 +++ b/Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1 @@ -1,6 +1,6 @@ Import-Module PSScriptAnalyzer -$violationMessage = [regex]::Escape("Parameter '`$passphrases' should use SecureString, otherwise this will expose sensitive information. See ConvertTo-SecureString for more information.") +$violationMessage = [regex]::Escape("Parameter '`$password' should use SecureString, otherwise this will expose sensitive information. See ConvertTo-SecureString for more information.") $violationName = "PSAvoidUsingPlainTextForPassword" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidUsingPlainTextForPassword.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/AvoidUsingUninitializedVariable.Tests.ps1 b/Tests/Rules/AvoidUsingUninitializedVariable.Tests.ps1 index 3fb75b360..4f499ed04 100644 --- a/Tests/Rules/AvoidUsingUninitializedVariable.Tests.ps1 +++ b/Tests/Rules/AvoidUsingUninitializedVariable.Tests.ps1 @@ -1,6 +1,6 @@ Import-Module PSScriptAnalyzer $AvoidUninitializedVariable = "PSAvoidUninitializedVariable" -$violationMessage = "Variable 'MyProgressPreference' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables." +$violationMessage = "Variable 'MyVerbosePreference' is not initialized. Non-global variables must be initialized. To fix a violation of this rule, please initialize non-global variables." $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidUsingUninitializedVariable.ps1 -IncludeRule $AvoidUninitializedVariable $noViolations = Invoke-ScriptAnalyzer $directory\AvoidUsingUninitializedVariableNoViolations.ps1 -IncludeRule $AvoidUninitializedVariable diff --git a/Tests/Rules/ProvideCommentHelp.tests.ps1 b/Tests/Rules/ProvideCommentHelp.tests.ps1 index 30a183710..a0ab714d4 100644 --- a/Tests/Rules/ProvideCommentHelp.tests.ps1 +++ b/Tests/Rules/ProvideCommentHelp.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "The cmdlet 'Verb-Files' does not have a help comment." +$violationMessage = "The cmdlet 'Comment' does not have a help comment." $violationName = "PSProvideCommentHelp" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\BadCmdlet.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/ProvideDefaultParameterValue.tests.ps1 b/Tests/Rules/ProvideDefaultParameterValue.tests.ps1 index c731d7974..5289fdc72 100644 --- a/Tests/Rules/ProvideDefaultParameterValue.tests.ps1 +++ b/Tests/Rules/ProvideDefaultParameterValue.tests.ps1 @@ -1,6 +1,6 @@ Import-Module PSScriptAnalyzer $violationName = "PSProvideDefaultParameterValue" -$violationMessage = "Parameter 'Param2' is not initialized. Parameters must have a default value. To fix a violation of this rule, please specify a default value for all parameters" +$violationMessage = "Parameter 'Param1' is not initialized. Parameters must have a default value. To fix a violation of this rule, please specify a default value for all parameters" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\ProvideDefaultParameterValue.ps1 | Where-Object {$_.RuleName -match $violationName} $noViolations = Invoke-ScriptAnalyzer $directory\ProvideDefaultParameterValueNoViolations.ps1 diff --git a/Tests/Rules/ReturnCorrectTypesForDSCFunctions.tests.ps1 b/Tests/Rules/ReturnCorrectTypesForDSCFunctions.tests.ps1 index f45163f88..4ba05667d 100644 --- a/Tests/Rules/ReturnCorrectTypesForDSCFunctions.tests.ps1 +++ b/Tests/Rules/ReturnCorrectTypesForDSCFunctions.tests.ps1 @@ -1,7 +1,7 @@ Import-Module -Verbose PSScriptAnalyzer $violationMessageDSCResource = "Test-TargetResource function in DSC Resource should return object of type System.Boolean instead of System.Collections.Hashtable" -$violationMessageDSCClass = "Test function in DSC Class FileResource should return object of type System.Boolean instead of type System.Int32" +$violationMessageDSCClass = "Get function in DSC Class FileResource should return object of type FileResource instead of type System.Collections.Hashtable" $violationName = "PSDSCReturnCorrectTypesForDSCFunctions" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\DSCResources\MSFT_WaitForAll\MSFT_WaitForAll.psm1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 b/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 index 61b5b34a2..f1d818893 100644 --- a/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 +++ b/Tests/Rules/UseDeclaredVarsMoreThanAssignments.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "The variable 'declaredVar' is assigned but never used." +$violationMessage = "The variable 'declaredVar2' is assigned but never used." $violationName = "PSUseDeclaredVarsMoreThanAssigments" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\UseDeclaredVarsMoreThanAssignments.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/UseOutputTypeCorrectly.tests.ps1 b/Tests/Rules/UseOutputTypeCorrectly.tests.ps1 index 666d281d0..236a5faa1 100644 --- a/Tests/Rules/UseOutputTypeCorrectly.tests.ps1 +++ b/Tests/Rules/UseOutputTypeCorrectly.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "The cmdlet 'Verb-Files' returns an object of type 'System.Double' but this type is not declared in the OutputType attribute." +$violationMessage = "The cmdlet 'Verb-Files' returns an object of type 'System.Collections.Hashtable' but this type is not declared in the OutputType attribute." $violationName = "PSUseOutputTypeCorrectly" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\BadCmdlet.ps1 | Where-Object {$_.RuleName -eq $violationName} diff --git a/Tests/Rules/UseVerboseMessageInDSCResource.Tests.ps1 b/Tests/Rules/UseVerboseMessageInDSCResource.Tests.ps1 index 9b2b3c5b8..bca6904e7 100644 --- a/Tests/Rules/UseVerboseMessageInDSCResource.Tests.ps1 +++ b/Tests/Rules/UseVerboseMessageInDSCResource.Tests.ps1 @@ -1,6 +1,6 @@ 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." +$violationMessage = "There is no call to Write-Verbose in DSC function ‘Test-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} From f4b99d483ee341655b44a9f5b1f13fddb4714475 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Thu, 18 Jun 2015 10:56:37 -0700 Subject: [PATCH 18/25] Remove countdownevent --- .../Commands/InvokeScriptAnalyzerCommand.cs | 60 ------------------- 1 file changed, 60 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index 014f01e18..1e5aa0512 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -276,7 +276,6 @@ private void ProcessPath(string path) Dictionary> ruleSuppressions; List includeRegexList; List excludeRegexList; - CountdownEvent cde; ConcurrentDictionary> ruleDictionary; /// @@ -673,65 +672,6 @@ private void AnalyzeFile(string filePath) } } - void bg_DoWork(object sender, DoWorkEventArgs e) - { - bool includeRegexMatch = false; - bool excludeRegexMatch = false; - - object[] parameters = e.Argument as object[]; - - IScriptRule scriptRule = parameters[0] as IScriptRule; - - foreach (Regex include in includeRegexList) - { - if (include.IsMatch(scriptRule.GetName())) - { - includeRegexMatch = true; - break; - } - } - - foreach (Regex exclude in excludeRegexList) - { - if (exclude.IsMatch(scriptRule.GetName())) - { - excludeRegexMatch = true; - break; - } - } - - List result = new List(); - - if ((includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch)) - { - //WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); - result.Add(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); - - // Ensure that any unhandled errors from Rules are converted to non-terminating errors - // We want the Engine to continue functioning even if one or more Rules throws an exception - try - { - var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, ast.Extent.File).ToList()); - foreach (var record in records.Item2) - { - diagnostics.Add(record); - } - foreach (var suppressedRec in records.Item1) - { - suppressed.Add(suppressedRec); - } - } - catch (Exception scriptRuleException) - { - result.Add(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, ast.Extent.File)); - } - } - - ruleDictionary[scriptRule.GetName()] = result; - - cde.Signal(); - } - #endregion } } \ No newline at end of file From 295f9a328757a0986f4cc4d9ec97cec0e2b59812 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Thu, 18 Jun 2015 11:47:26 -0700 Subject: [PATCH 19/25] Remove comment --- Engine/Commands/InvokeScriptAnalyzerCommand.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index 1e5aa0512..8936e2cd3 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -399,7 +399,6 @@ private void AnalyzeFile(string filePath) { List result = new List(); - //WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); result.Add(string.Format(CultureInfo.CurrentCulture, Strings.VerboseRunningMessage, scriptRule.GetName())); // Ensure that any unhandled errors from Rules are converted to non-terminating errors From 09d7bd9386511f459cf7606870a09fcff9d21de1 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Tue, 23 Jun 2015 14:31:34 -0700 Subject: [PATCH 20/25] Only raise NullComparisonRule if the RHS is an array or has unknown type --- Engine/Helper.cs | 2 +- Rules/PossibleIncorrectComparisonWithNull.cs | 65 +++++++++++++++++-- .../PossibleIncorrectComparisonWithNull.ps1 | 19 ++++++ ...sibleIncorrectComparisonWithNull.tests.ps1 | 4 +- ...ncorrectComparisonWithNullNoViolations.ps1 | 11 ++++ 5 files changed, 91 insertions(+), 10 deletions(-) diff --git a/Engine/Helper.cs b/Engine/Helper.cs index da5bbb890..66105b1b3 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -679,7 +679,7 @@ internal string GetTypeFromMemberExpressionAstHelper(MemberExpressionAst memberA /// /// /// - internal Type GetTypeFromAnalysis(VariableExpressionAst varAst, Ast ast) + public Type GetTypeFromAnalysis(VariableExpressionAst varAst, Ast ast) { try { diff --git a/Rules/PossibleIncorrectComparisonWithNull.cs b/Rules/PossibleIncorrectComparisonWithNull.cs index da97c13f6..cc589f28a 100644 --- a/Rules/PossibleIncorrectComparisonWithNull.cs +++ b/Rules/PossibleIncorrectComparisonWithNull.cs @@ -11,6 +11,7 @@ // using System; +using System.Linq; using System.Collections.Generic; using System.Management.Automation.Language; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; @@ -33,17 +34,67 @@ public class PossibleIncorrectComparisonWithNull : IScriptRule { public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - IEnumerable binExpressionAsts = ast.FindAll(testAst => testAst is BinaryExpressionAst, true); + IEnumerable binExpressionAsts = ast.FindAll(testAst => testAst is BinaryExpressionAst, false); - if (binExpressionAsts != null) { - foreach (BinaryExpressionAst binExpressionAst in binExpressionAsts) { - if ((binExpressionAst.Operator.Equals(TokenKind.Equals) || binExpressionAst.Operator.Equals(TokenKind.Ceq) - || binExpressionAst.Operator.Equals(TokenKind.Cne) || binExpressionAst.Operator.Equals(TokenKind.Ine) || binExpressionAst.Operator.Equals(TokenKind.Ieq)) - && binExpressionAst.Right.Extent.Text.Equals("$null", StringComparison.OrdinalIgnoreCase)) { - yield return new DiagnosticRecord(Strings.PossibleIncorrectComparisonWithNullError, binExpressionAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); + foreach (BinaryExpressionAst binExpressionAst in binExpressionAsts) { + if ((binExpressionAst.Operator.Equals(TokenKind.Equals) || binExpressionAst.Operator.Equals(TokenKind.Ceq) + || binExpressionAst.Operator.Equals(TokenKind.Cne) || binExpressionAst.Operator.Equals(TokenKind.Ine) || binExpressionAst.Operator.Equals(TokenKind.Ieq)) + && binExpressionAst.Right.Extent.Text.Equals("$null", StringComparison.OrdinalIgnoreCase)) + { + if (IncorrectComparisonWithNull(binExpressionAst, ast)) + { + yield return new DiagnosticRecord(Strings.PossibleIncorrectComparisonWithNullError, binExpressionAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); } } } + + IEnumerable funcAsts = ast.FindAll(item => item is FunctionDefinitionAst, true).Union(ast.FindAll(item => item is FunctionMemberAst, true)); + foreach (Ast funcAst in funcAsts) + { + IEnumerable binAsts = funcAst.FindAll(item => item is BinaryExpressionAst, true); + foreach (BinaryExpressionAst binAst in binAsts) + { + if (IncorrectComparisonWithNull(binAst, funcAst)) + { + yield return new DiagnosticRecord(Strings.PossibleIncorrectComparisonWithNullError, binAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); + } + } + } + } + + private bool IncorrectComparisonWithNull(BinaryExpressionAst binExpressionAst, Ast ast) + { + if ((binExpressionAst.Operator.Equals(TokenKind.Equals) || binExpressionAst.Operator.Equals(TokenKind.Ceq) + || binExpressionAst.Operator.Equals(TokenKind.Cne) || binExpressionAst.Operator.Equals(TokenKind.Ine) || binExpressionAst.Operator.Equals(TokenKind.Ieq)) + && binExpressionAst.Right.Extent.Text.Equals("$null", StringComparison.OrdinalIgnoreCase)) + { + if (binExpressionAst.Left.StaticType.IsArray) + { + return true; + } + else if (binExpressionAst.Left is VariableExpressionAst) + { + // ignores if the variable is a special variable + if (!Helper.Instance.HasSpecialVars((binExpressionAst.Left as VariableExpressionAst).VariablePath.UserPath)) + { + Type lhsType = Helper.Instance.GetTypeFromAnalysis(binExpressionAst.Left as VariableExpressionAst, ast); + if (lhsType == null) + { + return true; + } + else if (lhsType.IsArray || lhsType == typeof(object) || lhsType == typeof(Undetermined) || lhsType == typeof(Unreached)) + { + return true; + } + } + } + else if (binExpressionAst.Left.StaticType == typeof(object)) + { + return true; + } + } + + return false; } /// diff --git a/Tests/Rules/PossibleIncorrectComparisonWithNull.ps1 b/Tests/Rules/PossibleIncorrectComparisonWithNull.ps1 index a2b520739..fbfffd93d 100644 --- a/Tests/Rules/PossibleIncorrectComparisonWithNull.ps1 +++ b/Tests/Rules/PossibleIncorrectComparisonWithNull.ps1 @@ -1,4 +1,23 @@ function CompareWithNull { if ($DebugPreference -eq $null) { } +} + +if (@("dfd", "eee") -eq $null) +{ +} + +if ($randomUninitializedVariable -eq $null) +{ +} + +function Test +{ + $b = "dd", "ddfd"; + if ($b -ceq $null) + { + if ("dd","ee" -eq $null) + { + } + } } \ No newline at end of file diff --git a/Tests/Rules/PossibleIncorrectComparisonWithNull.tests.ps1 b/Tests/Rules/PossibleIncorrectComparisonWithNull.tests.ps1 index 4ef850c55..bdcb40902 100644 --- a/Tests/Rules/PossibleIncorrectComparisonWithNull.tests.ps1 +++ b/Tests/Rules/PossibleIncorrectComparisonWithNull.tests.ps1 @@ -7,8 +7,8 @@ $noViolations = Invoke-ScriptAnalyzer $directory\PossibleIncorrectComparisonWith Describe "PossibleIncorrectComparisonWithNull" { Context "When there are violations" { - It "has 1 possible incorrect comparison with null violation" { - $violations.Count | Should Be 1 + It "has 4 possible incorrect comparison with null violation" { + $violations.Count | Should Be 4 } It "has the correct description message" { diff --git a/Tests/Rules/PossibleIncorrectComparisonWithNullNoViolations.ps1 b/Tests/Rules/PossibleIncorrectComparisonWithNullNoViolations.ps1 index 5ac741e0a..6a34d3c80 100644 --- a/Tests/Rules/PossibleIncorrectComparisonWithNullNoViolations.ps1 +++ b/Tests/Rules/PossibleIncorrectComparisonWithNullNoViolations.ps1 @@ -1,4 +1,15 @@ function CompareWithNull { if ($null -eq $DebugPreference) { } + if ($DebugPreference -eq $null) { + } +} + +$a = 3 + +if ($a -eq $null) +{ + if (3 -eq $null) + { + } } \ No newline at end of file From 21167ca75eb2ad9458b027f67d4028d8cf78b7ee Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Wed, 24 Jun 2015 14:38:40 -0700 Subject: [PATCH 21/25] Update CHANGELOG.MD --- CHANGELOG.MD | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.MD b/CHANGELOG.MD index 4bec47113..c818a08fd 100644 --- a/CHANGELOG.MD +++ b/CHANGELOG.MD @@ -1,3 +1,30 @@ +## Released v1.0.2 (June.25, 2015) +###Features: +- Perf improvements in the Engine to execute rules concurrently. + + +###Rules: +- New rule to validate the presence of deprecated module manifest fields. +- Removed PSAvoidTrapStatement rule from the builtin set – since this is not deprecated and using trap is a better choice in certain scenarios. + + +###Fixes: +- Verbose Message rule applies to only DSC cmdlet based resources. +- Multiple fixes to AvoidUninitializedVariable to work with non-mandatory parameters, fix in the flow graphs for throw statements; support for missing preference variables; support for automatic variables. +- PSAvoidUsingInternalsURLs to work with xPath expressions. +- UseSingularNouns rule to raise warnings for plural phrases. +- Added .gitignore to exclude certain files from being reported as untracked. +- Revisited severity for DSC rules. +- PSUseOutputTypeCorrectly rule not to get triggered for functions returning system.void type. +- PSAvoidDefaultTrueValueSwitchParameter to work with switch attribute when supplied as fully qualified. +- Ignore PSObject and PSCustomObject for UseOutputTypeCorrectly rule. +- Only raise NullComparisonRule if the RHS is an array or has unknown type. +- PSUseDeclaredVarsMoreThanAssignments rule to be raised for script variables and for setting the property of a variable. +- Support for using PSUseCmdletCorrectly rule when mandatory parameters are supplied in a splatted hashtable. +- AvoidUsingPlainTextForPassword rule to be raised only strings or object types. +- Fix for PositionalParameterUsed method (Helper.cs) uses unsafe method to exclude ForEach-Object and Where-Object. + + ## Released v1.0.1 (May.8, 2015) ###Features: - Integrated with waffle.io for Project Management. @@ -18,7 +45,6 @@ ##Released v1.0.0 on Apr.24, 2015 - ###Features: - Finalized three levels of Severity - Error/Warning/Information. - Improved PSScriptAnalyzer engine behavior: emits non-terminating errors (Ex: for failed ast parse) and continues rule application when running on multiple scripts. From 610b12c3ac6e241ded00cec2e4643a2bb926e4ae Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Wed, 24 Jun 2015 14:39:04 -0700 Subject: [PATCH 22/25] Update PSScriptAnalyzer.psd1 --- Engine/PSScriptAnalyzer.psd1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Engine/PSScriptAnalyzer.psd1 b/Engine/PSScriptAnalyzer.psd1 index 2057d6ca3..8d26cea21 100644 --- a/Engine/PSScriptAnalyzer.psd1 +++ b/Engine/PSScriptAnalyzer.psd1 @@ -11,7 +11,7 @@ Author = 'Microsoft Corporation' RootModule = 'Microsoft.Windows.PowerShell.ScriptAnalyzer.dll' # Version number of this module. -ModuleVersion = '1.0.1' +ModuleVersion = '1.0.2' # ID used to uniquely identify this module GUID = '324fc715-36bf-4aee-8e58-72e9b4a08ad9' From 2938f244a4610407792c18fa2702b514fc6b9f48 Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Wed, 24 Jun 2015 15:45:24 -0700 Subject: [PATCH 23/25] Update CHANGELOG.MD --- CHANGELOG.MD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.MD b/CHANGELOG.MD index c818a08fd..b81273723 100644 --- a/CHANGELOG.MD +++ b/CHANGELOG.MD @@ -1,4 +1,4 @@ -## Released v1.0.2 (June.25, 2015) +## Released v1.0.2 (June.24, 2015) ###Features: - Perf improvements in the Engine to execute rules concurrently. From f30c7551dd12b06f0365df6e77d0334b1902397a Mon Sep 17 00:00:00 2001 From: Yuting Chen Date: Thu, 25 Jun 2015 13:42:33 -0700 Subject: [PATCH 24/25] Fix false positive for UsingInternalURL Add check if -replace is used and check if invocation expression is [String] or [System.String] --- Rules/AvoidUsingInternalURLs.cs | 19 +++++++++++++++++-- .../AvoidUsingInternalURLsNoViolations.ps1 | 4 +++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/Rules/AvoidUsingInternalURLs.cs b/Rules/AvoidUsingInternalURLs.cs index 99844de4f..90c880909 100644 --- a/Rules/AvoidUsingInternalURLs.cs +++ b/Rules/AvoidUsingInternalURLs.cs @@ -11,8 +11,10 @@ // using System; +using System.Collections; using System.Collections.Generic; using System.Linq; +using System.Linq.Expressions; using System.Management.Automation.Language; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; using System.ComponentModel.Composition; @@ -44,8 +46,19 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { foreach (StringConstantExpressionAst expressionAst in expressionAsts) { - //Check if XPath is used. If XPath is used, then we don't throw warnings. + Ast parentAst = expressionAst.Parent; + //Check if -replace is used, if it is string replace, we don't throw warnings. + Ast grandParentAst = parentAst.Parent; + if (grandParentAst is BinaryExpressionAst) + { + if ((grandParentAst as BinaryExpressionAst).Operator.Equals(TokenKind.Ireplace)) + { + continue; + } + } + + //Check if XPath is used. If XPath is used, then we don't throw warnings. if (parentAst is InvokeMemberExpressionAst) { InvokeMemberExpressionAst invocation = parentAst as InvokeMemberExpressionAst; @@ -55,7 +68,9 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) String.Equals(invocation.Member.ToString(), "SelectNodes",StringComparison.OrdinalIgnoreCase) || String.Equals(invocation.Member.ToString(), "Select", StringComparison.OrdinalIgnoreCase) || String.Equals(invocation.Member.ToString(), "Evaluate",StringComparison.OrdinalIgnoreCase) || - String.Equals(invocation.Member.ToString(), "Matches",StringComparison.OrdinalIgnoreCase)) + String.Equals(invocation.Member.ToString(), "Matches",StringComparison.OrdinalIgnoreCase) || + String.Equals(invocation.Expression.ToString(), "[System.String]",StringComparison.OrdinalIgnoreCase) || + String.Equals(invocation.Expression.ToString(), "[String]", StringComparison.OrdinalIgnoreCase)) { continue; } diff --git a/Tests/Rules/AvoidUsingInternalURLsNoViolations.ps1 b/Tests/Rules/AvoidUsingInternalURLsNoViolations.ps1 index 337bb0832..1d28e5baa 100644 --- a/Tests/Rules/AvoidUsingInternalURLsNoViolations.ps1 +++ b/Tests/Rules/AvoidUsingInternalURLsNoViolations.ps1 @@ -5,4 +5,6 @@ function Test { $filesNode = $infoXml.SelectSingleNode("//files") } -$sd = "O:BAG:BAD:(A;;0x800;;;WD)(A;;0x120fff;;;SY)(A;;0x120fff;;;LS)(A;;0x120fff;;;NS)(A;;0x120fff;;;BA)(A;;0xee5;;;LU)(A;;LC;;;MU)(A;;0x800;;;AG)" \ No newline at end of file +$sd = "O:BAG:BAD:(A;;0x800;;;WD)(A;;0x120fff;;;SY)(A;;0x120fff;;;LS)(A;;0x120fff;;;NS)(A;;0x120fff;;;BA)(A;;0xee5;;;LU)(A;;LC;;;MU)(A;;0x800;;;AG)" +$msg = [System.String]::Format("1:{0}", "test") + $internalId = $Id -replace '^([^/])','/$1' -as $Id.GetType() \ No newline at end of file From 1b6c3a25ceba1f03405c4ba7c096f2aceede49f6 Mon Sep 17 00:00:00 2001 From: "Raghu Shantha [MSFT]" Date: Thu, 25 Jun 2015 15:13:46 -0700 Subject: [PATCH 25/25] WriteVerbose only when analyzing valid powershell files --- Engine/Commands/InvokeScriptAnalyzerCommand.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index 8936e2cd3..2fa611404 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -255,12 +255,12 @@ private void ProcessPath(string path) } } else if (File.Exists(path)) - { - WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseFileMessage, path)); + { if ((path.Length > ps1Suffix.Length && path.Substring(path.Length - ps1Suffix.Length).Equals(ps1Suffix, StringComparison.OrdinalIgnoreCase)) || (path.Length > psm1Suffix.Length && path.Substring(path.Length - psm1Suffix.Length).Equals(psm1Suffix, StringComparison.OrdinalIgnoreCase)) || (path.Length > psd1Suffix.Length && path.Substring(path.Length - psd1Suffix.Length).Equals(psd1Suffix, StringComparison.OrdinalIgnoreCase))) { + WriteVerbose(string.Format(CultureInfo.CurrentCulture, Strings.VerboseFileMessage, path)); AnalyzeFile(path); } }