diff --git a/Engine/Extensions.cs b/Engine/Extensions.cs index 5e10b1d78..46b67bf65 100644 --- a/Engine/Extensions.cs +++ b/Engine/Extensions.cs @@ -178,5 +178,19 @@ public static bool GetValue(this NamedAttributeArgumentAst attrAst, out Expressi return false; } + + internal static bool TryGetPropertyValue(this PSObject psObject, string propertyName, out object value) + { + PSMemberInfo property = psObject.Properties[propertyName]; + + if (property is null) + { + value = default; + return false; + } + + value = property.Value; + return true; + } } } diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 0a8169173..d35ab7e0a 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -22,6 +22,7 @@ using System.Collections; using System.Diagnostics; using System.Text; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Extensions; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer { @@ -1263,13 +1264,6 @@ internal IEnumerable GetExternalRecord(Ast ast, Token[] token, foreach (var psobject in psobjects) { - DiagnosticSeverity severity; - IScriptExtent extent; - string message = string.Empty; - string ruleName = string.Empty; - string ruleSuppressionID = string.Empty; - IEnumerable suggestedCorrections; - if (psobject != null && psobject.ImmediateBaseObject != null) { // Because error stream is merged to output stream, @@ -1282,28 +1276,9 @@ internal IEnumerable GetExternalRecord(Ast ast, Token[] token, } // DiagnosticRecord may not be correctly returned from external rule. - try + if (TryConvertPSObjectToDiagnostic(psobject, filePath, out DiagnosticRecord diagnostic)) { - severity = (DiagnosticSeverity)Enum.Parse(typeof(DiagnosticSeverity), psobject.Properties["Severity"].Value.ToString()); - message = psobject.Properties["Message"].Value.ToString(); - extent = (IScriptExtent)psobject.Properties["Extent"].Value; - ruleName = psobject.Properties["RuleName"].Value.ToString(); - ruleSuppressionID = psobject.Properties["RuleSuppressionID"].Value?.ToString(); - suggestedCorrections = (IEnumerable)psobject.Properties["SuggestedCorrections"].Value; - } - catch (Exception ex) - { - this.outputWriter.WriteError(new ErrorRecord(ex, ex.HResult.ToString("X"), ErrorCategory.NotSpecified, this)); - continue; - } - - if (!string.IsNullOrEmpty(message)) - { - diagnostics.Add(new DiagnosticRecord(message, extent, ruleName, severity, filePath) - { - SuggestedCorrections = suggestedCorrections, - RuleSuppressionID = ruleSuppressionID, - }); + diagnostics.Add(diagnostic); } } } @@ -1660,6 +1635,57 @@ public EditableText Fix(EditableText text, Range range, bool skipParsing, out Ra return text; } + private bool TryConvertPSObjectToDiagnostic(PSObject psObject, string filePath, out DiagnosticRecord diagnostic) + { + string message = psObject.Properties["Message"]?.Value?.ToString(); + object extentValue = psObject.Properties["Extent"]?.Value; + string ruleName = psObject.Properties["RuleName"]?.Value?.ToString(); + string ruleSuppressionID = psObject.Properties["RuleSuppressionID"]?.Value?.ToString(); + CorrectionExtent[] suggestedCorrections = psObject.TryGetPropertyValue("SuggestedCorrections", out object correctionsValue) + ? LanguagePrimitives.ConvertTo(correctionsValue) + : null; + DiagnosticSeverity severity = psObject.TryGetPropertyValue("Severity", out object severityValue) + ? LanguagePrimitives.ConvertTo(severityValue) + : DiagnosticSeverity.Warning; + + bool isValid = true; + isValid &= CheckHasRequiredProperty("Message", message); + isValid &= CheckHasRequiredProperty("RuleName", ruleName); + + if (extentValue is not null && extentValue is not IScriptExtent) + { + this.outputWriter.WriteError( + new ErrorRecord( + new ArgumentException($"Property 'Extent' is expected to be of type '{typeof(IScriptExtent)}' but was instead of type '{extentValue.GetType()}'"), + "CustomRuleDiagnosticPropertyInvalidType", + ErrorCategory.InvalidArgument, + this)); + isValid = false; + } + + if (!isValid) + { + diagnostic = null; + return false; + } + + diagnostic = new DiagnosticRecord(message, (IScriptExtent)extentValue, ruleName, severity, filePath, ruleSuppressionID, suggestedCorrections); + return true; + } + + private bool CheckHasRequiredProperty(string propertyName, object propertyValue) + { + if (propertyValue is null) + { + var exception = new ArgumentNullException(propertyName, $"The '{propertyName}' property is required on custom rule diagnostics"); + this.outputWriter.WriteError(new ErrorRecord(exception, "CustomRuleDiagnosticPropertyMissing", ErrorCategory.InvalidArgument, this)); + return false; + } + + return true; + } + + private static Encoding GetFileEncoding(string path) { using (var stream = new FileStream(path, FileMode.Open)) diff --git a/Tests/Engine/CustomRuleNREAssets/CLMTest.ps1 b/Tests/Engine/CustomRuleNREAssets/CLMTest.ps1 new file mode 100644 index 000000000..ed3c800c1 --- /dev/null +++ b/Tests/Engine/CustomRuleNREAssets/CLMTest.ps1 @@ -0,0 +1,7 @@ +write-host "test" +$a = "asdf" +$a = $a.replace('s','ssssssss') +[math]::abs(-1) +Function ASDF1234{ + "asdf" +} diff --git a/Tests/Engine/CustomRuleNREAssets/MyCustom.psm1 b/Tests/Engine/CustomRuleNREAssets/MyCustom.psm1 new file mode 100644 index 000000000..34368748d --- /dev/null +++ b/Tests/Engine/CustomRuleNREAssets/MyCustom.psm1 @@ -0,0 +1,48 @@ +<# +.SYNOPSIS + Static methods are not allowed in constrained language mode. +.DESCRIPTION + Static methods are not allowed in constrained language mode. + To fix a violation of this rule, use a cmdlet or function instead of a static method. +.EXAMPLE + Test-StaticMethod -CommandAst $CommandAst +.INPUTS + [System.Management.Automation.Language.ScriptBlockAst] +.OUTPUTS + [PSCustomObject[]] +.NOTES + Reference: Output, CLM info. +#> +function Test-StaticMethod +{ + [CmdletBinding()] + [OutputType([PSCustomObject[]])] + Param + ( + [Parameter(Mandatory = $true)] + [ValidateNotNullOrEmpty()] + [System.Management.Automation.Language.ScriptBlockAst] + $ScriptBlockAst + ) + + Process + { + try + { + # Gets methods + + $invokedMethods = $ScriptBlockAst.FindAll({$args[0] -is [System.Management.Automation.Language.CommandExpressionAst] -and $args[0].Expression -match "^\[.*\]::" },$true) + foreach ($invokedMethod in $invokedMethods) + { + [PSCustomObject]@{Message = "Avoid Using Static Methods"; + Extent = $invokedMethod.Extent; + RuleName = $PSCmdlet.MyInvocation.InvocationName; + Severity = "Warning"} + } + } + catch + { + $PSCmdlet.ThrowTerminatingError($PSItem) + } + } +} diff --git a/Tests/Engine/CustomizedRule.tests.ps1 b/Tests/Engine/CustomizedRule.tests.ps1 index 842879801..ba0686d75 100644 --- a/Tests/Engine/CustomizedRule.tests.ps1 +++ b/Tests/Engine/CustomizedRule.tests.ps1 @@ -199,6 +199,19 @@ Describe "Test importing correct customized rules" { $violations[0].RuleSuppressionID | Should -Be "MyRuleSuppressionID" } + It "Does not throw an exception when optional diagnostic properties are not present" { + $violations = Invoke-ScriptAnalyzer -Path "$PSScriptRoot\CustomRuleNREAssets\CLMTest.ps1" -CustomizedRulePath "$PSScriptRoot\CustomRuleNREAssets\MyCustom.psm1" + + $violations.Count | Should -Be 1 + $violations[0].RuleName | Should -BeExactly 'MyCustom\Test-StaticMethod' + $violations[0].Severity | Should -Be 'Warning' + $violations[0].ScriptName | Should -BeExactly 'CLMTest.ps1' + $violations[0].Extent.StartLineNumber | Should -Be 4 + $violations[0].Message | Should -BeExactly 'Avoid Using Static Methods' + $violations[0].RuleSuppressionID | Should -BeNullOrEmpty + $violations[0].SuggestedCorrections | Should -BeNullOrEmpty + } + Context "Test Invoke-ScriptAnalyzer with customized rules - Advanced test cases" -Skip:$testingLibraryUsage { It "will show the custom rule in the results when given a rule folder path with trailing backslash" { # needs fixing for Linux