[xbuild] Replace string.ToLower() comparison with OrdinalIgnoreCase comparison

Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=10946

The scenario is the "turkish-i problem": Have an MSBuild Task Assembly
which calls ITaskItem.GetMetadata("Identity") (like, oh, the
Xamarin.Android build system...). Run in a Turkish locale (tr-TR), and
things fail badly:

	Error executing task AndroidComputeResPaths: System.ArgumentException: Invalid reserved metadata name
	  at Mono.XBuild.Utilities.ReservedNameUtils.GetReservedMetadata (System.String itemSpec, System.String metadataName, IDictionary metadata) [0x00000] in <filename unknown>:0
	  at Microsoft.Build.Utilities.TaskItem.GetMetadata (System.String metadataName) [0x00000] in <filename unknown>:0
	  at Xamarin.Android.Tasks.AndroidComputeResPaths.Execute () [0x00000] in <filename unknown>:0
	  at Microsoft.Build.BuildEngine.TaskEngine.Execute () [0x00000] in <filename unknown>:0
	  at Microsoft.Build.BuildEngine.BuildTask.Execute () [0x00000] in <filename unknown>:0

Wat? Well, in tr-TR, "Identity".ToLower() is "ıdentity", which
doesn't match match anything in GetReservedMetadata()'s `switch`
statement, so it throws an ArgumentException. *BOOM*.

So, if you need a culture-invariant comparison, USE IT.

Related: We could have just s/ToLower/ToLowerInvariant/g, which would
have fixed the problem, but would still result in lots of string
temporaries that aren't really necessary. Use the appropriate
string.Compare() or string.Equals() methods instead to avoid the
string temporary as well.

