diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index bf1e740c3..0b4b6f5f7 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -146,7 +146,7 @@ public string[] IncludeRule private string[] includeRule; /// - /// IncludeRule: Array of the severity types to be enabled. + /// Severity levels of the diagnostic records to include. /// [ValidateSet("Warning", "Error", "Information", "ParseError", IgnoreCase = true)] [Parameter(Mandatory = false)] diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 91bf188f2..7f9568259 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -1485,7 +1485,7 @@ public List AnalyzeScriptDefinition(string scriptDefinition, o var relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out List diagnosticRecords); // Add parse errors first if requested! - if ( relevantParseErrors != null && (severity == null || severity.Contains("ParseError", StringComparer.OrdinalIgnoreCase))) + if ( relevantParseErrors != null ) { var results = new List(); foreach ( ParseError parseError in relevantParseErrors ) @@ -1505,7 +1505,7 @@ public List AnalyzeScriptDefinition(string scriptDefinition, o // now, analyze the script definition diagnosticRecords.AddRange(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, null, skipVariableAnalysis, emitSuppressionErrors)); - return diagnosticRecords; + return FilterDiagnosticsBySeverity(diagnosticRecords).ToList(); } /// @@ -1876,7 +1876,7 @@ private IEnumerable AnalyzeFile(string filePath) // no parsing can really be done, but there are other rules to run (specifically encoding). if ( s_aboutHelpRegex.IsMatch(Path.GetFileName(filePath)) ) { - return diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath)); + return FilterDiagnosticsBySeverity(diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath))); } // Process script @@ -1899,7 +1899,7 @@ private IEnumerable AnalyzeFile(string filePath) IEnumerable relevantParseErrors = RemoveTypeNotFoundParseErrors(errors, out diagnosticRecords); // First, add all parse errors if they've been requested - if ( relevantParseErrors != null && (severity == null || severity.Contains("ParseError", StringComparer.OrdinalIgnoreCase))) + if ( relevantParseErrors != null ) { List results = new List(); foreach ( ParseError parseError in relevantParseErrors ) @@ -1916,7 +1916,7 @@ private IEnumerable AnalyzeFile(string filePath) diagnosticRecords.AddRange(results); } - return diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath)); + return FilterDiagnosticsBySeverity(diagnosticRecords.Concat(this.AnalyzeSyntaxTree(scriptAst, scriptTokens, filePath))); } private bool IsModuleNotFoundError(ParseError error) @@ -1925,42 +1925,26 @@ private bool IsModuleNotFoundError(ParseError error) && error.ErrorId.Equals("ModuleNotFoundDuringParse", StringComparison.OrdinalIgnoreCase); } - private bool IsSeverityAllowed(IEnumerable allowedSeverities, IRule rule) + private bool IsDiagnosticSeverityAllowed(DiagnosticRecord diagnostic) { return severity == null - || (allowedSeverities != null - && rule != null - && HasGetSeverity(rule) - && allowedSeverities.Contains((uint)rule.GetSeverity())); + || (diagnostic != null + && severity.Contains(diagnostic.Severity.ToString(), StringComparer.OrdinalIgnoreCase)); } - private static bool IsRuleEnabled(IRule rule) - { - var configurableRule = rule as ConfigurableRule; - return configurableRule == null || configurableRule.Enable; - } - - IEnumerable GetAllowedSeveritiesInInt() - { - return severity != null - ? severity.Select(item => (uint)Enum.Parse(typeof(DiagnosticSeverity), item, true)) - : null; - } - - bool HasMethod(T obj, string methodName) + private IEnumerable FilterDiagnosticsBySeverity(IEnumerable diagnostics) { - var type = obj.GetType(); - return type.GetMethod(methodName) != null; + return severity == null ? diagnostics : diagnostics.Where(IsDiagnosticSeverityAllowed); } - bool HasGetSeverity(T obj) + private static bool IsRuleEnabled(IRule rule) { - return HasMethod(obj, "GetSeverity"); + var configurableRule = rule as ConfigurableRule; + return configurableRule == null || configurableRule.Enable; } bool IsRuleAllowed(IRule rule) { - IEnumerable allowedSeverities = GetAllowedSeveritiesInInt(); bool includeRegexMatch = false; bool excludeRegexMatch = false; foreach (Regex include in includeRegexList) @@ -1981,12 +1965,8 @@ bool IsRuleAllowed(IRule rule) } } - bool helpRule = String.Equals(rule.GetName(), "PSUseUTF8EncodingForHelpFile", StringComparison.OrdinalIgnoreCase); - bool includeSeverity = IsSeverityAllowed(allowedSeverities, rule); - return (includeRule == null || includeRegexMatch) && (excludeRule == null || !excludeRegexMatch) - && IsSeverityAllowed(allowedSeverities, rule) && IsRuleEnabled(rule); } @@ -2358,13 +2338,14 @@ 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(); - return _suppressionPreference switch + IEnumerable selectedDiagnostics = _suppressionPreference switch { SuppressionPreference.SuppressedOnly => suppressed.OfType(), SuppressionPreference.Omit => diagnosticsList, SuppressionPreference.Include => diagnosticsList.Concat(suppressed.OfType()), _ => throw new ArgumentException($"SuppressionPreference has invalid value '{_suppressionPreference}'"), }; + return FilterDiagnosticsBySeverity(selectedDiagnostics); } } } diff --git a/Tests/Engine/CustomizedRule.tests.ps1 b/Tests/Engine/CustomizedRule.tests.ps1 index ce60f6dce..a38275f97 100644 --- a/Tests/Engine/CustomizedRule.tests.ps1 +++ b/Tests/Engine/CustomizedRule.tests.ps1 @@ -157,6 +157,60 @@ Describe "Test importing correct customized rules" { $customizedRulePath.Count | Should -Be 0 } + It "filters custom rule diagnostics by diagnostic severity" { + $customRuleContent = @' +function ReportError { + param ( + [System.Management.Automation.Language.CommandAst]$ast + ) + + if ($ast.GetCommandName() -eq 'Invoke-Something') { + New-Object -Typename 'Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic.DiagnosticRecord' ` + -ArgumentList 'This is an error', $ast.Extent, 'ReportError', Error, $ast.Extent.File, 'custom-id', $null + } +} +'@ + $customRuleScriptPath = Join-Path $TestDrive 'SeverityRule.psm1' + Set-Content -Path $customRuleScriptPath -Value $customRuleContent + (Get-ScriptAnalyzerRule -CustomRulePath $customRuleScriptPath).Severity | Should -Be 'Warning' + + $errors = Invoke-ScriptAnalyzer -ScriptDefinition 'Invoke-Something' -CustomRulePath $customRuleScriptPath -Severity Error + $errors.Count | Should -Be 1 + $errors[0].Severity | Should -Be 'Error' + + $warnings = Invoke-ScriptAnalyzer -ScriptDefinition 'Invoke-Something' -CustomRulePath $customRuleScriptPath -Severity Warning + $warnings.Count | Should -Be 0 + + $script = @' +function Suppressed { + [Diagnostics.CodeAnalysis.SuppressMessageAttribute('ReportError', '', Justification = 'test justification')] + param() + Invoke-Something +} +Invoke-Something +'@ + foreach ($mode in 'SuppressedOnly', 'IncludeSuppressed') { + $parameters = @{ + ScriptDefinition = $script + CustomRulePath = $customRuleScriptPath + ErrorAction = 'Stop' + $mode = $true + } + + $errors = @(Invoke-ScriptAnalyzer @parameters -Severity Error) + $errors.Count | Should -Be $(if ($mode -eq 'SuppressedOnly') { 1 } else { 2 }) + $suppressed = @($errors | Where-Object IsSuppressed) + $suppressed.Count | Should -Be 1 + $suppressed[0].Severity | Should -Be 'Error' + $suppressed[0].RuleSuppressionID | Should -BeExactly 'custom-id' + $suppressed[0].Suppression | Should -HaveCount 1 + $suppressed[0].Suppression[0].Justification | Should -BeExactly 'test justification' + + $warnings = @(Invoke-ScriptAnalyzer @parameters -Severity Warning) + $warnings.Count | Should -Be 0 + } + } + It "can suppress custom rule with rule name expression ''" -TestCases @( @{RuleNameExpression = '$MyInvocation.MyCommand.Name'; RuleName = 'WarningAboutDoSomething' } @{RuleNameExpression = '$MyInvocation.InvocationName'; RuleName = 'MyCustomRule\WarningAboutDoSomething' }