From f258a87b3523cf70c3f5bb62b1bcd682b154618d Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Wed, 2 Dec 2015 13:32:37 -0800 Subject: [PATCH 1/2] Throw error instead of warning for profiles --- .../Commands/InvokeScriptAnalyzerCommand.cs | 14 ++ Engine/ScriptAnalyzer.cs | 153 ++++++++++-------- Engine/Strings.Designer.cs | 47 +++++- Engine/Strings.resx | 17 +- Tests/Engine/GlobalSuppression.ps1 | 78 ++++++++- Tests/Engine/GlobalSuppression.test.ps1 | 32 +++- Tests/Engine/WrongProfile.ps1 | 9 ++ 7 files changed, 270 insertions(+), 80 deletions(-) create mode 100644 Tests/Engine/WrongProfile.ps1 diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index 5136555ed..dd9e2542d 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -169,6 +169,8 @@ public string Configuration } private string configuration; + private bool stopProcessing; + #endregion Parameters #region Overrides @@ -181,6 +183,12 @@ protected override void BeginProcessing() string[] rulePaths = Helper.ProcessCustomRulePaths(customRulePath, this.SessionState, recurseCustomRulePath); + if (!ScriptAnalyzer.Instance.ParseProfile(this.configuration, this.SessionState.Path, this)) + { + stopProcessing = true; + return; + } + ScriptAnalyzer.Instance.Initialize( this, rulePaths, @@ -196,6 +204,12 @@ protected override void BeginProcessing() /// protected override void ProcessRecord() { + if (stopProcessing) + { + stopProcessing = false; + return; + } + if (String.Equals(this.ParameterSetName, "File", StringComparison.OrdinalIgnoreCase)) { // throws Item Not Found Exception diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index c8f797a1c..ce3310985 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -151,49 +151,25 @@ public void Initialize( profile); } - private void Initialize( - IOutputWriter outputWriter, - PathIntrinsics path, - CommandInvocationIntrinsics invokeCommand, - string[] customizedRulePath, - string[] includeRuleNames, - string[] excludeRuleNames, - string[] severity, - bool suppressedOnly = false, - string profile = null) + internal bool ParseProfile(string profile, PathIntrinsics path, IOutputWriter writer) { - if (outputWriter == null) - { - throw new ArgumentNullException("outputWriter"); - } - - this.outputWriter = outputWriter; - - #region Verifies rule extensions and loggers path - - List paths = this.GetValidCustomRulePaths(customizedRulePath, path); + IEnumerable includeRuleList = new List(); + IEnumerable excludeRuleList = new List(); + IEnumerable severityList = new List(); - #endregion - - #region Initializes Rules - - this.severity = severity; - this.suppressedOnly = suppressedOnly; - this.includeRule = includeRuleNames; - this.excludeRule = excludeRuleNames; - this.includeRegexList = new List(); - this.excludeRegexList = new List(); + bool hasError = false; - #region ParseProfile if (!String.IsNullOrWhiteSpace(profile)) { try - { + { profile = path.GetResolvedPSPathFromPSPath(profile).First().Path; } catch { - this.outputWriter.WriteWarning(string.Format(CultureInfo.CurrentCulture, Strings.FileNotFound, profile)); + writer.WriteError(new ErrorRecord(new FileNotFoundException(string.Format(CultureInfo.CurrentCulture, Strings.FileNotFound, profile)), + Strings.ConfigurationFileNotFound, ErrorCategory.ResourceUnavailable, profile)); + hasError = true; } if (File.Exists(profile)) @@ -206,7 +182,9 @@ private void Initialize( // no hashtable, raise warning if (hashTableAsts.Count() == 0) { - this.outputWriter.WriteWarning(string.Format(CultureInfo.CurrentCulture, Strings.InvalidProfile, profile)); + writer.WriteError(new ErrorRecord(new ArgumentException(string.Format(CultureInfo.CurrentCulture, Strings.InvalidProfile, profile)), + Strings.ConfigurationFileHasNoHashTable, ErrorCategory.ResourceUnavailable, profile)); + hasError = true; } else { @@ -217,8 +195,9 @@ private void Initialize( if (!(kvp.Item1 is StringConstantExpressionAst)) { // first item (the key) should be a string - this.outputWriter.WriteWarning( - string.Format(CultureInfo.CurrentCulture, Strings.WrongKeyFormat, kvp.Item1.Extent.StartLineNumber, kvp.Item1.Extent.StartColumnNumber, profile)); + writer.WriteError(new ErrorRecord(new InvalidDataException(string.Format(CultureInfo.CurrentCulture, Strings.WrongKeyFormat, kvp.Item1.Extent.StartLineNumber, kvp.Item1.Extent.StartColumnNumber, profile)), + Strings.ConfigurationKeyNotAString, ErrorCategory.InvalidData, profile)); + hasError = true; continue; } @@ -241,10 +220,10 @@ private void Initialize( // Statements property is never null if (arrayExp.SubExpression != null) { - StatementAst stateAst = arrayExp.SubExpression.Statements.First(); + StatementAst stateAst = arrayExp.SubExpression.Statements.FirstOrDefault(); if (stateAst != null && stateAst is PipelineAst) { - CommandBaseAst cmdBaseAst = (stateAst as PipelineAst).PipelineElements.First(); + CommandBaseAst cmdBaseAst = (stateAst as PipelineAst).PipelineElements.FirstOrDefault(); if (cmdBaseAst != null && cmdBaseAst is CommandExpressionAst) { CommandExpressionAst cmdExpAst = cmdBaseAst as CommandExpressionAst; @@ -268,8 +247,9 @@ private void Initialize( // all the values in the array needs to be string if (!(element is StringConstantExpressionAst)) { - this.outputWriter.WriteWarning( - string.Format(CultureInfo.CurrentCulture, Strings.WrongValueFormat, element.Extent.StartLineNumber, element.Extent.StartColumnNumber, profile)); + writer.WriteError(new ErrorRecord(new InvalidDataException(string.Format(CultureInfo.CurrentCulture, Strings.WrongValueFormat, element.Extent.StartLineNumber, element.Extent.StartColumnNumber, profile)), + Strings.ConfigurationValueNotAString, ErrorCategory.InvalidData, profile)); + hasError = true; continue; } @@ -281,46 +261,30 @@ private void Initialize( if (rhsList.Count == 0) { - this.outputWriter.WriteWarning( - string.Format(CultureInfo.CurrentCulture, Strings.WrongValueFormat, kvp.Item2.Extent.StartLineNumber, kvp.Item2.Extent.StartColumnNumber, profile)); - break; + writer.WriteError(new ErrorRecord(new InvalidDataException(string.Format(CultureInfo.CurrentCulture, Strings.WrongValueFormat, kvp.Item2.Extent.StartLineNumber, kvp.Item2.Extent.StartColumnNumber, profile)), + Strings.ConfigurationValueWrongFormat, ErrorCategory.InvalidData, profile)); + hasError = true; + continue; } - switch ((kvp.Item1 as StringConstantExpressionAst).Value.ToLower()) + string key = (kvp.Item1 as StringConstantExpressionAst).Value.ToLower(); + + switch (key) { case "severity": - if (this.severity == null) - { - this.severity = rhsList.ToArray(); - } - else - { - this.severity = this.severity.Union(rhsList).ToArray(); - } + severityList = severityList.Union(rhsList); break; case "includerules": - if (this.includeRule == null) - { - this.includeRule = rhsList.ToArray(); - } - else - { - this.includeRule = this.includeRule.Union(rhsList).ToArray(); - } + includeRuleList = includeRuleList.Union(rhsList); break; case "excluderules": - if (this.excludeRule == null) - { - this.excludeRule = rhsList.ToArray(); - } - else - { - this.excludeRule = this.excludeRule.Union(rhsList).ToArray(); - } + excludeRuleList = excludeRuleList.Union(rhsList); break; default: - this.outputWriter.WriteWarning( - string.Format(CultureInfo.CurrentCulture, Strings.WrongKey, kvp.Item1.Extent.StartLineNumber, kvp.Item1.Extent.StartColumnNumber, profile)); + // keep writing warning here, we only stop processing if existing keys are wrong + writer.WriteWarning( + string.Format(CultureInfo.CurrentCulture, Strings.WrongKey, key, + kvp.Item1.Extent.StartLineNumber, kvp.Item1.Extent.StartColumnNumber, profile)); break; } } @@ -328,8 +292,51 @@ private void Initialize( } } + if (hasError) + { + return false; + } + + this.severity = (severityList.Count() == 0) ? null : severityList.ToArray(); + this.includeRule = (includeRuleList.Count() == 0) ? null : includeRuleList.ToArray(); + this.excludeRule = (excludeRuleList.Count() == 0) ? null : excludeRuleList.ToArray(); + + return true; + } + + private void Initialize( + IOutputWriter outputWriter, + PathIntrinsics path, + CommandInvocationIntrinsics invokeCommand, + string[] customizedRulePath, + string[] includeRuleNames, + string[] excludeRuleNames, + string[] severity, + bool suppressedOnly = false, + string profile = null) + { + if (outputWriter == null) + { + throw new ArgumentNullException("outputWriter"); + } + + this.outputWriter = outputWriter; + + #region Verifies rule extensions and loggers path + + List paths = this.GetValidCustomRulePaths(customizedRulePath, path); + #endregion + #region Initializes Rules + + this.suppressedOnly = suppressedOnly; + this.severity = this.severity == null ? severity : this.severity.Union(severity ?? new String[0]).ToArray(); + this.includeRule = this.includeRule == null ? includeRuleNames : this.includeRule.Union(includeRuleNames ?? new String[0]).ToArray(); + this.excludeRule = this.excludeRule == null ? excludeRuleNames : this.excludeRule.Union(excludeRuleNames ?? new String[0]).ToArray(); + this.includeRegexList = new List(); + this.excludeRegexList = new List(); + //Check wild card input for the Include/ExcludeRules and create regex match patterns if (this.includeRule != null) { @@ -339,6 +346,7 @@ private void Initialize( this.includeRegexList.Add(includeRegex); } } + if (this.excludeRule != null) { foreach (string rule in excludeRule) @@ -1446,7 +1454,10 @@ public IEnumerable AnalyzeSyntaxTree( if (severity != null) { var diagSeverity = severity.Select(item => Enum.Parse(typeof(DiagnosticSeverity), item, true)); - diagnosticsList = diagnostics.Where(item => diagSeverity.Contains(item.Severity)); + if (diagSeverity.Count() != 0) + { + diagnosticsList = diagnostics.Where(item => diagSeverity.Contains(item.Severity)); + } } return this.suppressedOnly ? diff --git a/Engine/Strings.Designer.cs b/Engine/Strings.Designer.cs index 619490de5..5b6596ba1 100644 --- a/Engine/Strings.Designer.cs +++ b/Engine/Strings.Designer.cs @@ -87,6 +87,51 @@ internal static string CommandInfoNotFound { } } + /// + /// Looks up a localized string similar to ConfigurationFileHasNoHashTable. + /// + internal static string ConfigurationFileHasNoHashTable { + get { + return ResourceManager.GetString("ConfigurationFileHasNoHashTable", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to ConfigurationFileNotFound. + /// + internal static string ConfigurationFileNotFound { + get { + return ResourceManager.GetString("ConfigurationFileNotFound", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to ConfigurationKeyNotAString. + /// + internal static string ConfigurationKeyNotAString { + get { + return ResourceManager.GetString("ConfigurationKeyNotAString", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to ConfigurationValueNotAString. + /// + internal static string ConfigurationValueNotAString { + get { + return ResourceManager.GetString("ConfigurationValueNotAString", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to ConfigurationValueWrongFormat. + /// + internal static string ConfigurationValueWrongFormat { + get { + return ResourceManager.GetString("ConfigurationValueWrongFormat", resourceCulture); + } + } + /// /// Looks up a localized string similar to Writes all diagnostics to WriteObject.. /// @@ -322,7 +367,7 @@ internal static string VerboseScriptDefinitionMessage { } /// - /// Looks up a localized string similar to {0} is not a valid key in the profile hashtable: line {0} column {1} in file {2}. + /// Looks up a localized string similar to {0} is not a valid key in the profile hashtable: line {1} column {2} in file {3}. This key value pair will not be processed.. /// internal static string WrongKey { get { diff --git a/Engine/Strings.resx b/Engine/Strings.resx index 9de1cef68..c998d8b36 100644 --- a/Engine/Strings.resx +++ b/Engine/Strings.resx @@ -193,7 +193,7 @@ Cannot find any DiagnosticRecord with the Rule Suppression ID {0}. - {0} is not a valid key in the profile hashtable: line {0} column {1} in file {2} + {0} is not a valid key in the profile hashtable: line {1} column {2} in file {3}. This key value pair will not be processed. Key in the profile hashtable should be a string: line {0} column {1} in file {2} @@ -216,4 +216,19 @@ Analyzing Script Definition. + + ConfigurationFileHasNoHashTable + + + ConfigurationFileNotFound + + + ConfigurationKeyNotAString + + + ConfigurationValueNotAString + + + ConfigurationValueWrongFormat + \ No newline at end of file diff --git a/Tests/Engine/GlobalSuppression.ps1 b/Tests/Engine/GlobalSuppression.ps1 index a777debc5..910007e7d 100644 --- a/Tests/Engine/GlobalSuppression.ps1 +++ b/Tests/Engine/GlobalSuppression.ps1 @@ -4,4 +4,80 @@ gcm "blah" $a = "//internal" -Get-Alias -ComputerName dfd \ No newline at end of file +Get-Alias -ComputerName dfd + +<# +.Synopsis + Short description +.DESCRIPTION + Long description +.EXAMPLE + Example of how to use this cmdlet +.EXAMPLE + Another example of how to use this cmdlet +.INPUTS + Inputs to this cmdlet (if any) +.OUTPUTS + Output from this cmdlet (if any) +.NOTES + General notes +.COMPONENT + The component this cmdlet belongs to +.ROLE + The role this cmdlet belongs to +.FUNCTIONALITY + The functionality that best describes this cmdlet +#> +function Verb-Noun +{ + [CmdletBinding(DefaultParameterSetName='Parameter Set 1', + SupportsShouldProcess=$true, + PositionalBinding=$false, + HelpUri = 'http://www.microsoft.com/', + ConfirmImpact='Medium')] + [Alias()] + Param + ( + # Param1 help description + [Parameter(Mandatory=$true, + ValueFromPipeline=$true, + ValueFromPipelineByPropertyName=$true, + ValueFromRemainingArguments=$false, + Position=0, + ParameterSetName='Parameter Set 1')] + [ValidateNotNull()] + [ValidateNotNullOrEmpty()] + [ValidateCount(0,5)] + [ValidateSet("sun", "moon", "earth")] + [Alias("p1")] + $Param1, + + # Param2 help description + [Parameter(ParameterSetName='Parameter Set 1')] + [AllowNull()] + [AllowEmptyCollection()] + [AllowEmptyString()] + [ValidateScript({$true})] + [ValidateRange(0,5)] + [int] + $Param2, + + # Param3 help description + [Parameter(ParameterSetName='Another Parameter Set')] + [ValidatePattern("[a-z]*")] + [ValidateLength(0,15)] + [String] + $Param3 + ) + + Begin + { + } + Process + { + return 1 + } + End + { + } +} \ No newline at end of file diff --git a/Tests/Engine/GlobalSuppression.test.ps1 b/Tests/Engine/GlobalSuppression.test.ps1 index e5aeb87cd..e4eb88050 100644 --- a/Tests/Engine/GlobalSuppression.test.ps1 +++ b/Tests/Engine/GlobalSuppression.test.ps1 @@ -46,18 +46,38 @@ Describe "GlobalSuppression" { } Context "Severity" { - It "Raises 1 violation for internal url without profile" { - $withoutProfile = $violations | Where-Object { $_.RuleName -eq "PSAvoidUsingInternalURLs" } + It "Raises 1 violation for use output type correctly without profile" { + $withoutProfile = $violations | Where-Object { $_.RuleName -eq "PSUseOutputTypeCorrectly" } $withoutProfile.Count | Should Be 1 - $withoutProfile = $violationsUsingScriptDefinition | Where-Object { $_.RuleName -eq "PSAvoidUsingInternalURLs" } + $withoutProfile = $violationsUsingScriptDefinition | Where-Object { $_.RuleName -eq "PSUseOutputTypeCorrectly" } $withoutProfile.Count | Should Be 1 } - It "Does not raise any violations for internal urls with profile" { - $withProfile = $suppression | Where-Object { $_.RuleName -eq "PSAvoidUsingInternalURLs" } + It "Does not raise any violations for use output type correctly with profile" { + $withProfile = $suppression | Where-Object { $_.RuleName -eq "PSUseOutputTypeCorrectly" } $withProfile.Count | Should be 0 - $withProfile = $suppressionUsingScriptDefinition | Where-Object { $_.RuleName -eq "PSAvoidUsingInternalURLs" } + $withProfile = $suppressionUsingScriptDefinition | Where-Object { $_.RuleName -eq "PSUseOutputTypeCorrectly" } $withProfile.Count | Should be 0 } } + + Context "Error Case" { + It "Raises Error for file not found" { + $invokeWithError = Invoke-ScriptAnalyzer "$directory\GlobalSuppression.ps1" -Configuration ".\ThisFileDoesNotExist.ps1" -ErrorAction SilentlyContinue + $invokeWithError.Count | should be 0 + $Error[0].FullyQualifiedErrorId | should match "ConfigurationFileNotFound,Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands.InvokeScriptAnalyzerCommand" + } + + It "Raises Error for file with no hash table" { + $invokeWithError = Invoke-ScriptAnalyzer "$directory\GlobalSuppression.ps1" -Configuration "$directory\GlobalSuppression.ps1" -ErrorAction SilentlyContinue + $invokeWithError.Count | should be 0 + $Error[0].FullyQualifiedErrorId | should match "ConfigurationFileHasNoHashTable,Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands.InvokeScriptAnalyzerCommand" + } + + It "Raises Error for wrong profile" { + $invokeWithError = Invoke-ScriptAnalyzer "$directory\GlobalSuppression.ps1" -Configuration "$directory\WrongProfile.ps1" -ErrorAction SilentlyContinue + $invokeWithError.Count | should be 0 + $Error[0].FullyQualifiedErrorId | should match "ConfigurationValueWrongFormat,Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands.InvokeScriptAnalyzerCommand" + } + } } \ No newline at end of file diff --git a/Tests/Engine/WrongProfile.ps1 b/Tests/Engine/WrongProfile.ps1 new file mode 100644 index 000000000..b11fbd07b --- /dev/null +++ b/Tests/Engine/WrongProfile.ps1 @@ -0,0 +1,9 @@ +@{ + Severity='Warning' + IncludeRules=@('PSAvoidUsingCmdletAliases', + 'PSAvoidUsingPositionalParameters', + 'PSAvoidUsingInternalURLs' + 'PSAvoidUninitializedVariable') + ExcludeRules=@(1) + Exclude=@('blah') +} \ No newline at end of file From ac11a2d7fa1116c8c521914b125587df7cb26d31 Mon Sep 17 00:00:00 2001 From: Quoc Truong Date: Wed, 2 Dec 2015 15:05:27 -0800 Subject: [PATCH 2/2] Change remaining warning to error --- Engine/ScriptAnalyzer.cs | 8 ++++---- Engine/Strings.Designer.cs | 11 ++++++++++- Engine/Strings.resx | 5 ++++- Tests/Engine/GlobalSuppression.test.ps1 | 2 +- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index ce3310985..ae4dde1de 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -281,10 +281,10 @@ internal bool ParseProfile(string profile, PathIntrinsics path, IOutputWriter wr excludeRuleList = excludeRuleList.Union(rhsList); break; default: - // keep writing warning here, we only stop processing if existing keys are wrong - writer.WriteWarning( - string.Format(CultureInfo.CurrentCulture, Strings.WrongKey, key, - kvp.Item1.Extent.StartLineNumber, kvp.Item1.Extent.StartColumnNumber, profile)); + writer.WriteError(new ErrorRecord( + new InvalidDataException(string.Format(CultureInfo.CurrentCulture, Strings.WrongKey, key, kvp.Item1.Extent.StartLineNumber, kvp.Item1.Extent.StartColumnNumber, profile)), + Strings.WrongConfigurationKey, ErrorCategory.InvalidData, profile)); + hasError = true; break; } } diff --git a/Engine/Strings.Designer.cs b/Engine/Strings.Designer.cs index 5b6596ba1..a0966e77a 100644 --- a/Engine/Strings.Designer.cs +++ b/Engine/Strings.Designer.cs @@ -367,7 +367,16 @@ internal static string VerboseScriptDefinitionMessage { } /// - /// Looks up a localized string similar to {0} is not a valid key in the profile hashtable: line {1} column {2} in file {3}. This key value pair will not be processed.. + /// Looks up a localized string similar to WrongConfigurationKey. + /// + internal static string WrongConfigurationKey { + get { + return ResourceManager.GetString("WrongConfigurationKey", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to {0} is not a valid key in the profile hashtable: line {1} column {2} in file {3}. Valid keys are ExcludeRules, IncludeRules and Severity.. /// internal static string WrongKey { get { diff --git a/Engine/Strings.resx b/Engine/Strings.resx index c998d8b36..7907b2efa 100644 --- a/Engine/Strings.resx +++ b/Engine/Strings.resx @@ -193,7 +193,7 @@ Cannot find any DiagnosticRecord with the Rule Suppression ID {0}. - {0} is not a valid key in the profile hashtable: line {1} column {2} in file {3}. This key value pair will not be processed. + {0} is not a valid key in the profile hashtable: line {1} column {2} in file {3}. Valid keys are ExcludeRules, IncludeRules and Severity. Key in the profile hashtable should be a string: line {0} column {1} in file {2} @@ -231,4 +231,7 @@ ConfigurationValueWrongFormat + + WrongConfigurationKey + \ No newline at end of file diff --git a/Tests/Engine/GlobalSuppression.test.ps1 b/Tests/Engine/GlobalSuppression.test.ps1 index e4eb88050..9608071ed 100644 --- a/Tests/Engine/GlobalSuppression.test.ps1 +++ b/Tests/Engine/GlobalSuppression.test.ps1 @@ -77,7 +77,7 @@ Describe "GlobalSuppression" { It "Raises Error for wrong profile" { $invokeWithError = Invoke-ScriptAnalyzer "$directory\GlobalSuppression.ps1" -Configuration "$directory\WrongProfile.ps1" -ErrorAction SilentlyContinue $invokeWithError.Count | should be 0 - $Error[0].FullyQualifiedErrorId | should match "ConfigurationValueWrongFormat,Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands.InvokeScriptAnalyzerCommand" + $Error[0].FullyQualifiedErrorId | should match "WrongConfigurationKey,Microsoft.Windows.PowerShell.ScriptAnalyzer.Commands.InvokeScriptAnalyzerCommand" } } } \ No newline at end of file