diff --git a/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/BuildItem.cs b/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/BuildItem.cs
index feac4e6833338792855b42edaebe4e10f98c8850..3421813ce8db754f87759d7f5edb70b516957531 100644 (file)
--- a/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/BuildItem.cs
+++ b/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/BuildItem.cs
@@ -125,7 +125,9 @@ namespace Microsoft.Build.BuildEngine {
                {
                        if (ReservedNameUtils.IsReservedMetadataName (metadataName)) {
                                string metadata = ReservedNameUtils.GetReservedMetadata (FinalItemSpec, metadataName, evaluatedMetadata);
-                               return (metadataName.ToLower () == "fullpath") ? MSBuildUtils.Escape (metadata) : metadata;
+                               return string.Equals (metadataName, "fullpath", StringComparison.OrdinalIgnoreCase)
+                                               ? MSBuildUtils.Escape (metadata)
+                                               : metadata;
                        }
 
                        if (evaluatedMetadata.Contains (metadataName))
@@ -138,7 +140,9 @@ namespace Microsoft.Build.BuildEngine {
                {
                        if (ReservedNameUtils.IsReservedMetadataName (metadataName)) {
                                string metadata = ReservedNameUtils.GetReservedMetadata (FinalItemSpec, metadataName, unevaluatedMetadata);
-                               return (metadataName.ToLower () == "fullpath") ? MSBuildUtils.Escape (metadata) : metadata;
+                               return string.Equals (metadataName, "fullpath", StringComparison.OrdinalIgnoreCase)
+                                       ? MSBuildUtils.Escape (metadata)
+                                       : metadata;
                        } else if (unevaluatedMetadata.Contains (metadataName))
                                return (string) unevaluatedMetadata [metadataName];
                        else

diff --git a/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/ConsoleLogger.cs b/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/ConsoleLogger.cs
index 1b6b4003c0466345b14be069543df29e15be372f..2a62f4b388c2caee9b4635338a637442eafe33d8 100644 (file)
--- a/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/ConsoleLogger.cs
+++ b/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/ConsoleLogger.cs
@@ -149,7 +149,7 @@ namespace Microsoft.Build.BuildEngine {
 
                bool TryParseConsoleColor (string color_str, ref ConsoleColor color)
                {
-                       switch (color_str.ToLower ()) {
+                       switch (color_str.ToLowerInvariant ()) {
                        case "black": color = ConsoleColor.Black; break;
 
                        case "blue": color = ConsoleColor.DarkBlue; break;

diff --git a/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/Project.cs b/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/Project.cs
index e223bbc05cdd2851aecc7609620d36ede27cf33a..5c66cf723089374fc1e859b53e6bc6a444b075d4 100644 (file)
--- a/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/Project.cs
+++ b/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/Project.cs
@@ -390,7 +390,7 @@ namespace Microsoft.Build.BuildEngine {
                internal string GetKeyForTarget (string target_name, bool include_global_properties)
                {
                        // target name is case insensitive
-                       return fullFileName + ":" + target_name.ToLower () +
+                       return fullFileName + ":" + target_name.ToLowerInvariant () +
                                        (include_global_properties ? (":" + GlobalPropertiesToString (GlobalProperties))
                                                                   : String.Empty);
                }

diff --git a/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/CreateCSharpManifestResourceName.cs b/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/CreateCSharpManifestResourceName.cs
index 1a0f7f52ea0c132ecd832bf60dc3198e2b063257..02dcec791e64cffe2b55f7e6bb58c5befa40bca2 100644 (file)
--- a/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/CreateCSharpManifestResourceName.cs
+++ b/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/CreateCSharpManifestResourceName.cs
@@ -41,7 +41,7 @@ namespace Microsoft.Build.Tasks {
 
                protected override bool IsSourceFile (string fileName)
                {
-                       return Path.GetExtension (fileName).ToLower () == ".cs";
+                       return string.Equals (Path.GetExtension (fileName), ".cs", StringComparison.OrdinalIgnoreCase);
                }
                
                protected override string CreateManifestName (string fileName,

diff --git a/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/CreateVisualBasicManifestResourceName.cs b/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/CreateVisualBasicManifestResourceName.cs
index ffa62a2bfa4c16552ce11045391f8204cecbb148..dc615f99ef04266d84024f88b140d5022dc13dbd 100644 (file)
--- a/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/CreateVisualBasicManifestResourceName.cs
+++ b/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/CreateVisualBasicManifestResourceName.cs
@@ -101,7 +101,7 @@ namespace Microsoft.Build.Tasks {
 
                protected override bool IsSourceFile (string fileName)
                {
-                       return Path.GetExtension (fileName).ToLower () == ".vb";
+                       return string.Equals (Path.GetExtension (fileName), ".vb", StringComparison.OrdinalIgnoreCase);
                }
 
                /* Special parser for VB.NET files

diff --git a/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/GenerateResource.cs b/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/GenerateResource.cs
index 5610a8c989d797141ffc0f1f7d372fa22417bcfa..98f614613c2f001734a5ff9c2d1a50c4c1e3b91f 100644 (file)
--- a/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/GenerateResource.cs
+++ b/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/GenerateResource.cs
@@ -186,7 +186,7 @@ namespace Microsoft.Build.Tasks {
                private IResourceReader GetReader (Stream stream, string name)
                {
                        string format = Path.GetExtension (name);
-                       switch (format.ToLower ()) {
+                       switch (format.ToLowerInvariant ()) {
                        case ".po":
                                return new PoResourceReader (stream);
                        case ".txt":
@@ -210,7 +210,7 @@ namespace Microsoft.Build.Tasks {
                private IResourceWriter GetWriter (Stream stream, string name)
                {
                        string format = Path.GetExtension (name);
-                       switch (format.ToLower ()) {
+                       switch (format.ToLowerInvariant ()) {
                        case ".po":
                                return new PoResourceWriter (stream);
                        case ".txt":

diff --git a/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/LibraryPcFileCache.cs b/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/LibraryPcFileCache.cs
index 1a2a2fb9f611f3af5c05470b4a02c2f98e450b60..f29a591f56af939ef3978c62d8e42661b2566343 100644 (file)
--- a/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/LibraryPcFileCache.cs
+++ b/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/LibraryPcFileCache.cs
@@ -145,8 +145,9 @@ namespace Mono.PkgConfig
                        
                        value = file.GetVariable ("GacPackage");
                        if (value != null) {
-                               value = value.ToLower ();
-                               pinfo.IsGacPackage = value == "yes" || value == "true";
+                               pinfo.IsGacPackage = 
+                                       string.Equals (value, "yes", StringComparison.OrdinalIgnoreCase) ||
+                                       string.Equals (value, "true", StringComparison.OrdinalIgnoreCase);
                                gacPackageSet = true;
                        }
        
@@ -191,9 +192,10 @@ namespace Mono.PkgConfig
                        List<string> libdirs = new List<string> ();
                        List<string> retval = new List<string> ();
                        foreach (string piece in line.Split (' ')) {
-                               if (piece.ToLower ().Trim ().StartsWith ("/r:") || piece.ToLower ().Trim ().StartsWith ("-r:")) {
+                               if (IsReferenceParameter (piece)) {
                                        references.Add (piece.Substring (3).Trim ());
-                               } else if (piece.ToLower ().Trim ().StartsWith ("/lib:") || piece.ToLower ().Trim ().StartsWith ("-lib:")) {
+                               } else if (piece.TrimStart ().StartsWith ("/lib:", StringComparison.OrdinalIgnoreCase) ||
+                                               piece.TrimStart ().StartsWith ("-lib:", StringComparison.OrdinalIgnoreCase)) {
                                        libdirs.Add (piece.Substring (5).Trim ());
                                }
                        }
@@ -208,6 +210,12 @@ namespace Mono.PkgConfig
        
                        return retval;
                }
+
+               static bool IsReferenceParameter (string value)
+               {
+                       return value.TrimStart ().StartsWith ("/r:", StringComparison.OrdinalIgnoreCase) ||
+                               value.TrimStart ().StartsWith ("-r:", StringComparison.OrdinalIgnoreCase);
+               }
                
                List<string> GetAssembliesFromLibrariesVar (string line)
                {
@@ -223,7 +231,7 @@ namespace Mono.PkgConfig
                {
                        List<string> references = new List<string> ();
                        foreach (string reference in line.Split (' ')) {
-                               if (reference.ToLower ().Trim ().StartsWith ("/r:") || reference.ToLower ().Trim ().StartsWith ("-r:")) {
+                               if (IsReferenceParameter (reference)) {
                                        string final_ref = reference.Substring (3).Trim ();
                                        references.Add (final_ref);
                                }
@@ -233,10 +241,10 @@ namespace Mono.PkgConfig
                
                public static string NormalizeAsmName (string name)
                {
-                       int i = name.ToLower ().IndexOf (", publickeytoken=null");
+                       int i = name.IndexOf (", publickeytoken=null", StringComparison.OrdinalIgnoreCase);
                        if (i != -1)
                                name = name.Substring (0, i).Trim ();
-                       i = name.ToLower ().IndexOf (", processorarchitecture=");
+                       i = name.IndexOf (", processorarchitecture=", StringComparison.OrdinalIgnoreCase);
                        if (i != -1)
                                name = name.Substring (0, i).Trim ();
                        return name;
@@ -304,7 +312,7 @@ namespace Mono.PkgConfig
                        }
                        string fn = aname.ToString ();
                        string key = "publickeytoken=";
-                       int i = fn.ToLower().IndexOf (key) + key.Length;
+                       int i = fn.IndexOf (key, StringComparison.OrdinalIgnoreCase) + key.Length;
                        int j = fn.IndexOf (',', i);
                        if (j == -1) j = fn.Length;
                        PublicKeyToken = fn.Substring (i, j - i);

diff --git a/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/ManagedCompiler.cs b/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/ManagedCompiler.cs
index 08cd77068ef22d9c6034572b61552cd188849663..a2c0cfb3904dfa7f48fa18a14196085982cad3f9 100644 (file)
--- a/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/ManagedCompiler.cs
+++ b/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/ManagedCompiler.cs
@@ -285,7 +285,7 @@ namespace Microsoft.Build.Tasks {
                        get {
                                if (Bag.Contains ("TargetType")) {
                                        string s = (string) Bag ["TargetType"];
-                                       return s.ToLower ();
+                                       return s.ToLowerInvariant ();
                                } else
                                        return null;
                        }

diff --git a/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/Message.cs b/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/Message.cs
index 8f0783b3f2410047a264bdade1f9d5cc73ef698a..dc949b8e4ecc6b3c9c20debe06c54043f2c2f1f4 100644 (file)
--- a/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/Message.cs
+++ b/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/Message.cs
@@ -27,6 +27,7 @@
 
 #if NET_2_0
 
+using System;
 using Microsoft.Build.Framework; 
 using Microsoft.Build.Tasks;
 
@@ -49,11 +50,11 @@ namespace Microsoft.Build.Tasks {
                        
                        if (importance == null)
                                messageImportance = MessageImportance.Normal;
-                       else if (importance.ToLower () == "low")
+                       else if (string.Equals ("low", importance, StringComparison.OrdinalIgnoreCase))
                                messageImportance = MessageImportance.Low;
-                       else if (importance.ToLower () == "normal")
+                       else if (string.Equals ("normal", importance, StringComparison.OrdinalIgnoreCase))
                                messageImportance = MessageImportance.Normal;
-                       else if (importance.ToLower () == "high")
+                       else if (string.Equals ("high", importance, StringComparison.OrdinalIgnoreCase))
                                messageImportance = MessageImportance.High;
                        else {
                                return false;

diff --git a/mcs/class/Microsoft.Build.Utilities/Mono.XBuild.Utilities/ReservedNameUtils.cs b/mcs/class/Microsoft.Build.Utilities/Mono.XBuild.Utilities/ReservedNameUtils.cs
index 9547b0d663f996d006628c379cd5ffc6b98b71e2..f2712566d594d5fca282c6c2765ad57f4d6e8da1 100644 (file)
--- a/mcs/class/Microsoft.Build.Utilities/Mono.XBuild.Utilities/ReservedNameUtils.cs
+++ b/mcs/class/Microsoft.Build.Utilities/Mono.XBuild.Utilities/ReservedNameUtils.cs
@@ -75,7 +75,7 @@ namespace Mono.XBuild.Utilities {
                        if (String.IsNullOrEmpty (itemSpec))
                                return String.Empty;
                
-                       switch (metadataName.ToLower ()) {
+                       switch (metadataName.ToLowerInvariant ()) {
                        case "fullpath":
                                return Path.GetFullPath (itemSpec);
                        case "rootdir":