Skip to content

Commit

Permalink
Fix PSSA for Turkish culture and tests when culture is not en-US (#1099)
Browse files Browse the repository at this point in the history 
* Fix Parsing of settings that occurs when using turkish culture

* use german culture before running tests to exhibit failures

* set ui culture as well

* fix communityanalyzer for turkish culture

* fix ProvideCommentHelp for turkish culture (i problem again)

* update comments

* fix for import-localizedDatabug in pscore (opened issue 8219 in ps core repo)

* change to UI culture

* use invariant culture comparison instead of tolowerinvariant and revert change where it is not needed

* finish with comments

* Use OrdinalIgnoreCase

* revert accidental change from 2 commits before

* trigger ci

* Apply suggestions from code review

Co-Authored-By: bergmeister <[email protected]>

* Use else-less approach to address PR comment
  • Loading branch information
@bergmeister
bergmeister authored Mar 8, 2019
1 parent a15d260 commit a68b139
Show file tree
Hide file tree
Showing 11 changed files with 156 additions and 152 deletions.
2 changes: 1 addition & 1 deletion Engine/Generic/ModuleDependencyHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ public ModuleDependencyHandler(
? "PSScriptAnalyzer"
: pssaAppDataPath);

modulesFound = new Dictionary<string, PSObject>();
modulesFound = new Dictionary<string, PSObject>(StringComparer.OrdinalIgnoreCase);

// TODO Add PSSA Version in the path
symLinkPath = Path.Combine(pssaAppDataPath, symLinkName);
Expand Down
115 changes: 52 additions & 63 deletions Engine/Generic/RuleSuppression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,61 +202,56 @@ public RuleSuppression(AttributeAst attrAst, int start, int end)
break;
}

switch (name.ArgumentName.ToLower())
string argumentName = name.ArgumentName;
if (argumentName.Equals("rulename", StringComparison.OrdinalIgnoreCase))
{
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.OrdinalIgnoreCase))
{
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.OrdinalIgnoreCase))
{
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.OrdinalIgnoreCase))
{
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.OrdinalIgnoreCase))
{
if (!String.IsNullOrWhiteSpace(Justification))
{
Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name);
}

default:
break;
Justification = (name.Argument as StringConstantExpressionAst).Value;
}
}
}
Expand Down Expand Up @@ -354,23 +349,17 @@ public static List<RuleSuppression> GetSuppressions(IEnumerable<AttributeAst> at
Regex reg = new Regex(String.Format("^{0}$", ruleSupp.Target.Replace(@"*", ".*")), RegexOptions.IgnoreCase);
IEnumerable<Ast> targetAsts = null;

switch (ruleSupp.Scope.ToLower())
string scope = ruleSupp.Scope;
if (scope.Equals("function", StringComparison.OrdinalIgnoreCase))
{
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.OrdinalIgnoreCase))
{
targetAsts = scopeAst.FindAll(ast => ast is TypeDefinitionAst && reg.IsMatch((ast as TypeDefinitionAst).Name), true);
}
#endif

if (targetAsts != null)
{
Expand Down
140 changes: 66 additions & 74 deletions Engine/ScriptAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,21 +283,18 @@ private bool AddProfileItem(
Debug.Assert(includeRuleList != null);
Debug.Assert(excludeRuleList != null);

switch (key.ToLower())
if (key.Equals("severity", StringComparison.OrdinalIgnoreCase))
{
case "severity":
severityList.AddRange(values);
break;
case "includerules":
includeRuleList.AddRange(values);
break;
case "excluderules":
excludeRuleList.AddRange(values);
break;
default:
return false;
severityList.AddRange(values);
return true;
}
return true;
if (key.Equals("includerules", StringComparison.OrdinalIgnoreCase))
{
includeRuleList.AddRange(values);
return true;
}

return false;
}

private Dictionary<string, object> GetDictionaryFromHashTableAst(
Expand Down Expand Up @@ -510,75 +507,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<string>
|| 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<string> values = new List<string>();
if (value is string)
{
values.Add(value as string);
}
else if (value is IEnumerable<string>)
{
values.Union(value as IEnumerable<string>);
}
else if (value.GetType().IsArray)
{
// for array case, sometimes we won't be able to cast it directly to IEnumerable<string>
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;

case "rules":
break;
object value = settings[settingKey];

default:
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
|| !(value is string
|| value is IEnumerable<string>
|| 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<string>();
if (value is string)
{
values.Add(value as string);
}
else if (value is IEnumerable<string>)
{
values.Union(value as IEnumerable<string>);
}
else if (value.GetType().IsArray)
{
// for array case, sometimes we won't be able to cast it directly to IEnumerable<string>
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.OrdinalIgnoreCase))
{
writer.WriteError(
new ErrorRecord(
new InvalidDataException(string.Format(CultureInfo.CurrentCulture, Strings.WrongKeyHashTable, settingKey)),
Strings.WrongConfigurationKey,
ErrorCategory.InvalidData,
profile));
hasError = true;
}
}
return hasError;
Expand Down
4 changes: 2 additions & 2 deletions Engine/Settings.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(); // ToLowerInvariant is important to also work with turkish culture, see https://github.com/PowerShell/PSScriptAnalyzer/issues/1095
object val = settings[key];
switch (key)
{
Expand Down Expand Up @@ -514,7 +514,7 @@ private static object GetSafeValueFromExpressionAst(ExpressionAst exprAst)

case VariableExpressionAst varExprAst:
// $true and $false are VariableExpressionAsts, so look for them here
switch (varExprAst.VariablePath.UserPath.ToLower())
switch (varExprAst.VariablePath.UserPath.ToLowerInvariant())
{
case "true":
return true;
Expand Down
8 changes: 4 additions & 4 deletions PSCompatibilityAnalyzer/Microsoft.PowerShell.CrossCompatibility/Utility/PlatformNaming.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ public static string GetPlatformName(PlatformData platform)
{
string psVersion = platform.PowerShell.Version?.ToString();
string osVersion = platform.OperatingSystem.Version;
string osArch = platform.OperatingSystem.Architecture.ToString().ToLower();
string pArch = platform.PowerShell.ProcessArchitecture.ToString().ToLower();
string dotnetVersion = platform.Dotnet.ClrVersion.ToString().ToLower();
string dotnetEdition = platform.Dotnet.Runtime.ToString().ToLower();
string osArch = platform.OperatingSystem.Architecture.ToString().ToLowerInvariant();
string pArch = platform.PowerShell.ProcessArchitecture.ToString().ToLowerInvariant();
string dotnetVersion = platform.Dotnet.ClrVersion.ToString().ToLowerInvariant();
string dotnetEdition = platform.Dotnet.Runtime.ToString().ToLowerInvariant();

string[] platformNameComponents;
switch (platform.OperatingSystem.Family)
Expand Down
2 changes: 1 addition & 1 deletion Rules/ProvideCommentHelp.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()); // 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));
Expand Down
2 changes: 1 addition & 1 deletion Rules/UseShouldProcessCorrectly.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ public override bool Equals(Object other)
/// </summary>
public override int GetHashCode()
{
return name.ToLower().GetHashCode();
return name.ToLowerInvariant().GetHashCode();
}
}

Expand Down
2 changes: 1 addition & 1 deletion ScriptRuleDocumentation.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,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
}
Expand Down
Loading

0 comments on commit a68b139

Please sign in to comment.