From 010a8621170652d61039fad376485aee3846c999 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 6 Nov 2018 19:56:20 +0000 Subject: [PATCH 01/15] Fix Parsing of settings that occurs when using turkish culture --- Engine/Settings.cs | 2 +- Tests/Engine/InvokeFormatter.tests.ps1 | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/Engine/Settings.cs b/Engine/Settings.cs index 4f9dcd546..eba05b0b3 100644 --- a/Engine/Settings.cs +++ b/Engine/Settings.cs @@ -395,7 +395,7 @@ private void parseSettingsHashtable(Hashtable settingsHashtable) var settings = GetDictionaryFromHashtable(settingsHashtable); foreach (var settingKey in settings.Keys) { - var key = settingKey.ToLower(); + var key = settingKey.ToLowerInvariant(); // Invariant is important to also work with turkish culture, see https://github.com/PowerShell/PSScriptAnalyzer/issues/1095 object val = settings[key]; switch (key) { diff --git a/Tests/Engine/InvokeFormatter.tests.ps1 b/Tests/Engine/InvokeFormatter.tests.ps1 index a4d841657..894237d68 100644 --- a/Tests/Engine/InvokeFormatter.tests.ps1 +++ b/Tests/Engine/InvokeFormatter.tests.ps1 @@ -85,6 +85,17 @@ function foo { Invoke-Formatter -ScriptDefinition $def | Should -Be $expected } + + It "Does not throw when using turkish culture - https://github.com/PowerShell/PSScriptAnalyzer/issues/1095" { + $initialCulture = [System.Threading.Thread]::CurrentThread.CurrentCulture + try { + [System.Threading.Thread]::CurrentThread.CurrentCulture = [cultureinfo]::CreateSpecificCulture('tr-TR') + Invoke-Formatter ' foo' | Should -Be 'foo' + } + finally { + [System.Threading.Thread]::CurrentThread.CurrentCulture = $initialCulture + } + } } } From 57ab21c2fa28592b38ebe2967497b8cbf297ffd7 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 8 Nov 2018 08:59:17 +0000 Subject: [PATCH 02/15] use german culture before running tests to exhibit failures --- tools/appveyor.psm1 | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/appveyor.psm1 b/tools/appveyor.psm1 index cc8e15f81..a4df1ca2e 100644 --- a/tools/appveyor.psm1 +++ b/tools/appveyor.psm1 @@ -56,6 +56,7 @@ function Invoke-AppveyorTest { Copy-Item "${CheckoutPath}\out\PSScriptAnalyzer" "$modulePath\" -Recurse -Force $testResultsFile = ".\TestResults.xml" $testScripts = "${CheckoutPath}\Tests\Engine","${CheckoutPath}\Tests\Rules","${CheckoutPath}\Tests\Documentation" + [System.Threading.Thread]::CurrentThread.CurrentCulture = [cultureinfo]::CreateSpecificCulture('de-DE') $testResults = Invoke-Pester -Script $testScripts -OutputFormat NUnitXml -OutputFile $testResultsFile -PassThru (New-Object 'System.Net.WebClient').UploadFile("https://ci.appveyor.com/api/testresults/nunit/${env:APPVEYOR_JOB_ID}", (Resolve-Path $testResultsFile)) if ($testResults.FailedCount -gt 0) { From dcfe1cfc50bc7287896c363319d0a0ff2932040b Mon Sep 17 00:00:00 2001 From: "Christoph Bergmeister [MVP]" Date: Thu, 8 Nov 2018 12:58:27 +0000 Subject: [PATCH 03/15] set ui culture as well --- tools/appveyor.psm1 | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/appveyor.psm1 b/tools/appveyor.psm1 index a4df1ca2e..1a9b2cd4a 100644 --- a/tools/appveyor.psm1 +++ b/tools/appveyor.psm1 @@ -57,6 +57,7 @@ function Invoke-AppveyorTest { $testResultsFile = ".\TestResults.xml" $testScripts = "${CheckoutPath}\Tests\Engine","${CheckoutPath}\Tests\Rules","${CheckoutPath}\Tests\Documentation" [System.Threading.Thread]::CurrentThread.CurrentCulture = [cultureinfo]::CreateSpecificCulture('de-DE') + [System.Threading.Thread]::CurrentThread.CurrentUICulture = [cultureinfo]::CreateSpecificCulture('de-DE') $testResults = Invoke-Pester -Script $testScripts -OutputFormat NUnitXml -OutputFile $testResultsFile -PassThru (New-Object 'System.Net.WebClient').UploadFile("https://ci.appveyor.com/api/testresults/nunit/${env:APPVEYOR_JOB_ID}", (Resolve-Path $testResultsFile)) if ($testResults.FailedCount -gt 0) { From 5fea008381889e2f008f18b5bd6b8c17e9da7b25 Mon Sep 17 00:00:00 2001 From: "Christoph Bergmeister [MVP]" Date: Thu, 8 Nov 2018 18:39:08 +0000 Subject: [PATCH 04/15] fix communityanalyzer for turkish culture --- Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 b/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 index d5b39baca..cbae02dee 100644 --- a/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 +++ b/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 @@ -167,7 +167,7 @@ function Measure-RequiresModules [System.Management.Automation.Language.CommandAst]$cmdAst = $Ast; if ($null -ne $cmdAst.GetCommandName()) { - if ($cmdAst.GetCommandName().ToLower() -eq "import-module") + if ($cmdAst.GetCommandName().ToLowerInvariant() -eq "import-module") { $returnValue = $true } From a8df05d9186a2e9b6a090966f774d5134ff1ef9d Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 8 Nov 2018 18:54:38 +0000 Subject: [PATCH 05/15] fix ProvideCommentHelp for turkish culture (i problem again) --- Rules/ProvideCommentHelp.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index 3384debef..ba4e20306 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -419,7 +419,7 @@ public override string ToString() public virtual string ToString(int? tabStop) { var sb = new StringBuilder(); - sb.Append(".").AppendLine(Name.ToUpper()); + sb.Append(".").AppendLine(Name.ToUpperInvariant()); if (!String.IsNullOrWhiteSpace(Description)) { sb.Append(Snippetify(tabStop, Description)); From c33c38b59f87870323c1fd5d2342e8323dc5137b Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 8 Nov 2018 21:17:17 +0000 Subject: [PATCH 06/15] update comments --- Engine/Settings.cs | 2 +- Rules/ProvideCommentHelp.cs | 2 +- .../CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Engine/Settings.cs b/Engine/Settings.cs index eba05b0b3..8e5b406d8 100644 --- a/Engine/Settings.cs +++ b/Engine/Settings.cs @@ -395,7 +395,7 @@ private void parseSettingsHashtable(Hashtable settingsHashtable) var settings = GetDictionaryFromHashtable(settingsHashtable); foreach (var settingKey in settings.Keys) { - var key = settingKey.ToLowerInvariant(); // Invariant is important to also work with turkish culture, see https://github.com/PowerShell/PSScriptAnalyzer/issues/1095 + var key = settingKey.ToLowerInvariant(); // ToLowerInvariant is important to also work with turkish culture, see https://github.com/PowerShell/PSScriptAnalyzer/issues/1095 object val = settings[key]; switch (key) { diff --git a/Rules/ProvideCommentHelp.cs b/Rules/ProvideCommentHelp.cs index ba4e20306..63e8d6438 100644 --- a/Rules/ProvideCommentHelp.cs +++ b/Rules/ProvideCommentHelp.cs @@ -419,7 +419,7 @@ public override string ToString() public virtual string ToString(int? tabStop) { var sb = new StringBuilder(); - sb.Append(".").AppendLine(Name.ToUpperInvariant()); + sb.Append(".").AppendLine(Name.ToUpperInvariant()); // ToUpperInvariant is important to also work with turkish culture, see https://github.com/PowerShell/PSScriptAnalyzer/issues/1095 if (!String.IsNullOrWhiteSpace(Description)) { sb.Append(Snippetify(tabStop, Description)); diff --git a/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 b/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 index cbae02dee..b2d3aa1e9 100644 --- a/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 +++ b/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 @@ -167,6 +167,7 @@ function Measure-RequiresModules [System.Management.Automation.Language.CommandAst]$cmdAst = $Ast; if ($null -ne $cmdAst.GetCommandName()) { + # ToLowerInvariant is important to also work with turkish culture, see https://github.com/PowerShell/PSScriptAnalyzer/issues/1095 if ($cmdAst.GetCommandName().ToLowerInvariant() -eq "import-module") { $returnValue = $true @@ -225,8 +226,8 @@ function Measure-RequiresModules } -# The two rules in the following if block use StaticParameterBinder class. -# StaticParameterBinder class was introduced in PSv4. +# The two rules in the following if block use StaticParameterBinder class. +# StaticParameterBinder class was introduced in PSv4. if ($PSVersionTable.PSVersion -ge [Version]'4.0.0') { <# From 479ed3a9c3e4dcdc72a5068eb450e09751ecd682 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Fri, 9 Nov 2018 08:20:52 +0000 Subject: [PATCH 07/15] fix for import-localizedDatabug in pscore (opened issue 8219 in ps core repo) --- .../CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 b/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 index b2d3aa1e9..17ff14871 100644 --- a/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 +++ b/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 @@ -1,7 +1,14 @@ #Requires -Version 3.0 # Import Localized Data -Import-LocalizedData -BindingVariable Messages +if ([System.Threading.Thread]::CurrentThread.CurrentCulture.Name -ne 'en-US') +{ + Import-LocalizedData -BindingVariable Messages -UICulture 'en-US' +} +else +{ + Import-LocalizedData -BindingVariable Messages +} <# .SYNOPSIS From 227b64ff04d29376a2549f27eede7222f66aa9d4 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Fri, 9 Nov 2018 08:21:28 +0000 Subject: [PATCH 08/15] change to UI culture --- Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 b/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 index 17ff14871..d1cbe9e86 100644 --- a/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 +++ b/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 @@ -1,7 +1,7 @@ #Requires -Version 3.0 # Import Localized Data -if ([System.Threading.Thread]::CurrentThread.CurrentCulture.Name -ne 'en-US') +if ([System.Threading.Thread]::CurrentThread.CurrentUICulture.Name -ne 'en-US') { Import-LocalizedData -BindingVariable Messages -UICulture 'en-US' } From cee22f91f49df951668054b124cb8a56675b8fba Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 12 Nov 2018 18:03:43 +0000 Subject: [PATCH 09/15] use invariant culture comparison instead of tolowerinvariant and revert change where it is not needed --- Engine/Generic/ModuleDependencyHandler.cs | 3 +- Engine/Generic/RuleSuppression.cs | 115 +++++++------- Engine/ScriptAnalyzer.cs | 142 +++++++++--------- Engine/Settings.cs | 17 +-- Rules/UseShouldProcessCorrectly.cs | 2 +- ScriptRuleDocumentation.md | 2 +- .../CommunityAnalyzerRules.psm1 | 5 +- 7 files changed, 129 insertions(+), 157 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 0d0833aaf..adcd51660 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -286,7 +286,7 @@ public ModuleDependencyHandler( ? "PSScriptAnalyzer" : pssaAppDataPath); - modulesFound = new Dictionary(); + modulesFound = new Dictionary(StringComparer.InvariantCultureIgnoreCase); // TODO Add PSSA Version in the path symLinkPath = Path.Combine(pssaAppDataPath, symLinkName); @@ -303,7 +303,6 @@ public ModuleDependencyHandler( public PSObject FindModule(string moduleName) { ThrowIfNull(moduleName, "moduleName"); - moduleName = moduleName.ToLower(); if (modulesFound.ContainsKey(moduleName)) { return modulesFound[moduleName]; diff --git a/Engine/Generic/RuleSuppression.cs b/Engine/Generic/RuleSuppression.cs index ac59d43b4..3660c6d8d 100644 --- a/Engine/Generic/RuleSuppression.cs +++ b/Engine/Generic/RuleSuppression.cs @@ -202,61 +202,56 @@ public RuleSuppression(AttributeAst attrAst, int start, int end) break; } - switch (name.ArgumentName.ToLower()) + var argumentName = name.ArgumentName; + if (argumentName.Equals("rulename", StringComparison.InvariantCultureIgnoreCase)) { - case "rulename": - if (!String.IsNullOrWhiteSpace(RuleName)) - { - Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name); - } - - RuleName = (name.Argument as StringConstantExpressionAst).Value; - goto default; - - case "rulesuppressionid": - if (!String.IsNullOrWhiteSpace(RuleSuppressionID)) - { - Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name); - } - - RuleSuppressionID = (name.Argument as StringConstantExpressionAst).Value; - goto default; - - case "scope": - if (!String.IsNullOrWhiteSpace(Scope)) - { - Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name); - } - - Scope = (name.Argument as StringConstantExpressionAst).Value; - - if (!scopeSet.Contains(Scope)) - { - Error = Strings.WrongScopeArgumentSuppressionAttributeError; - } + if (!String.IsNullOrWhiteSpace(RuleName)) + { + Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name); + } - goto default; + RuleName = (name.Argument as StringConstantExpressionAst).Value; + } + else if (argumentName.Equals("rulesuppressionid", StringComparison.InvariantCultureIgnoreCase)) + { + if (!String.IsNullOrWhiteSpace(RuleName)) + { + Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name); + } - case "target": - if (!String.IsNullOrWhiteSpace(Target)) - { - Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name); - } + RuleName = (name.Argument as StringConstantExpressionAst).Value; + } + else if (argumentName.Equals("scope", StringComparison.InvariantCultureIgnoreCase)) + { + if (!String.IsNullOrWhiteSpace(Scope)) + { + Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name); + } - Target = (name.Argument as StringConstantExpressionAst).Value; - goto default; + Scope = (name.Argument as StringConstantExpressionAst).Value; - case "justification": - if (!String.IsNullOrWhiteSpace(Justification)) - { - Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name); - } + if (!scopeSet.Contains(Scope)) + { + Error = Strings.WrongScopeArgumentSuppressionAttributeError; + } + } + else if (argumentName.Equals("target", StringComparison.InvariantCultureIgnoreCase)) + { + if (!String.IsNullOrWhiteSpace(Target)) + { + Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name); + } - Justification = (name.Argument as StringConstantExpressionAst).Value; - goto default; + Target = (name.Argument as StringConstantExpressionAst).Value; + } + else if (argumentName.Equals("justification", StringComparison.InvariantCultureIgnoreCase)) + { + if (!String.IsNullOrWhiteSpace(Justification)) + { + Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name); + } - default: - break; + Justification = (name.Argument as StringConstantExpressionAst).Value; } } } @@ -354,23 +349,17 @@ public static List GetSuppressions(IEnumerable at Regex reg = new Regex(String.Format("^{0}$", ruleSupp.Target.Replace(@"*", ".*")), RegexOptions.IgnoreCase); IEnumerable targetAsts = null; - switch (ruleSupp.Scope.ToLower()) + var scope = ruleSupp.Scope; + if (scope.Equals("function", StringComparison.InvariantCultureIgnoreCase)) { - case "function": - targetAsts = scopeAst.FindAll(item => item is FunctionDefinitionAst && reg.IsMatch((item as FunctionDefinitionAst).Name), true); - goto default; - - #if !(PSV3||PSV4) - - case "class": - targetAsts = scopeAst.FindAll(item => item is TypeDefinitionAst && reg.IsMatch((item as TypeDefinitionAst).Name), true); - goto default; - - #endif - - default: - break; + targetAsts = scopeAst.FindAll(ast => ast is FunctionDefinitionAst && reg.IsMatch((ast as FunctionDefinitionAst).Name), true); + } + #if !(PSV3 || PSV4) + else if (scope.Equals("class", StringComparison.InvariantCultureIgnoreCase)) + { + targetAsts = scopeAst.FindAll(ast => ast is TypeDefinitionAst && reg.IsMatch((ast as TypeDefinitionAst).Name), true); } + #endif if (targetAsts != null) { diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index ae6e22255..1f1e108bc 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -282,20 +282,19 @@ private bool AddProfileItem( Debug.Assert(includeRuleList != null); Debug.Assert(excludeRuleList != null); - switch (key.ToLower()) + if (key.Equals("severity", StringComparison.InvariantCultureIgnoreCase)) { - case "severity": - severityList.AddRange(values); - break; - case "includerules": - includeRuleList.AddRange(values); - break; - case "excluderules": - excludeRuleList.AddRange(values); - break; - default: - return false; + severityList.AddRange(values); + } + else if (key.Equals("includerules", StringComparison.InvariantCultureIgnoreCase)) + { + includeRuleList.AddRange(values); } + else + { + return false; + } + return true; } @@ -509,75 +508,70 @@ private bool ParseProfileHashtable(Hashtable profile, PathIntrinsics path, IOutp settings.Keys.CopyTo(settingsKeys, 0); foreach (var settingKey in settingsKeys) { - var key = settingKey.ToLower(); - object value = settings[key]; - switch (key) - { - case "severity": - case "includerules": - case "excluderules": - // value must be either string or collections of string or array - if (value == null - || !(value is string - || value is IEnumerable - || value.GetType().IsArray)) - { - writer.WriteError( - new ErrorRecord( - new InvalidDataException(string.Format(CultureInfo.CurrentCulture, Strings.WrongValueHashTable, value, key)), - Strings.WrongConfigurationKey, - ErrorCategory.InvalidData, - profile)); - hasError = true; - break; - } - List values = new List(); - if (value is string) - { - values.Add(value as string); - } - else if (value is IEnumerable) - { - values.Union(value as IEnumerable); - } - else if (value.GetType().IsArray) - { - // for array case, sometimes we won't be able to cast it directly to IEnumerable - foreach (var val in value as IEnumerable) - { - if (val is string) - { - values.Add(val as string); - } - else - { - writer.WriteError( - new ErrorRecord( - new InvalidDataException(string.Format(CultureInfo.CurrentCulture, Strings.WrongValueHashTable, val, key)), - Strings.WrongConfigurationKey, - ErrorCategory.InvalidData, - profile)); - hasError = true; - break; - } - } - } - AddProfileItem(key, values, severityList, includeRuleList, excludeRuleList); - settings[key] = values; - break; + object value = settings[settingKey]; - case "rules": - break; - - default: + if (settingKey.Equals("severity", StringComparison.InvariantCultureIgnoreCase) || + settingKey.Equals("includerules", StringComparison.InvariantCultureIgnoreCase) || + settingKey.Equals("excluderules", StringComparison.InvariantCultureIgnoreCase)) + { + // value must be either string or collections of string or array + if (value == null + || !(value is string + || value is IEnumerable + || value.GetType().IsArray)) + { writer.WriteError( new ErrorRecord( - new InvalidDataException(string.Format(CultureInfo.CurrentCulture, Strings.WrongKeyHashTable, key)), + new InvalidDataException(string.Format(CultureInfo.CurrentCulture, Strings.WrongValueHashTable, value, settingKey)), Strings.WrongConfigurationKey, ErrorCategory.InvalidData, profile)); hasError = true; break; + } + var values = new List(); + if (value is string) + { + values.Add(value as string); + } + else if (value is IEnumerable) + { + values.Union(value as IEnumerable); + } + else if (value.GetType().IsArray) + { + // for array case, sometimes we won't be able to cast it directly to IEnumerable + foreach (var val in value as IEnumerable) + { + if (val is string) + { + values.Add(val as string); + } + else + { + writer.WriteError( + new ErrorRecord( + new InvalidDataException(string.Format(CultureInfo.CurrentCulture, Strings.WrongValueHashTable, val, settingKey)), + Strings.WrongConfigurationKey, + ErrorCategory.InvalidData, + profile)); + hasError = true; + break; + } + } + } + AddProfileItem(settingKey, values, severityList, includeRuleList, excludeRuleList); + settings[settingKey] = values; + } + else if (settingKey.Equals("excluderules", StringComparison.InvariantCultureIgnoreCase)) + { + writer.WriteError( + new ErrorRecord( + new InvalidDataException(string.Format(CultureInfo.CurrentCulture, Strings.WrongKeyHashTable, settingKey)), + Strings.WrongConfigurationKey, + ErrorCategory.InvalidData, + profile)); + hasError = true; } } return hasError; @@ -1843,7 +1837,7 @@ private IEnumerable AnalyzeFile(string filePath) if (File.Exists(filePath)) { // processing for non help script - if (!(Path.GetFileName(filePath).ToLower().StartsWith("about_") && Path.GetFileName(filePath).ToLower().EndsWith(".help.txt"))) + if (!(Path.GetFileName(filePath).StartsWith("about_", StringComparison.InvariantCultureIgnoreCase) && Path.GetFileName(filePath).EndsWith(".help.txt", StringComparison.InvariantCultureIgnoreCase))) { try { diff --git a/Engine/Settings.cs b/Engine/Settings.cs index 8e5b406d8..6969a8873 100644 --- a/Engine/Settings.cs +++ b/Engine/Settings.cs @@ -536,21 +536,12 @@ private Hashtable GetHashtableFromHashTableAst(HashtableAst hashTableAst) } else if (pureExp is VariableExpressionAst) { - var varExprAst = (VariableExpressionAst)pureExp; - switch (varExprAst.VariablePath.UserPath.ToLower()) + var variableExpressionAst = (VariableExpressionAst)pureExp; + if (!bool.TryParse(variableExpressionAst.VariablePath.UserPath, out bool booleanValue)) { - case "true": - output[key] = true; - break; - - case "false": - output[key] = false; - break; - - default: - ThrowInvalidDataException(varExprAst.Extent); - break; + ThrowInvalidDataException(variableExpressionAst.Extent); } + output[key] = booleanValue; continue; } diff --git a/Rules/UseShouldProcessCorrectly.cs b/Rules/UseShouldProcessCorrectly.cs index 6be9db87e..6cacc2634 100644 --- a/Rules/UseShouldProcessCorrectly.cs +++ b/Rules/UseShouldProcessCorrectly.cs @@ -451,7 +451,7 @@ public override bool Equals(Object other) /// public override int GetHashCode() { - return name.ToLower().GetHashCode(); + return name.ToLowerInvariant().GetHashCode(); } } diff --git a/ScriptRuleDocumentation.md b/ScriptRuleDocumentation.md index 45e558ef7..223dff5dc 100644 --- a/ScriptRuleDocumentation.md +++ b/ScriptRuleDocumentation.md @@ -153,7 +153,7 @@ function Measure-RequiresRunAsAdministrator if ($Ast -is [System.Management.Automation.Language.AssignmentStatementAst]) { [System.Management.Automation.Language.AssignmentStatementAst]$asAst = $Ast - if ($asAst.Right.ToString().ToLower() -eq '[system.security.principal.windowsbuiltinrole]::administrator') + if ($asAst.Right.ToString() -eq '[system.security.principal.windowsbuiltinrole]::administrator') { $returnValue = $true } diff --git a/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 b/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 index d1cbe9e86..5ed8f6ac0 100644 --- a/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 +++ b/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 @@ -78,7 +78,7 @@ function Measure-RequiresRunAsAdministrator if ($ast -is [System.Management.Automation.Language.AssignmentStatementAst]) { [System.Management.Automation.Language.AssignmentStatementAst]$asAst = $Ast; - if ($asAst.Right.ToString().ToLower() -eq "[system.security.principal.windowsbuiltinrole]::administrator") + if ($asAst.Right.ToString() -eq "[system.security.principal.windowsbuiltinrole]::administrator") { $returnValue = $true } @@ -174,8 +174,7 @@ function Measure-RequiresModules [System.Management.Automation.Language.CommandAst]$cmdAst = $Ast; if ($null -ne $cmdAst.GetCommandName()) { - # ToLowerInvariant is important to also work with turkish culture, see https://github.com/PowerShell/PSScriptAnalyzer/issues/1095 - if ($cmdAst.GetCommandName().ToLowerInvariant() -eq "import-module") + if ($cmdAst.GetCommandName() -eq "import-module") { $returnValue = $true } From 079b98dfdd00c58e175fff00d93e7c50c8914239 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 12 Nov 2018 18:29:28 +0000 Subject: [PATCH 10/15] finish with comments --- .../CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 | 1 + tools/appveyor.psm1 | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 b/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 index 5ed8f6ac0..f9abf9950 100644 --- a/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 +++ b/Tests/Engine/CommunityAnalyzerRules/CommunityAnalyzerRules.psm1 @@ -1,6 +1,7 @@ #Requires -Version 3.0 # Import Localized Data +# Explicit culture needed for culture that do not match when using PowerShell Core: https://github.com/PowerShell/PowerShell/issues/8219 if ([System.Threading.Thread]::CurrentThread.CurrentUICulture.Name -ne 'en-US') { Import-LocalizedData -BindingVariable Messages -UICulture 'en-US' diff --git a/tools/appveyor.psm1 b/tools/appveyor.psm1 index 1a9b2cd4a..a6c2449b3 100644 --- a/tools/appveyor.psm1 +++ b/tools/appveyor.psm1 @@ -56,8 +56,9 @@ function Invoke-AppveyorTest { Copy-Item "${CheckoutPath}\out\PSScriptAnalyzer" "$modulePath\" -Recurse -Force $testResultsFile = ".\TestResults.xml" $testScripts = "${CheckoutPath}\Tests\Engine","${CheckoutPath}\Tests\Rules","${CheckoutPath}\Tests\Documentation" - [System.Threading.Thread]::CurrentThread.CurrentCulture = [cultureinfo]::CreateSpecificCulture('de-DE') - [System.Threading.Thread]::CurrentThread.CurrentUICulture = [cultureinfo]::CreateSpecificCulture('de-DE') + # Change culture to test that PSSA works well with different locales + [System.Threading.Thread]::CurrentThread.CurrentCulture = [cultureinfo]::CreateSpecificCulture('tr-TR') + [System.Threading.Thread]::CurrentThread.CurrentUICulture = [cultureinfo]::CreateSpecificCulture('tr-TR') $testResults = Invoke-Pester -Script $testScripts -OutputFormat NUnitXml -OutputFile $testResultsFile -PassThru (New-Object 'System.Net.WebClient').UploadFile("https://ci.appveyor.com/api/testresults/nunit/${env:APPVEYOR_JOB_ID}", (Resolve-Path $testResultsFile)) if ($testResults.FailedCount -gt 0) { From 6d28bcc187d2917cb77c0e695a6190f637d13beb Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 15 Dec 2018 12:19:20 +0000 Subject: [PATCH 11/15] Use OrdinalIgnoreCase --- Engine/Generic/ModuleDependencyHandler.cs | 2 +- Engine/Generic/RuleSuppression.cs | 14 +++++++------- Engine/ScriptAnalyzer.cs | 14 +++++++------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Engine/Generic/ModuleDependencyHandler.cs b/Engine/Generic/ModuleDependencyHandler.cs index 53e3999ac..0d9dc3444 100644 --- a/Engine/Generic/ModuleDependencyHandler.cs +++ b/Engine/Generic/ModuleDependencyHandler.cs @@ -271,7 +271,7 @@ public ModuleDependencyHandler( ? "PSScriptAnalyzer" : pssaAppDataPath); - modulesFound = new Dictionary(StringComparer.InvariantCultureIgnoreCase); + modulesFound = new Dictionary(StringComparer.OrdinalIgnoreCase); // TODO Add PSSA Version in the path symLinkPath = Path.Combine(pssaAppDataPath, symLinkName); diff --git a/Engine/Generic/RuleSuppression.cs b/Engine/Generic/RuleSuppression.cs index 3660c6d8d..b4bb492d7 100644 --- a/Engine/Generic/RuleSuppression.cs +++ b/Engine/Generic/RuleSuppression.cs @@ -203,7 +203,7 @@ public RuleSuppression(AttributeAst attrAst, int start, int end) } var argumentName = name.ArgumentName; - if (argumentName.Equals("rulename", StringComparison.InvariantCultureIgnoreCase)) + if (argumentName.Equals("rulename", StringComparison.OrdinalIgnoreCase)) { if (!String.IsNullOrWhiteSpace(RuleName)) { @@ -212,7 +212,7 @@ public RuleSuppression(AttributeAst attrAst, int start, int end) RuleName = (name.Argument as StringConstantExpressionAst).Value; } - else if (argumentName.Equals("rulesuppressionid", StringComparison.InvariantCultureIgnoreCase)) + else if (argumentName.Equals("rulesuppressionid", StringComparison.OrdinalIgnoreCase)) { if (!String.IsNullOrWhiteSpace(RuleName)) { @@ -221,7 +221,7 @@ public RuleSuppression(AttributeAst attrAst, int start, int end) RuleName = (name.Argument as StringConstantExpressionAst).Value; } - else if (argumentName.Equals("scope", StringComparison.InvariantCultureIgnoreCase)) + else if (argumentName.Equals("scope", StringComparison.OrdinalIgnoreCase)) { if (!String.IsNullOrWhiteSpace(Scope)) { @@ -235,7 +235,7 @@ public RuleSuppression(AttributeAst attrAst, int start, int end) Error = Strings.WrongScopeArgumentSuppressionAttributeError; } } - else if (argumentName.Equals("target", StringComparison.InvariantCultureIgnoreCase)) + else if (argumentName.Equals("target", StringComparison.OrdinalIgnoreCase)) { if (!String.IsNullOrWhiteSpace(Target)) { @@ -244,7 +244,7 @@ public RuleSuppression(AttributeAst attrAst, int start, int end) Target = (name.Argument as StringConstantExpressionAst).Value; } - else if (argumentName.Equals("justification", StringComparison.InvariantCultureIgnoreCase)) + else if (argumentName.Equals("justification", StringComparison.OrdinalIgnoreCase)) { if (!String.IsNullOrWhiteSpace(Justification)) { @@ -350,12 +350,12 @@ public static List GetSuppressions(IEnumerable at IEnumerable targetAsts = null; var scope = ruleSupp.Scope; - if (scope.Equals("function", StringComparison.InvariantCultureIgnoreCase)) + if (scope.Equals("function", StringComparison.OrdinalIgnoreCase)) { targetAsts = scopeAst.FindAll(ast => ast is FunctionDefinitionAst && reg.IsMatch((ast as FunctionDefinitionAst).Name), true); } #if !(PSV3 || PSV4) - else if (scope.Equals("class", StringComparison.InvariantCultureIgnoreCase)) + else if (scope.Equals("class", StringComparison.OrdinalIgnoreCase)) { targetAsts = scopeAst.FindAll(ast => ast is TypeDefinitionAst && reg.IsMatch((ast as TypeDefinitionAst).Name), true); } diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index bc9cea9a0..2b3ae3ed2 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -282,11 +282,11 @@ private bool AddProfileItem( Debug.Assert(includeRuleList != null); Debug.Assert(excludeRuleList != null); - if (key.Equals("severity", StringComparison.InvariantCultureIgnoreCase)) + if (key.Equals("severity", StringComparison.OrdinalIgnoreCase)) { severityList.AddRange(values); } - else if (key.Equals("includerules", StringComparison.InvariantCultureIgnoreCase)) + else if (key.Equals("includerules", StringComparison.OrdinalIgnoreCase)) { includeRuleList.AddRange(values); } @@ -510,9 +510,9 @@ private bool ParseProfileHashtable(Hashtable profile, PathIntrinsics path, IOutp { object value = settings[settingKey]; - if (settingKey.Equals("severity", StringComparison.InvariantCultureIgnoreCase) || - settingKey.Equals("includerules", StringComparison.InvariantCultureIgnoreCase) || - settingKey.Equals("excluderules", StringComparison.InvariantCultureIgnoreCase)) + if (settingKey.Equals("severity", StringComparison.OrdinalIgnoreCase) || + settingKey.Equals("includerules", StringComparison.OrdinalIgnoreCase) || + settingKey.Equals("excluderules", StringComparison.OrdinalIgnoreCase)) { // value must be either string or collections of string or array if (value == null @@ -563,7 +563,7 @@ private bool ParseProfileHashtable(Hashtable profile, PathIntrinsics path, IOutp AddProfileItem(settingKey, values, severityList, includeRuleList, excludeRuleList); settings[settingKey] = values; } - else if (settingKey.Equals("excluderules", StringComparison.InvariantCultureIgnoreCase)) + else if (settingKey.Equals("excluderules", StringComparison.OrdinalIgnoreCase)) { writer.WriteError( new ErrorRecord( @@ -1837,7 +1837,7 @@ private IEnumerable AnalyzeFile(string filePath) if (File.Exists(filePath)) { // processing for non help script - if (!(Path.GetFileName(filePath).StartsWith("about_", StringComparison.InvariantCultureIgnoreCase) && Path.GetFileName(filePath).EndsWith(".help.txt", StringComparison.InvariantCultureIgnoreCase))) + if (!(Path.GetFileName(filePath).StartsWith("about_", StringComparison.OrdinalIgnoreCase) && Path.GetFileName(filePath).EndsWith(".help.txt", StringComparison.OrdinalIgnoreCase))) { try { From 7ffbf65f3de1f40602b3f154da3f7914038299f9 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 15 Dec 2018 12:20:26 +0000 Subject: [PATCH 12/15] revert accidental change from 2 commits before --- Rules/AlignAssignmentStatement.cs | 4 ---- Rules/UseConsistentWhitespace.cs | 3 --- 2 files changed, 7 deletions(-) diff --git a/Rules/AlignAssignmentStatement.cs b/Rules/AlignAssignmentStatement.cs index 1b5bd7c24..d8b1623d6 100644 --- a/Rules/AlignAssignmentStatement.cs +++ b/Rules/AlignAssignmentStatement.cs @@ -135,8 +135,6 @@ public override SourceType GetSourceType() { return SourceType.Builtin; } - - #region AllignAssignment statement private IEnumerable FindHashtableViolations(TokenOperations tokenOps) { var hashtableAsts = tokenOps.Ast.FindAll(ast => ast is HashtableAst, true); @@ -331,7 +329,5 @@ private bool HasPropertiesOnSeparateLines(IEnumerable AnalyzeScript(Ast ast, string file } var tokenOperations = new TokenOperations(Helper.Instance.Tokens, ast); - var tokenOperationsWithoutHashtable = new TokenOperations(Helper.Instance.Tokens, ast); - - var diagnosticRecords = Enumerable.Empty(); foreach (var violationFinder in violationFinders) { From e6f4ca3c03cc47403cc345236161ac5cad11b47c Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 10 Jan 2019 09:17:37 +0000 Subject: [PATCH 13/15] trigger ci From 50e60878c4b958e7dceedcb63d21ba3e58123307 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 5 Mar 2019 20:33:57 +0000 Subject: [PATCH 14/15] Apply suggestions from code review Co-Authored-By: bergmeister --- Engine/Generic/RuleSuppression.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Engine/Generic/RuleSuppression.cs b/Engine/Generic/RuleSuppression.cs index b4bb492d7..7e865ea6e 100644 --- a/Engine/Generic/RuleSuppression.cs +++ b/Engine/Generic/RuleSuppression.cs @@ -202,7 +202,7 @@ public RuleSuppression(AttributeAst attrAst, int start, int end) break; } - var argumentName = name.ArgumentName; + string argumentName = name.ArgumentName; if (argumentName.Equals("rulename", StringComparison.OrdinalIgnoreCase)) { if (!String.IsNullOrWhiteSpace(RuleName)) @@ -349,7 +349,7 @@ public static List GetSuppressions(IEnumerable at Regex reg = new Regex(String.Format("^{0}$", ruleSupp.Target.Replace(@"*", ".*")), RegexOptions.IgnoreCase); IEnumerable targetAsts = null; - var scope = ruleSupp.Scope; + string scope = ruleSupp.Scope; if (scope.Equals("function", StringComparison.OrdinalIgnoreCase)) { targetAsts = scopeAst.FindAll(ast => ast is FunctionDefinitionAst && reg.IsMatch((ast as FunctionDefinitionAst).Name), true); From db4af7b7b1ab1ab252667cbbded24a2cbb2b4a5f Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Tue, 5 Mar 2019 20:49:03 +0000 Subject: [PATCH 15/15] Use else-less approach to address PR comment --- Engine/ScriptAnalyzer.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index 0ee5de7b3..1af5be1cf 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -286,17 +286,15 @@ private bool AddProfileItem( if (key.Equals("severity", StringComparison.OrdinalIgnoreCase)) { severityList.AddRange(values); + return true; } - else if (key.Equals("includerules", StringComparison.OrdinalIgnoreCase)) + if (key.Equals("includerules", StringComparison.OrdinalIgnoreCase)) { includeRuleList.AddRange(values); - } - else - { - return false; + return true; } - return true; + return false; } private Dictionary GetDictionaryFromHashTableAst(