From bbfd1d9054a95d229a2cdcb0988ae3fe1f6af7fb Mon Sep 17 00:00:00 2001 From: Charlie Schmidt Date: Fri, 22 Jul 2016 13:05:00 -0400 Subject: [PATCH 1/6] Suppress External Rules (#585) * External/Custom rule supression * Revert to the previous GetExternalRecord() call on all external rules, rather than 1-by-1 to continue to take advantage of the runspacepool stuff. New SupressRule() function that takes a single DiagnosticRecord and list of suppressions - bounces the RuleName of the record against that list. --- Engine/ScriptAnalyzer.cs | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index b39db8b87..b864f9e26 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1454,6 +1454,23 @@ private Tuple, List> SuppressRule( } return records; } + private Tuple, List> SuppressRule( + DiagnosticRecord ruleDiagnosticRecord, + Dictionary> ruleSuppressions + ) + { + List suppressRuleErrors; + var records = Helper.Instance.SuppressRule( + ruleDiagnosticRecord.RuleName, + ruleSuppressions, + new List { ruleDiagnosticRecord }, + out suppressRuleErrors); + foreach (var error in suppressRuleErrors) + { + this.outputWriter.WriteError(error); + } + return records; + } /// /// Analyzes the syntax tree of a script file that has already been parsed. @@ -1752,13 +1769,21 @@ public IEnumerable AnalyzeSyntaxTree( } } - foreach (var record in this.GetExternalRecord(scriptAst, scriptTokens, exRules.ToArray(), fileName)) + foreach (var ruleRecord in this.GetExternalRecord(scriptAst, scriptTokens, exRules.ToArray(), fileName)) { - diagnostics.Add(record); + var records = SuppressRule(ruleRecord, ruleSuppressions); + foreach (var record in records.Item2) + { + diagnostics.Add(record); + } + foreach (var suppressedRec in records.Item1) + { + suppressed.Add(suppressedRec); + } } } - -#endregion + + #endregion // Need to reverse the concurrentbag to ensure that results are sorted in the increasing order of line numbers IEnumerable diagnosticsList = diagnostics.Reverse(); From e847cf9cca8ea932c67deb3a3336bb86aba55c10 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 1 Aug 2016 14:40:44 -0700 Subject: [PATCH 2/6] Add test for suppressing external rule --- Tests/Engine/RuleSuppression.tests.ps1 | 32 ++++++++++++++++++++------ 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index 0ad171eab..180d604f8 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -114,13 +114,31 @@ function SuppressPwdParam() if (!$testingLibraryUsage) { - Context "Bad Rule Suppression" { - It "Throws a non-terminating error" { - Invoke-ScriptAnalyzer -ScriptDefinition $ruleSuppressionBad -IncludeRule "PSAvoidUsingUserNameAndPassWordParams" -ErrorVariable errorRecord -ErrorAction SilentlyContinue - $errorRecord.Count | Should Be 1 - $errorRecord.FullyQualifiedErrorId | Should match "suppression message attribute error" - } - } + Context "Bad Rule Suppression" { + It "Throws a non-terminating error" { + Invoke-ScriptAnalyzer -ScriptDefinition $ruleSuppressionBad -IncludeRule "PSAvoidUsingUserNameAndPassWordParams" -ErrorVariable errorRecord -ErrorAction SilentlyContinue + $errorRecord.Count | Should Be 1 + $errorRecord.FullyQualifiedErrorId | Should match "suppression message attribute error" + } + } + + Context "External Rule Suppression" { + $externalRuleSuppression = @' +[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('CommunityAnalyzerRules\Measure-WriteHost','')] +param() # without the param block, powershell parser throws up! +Write-Host "write-host" +'@ + It "Suppresses violation of an external ast rule" { + Invoke-ScriptAnalyzer ` + -ScriptDefinition $externalRuleSuppression ` + -CustomRulePath "CommunityAnalyzerRules/" ` + -OutVariable ruleViolations ` + -SuppressedOnly + $ruleViolations.Count | Should Be 1 + } + } + + } } From 791e2c7481a36ffef3da5d61083258c140113c20 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 1 Aug 2016 15:09:56 -0700 Subject: [PATCH 3/6] Remove SuppressRule method redundancy --- Engine/ScriptAnalyzer.cs | 29 +++++++++++++++----------- Tests/Engine/RuleSuppression.tests.ps1 | 2 -- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index b864f9e26..4d9eacc63 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1454,22 +1454,27 @@ private Tuple, List> SuppressRule( } return records; } + + /// + /// Wrapper around SuppressRule method. + /// + /// This enables suppressing external rules (written in PowerShell). + /// Since there can be an inconsistency between the rule name in + /// diagnostic record and the actual rule (function) name, we use + /// the diagnostic record rule name for suppression + /// + /// + /// + /// Returns a tuple of suppressed and diagnostic records private Tuple, List> SuppressRule( - DiagnosticRecord ruleDiagnosticRecord, - Dictionary> ruleSuppressions + Dictionary> ruleSuppressions, + DiagnosticRecord ruleDiagnosticRecord ) { - List suppressRuleErrors; - var records = Helper.Instance.SuppressRule( + return SuppressRule( ruleDiagnosticRecord.RuleName, ruleSuppressions, - new List { ruleDiagnosticRecord }, - out suppressRuleErrors); - foreach (var error in suppressRuleErrors) - { - this.outputWriter.WriteError(error); - } - return records; + new List { ruleDiagnosticRecord }); } /// @@ -1771,7 +1776,7 @@ public IEnumerable AnalyzeSyntaxTree( foreach (var ruleRecord in this.GetExternalRecord(scriptAst, scriptTokens, exRules.ToArray(), fileName)) { - var records = SuppressRule(ruleRecord, ruleSuppressions); + var records = SuppressRule(ruleSuppressions, ruleRecord); foreach (var record in records.Item2) { diagnostics.Add(record); diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index 180d604f8..04fdd0b87 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -137,8 +137,6 @@ Write-Host "write-host" $ruleViolations.Count | Should Be 1 } } - - } } From f509cbfe8fb5c9b8842aae685f45c57dcfaab864 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 1 Aug 2016 15:58:34 -0700 Subject: [PATCH 4/6] Set CustomRulePath to absolute path in test --- Tests/Engine/RuleSuppression.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Engine/RuleSuppression.tests.ps1 b/Tests/Engine/RuleSuppression.tests.ps1 index 04fdd0b87..3f31e4619 100644 --- a/Tests/Engine/RuleSuppression.tests.ps1 +++ b/Tests/Engine/RuleSuppression.tests.ps1 @@ -131,7 +131,7 @@ Write-Host "write-host" It "Suppresses violation of an external ast rule" { Invoke-ScriptAnalyzer ` -ScriptDefinition $externalRuleSuppression ` - -CustomRulePath "CommunityAnalyzerRules/" ` + -CustomRulePath (Join-Path $directory "CommunityAnalyzerRules") ` -OutVariable ruleViolations ` -SuppressedOnly $ruleViolations.Count | Should Be 1 From eda6fdd2f15521626dc0612f2b772b90672c72a4 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 1 Aug 2016 16:00:05 -0700 Subject: [PATCH 5/6] Add check for null profile value --- build.ps1 | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/build.ps1 b/build.ps1 index 22d1337d0..53a8a8a63 100644 --- a/build.ps1 +++ b/build.ps1 @@ -110,8 +110,12 @@ if ($BuildDocs) New-ExternalHelp -Path $markdownDocsPath -OutputPath $outputDocsPath -Force -Verbose:$verbosity } - -$moduleRootPath = Join-Path (Split-Path $profile) 'Modules' +# Appyeyor errors out due to $profile being null. Hence... +$moduleRootPath = "$HOME/Documents/WindowsPowerShell/Modules" +if ($profile -ne $null) +{ + $moduleRootPath = Join-Path (Split-Path $profile) 'Modules' +} $modulePSSAPath = Join-Path $moduleRootPath 'PSScriptAnalyzer' if ($Install) { From 51cefbc77222da9686404bcb7c78b7a9a3c2db34 Mon Sep 17 00:00:00 2001 From: Kapil Borle Date: Mon, 1 Aug 2016 22:25:01 -0700 Subject: [PATCH 6/6] Fix typo in build script comment --- build.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.ps1 b/build.ps1 index 53a8a8a63..9af1a0d60 100644 --- a/build.ps1 +++ b/build.ps1 @@ -110,7 +110,7 @@ if ($BuildDocs) New-ExternalHelp -Path $markdownDocsPath -OutputPath $outputDocsPath -Force -Verbose:$verbosity } -# Appyeyor errors out due to $profile being null. Hence... +# Appveyor errors out due to $profile being null. Hence... $moduleRootPath = "$HOME/Documents/WindowsPowerShell/Modules" if ($profile -ne $null) {