Skip to content

Fix PSSA for Turkish culture and tests when culture is not en-US #1099

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Mar 8, 2019
Merged

Fix PSSA for Turkish culture and tests when culture is not en-US #1099

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
010a862
Fix Parsing of settings that occurs when using turkish culture
bergmeister Nov 6, 2018
57ab21c
use german culture before running tests to exhibit failures
bergmeister Nov 8, 2018
dcfe1cf
set ui culture as well
bergmeister Nov 8, 2018
5fea008
fix communityanalyzer for turkish culture
bergmeister Nov 8, 2018
a8df05d
fix ProvideCommentHelp for turkish culture (i problem again)
bergmeister Nov 8, 2018
c33c38b
update comments
bergmeister Nov 8, 2018
479ed3a
fix for import-localizedDatabug in pscore (opened issue 8219 in ps co…
bergmeister Nov 9, 2018
227b64f
change to UI culture
bergmeister Nov 9, 2018
cee22f9
use invariant culture comparison instead of tolowerinvariant and reve…
bergmeister Nov 12, 2018
079b98d
finish with comments
bergmeister Nov 12, 2018
bd97fcb
Merge branch 'development' of https://github.com/PowerShell/PSScriptA…
bergmeister Dec 15, 2018
6d28bcc
Use OrdinalIgnoreCase
bergmeister Dec 15, 2018
7ffbf65
revert accidental change from 2 commits before
bergmeister Dec 15, 2018
9ae39ba
Merge branch 'development' of https://github.com/PowerShell/PSScriptA…
bergmeister Jan 9, 2019
e6f4ca3
trigger ci
bergmeister Jan 10, 2019
eae7ff0
Merge branch 'development' into CultureFixes
bergmeister Mar 2, 2019
50e6087
Apply suggestions from code review
rjmholt Mar 5, 2019
7540baf
Merge branch 'development' of https://github.com/PowerShell/PSScriptA…
bergmeister Mar 5, 2019
db4af7b
Use else-less approach to address PR comment
bergmeister Mar 5, 2019
e1752bc
Merge branch 'CultureFixes' of https://github.com/bergmeister/PSScrip…
bergmeister Mar 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Engine/Generic/ModuleDependencyHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ public ModuleDependencyHandler(
? "PSScriptAnalyzer"
: pssaAppDataPath);

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

// TODO Add PSSA Version in the path
symLinkPath = Path.Combine(pssaAppDataPath, symLinkName);
Expand All @@ -303,7 +303,6 @@ public ModuleDependencyHandler(
public PSObject FindModule(string moduleName)
{
ThrowIfNull(moduleName, "moduleName");
moduleName = moduleName.ToLower();
if (modulesFound.ContainsKey(moduleName))
{
return modulesFound[moduleName];
Expand Down
115 changes: 52 additions & 63 deletions Engine/Generic/RuleSuppression.cs
View file
Open in desktop
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())
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;
}
}
}
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())
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)
{
Expand Down
142 changes: 68 additions & 74 deletions Engine/ScriptAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It strikes me that it would be simpler to just return true here and in the next clause and just return false at the end by default

Copy link
Collaborator Author

@bergmeister bergmeister Mar 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I agree, I''ve addressed that in this commit

}
else if (key.Equals("includerules", StringComparison.InvariantCultureIgnoreCase))
{
includeRuleList.AddRange(values);
}
else
{
return false;
}

return true;
}

Expand Down Expand Up @@ -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<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;
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<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.InvariantCultureIgnoreCase))
{
writer.WriteError(
new ErrorRecord(
new InvalidDataException(string.Format(CultureInfo.CurrentCulture, Strings.WrongKeyHashTable, settingKey)),
Strings.WrongConfigurationKey,
ErrorCategory.InvalidData,
profile));
hasError = true;
}
}
return hasError;
Expand Down Expand Up @@ -1843,7 +1837,7 @@ private IEnumerable<DiagnosticRecord> 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
{
Expand Down
19 changes: 5 additions & 14 deletions Engine/Settings.cs
View file
Open in desktop
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 @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion Rules/ProvideCommentHelp.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading