From 3139f057aa6b2500edd29419bf03433d93eed630 Mon Sep 17 00:00:00 2001 From: lipingma Date: Fri, 16 Jul 2021 16:25:48 +0800 Subject: [PATCH 1/2] add-suppressions --- .../Commands/InvokeScriptAnalyzerCommand.cs | 15 +- Engine/Generic/RuleSuppression.cs | 19 + Engine/Generic/SuppressedRecord.cs | 21 +- Engine/Helper.cs | 151 ++++---- Engine/ScriptAnalyzer.cs | 17 +- Engine/ScriptAnalyzer.format.ps1xml | 7 + Engine/ScriptAnalyzer.types.ps1xml | 9 +- README.md | 5 +- Tests/Engine/LibraryUsage.tests.ps1 | 6 +- Tests/Engine/RuleSuppression.tests.ps1 | 338 ++++++++++++++++-- docs/markdown/Invoke-ScriptAnalyzer.md | 82 ++++- 11 files changed, 550 insertions(+), 120 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index f5598d5bd..46eac3c8a 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -167,6 +167,18 @@ public SwitchParameter SuppressedOnly } private bool suppressedOnly; + /// + /// ShowAll: Show the suppressed and non-suppressed message + /// + [Parameter(Mandatory = false)] + [SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")] + public SwitchParameter IncludeSuppressions + { + get { return includeSuppressions; } + set { includeSuppressions = value; } + } + private bool includeSuppressions; + /// /// Resolves rule violations automatically where possible. /// @@ -341,7 +353,8 @@ protected override void BeginProcessing() this.excludeRule, this.severity, combRulePaths == null || combIncludeDefaultRules, - this.suppressedOnly); + this.suppressedOnly, + this.includeSuppressions); } /// diff --git a/Engine/Generic/RuleSuppression.cs b/Engine/Generic/RuleSuppression.cs index d8a41f974..25303f2c7 100644 --- a/Engine/Generic/RuleSuppression.cs +++ b/Engine/Generic/RuleSuppression.cs @@ -97,6 +97,15 @@ public string Justification set; } + /// + /// Returns the kind of the suppression + /// + public SuppressionKind Kind + { + get; + set; + } + private static HashSet scopeSet; /// @@ -376,5 +385,15 @@ public static List GetSuppressions(IEnumerable at return result; } + + /// + /// A string that indicates where the suppression is persisted. + /// + public enum SuppressionKind + { + None, + InSource, + External + } } } diff --git a/Engine/Generic/SuppressedRecord.cs b/Engine/Generic/SuppressedRecord.cs index ee50ea48b..c91aafd43 100644 --- a/Engine/Generic/SuppressedRecord.cs +++ b/Engine/Generic/SuppressedRecord.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System.Collections.Generic; + namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic { /// @@ -11,20 +13,29 @@ public class SuppressedRecord : DiagnosticRecord /// /// The rule suppression of this record /// - public RuleSuppression Suppression + public IList Suppressions { - get; - set; + get + { + if (suppressions == null) suppressions = new List(); + + return suppressions; + } + set + { + suppressions = value; + } } + private IList suppressions; /// /// Creates a suppressed record based on a diagnostic record and the rule suppression /// /// /// - public SuppressedRecord(DiagnosticRecord record, RuleSuppression suppression) + public SuppressedRecord(DiagnosticRecord record, IList suppressions) { - Suppression = suppression; + Suppressions = suppressions; if (record != null) { RuleName = record.RuleName; diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 743cb4d68..30b6088f3 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -1348,50 +1348,47 @@ public Tuple, List> SuppressRule( } List ruleSuppressions = ruleSuppressionsDict[ruleName]; - var offsetArr = GetOffsetArray(diagnostics); - int recordIndex = 0; - int startRecord = 0; - bool[] suppressed = new bool[diagnostics.Count]; - foreach (RuleSuppression ruleSuppression in ruleSuppressions) - { - int suppressionCount = 0; - while (startRecord < diagnostics.Count - // && diagnostics[startRecord].Extent.StartOffset < ruleSuppression.StartOffset) - // && diagnostics[startRecord].Extent.StartLineNumber < ruleSuppression.st) - && offsetArr[startRecord] != null && offsetArr[startRecord].Item1 < ruleSuppression.StartOffset) - { - startRecord += 1; - } + bool[] applied = new bool[ruleSuppressions.Count]; - // at this point, start offset of startRecord is greater or equals to rulesuppression.startoffset - recordIndex = startRecord; - - while (recordIndex < diagnostics.Count) + foreach(DiagnosticRecord diagnostic in diagnostics) + { + var curOffset = GetOffsetArray(diagnostic); + List suppressions = new List(); + for (int ruleSuppressionIndex = 0; ruleSuppressionIndex < ruleSuppressions.Count; ruleSuppressionIndex++) { - DiagnosticRecord record = diagnostics[recordIndex]; - var curOffset = offsetArr[recordIndex]; - - //if (record.Extent.EndOffset > ruleSuppression.EndOffset) - if (curOffset != null && curOffset.Item2 > ruleSuppression.EndOffset) + RuleSuppression ruleSuppression = ruleSuppressions[ruleSuppressionIndex]; + //if (diagnostic.Extent.StartOffset < ruleSuppression.StartOffset||diagnostic.Extent.EndOffset > ruleSuppression.EndOffset) + if (curOffset != null && (curOffset.Item1 < ruleSuppression.StartOffset || curOffset.Item2 > ruleSuppression.EndOffset)) { - break; + continue; } // we suppress if there is no suppression id or if there is suppression id and it matches - if (string.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID) - || (!String.IsNullOrWhiteSpace(record.RuleSuppressionID) && - string.Equals(ruleSuppression.RuleSuppressionID, record.RuleSuppressionID, StringComparison.OrdinalIgnoreCase))) + if (string.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID) || + (!String.IsNullOrWhiteSpace(diagnostic.RuleSuppressionID) && + string.Equals(ruleSuppression.RuleSuppressionID, diagnostic.RuleSuppressionID, StringComparison.OrdinalIgnoreCase))) { - suppressed[recordIndex] = true; - suppressedRecords.Add(new SuppressedRecord(record, ruleSuppression)); - suppressionCount += 1; + ruleSuppression.Kind = RuleSuppression.SuppressionKind.InSource; + suppressions.Add(ruleSuppression); + applied[ruleSuppressionIndex] = true; } + } - recordIndex += 1; + if (suppressions.Count() != 0) + { + suppressedRecords.Add(new SuppressedRecord(diagnostic, suppressions)); } + else + { + unSuppressedRecords.Add(diagnostic); + } + } - // If we cannot found any error but the rulesuppression has a rulesuppressionid then it must be used wrongly - if (!String.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID) && suppressionCount == 0) + // If we cannot found any error but the rulesuppression has a rulesuppressionid then it must be used wrongly + for (int ruleSuppressionIndex = 0; ruleSuppressionIndex < ruleSuppressions.Count; ruleSuppressionIndex++) + { + RuleSuppression ruleSuppression = ruleSuppressions[ruleSuppressionIndex]; + if ((!String.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID)) && !applied[ruleSuppressionIndex]) { // checks whether are given a string or a file path if (String.IsNullOrWhiteSpace(diagnostics.First().Extent.File)) @@ -1409,70 +1406,56 @@ public Tuple, List> SuppressRule( } } - for (int i = 0; i < suppressed.Length; i += 1) - { - if (!suppressed[i]) - { - unSuppressedRecords.Add(diagnostics[i]); - } - } - return result; } - private Tuple[] GetOffsetArray(List diagnostics) + private Tuple GetOffsetArray(DiagnosticRecord diagnostic) { Func> GetTuple = (x, y) => new Tuple(x, y); - Func> GetDefaultTuple = () => GetTuple(0, 0); - var offsets = new Tuple[diagnostics.Count]; - for (int k = 0; k < diagnostics.Count; k++) + var offset = new Tuple(0, 0); + var ext = diagnostic.Extent; + if (ext == null) { - var ext = diagnostics[k].Extent; - if (ext == null) + return null; + } + if (ext.StartOffset == 0 && ext.EndOffset == 0) + { + // check if line and column number correspond to 0 offsets + if (ext.StartLineNumber == 1 + && ext.StartColumnNumber == 1 + && ext.EndLineNumber == 1 + && ext.EndColumnNumber == 1) { - continue; + return offset; } - if (ext.StartOffset == 0 && ext.EndOffset == 0) + // created using the ScriptExtent constructor, which sets + // StartOffset and EndOffset to 0 + // find the token the corresponding start line and column number + var startToken = Tokens.Where(x + => x.Extent.StartLineNumber == ext.StartLineNumber + && x.Extent.StartColumnNumber == ext.StartColumnNumber) + .FirstOrDefault(); + if (startToken == null) { - // check if line and column number correspond to 0 offsets - if (ext.StartLineNumber == 1 - && ext.StartColumnNumber == 1 - && ext.EndLineNumber == 1 - && ext.EndColumnNumber == 1) - { - offsets[k] = GetDefaultTuple(); - continue; - } - // created using the ScriptExtent constructor, which sets - // StartOffset and EndOffset to 0 - // find the token the corresponding start line and column number - var startToken = Tokens.Where(x - => x.Extent.StartLineNumber == ext.StartLineNumber - && x.Extent.StartColumnNumber == ext.StartColumnNumber) - .FirstOrDefault(); - if (startToken == null) - { - offsets[k] = GetDefaultTuple(); - continue; - } - var endToken = Tokens.Where(x - => x.Extent.EndLineNumber == ext.EndLineNumber - && x.Extent.EndColumnNumber == ext.EndColumnNumber) - .FirstOrDefault(); - if (endToken == null) - { - offsets[k] = GetDefaultTuple(); - continue; - } - offsets[k] = GetTuple(startToken.Extent.StartOffset, endToken.Extent.EndOffset); + return offset; } - else + var endToken = Tokens.Where(x + => x.Extent.EndLineNumber == ext.EndLineNumber + && x.Extent.EndColumnNumber == ext.EndColumnNumber) + .FirstOrDefault(); + if (endToken == null) { - // Extent has valid offsets - offsets[k] = GetTuple(ext.StartOffset, ext.EndOffset); + return offset; } + offset = GetTuple(startToken.Extent.StartOffset, endToken.Extent.EndOffset); + } + else + { + // Extent has valid offsets + offset = GetTuple(ext.StartOffset, ext.EndOffset); } - return offsets; + + return offset; } public static string[] ProcessCustomRulePaths(string[] rulePaths, SessionState sessionState, bool recurse = false) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index ffe277375..be0ebbfab 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -42,6 +42,7 @@ public sealed class ScriptAnalyzer List includeRegexList; List excludeRegexList; bool suppressedOnly; + bool includeSuppressions; #if !PSV3 ModuleDependencyHandler moduleHandler; #endif @@ -118,7 +119,8 @@ internal void Initialize( string[] excludeRuleNames = null, string[] severity = null, bool includeDefaultRules = false, - bool suppressedOnly = false) + bool suppressedOnly = false, + bool includeSuppressions = false) where TCmdlet : PSCmdlet, IOutputWriter { if (cmdlet == null) @@ -135,7 +137,8 @@ internal void Initialize( excludeRuleNames, severity, includeDefaultRules, - suppressedOnly); + suppressedOnly, + includeSuppressions); } /// @@ -150,6 +153,7 @@ public void Initialize( string[] severity = null, bool includeDefaultRules = false, bool suppressedOnly = false, + bool includeSuppressions = false, string profile = null) { if (runspace == null) @@ -174,6 +178,7 @@ public void Initialize( severity, includeDefaultRules, suppressedOnly, + includeSuppressions, profile); } @@ -188,6 +193,7 @@ public void CleanUp() includeRegexList = null; excludeRegexList = null; suppressedOnly = false; + includeSuppressions = false; } /// @@ -672,6 +678,7 @@ private void Initialize( string[] severity, bool includeDefaultRules = false, bool suppressedOnly = false, + bool includeSuppressions = false, string profile = null) { if (outputWriter == null) @@ -731,6 +738,7 @@ private void Initialize( } this.suppressedOnly = suppressedOnly; + this.includeSuppressions = includeSuppressions; this.includeRegexList = new List(); this.excludeRegexList = new List(); @@ -2323,9 +2331,12 @@ public IEnumerable AnalyzeSyntaxTree( // Need to reverse the concurrentbag to ensure that results are sorted in the increasing order of line numbers IEnumerable diagnosticsList = diagnostics.Reverse(); + IEnumerable suppressedList = suppressed.OfType(); + IEnumerable allRecordsList = diagnosticsList.Concat(suppressedList); return this.suppressedOnly ? - suppressed.OfType() : + suppressedList : this.includeSuppressions ? + allRecordsList : diagnosticsList; } } diff --git a/Engine/ScriptAnalyzer.format.ps1xml b/Engine/ScriptAnalyzer.format.ps1xml index e6d78fc28..244403b35 100644 --- a/Engine/ScriptAnalyzer.format.ps1xml +++ b/Engine/ScriptAnalyzer.format.ps1xml @@ -81,6 +81,10 @@ 60 + + 10 + + @@ -101,6 +105,9 @@ Justification + + Kind + diff --git a/Engine/ScriptAnalyzer.types.ps1xml b/Engine/ScriptAnalyzer.types.ps1xml index da989657e..852ee05fa 100644 --- a/Engine/ScriptAnalyzer.types.ps1xml +++ b/Engine/ScriptAnalyzer.types.ps1xml @@ -50,7 +50,13 @@ Justification - $this.Suppression.Justification + $this.Suppressions.Justification + + + + Kind + + $this.Suppressions.Kind @@ -64,6 +70,7 @@ Line Column Justification + Kind diff --git a/README.md b/README.md index 56c04c1c1..e080de252 100644 --- a/README.md +++ b/README.md @@ -54,9 +54,9 @@ Usage ``` PowerShell Get-ScriptAnalyzerRule [-CustomRulePath ] [-RecurseCustomRulePath] [-Name ] [-Severity ] [] -Invoke-ScriptAnalyzer [-Path] [-CustomRulePath ] [-RecurseCustomRulePath] [-ExcludeRule ] [-IncludeDefaultRules] [-IncludeRule ] [-Severity ] [-Recurse] [-SuppressedOnly] [-Fix] [-EnableExit] [-ReportSummary] [-Settings ] [-SaveDscDependency] [] +Invoke-ScriptAnalyzer [-Path] [-CustomRulePath ] [-RecurseCustomRulePath] [-ExcludeRule ] [-IncludeDefaultRules] [-IncludeRule ] [-Severity ] [-Recurse] [-SuppressedOnly] [-IncludeSuppressions] [-Fix] [-EnableExit] [-ReportSummary] [-Settings ] [-SaveDscDependency] [] -Invoke-ScriptAnalyzer [-ScriptDefinition] [-CustomRulePath ] [-RecurseCustomRulePath] [-ExcludeRule ] [-IncludeDefaultRules] [-IncludeRule ] [-Severity ] [-Recurse] [-SuppressedOnly] [-EnableExit] [-ReportSummary] [-Settings ] [-SaveDscDependency] [] +Invoke-ScriptAnalyzer [-ScriptDefinition] [-CustomRulePath ] [-RecurseCustomRulePath] [-ExcludeRule ] [-IncludeDefaultRules] [-IncludeRule ] [-Severity ] [-Recurse] [-SuppressedOnly] [-IncludeSuppressions] [-EnableExit] [-ReportSummary] [-Settings ] [-SaveDscDependency] [] Invoke-Formatter [-ScriptDefinition] [[-Settings] ] [[-Range] ] [] ``` @@ -458,6 +458,7 @@ Microsoft.Windows.PowerShell.ScriptAnalyzer.IOutputWriter outputWriter, [string[] excludeRuleNames = null], [string[] severity = null], [bool suppressedOnly = false], +[bool IncludeSuppressions = false], [string profile = null]) public System.Collections.Generic.IEnumerable AnalyzePath(string path, diff --git a/Tests/Engine/LibraryUsage.tests.ps1 b/Tests/Engine/LibraryUsage.tests.ps1 index 4dbc34c43..3797da112 100644 --- a/Tests/Engine/LibraryUsage.tests.ps1 +++ b/Tests/Engine/LibraryUsage.tests.ps1 @@ -41,6 +41,9 @@ Describe 'Library Usage' -Skip:$IsCoreCLR { [Parameter(Mandatory = $false)] [switch] $SuppressedOnly, + + [Parameter(Mandatory = $false)] + [switch] $IncludeSuppressions, [Parameter(Mandatory = $false)] [switch] $Fix, @@ -72,7 +75,8 @@ Describe 'Library Usage' -Skip:$IsCoreCLR { $ExcludeRule, $Severity, $IncludeDefaultRules.IsPresent, - $SuppressedOnly.IsPresent + $SuppressedOnly.IsPresent, + $IncludeSuppressions.IsPresent ); if ($PSCmdlet.ParameterSetName -eq "File") diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index 49e2570db..58ab8edd0 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -55,14 +55,14 @@ Describe "RuleSuppressionWithoutScope" { } It "Suppresses rule with extent created using ScriptExtent constructor" { - Invoke-ScriptAnalyzer ` - -ScriptDefinition $ruleSuppressionAvoidUsernameAndPassword ` - -IncludeRule "PSAvoidUsingUserNameAndPassWordParams" ` - -OutVariable ruleViolations ` - -SuppressedOnly - $ruleViolations.Count | Should -Be 1 - } + $suppressed = Invoke-ScriptAnalyzer -ScriptDefinition $ruleSuppressionAvoidUsernameAndPassword -IncludeRule "PSAvoidUsingUserNameAndPassWordParams" -SuppressedOnly + $suppressed.Count | Should -Be 1 + } + It "All PSAvoidUsingUserNameAndPassWordParams violations with extent created using ScriptExtent constructor" { + $allrecords = Invoke-ScriptAnalyzer -ScriptDefinition $ruleSuppressionAvoidUsernameAndPassword -IncludeRule "PSAvoidUsingUserNameAndPassWordParams" -IncludeSuppressions + $allrecords.Count | Should -Be 1 + } } Context "Script" { @@ -93,12 +93,80 @@ function SuppressPwdParam() ) } '@ - Invoke-ScriptAnalyzer ` - -ScriptDefinition $ruleSuppressionIdAvoidPlainTextPassword ` - -IncludeRule "PSAvoidUsingPlainTextForPassword" ` - -OutVariable ruleViolations ` - -SuppressedOnly - $ruleViolations.Count | Should -Be 1 + $suppressed = Invoke-ScriptAnalyzer -ScriptDefinition $ruleSuppressionIdAvoidPlainTextPassword -IncludeRule "PSAvoidUsingPlainTextForPassword" -SuppressedOnly + $suppressed.Count | Should -Be 1 + } + + It "All PSAvoidUsingPlainTextForPassword violations including suppresses violation for the given ID" { + $ruleSuppressionIdAvoidPlainTextPassword1 = @' +function SuppressPwdParam() +{ + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingPlainTextForPassword", "password1")] + param( + [string] $password1 + ) +} +'@ + $allrecords1 = Invoke-ScriptAnalyzer -ScriptDefinition $ruleSuppressionIdAvoidPlainTextPassword1 -IncludeRule "PSAvoidUsingPlainTextForPassword" -IncludeSuppressions + $allrecords1.Count | Should -Be 1 + + $ruleSuppressionIdAvoidPlainTextPassword2 = @' +function SuppressPwdParam() +{ + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingPlainTextForPassword", "password1")] + param( + [string] $password1, + [string] $password2 + ) +} +'@ + $allrecords2 = Invoke-ScriptAnalyzer -ScriptDefinition $ruleSuppressionIdAvoidPlainTextPassword2 -IncludeRule "PSAvoidUsingPlainTextForPassword" -IncludeSuppressions + $allrecords2.Count | Should -Be 2 + } + + It "PSAvoidUsingPlainTextForPassword violation for the given ID with sereval suppressions" { + $ruleSuppressionIdAvoidPlainTextPassword = @' +function SuppressPwdParam() +{ + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingPlainTextForPassword", "password1",Justification="a")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingPlainTextForPassword", "password1",Justification="b")] + param( + [string] $password1, + [string] $password2 + ) +} +'@ + $suppressed = Invoke-ScriptAnalyzer -ScriptDefinition $ruleSuppressionIdAvoidPlainTextPassword -IncludeRule "PSAvoidUsingPlainTextForPassword" -SuppressedOnly + $suppressed.Count | Should -Be 1 + } + + It "All PSAvoidUsingPlainTextForPassword violations including those for the given ID and suppressed several times" { + $ruleSuppressionIdAvoidPlainTextPassword1 = @' +function SuppressPwdParam() +{ + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingPlainTextForPassword", "password1",Justification="a")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingPlainTextForPassword", "password1",Justification="b")] + param( + [string] $password1 + ) +} +'@ + $allrecords1 = Invoke-ScriptAnalyzer -ScriptDefinition $ruleSuppressionIdAvoidPlainTextPassword1 -IncludeRule "PSAvoidUsingPlainTextForPassword" -IncludeSuppressions + $allrecords1.Count | Should -Be 1 + + $ruleSuppressionIdAvoidPlainTextPassword2 = @' +function SuppressPwdParam() +{ + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingPlainTextForPassword", "password1",Justification="a")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingPlainTextForPassword", "password1",Justification="b")] + param( + [string] $password1, + [string] $password2 + ) +} +'@ + $allrecords2 = Invoke-ScriptAnalyzer -ScriptDefinition $ruleSuppressionIdAvoidPlainTextPassword2 -IncludeRule "PSAvoidUsingPlainTextForPassword" -IncludeSuppressions + $allrecords2.Count | Should -Be 2 } } @@ -107,6 +175,11 @@ function SuppressPwdParam() $suppressedRule = Invoke-ScriptAnalyzer -ScriptDefinition $ruleSuppressionInConfiguration -SuppressedOnly $suppressedRule.Count | Should -Be 1 } + + It "All rule violations" -skip:($IsLinux -or $IsMacOS -or ($PSVersionTable.PSVersion.Major -lt 5)) { + $allrecords = Invoke-ScriptAnalyzer -ScriptDefinition $ruleSuppressionInConfiguration -IncludeSuppressions + $allrecords.Count | Should -Be 3 + } } Context "Bad Rule Suppression" -Skip:$testingLibraryUsage { @@ -124,12 +197,40 @@ function SuppressPwdParam() param() # without the param block, powershell parser throws up! Write-Host "write-host" '@ - Invoke-ScriptAnalyzer ` - -ScriptDefinition $externalRuleSuppression ` - -CustomRulePath (Join-Path $PSScriptRoot "CommunityAnalyzerRules") ` - -OutVariable ruleViolations ` - -SuppressedOnly - $ruleViolations.Count | Should -Be 1 + $suppressed = Invoke-ScriptAnalyzer -ScriptDefinition $externalRuleSuppression -CustomRulePath (Join-Path $PSScriptRoot "CommunityAnalyzerRules") -SuppressedOnly + $suppressed.Count | Should -Be 1 + } + + It "Violations of an external ast rule including suppresses violation" { + $externalRuleSuppression = @' + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('CommunityAnalyzerRules\Measure-WriteHost','')] + param() # without the param block, powershell parser throws up! + Write-Host "write-host" +'@ + $allrecords = Invoke-ScriptAnalyzer -ScriptDefinition $externalRuleSuppression -CustomRulePath (Join-Path $PSScriptRoot "CommunityAnalyzerRules") -IncludeSuppressions + $allrecords.Count | Should -Be 1 + } + + It "Violation of an external ast rule with sereval suppressions" { + $externalRuleSuppression = @' + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('CommunityAnalyzerRules\Measure-WriteHost','',Justification="a")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('CommunityAnalyzerRules\Measure-WriteHost','',Justification="b")] + param() # without the param block, powershell parser throws up! + Write-Host "write-host" +'@ + $suppressed = Invoke-ScriptAnalyzer -ScriptDefinition $externalRuleSuppression -CustomRulePath (Join-Path $PSScriptRoot "CommunityAnalyzerRules") -SuppressedOnly + $suppressed.Count | Should -Be 1 + } + + It "Violations of an external ast rule including violation suppressed sereval times" { + $externalRuleSuppression = @' + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('CommunityAnalyzerRules\Measure-WriteHost','',Justification="a")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('CommunityAnalyzerRules\Measure-WriteHost','',Justification="b")] + param() # without the param block, powershell parser throws up! + Write-Host "write-host" +'@ + $allrecords = Invoke-ScriptAnalyzer -ScriptDefinition $externalRuleSuppression -CustomRulePath (Join-Path $PSScriptRoot "CommunityAnalyzerRules") -IncludeSuppressions + $allrecords.Count | Should -Be 1 } } } @@ -168,6 +269,79 @@ Describe "RuleSuppressionWithScope" { $suppressed = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule 'PSAvoidUsingWriteHost' -SuppressedOnly $suppressed.Count | Should -Be 2 } + It "all objects violating PSAvoidUsingWriteHost including suppresses objects that match the regular expression" { + $scriptDef = @' + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='start-ba[rz]')] + param() + function start-foo { + write-host "start-foo" + } + + function start-bar { + write-host "start-bar" + } + + function start-baz { + write-host "start-baz" + } + + function start-bam { + write-host "start-bam" + } +'@ + $allrecords = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule 'PSAvoidUsingWriteHost' -IncludeSuppressions + $allrecords.Count | Should -Be 4 + } + + It "objects that match the regular expression with several suppressions" { + $scriptDef = @' + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='start-ba[rz]',Justification="a")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='start-bar',Justification="b")] + param() + function start-foo { + write-host "start-foo" + } + + function start-bar { + write-host "start-bar" + } + + function start-baz { + write-host "start-baz" + } + + function start-bam { + write-host "start-bam" + } +'@ + $suppressed = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule 'PSAvoidUsingWriteHost' -SuppressedOnly + $suppressed.Count | Should -Be 2 + } + + It "all objects violating PSAvoidUsingWriteHost including those that match the regular expression and suppressed several times" { + $scriptDef = @' + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='start-ba[rz]',Justification="a")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='start-bar',Justification="b")] + param() + function start-foo { + write-host "start-foo" + } + + function start-bar { + write-host "start-bar" + } + + function start-baz { + write-host "start-baz" + } + + function start-bam { + write-host "start-bam" + } +'@ + $allrecords = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule 'PSAvoidUsingWriteHost' -IncludeSuppressions + $allrecords.Count | Should -Be 4 + } It "suppresses objects that match glob pattern with glob in the end" { $scriptDef = @' @@ -189,7 +363,69 @@ Describe "RuleSuppressionWithScope" { $suppressed.Count | Should -Be 2 } - It "suppresses objects that match glob pattern with glob in the begining" { + It "all objects violating PSAvoidUsingWriteHost including suppresses objects that match glob pattern with glob in the end" { + $scriptDef = @' + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='start-*')] + param() + function start-foo { + write-host "start-foo" + } + + function start-bar { + write-host "start-bar" + } + + function stop-bar { + write-host "stop-bar" + } +'@ + $allrecords = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule 'PSAvoidUsingWriteHost' -IncludeSuppressions + $allrecords.Count | Should -Be 3 + } + + It "objects that match glob pattern with glob in the end and suppressed several times" { + $scriptDef = @' + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='start-*',Justification="a")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='start-*',Justification="b")] + param() + function start-foo { + write-host "start-foo" + } + + function start-bar { + write-host "start-bar" + } + + function stop-bar { + write-host "stop-bar" + } +'@ + $suppressed = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule 'PSAvoidUsingWriteHost' -SuppressedOnly + $suppressed.Count | Should -Be 2 + } + + It "all objects violating PSAvoidUsingWriteHost including those that match glob pattern with glob in the end and suppressed several times" { + $scriptDef = @' + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='start-*',Justification="a")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='start-*',Justification="b")] + param() + function start-foo { + write-host "start-foo" + } + + function start-bar { + write-host "start-bar" + } + + function stop-bar { + write-host "stop-bar" + } +'@ + $allrecords = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule 'PSAvoidUsingWriteHost' -IncludeSuppressions + $allrecords.Count | Should -Be 3 + } + + It "suppresses objects that match glob pattern with glob in the beginning" { $scriptDef = @' [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='*-bar')] param() @@ -208,5 +444,67 @@ Describe "RuleSuppressionWithScope" { $suppressed = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule 'PSAvoidUsingWriteHost' -SuppressedOnly $suppressed.Count | Should -Be 1 } + + It "all objects violating PSAvoidUsingWriteHost including suppresses objects that match glob pattern with glob in the beginning" { + $scriptDef = @' + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='*-bar')] + param() + function start-foo { + write-host "start-foo" + } + + function start-bar { + write-host "start-bar" + } + + function start-baz { + write-host "start-baz" + } +'@ + $allrecords = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule 'PSAvoidUsingWriteHost' -IncludeSuppressions + $allrecords.Count | Should -Be 3 + } + + It "objects that match glob pattern with glob in the beginning and suppressed several times" { + $scriptDef = @' + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='*-bar',Justification="a")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='*-bar',Justification="b")] + param() + function start-foo { + write-host "start-foo" + } + + function start-bar { + write-host "start-bar" + } + + function start-baz { + write-host "start-baz" + } +'@ + $suppressed = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule 'PSAvoidUsingWriteHost' -SuppressedOnly + $suppressed.Count | Should -Be 1 + } + + It "all objects violating PSAvoidUsingWriteHost including those that match glob pattern with glob in the beginning and suppressed several times" { + $scriptDef = @' + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='*-bar',Justification="a")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Scope='Function', Target='*-bar',Justification="b")] + param() + function start-foo { + write-host "start-foo" + } + + function start-bar { + write-host "start-bar" + } + + function start-baz { + write-host "start-baz" + } +'@ + $allrecords = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDef -IncludeRule 'PSAvoidUsingWriteHost' -IncludeSuppressions + $allrecords.Count | Should -Be 3 + } } } diff --git a/docs/markdown/Invoke-ScriptAnalyzer.md b/docs/markdown/Invoke-ScriptAnalyzer.md index 4150da099..2e403062a 100644 --- a/docs/markdown/Invoke-ScriptAnalyzer.md +++ b/docs/markdown/Invoke-ScriptAnalyzer.md @@ -12,14 +12,14 @@ Evaluates a script or module based on selected best practice rules ### UNNAMED_PARAMETER_SET_1 ``` Invoke-ScriptAnalyzer [-Path] [-CustomRulePath ] [-RecurseCustomRulePath] - [-ExcludeRule ] [-IncludeRule ] [-Severity ] [-Recurse] [-SuppressedOnly] [-Fix] [-EnableExit] [-ReportSummary] + [-ExcludeRule ] [-IncludeRule ] [-Severity ] [-Recurse] [-SuppressedOnly] [-IncludeSuppressions] [-Fix] [-EnableExit] [-ReportSummary] [-Settings ] ``` ### UNNAMED_PARAMETER_SET_2 ``` Invoke-ScriptAnalyzer [-ScriptDefinition] [-CustomRulePath ] [-RecurseCustomRulePath] - [-ExcludeRule ] [-IncludeRule ] [-Severity ] [-Recurse] [-SuppressedOnly] [-EnableExit] [-ReportSummary] + [-ExcludeRule ] [-IncludeRule ] [-Severity ] [-Recurse] [-SuppressedOnly] [-IncludeSuppressions] [-EnableExit] [-ReportSummary] [-Settings ] ``` @@ -39,6 +39,8 @@ This feature should be used only when absolutely necessary. To get rules that were suppressed, run Invoke-ScriptAnalyzer with the -SuppressedOnly parameter. For instructions on suppressing a rule, see the description of the SuppressedOnly parameter. +To get all violations including suppressed one, run Invoke-ScriptAnalyzer with the -IncludeSuppressions parameter. + For usage in CI systems, the -EnableExit exits the shell with an exit code equal to the number of error records. PSScriptAnalyzer is an open-source project. @@ -142,6 +144,61 @@ The output reports the suppressed rules. ### -------------------------- EXAMPLE 8 -------------------------- ``` +function Get-Widgets +{ + [CmdletBinding()] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSUseSingularNouns", "")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingCmdletAliases", "", Justification="Resolution in progress.")] + Param() + + dir $pshome + ... +} + +PS C:\> Invoke-ScriptAnalyzer -Path .\Get-Widgets.ps1 + +RuleName Severity FileName Line Message +-------- -------- -------- ---- ------- +PSProvideCommentHelp Information ManageProf 14 The cmdlet 'Get-Widget' does not have a help comment. + iles.psm1 + +PS C:\> Invoke-ScriptAnalyzer -Path .\Get-Widgets.ps1 -SuppressedOnly + +Rule Name Severity File Name Line Justification +--------- -------- --------- ---- ------------- +PSAvoidUsingCmdletAliases Warning ManageProf 21 Resolution in progress. + iles.psm1 +PSUseSingularNouns Warning ManageProf 14 + iles.psm1 + +PS C:\> Invoke-ScriptAnalyzer -Path .\Get-Widgets.ps1 -IncludeSuppressions + +Rule Name Severity File Name Line Justification +--------- -------- --------- ---- ------------- +PSProvideCommentHelp Information ManageProf 14 The cmdlet 'Get-Widget' does not have a help comment. + iles.psm1 +PSAvoidUsingCmdletAliases Warning ManageProf 21 Resolution in progress. + iles.psm1 +PSUseSingularNouns Warning ManageProf 14 + iles.psm1 +``` + +This example shows how to discover all rule violations that are nonsuppressed and suppressed. + +The first command runs Script Analyzer on the script that contains the Get-Widgets function. +The output reports a rule +violation, but neither of the suppressed rules is listed, even though they are violated. + +The second command uses the SuppressedOnly parameter to discover the rules that are supressed in the Get-Widgets.ps1 +file. +The output reports the suppressed rules. + +The third command uses the IncludeSuppressions parameter to discover all rule violations even though they are violated in the Get-Widgets.ps1 +file. +The output reports the nonsuppressed and suppressed rules. + +### -------------------------- EXAMPLE 9 -------------------------- +``` # In .\ScriptAnalyzerProfile.txt @{ Severity = @('Error', 'Warning') @@ -162,7 +219,7 @@ Script Analyzer profile. If you include a conflicting parameter in the Invoke-ScriptAnalyzer command, such as '-Severity Error', Invoke-ScriptAnalyzer uses the profile value and ignores the parameter. -### -------------------------- EXAMPLE 9 -------------------------- +### -------------------------- EXAMPLE 10 -------------------------- ``` Invoke-ScriptAnalyzer -ScriptDefinition "function Get-Widgets {Write-Host 'Hello'}" @@ -379,6 +436,25 @@ Accept pipeline input: False Accept wildcard characters: False ``` +### -IncludeSuppressions +Returns all rule violations even though they are suppressed, instead of analyzing the files in the path. + +When you used IncludeSuppressions, Invoke-ScriptAnalyzer returns both SuppressedRecord and DiagnosticRecord objects +(Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.SuppressedRecord) +(Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord). + +```yaml +Type: SwitchParameter +Parameter Sets: (All) +Aliases: + +Required: False +Position: Named +Default value: False +Accept pipeline input: False +Accept wildcard characters: False +``` + ### -Fix Fixes certain warnings which contain a fix in their DiagnosticRecord. From 791bd0c68fb2120aad5877e1aa0af1ad0780d51d Mon Sep 17 00:00:00 2001 From: Liping Ma Date: Fri, 23 Jul 2021 21:22:46 +0800 Subject: [PATCH 2/2] parametersets --- .../Commands/InvokeScriptAnalyzerCommand.cs | 54 +++++++++++-------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index 46eac3c8a..176d4142d 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -21,7 +21,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands /// [Cmdlet(VerbsLifecycle.Invoke, "ScriptAnalyzer", - DefaultParameterSetName = "File", + DefaultParameterSetName = "FilePartial", SupportsShouldProcess = true, HelpUri = "https://go.microsoft.com/fwlink/?LinkId=525914")] [OutputType(typeof(DiagnosticRecord))] @@ -37,7 +37,12 @@ public class InvokeScriptAnalyzerCommand : PSCmdlet, IOutputWriter /// Path: The path to the file or folder to invoke PSScriptAnalyzer on. /// [Parameter(Position = 0, - ParameterSetName = "File", + ParameterSetName = "FilePartial", + Mandatory = true, + ValueFromPipeline = true, + ValueFromPipelineByPropertyName = true)] + [Parameter(Position = 0, + ParameterSetName = "FileAll", Mandatory = true, ValueFromPipeline = true, ValueFromPipelineByPropertyName = true)] @@ -54,7 +59,12 @@ public string Path /// ScriptDefinition: a script definition in the form of a string to run rules on. /// [Parameter(Position = 0, - ParameterSetName = "ScriptDefinition", + ParameterSetName = "ScriptDefinitionPartial", + Mandatory = true, + ValueFromPipeline = true, + ValueFromPipelineByPropertyName = true)] + [Parameter(Position = 0, + ParameterSetName = "ScriptDefinitionAll", Mandatory = true, ValueFromPipeline = true, ValueFromPipelineByPropertyName = true)] @@ -158,31 +168,24 @@ public SwitchParameter Recurse /// /// ShowSuppressed: Show the suppressed message /// - [Parameter(Mandatory = false)] + [Parameter(Mandatory = false, ParameterSetName = "FilePartial")] + [Parameter(Mandatory = false, ParameterSetName = "ScriptDefinitionPartial")] [SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")] - public SwitchParameter SuppressedOnly - { - get { return suppressedOnly; } - set { suppressedOnly = value; } - } - private bool suppressedOnly; + public SwitchParameter SuppressedOnly { get; set; } /// /// ShowAll: Show the suppressed and non-suppressed message /// - [Parameter(Mandatory = false)] + [Parameter(Mandatory = true, ParameterSetName = "FileAll")] + [Parameter(Mandatory = true, ParameterSetName = "ScriptDefinitionAll")] [SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")] - public SwitchParameter IncludeSuppressions - { - get { return includeSuppressions; } - set { includeSuppressions = value; } - } - private bool includeSuppressions; + public SwitchParameter IncludeSuppressions { get; set; } /// /// Resolves rule violations automatically where possible. /// - [Parameter(Mandatory = false, ParameterSetName = "File")] + [Parameter(Mandatory = false, ParameterSetName = "FilePartial")] + [Parameter(Mandatory = false, ParameterSetName = "FileAll")] public SwitchParameter Fix { get { return fix; } @@ -353,8 +356,8 @@ protected override void BeginProcessing() this.excludeRule, this.severity, combRulePaths == null || combIncludeDefaultRules, - this.suppressedOnly, - this.includeSuppressions); + this.SuppressedOnly, + this.IncludeSuppressions); } /// @@ -433,7 +436,7 @@ private void ProcessInput() WriteToOutput(diagnosticsList); } } - else if (String.Equals(this.ParameterSetName, "ScriptDefinition", StringComparison.OrdinalIgnoreCase)) + else if (IsScriptParameterSet()) { diagnosticsList = ScriptAnalyzer.Instance.AnalyzeScriptDefinition(scriptDefinition, out _, out _); WriteToOutput(diagnosticsList); @@ -512,7 +515,12 @@ private void ProcessPath() private bool IsFileParameterSet() { - return String.Equals(this.ParameterSetName, "File", StringComparison.OrdinalIgnoreCase); + return String.Equals(this.ParameterSetName, "FilePartial", StringComparison.OrdinalIgnoreCase) || String.Equals(this.ParameterSetName, "FileAll", StringComparison.OrdinalIgnoreCase); + } + + private bool IsScriptParameterSet() + { + return String.Equals(this.ParameterSetName, "ScriptDefinitionPartial", StringComparison.OrdinalIgnoreCase) || String.Equals(this.ParameterSetName, "ScriptDefinitionAll", StringComparison.OrdinalIgnoreCase); } private bool OverrideSwitchParam(bool paramValue, string paramName) @@ -524,4 +532,4 @@ private bool OverrideSwitchParam(bool paramValue, string paramName) #endregion // Private Methods } -} +} \ No newline at end of file