[generator] Fix reserved keywords binary search (#977)

simonrozsival · web-flow · commit 61cdb40dcc8b · 2022-05-04T13:19:20.000-04:00
Context: 7068f4b Context: https://docs.microsoft.com/en-us/dotnet/core/extensions/performing-culture-insensitive-string-operations-in-arrays Commit 7068f4b enabled string code analyzers, because in e.g. the `sr` locale, `"Ljava".StartsWith("L")` would be *false*, because `Lj` was a language-specific digraph, and "special-cased". For intended behavior, we need to use `.StartsWith("L", StringComparison.Ordinal)`. There is a ["similar yet different" problem][0] with `Array.Sort()` and `Array.BinarySearch()` and string arrays: a locale-specific comparison is performed by default: var reserved_keywords = new [] { "as", "catch", "char", "checked", "class", "const", }; var index = Array.BinarySearch (reserved_keywords, "checked"); The *expectation* is that `index` is 3. In the `cs-CZ` locale, "ch" is a digraph, and `index` is -7, i.e. was not found. There is currently no Code Analysis check for this scenario. Review all `Array.Sort()` and `Array.BinarySearch()` invocations so that an appropriate `IComparer` instance is provided. This allows e.g. `checked` to be properly `@escaped` when created a binding in a machine with a `cs-CZ` locale. [0]: https://docs.microsoft.com/en-us/dotnet/core/extensions/performing-culture-insensitive-string-operations-in-arrays
diff --git a/tests/generator-Tests/Unit-Tests/CodeGenerationOptionsTests.cs b/tests/generator-Tests/Unit-Tests/CodeGenerationOptionsTests.cs
@@ -1,4 +1,5 @@
-using System;
+using System.Collections.Generic;
+using System.Linq;
 using MonoDroid.Generation;
 using NUnit.Framework;
 
@@ -79,5 +80,17 @@ public void GetTypeReferenceName_Nullable ()
 
 			Assert.AreEqual ("uint", opt.GetTypeReferenceName (primitive_uint));
 		}
+
+		[Test, TestCaseSource (nameof (ReservedKeywords))]
+		[SetCulture ("cs-CZ")]
+		public void GetSafeIdentifierCultureInvariant (string keyword)
+		{
+			var opt = new CodeGenerationOptions ();
+
+			Assert.AreEqual ($"{keyword}_", opt.GetSafeIdentifier (keyword));
+		}
+
+		private static IEnumerable<TestCaseData> ReservedKeywords
+			=> TypeNameUtilities.reserved_keywords.Select (keyword => new TestCaseData (keyword));
 	}
 }
diff --git a/tests/generator-Tests/Unit-Tests/TypeNameUtilitiesTests.cs b/tests/generator-Tests/Unit-Tests/TypeNameUtilitiesTests.cs
@@ -1,4 +1,5 @@
-using System;
+using System.Collections.Generic;
+using System.Linq;
 using MonoDroid.Generation;
 using NUnit.Framework;
 
@@ -19,5 +20,17 @@ public void MangleName ()
 			Assert.AreEqual ("byte_var", TypeNameUtilities.MangleName ("byte_var"));
 			Assert.AreEqual ("foo", TypeNameUtilities.MangleName ("foo"));
 		}
+
+		[Test, TestCaseSource (nameof (ReservedKeywords))]
+		[SetCulture ("cs-CZ")]
+		public void MangleNameCutlureInvariant (string keyword)
+		{
+			Assert.AreEqual ($"@{keyword}", TypeNameUtilities.MangleName (keyword));
+		}
+
+		private static IEnumerable<TestCaseData> ReservedKeywords
+			=> TypeNameUtilities.reserved_keywords
+				.Where (keyword => keyword != "event") // "event" is a special case which is mapped to "e" instead of "@event"
+				.Select (keyword => new TestCaseData (keyword));
 	}
 }
diff --git a/tools/generator/CodeGenerationOptions.cs b/tools/generator/CodeGenerationOptions.cs
@@ -267,7 +267,7 @@ public string GetSafeIdentifier (string name)
 			// (ReturnValue.ToNative() takes an argument which could be either an expression or mere symbol.)
 			if (name [name.Length-1] != ')' && !name.Contains ('.') && !name.StartsWith ("@", StringComparison.Ordinal)) {
 				if (!IdentifierValidator.IsValidIdentifier (name) ||
-						Array.BinarySearch (TypeNameUtilities.reserved_keywords, name) >= 0) {
+						Array.BinarySearch (TypeNameUtilities.reserved_keywords, name, StringComparer.Ordinal) >= 0) {
 					name = name + "_";
 				}
 			}
diff --git a/tools/generator/Utilities/TypeNameUtilities.cs b/tools/generator/Utilities/TypeNameUtilities.cs
@@ -80,8 +80,8 @@ public static string MangleName (string name)
 		{
 			if (name == "event")
 				return "e";
-			
-			if (Array.BinarySearch (reserved_keywords, name) >= 0)
+
+			if (Array.BinarySearch (reserved_keywords, name, StringComparer.Ordinal) >= 0)
 				return "@" + name;
 
 			return name;

Original file line number	Diff line number	Diff line change
`@@ -267,7 +267,7 @@ public string GetSafeIdentifier (string name)`
`267`	`267`	`// (ReturnValue.ToNative() takes an argument which could be either an expression or mere symbol.)`
`268`	`268`	`if (name [name.Length-1] != ')' && !name.Contains ('.') && !name.StartsWith ("@", StringComparison.Ordinal)) {`
`269`	`269`	`if (!IdentifierValidator.IsValidIdentifier (name) \|\|`
`270`		`- Array.BinarySearch (TypeNameUtilities.reserved_keywords, name) >= 0) {`
	`270`	`+ Array.BinarySearch (TypeNameUtilities.reserved_keywords, name, StringComparer.Ordinal) >= 0) {`
`271`	`271`	`name = name + "_";`
`272`	`272`	`}`
`273`	`273`	`}`