Skip to content

Commit 04e4bb1

Browse files
[build] enable CA1305 for most projects (#7993)
Context: b1079a0 Context: https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1305 Context: https://learn.microsoft.com/dotnet/csharp/tutorials/string-interpolation#how-to-create-a-culture-specific-result-string-with-string-interpolation In b1079a0, we found a real bug where omitting `CultureInfo.InvariantCulture` caused problems under certain locales. Let's take this a step further by making `CA1305` a build error. In every case I found, it seemed that invariant culture was fine, and some might be bugs? This also catches cases like this that use the current culture: $"My integer: {x}" Where we should do this instead: FormattableString.Invariant($"My integer: {x}") Only a single instance for displaying a localized string/error message from MSBuild seemed like it needed the current culture: string.Format (CultureInfo.CurrentCulture, Resources.XA2000, assembly); And this one might not even matter, as `assembly` is a string. For on-device app execution, add a new `Android.Util.Log.FormatProvider` property, which is used by all the log methods. This mirrors [`TextWriter.FormatProvider`][1]: namespace Android.Util { partial class Log { public static IFormatProvider FormatProvider { get; set; } = CultureInfo.CurrentCulture; public static int Debug (string tag, string format, params object[] args) => Debug (tag, string.Format (FormatProvider, format, args)); } } Some projects that are not shipped I didn't enable yet, to keep this change smaller: * Projects in `build-tools` * MSBuild test projects * Use `src/.editorconfig`; see also [`.editorconfig` File hierarchy and precedence][0] Finally, `build-tools/automation/guardian/source.gdnsuppress` needed to be updated because the existing `Log.Wtf()` methods were changed to use the new `Log.FormatProvider` property. [0]: https://learn.microsoft.com/visualstudio/ide/create-portable-custom-editor-options#file-hierarchy-and-precedence [1]: https://learn.microsoft.com/en-us/dotnet/api/system.io.textwriter.formatprovider?view=net-8.0
1 parent d9938ba commit 04e4bb1

File tree

28 files changed

+124
-114
lines changed

28 files changed

+124
-114
lines changed

build-tools/automation/guardian/source.gdnsuppress

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"default": {
55
"name": "default",
66
"createdDate": "2023-02-22 23:55:29Z",
7-
"lastUpdatedDate": "2023-02-22 23:55:29Z"
7+
"lastUpdatedDate": "2023-05-04 13:54:18Z"
88
}
99
},
1010
"results": {
@@ -78,8 +78,8 @@
7878
"expirationDate": null,
7979
"type": null
8080
},
81-
"58fab4dfef38677720e955e546a6af108332c65daafb0d043ad9d93442300a30": {
82-
"signature": "58fab4dfef38677720e955e546a6af108332c65daafb0d043ad9d93442300a30",
81+
"6d1fb3a483eb491710d6a09ed0b4bab47f13942d0c6fc744e6683614a66604ab": {
82+
"signature": "6d1fb3a483eb491710d6a09ed0b4bab47f13942d0c6fc744e6683614a66604ab",
8383
"alternativeSignatures": [],
8484
"target": "src/Mono.Android/Android.Util/Log.cs",
8585
"memberOf": [
@@ -134,8 +134,8 @@
134134
"expirationDate": null,
135135
"type": null
136136
},
137-
"06af52be6b6f87455b1db2eb6e631e783f1dacaf607c9b5f34cdee669992c8b5": {
138-
"signature": "06af52be6b6f87455b1db2eb6e631e783f1dacaf607c9b5f34cdee669992c8b5",
137+
"1b38e026fae90da4ae2fe9151c9c1ebd73c8b3c2c5f072ceae390a3ceec2fb97": {
138+
"signature": "1b38e026fae90da4ae2fe9151c9c1ebd73c8b3c2c5f072ceae390a3ceec2fb97",
139139
"alternativeSignatures": [],
140140
"target": "src/Mono.Android/Android.Util/Log.cs",
141141
"memberOf": [

src/.editorconfig

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[*.{cs,vb}]
2+
dotnet_diagnostic.CA1305.severity = error # Specify IFormatProvider

src/Mono.Android.Export/Mono.CodeGeneration/CodeCustomAttribute.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public static CodeCustomAttribute Create (Type attributeType, Type [] ctorArgTyp
4242
if (members [i] == null)
4343
members [i] = attributeType.GetProperty (namedArgNames [i]);
4444
if (members [i] == null)
45-
throw new ArgumentException (String.Format ("Named argument {0} was not found in attribute type {1}", namedArgNames [i], attributeType));
45+
throw new ArgumentException (FormattableString.Invariant ($"Named argument {namedArgNames [i]} was not found in attribute type {attributeType}"));
4646
}
4747

4848
CodeLiteral [] args = new CodeLiteral [ctorArgs.Length];
@@ -64,7 +64,7 @@ public static CodeCustomAttribute Create (Type attributeType, Type [] ctorArgTyp
6464
ArrayList fvalues = new ArrayList ();
6565
for (int i = 0; i < members.Length; i++) {
6666
if (members [i] == null)
67-
throw new ArgumentException (String.Format ("MemberInfo at {0} was null for type {1}.", i, attributeType));
67+
throw new ArgumentException (FormattableString.Invariant ($"MemberInfo at {i} was null for type {attributeType}."));
6868
if (members [i] is PropertyInfo) {
6969
props.Add ((PropertyInfo) members [i]);
7070
pvalues.Add (values [i].Value);

src/Mono.Android/Android.Graphics/Color.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Diagnostics.CodeAnalysis;
4+
using System.Globalization;
45
using System.Linq.Expressions;
56
using System.Reflection;
67
using System.Text;
@@ -64,7 +65,7 @@ public int ToArgb ()
6465

6566
public override string ToString ()
6667
{
67-
return String.Format ("Color [A={0}, R={1}, G={2}, B={3}]", A, R, G, B);
68+
return FormattableString.Invariant ($"Color [A={A}, R={R}, G={G}, B={B}]");
6869
}
6970

7071
public override bool Equals (object obj)
@@ -217,9 +218,8 @@ private static void CheckRGBValues (int red, int green, int blue)
217218

218219
private static ArgumentException CreateColorArgumentException (int value, string color)
219220
{
220-
return new ArgumentException (string.Format ("'{0}' is not a valid"
221-
+ " value for '{1}'. '{1}' should be greater or equal to 0 and"
222-
+ " less than or equal to 255.", value, color));
221+
return new ArgumentException (FormattableString.Invariant (
222+
$"'{value}' is not a valid value for '{color}'. '{color}' should be greater or equal to 0 and less than or equal to 255."));
223223
}
224224

225225
public static Color ParseColor (string colorString)

src/Mono.Android/Android.Runtime/AndroidEnvironment.cs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,8 @@ static bool TrustEvaluateSsl (List <byte[]> certsRawData)
188188
if (certStore == null)
189189
return null;
190190

191-
var alias = string.Format ("{0}:{1:x8}.0", userStore ? "user" : "system", hash);
191+
var store = userStore ? "user" : "system";
192+
var alias = FormattableString.Invariant ($"{store}:{hash:x8}.0");
192193
var certificate = certStore.GetCertificate (alias);
193194
if (certificate == null)
194195
return null;
@@ -220,7 +221,7 @@ static void NotifyTimeZoneChanged ()
220221
try {
221222
clearInfo.Method ();
222223
} catch (Exception e) {
223-
Logger.Log (LogLevel.Warn, "MonoAndroid", string.Format ("Ignoring exception from {0}: {1}", clearInfo.Description, e));
224+
Logger.Log (LogLevel.Warn, "MonoAndroid", FormattableString.Invariant ($"Ignoring exception from {clearInfo.Description}: {e}"));
224225
}
225226
}
226227
}
@@ -426,11 +427,7 @@ public Uri GetProxy (Uri destination)
426427
if (address == null) // FIXME
427428
return destination;
428429

429-
#if ANDROID_19
430-
return new Uri (string.Format ("http://{0}:{1}/", address.HostString, address.Port));
431-
#else
432-
return new Uri (string.Format ("http://{0}:{1}/", address.HostName, address.Port));
433-
#endif
430+
return new Uri (FormattableString.Invariant ($"http://{address.HostString}:{address.Port}/"));
434431
}
435432

436433
public bool IsBypassed (Uri host)

src/Mono.Android/Android.Runtime/AndroidRuntime.cs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Diagnostics;
4+
using System.Globalization;
45
using System.Runtime.CompilerServices;
56
using System.Runtime.InteropServices;
67
using System.Runtime.Versioning;
@@ -165,7 +166,7 @@ public override IntPtr ReleaseLocalReference (ref JniObjectReference value, ref
165166

166167
public override void WriteGlobalReferenceLine (string format, params object?[] args)
167168
{
168-
RuntimeNativeMethods._monodroid_gref_log (string.Format (format, args));
169+
RuntimeNativeMethods._monodroid_gref_log (string.Format (CultureInfo.InvariantCulture, format, args));
169170
}
170171

171172
public override JniObjectReference CreateGlobalReference (JniObjectReference value)
@@ -530,7 +531,7 @@ public void RegisterNativeMembers (JniType nativeClass, Type type, ReadOnlySpan<
530531
}
531532

532533
if (minfo == null)
533-
throw new InvalidOperationException (String.Format ("Specified managed method '{0}' was not found. Signature: {1}", mname.ToString (), signature.ToString ()));
534+
throw new InvalidOperationException (FormattableString.Invariant ($"Specified managed method '{mname.ToString ()}' was not found. Signature: {signature.ToString ()}"));
534535
callback = CreateDynamicCallback (minfo);
535536
needToRegisterNatives = true;
536537
} else {
@@ -659,9 +660,8 @@ internal void AddPeer (IJavaPeerable value, JniObjectReference reference, IntPtr
659660
if (JniEnvironment.Types.IsSameObject (value.PeerReference, target!.PeerReference)) {
660661
found = true;
661662
if (Logger.LogGlobalRef) {
662-
Logger.Log (LogLevel.Info, "monodroid-gref",
663-
string.Format ("warning: not replacing previous registered handle {0} with handle {1} for key_handle 0x{2}",
664-
target.PeerReference.ToString (), reference.ToString (), hash.ToString ("x")));
663+
Logger.Log (LogLevel.Info, "monodroid-gref", FormattableString.Invariant (
664+
$"warning: not replacing previous registered handle {target.PeerReference} with handle {reference} for key_handle 0x{hash:x}"));
665665
}
666666
}
667667
}
@@ -704,10 +704,9 @@ internal void AddPeer (IJavaPeerable value, IntPtr handle, JniHandleOwnership tr
704704
}
705705

706706
if (Logger.LogGlobalRef) {
707-
RuntimeNativeMethods._monodroid_gref_log ("handle 0x" + handleField.ToString ("x") +
708-
"; key_handle 0x" + hash.ToString ("x") +
709-
": Java Type: `" + JNIEnv.GetClassNameFromInstance (handleField) + "`; " +
710-
"MCW type: `" + value.GetType ().FullName + "`\n");
707+
RuntimeNativeMethods._monodroid_gref_log (
708+
FormattableString.Invariant (
709+
$"handle 0x{handleField:x}; key_handle 0x{hash:x}: Java Type: `{JNIEnv.GetClassNameFromInstance (handleField)}`; MCW type: `{value.GetType ().FullName}`\n"));
711710
}
712711
}
713712

src/Mono.Android/Android.Runtime/JNIEnv.cs

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Diagnostics;
4+
using System.Globalization;
45
using System.IO;
56
using System.Reflection;
67
using System.Runtime.CompilerServices;
@@ -262,8 +263,8 @@ public static unsafe void InvokeConstructor (IntPtr instance, string jniCtorSign
262263
try {
263264
IntPtr ctor = JNIEnv.GetMethodID (lrefClass, "<init>", jniCtorSignature);
264265
if (ctor == IntPtr.Zero)
265-
throw new ArgumentException (string.Format ("Could not find constructor JNI signature '{0}' on type '{1}'.",
266-
jniCtorSignature, Java.Interop.TypeManager.GetClassName (lrefClass)));
266+
throw new ArgumentException (FormattableString.Invariant (
267+
$"Could not find constructor JNI signature '{jniCtorSignature}' on type '{Java.Interop.TypeManager.GetClassName (lrefClass)}'."));
267268
CallNonvirtualVoidMethod (instance, lrefClass, ctor, constructorParameters);
268269
} finally {
269270
DeleteLocalRef (lrefClass);
@@ -280,8 +281,8 @@ public static unsafe IntPtr CreateInstance (IntPtr jniClass, string signature, J
280281
{
281282
IntPtr ctor = JNIEnv.GetMethodID (jniClass, "<init>", signature);
282283
if (ctor == IntPtr.Zero)
283-
throw new ArgumentException (string.Format ("Could not find constructor JNI signature '{0}' on type '{1}'.",
284-
signature, Java.Interop.TypeManager.GetClassName (jniClass)));
284+
throw new ArgumentException (FormattableString.Invariant (
285+
$"Could not find constructor JNI signature '{signature}' on type '{Java.Interop.TypeManager.GetClassName (jniClass)}'."));
285286
return JNIEnv.NewObject (jniClass, ctor, constructorParameters);
286287
}
287288

@@ -627,9 +628,8 @@ static void AssertCompatibleArrayTypes (Type sourceType, IntPtr destArray)
627628
IntPtr lrefDest = GetObjectClass (destArray);
628629
try {
629630
if (!IsAssignableFrom (grefSource, lrefDest)) {
630-
throw new InvalidCastException (string.Format ("Unable to cast from '{0}' to '{1}'.",
631-
Java.Interop.TypeManager.GetClassName (grefSource),
632-
Java.Interop.TypeManager.GetClassName (lrefDest)));
631+
throw new InvalidCastException (FormattableString.Invariant (
632+
$"Unable to cast from '{Java.Interop.TypeManager.GetClassName (grefSource)}' to '{Java.Interop.TypeManager.GetClassName (lrefDest)}'."));
633633
}
634634
} finally {
635635
DeleteGlobalRef (grefSource);
@@ -643,9 +643,8 @@ static void AssertCompatibleArrayTypes (IntPtr sourceArray, Type destType)
643643
IntPtr lrefSource = GetObjectClass (sourceArray);
644644
try {
645645
if (!IsAssignableFrom (lrefSource, grefDest)) {
646-
throw new InvalidCastException (string.Format ("Unable to cast from '{0}' to '{1}'.",
647-
Java.Interop.TypeManager.GetClassName (lrefSource),
648-
Java.Interop.TypeManager.GetClassName (grefDest)));
646+
throw new InvalidCastException (FormattableString.Invariant (
647+
$"Unable to cast from '{Java.Interop.TypeManager.GetClassName (lrefSource)}' to '{Java.Interop.TypeManager.GetClassName (grefDest)}'."));
649648
}
650649
} finally {
651650
DeleteGlobalRef (grefDest);
@@ -1145,7 +1144,7 @@ static int _GetArrayLength (IntPtr array_ptr)
11451144
ret [i] = value;
11461145
ret [i] = targetType == null || targetType.IsInstanceOfType (value)
11471146
? value
1148-
: Convert.ChangeType (value, targetType);
1147+
: Convert.ChangeType (value, targetType, CultureInfo.InvariantCulture);
11491148
}
11501149

11511150
return ret;

src/Mono.Android/Android.Runtime/XmlReaderPullParser.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -461,8 +461,9 @@ public string PositionDescription {
461461
var xi = r as IXmlLineInfo;
462462
var loc = xi == null || !xi.HasLineInfo () ?
463463
"(location N/A)" :
464-
String.Format ("({0}, {1})", xi.LineNumber, xi.LinePosition);
465-
return String.Format ("Node {0} at {1} {2}", r.NodeType, String.IsNullOrEmpty (r.BaseURI) ? null : r.BaseURI, loc);
464+
FormattableString.Invariant ($"({xi.LineNumber}, {xi.LinePosition})");
465+
var uri = string.IsNullOrEmpty (r.BaseURI) ? null : r.BaseURI;
466+
return FormattableString.Invariant ($"Node {r.NodeType} at {uri} {loc}");
466467
}
467468
}
468469

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,22 @@
11
using System;
2+
using System.Globalization;
23
using Android.Runtime;
34

45
namespace Android.Util {
56

67
public partial class Log {
78

9+
/// <summary>
10+
/// IFormatProvider passed to any underlying string.Format() calls. Defaults to System.Globalization.CultureInfo.CurrentCulture.
11+
/// </summary>
12+
#if ANDROID_34
13+
public
14+
#endif // ANDROID_34
15+
static IFormatProvider FormatProvider { get; set; } = CultureInfo.CurrentCulture;
16+
817
public static int Debug (string tag, string format, params object[] args)
918
{
10-
return Debug (tag, string.Format (format, args));
19+
return Debug (tag, string.Format (FormatProvider, format, args));
1120
}
1221

1322
public static int Debug (string tag, Java.Lang.Throwable tr, string msg)
@@ -17,12 +26,12 @@ public static int Debug (string tag, Java.Lang.Throwable tr, string msg)
1726

1827
public static int Debug (string tag, Java.Lang.Throwable tr, string format, params object[] args)
1928
{
20-
return Debug (tag, string.Format (format, args), tr);
29+
return Debug (tag, string.Format (FormatProvider, format, args), tr);
2130
}
2231

2332
public static int Error (string tag, string format, params object[] args)
2433
{
25-
return Error (tag, string.Format (format, args));
34+
return Error (tag, string.Format (FormatProvider, format, args));
2635
}
2736

2837
public static int Error (string tag, Java.Lang.Throwable tr, string msg)
@@ -32,12 +41,12 @@ public static int Error (string tag, Java.Lang.Throwable tr, string msg)
3241

3342
public static int Error (string tag, Java.Lang.Throwable tr, string format, params object[] args)
3443
{
35-
return Error (tag, string.Format (format, args), tr);
44+
return Error (tag, string.Format (FormatProvider, format, args), tr);
3645
}
3746

3847
public static int Info (string tag, string format, params object[] args)
3948
{
40-
return Info (tag, string.Format (format, args));
49+
return Info (tag, string.Format (FormatProvider, format, args));
4150
}
4251

4352
public static int Info (string tag, Java.Lang.Throwable tr, string msg)
@@ -47,17 +56,17 @@ public static int Info (string tag, Java.Lang.Throwable tr, string msg)
4756

4857
public static int Info (string tag, Java.Lang.Throwable tr, string format, params object[] args)
4958
{
50-
return Info (tag, string.Format (format, args), tr);
59+
return Info (tag, string.Format (FormatProvider, format, args), tr);
5160
}
5261

5362
public static int WriteLine (LogPriority priority, string tag, string format, params object[] args)
5463
{
55-
return WriteLine (priority, tag, string.Format (format, args));
64+
return WriteLine (priority, tag, string.Format (FormatProvider, format, args));
5665
}
5766

5867
public static int Verbose (string tag, string format, params object[] args)
5968
{
60-
return Verbose (tag, string.Format (format, args));
69+
return Verbose (tag, string.Format (FormatProvider, format, args));
6170
}
6271

6372
public static int Verbose (string tag, Java.Lang.Throwable tr, string msg)
@@ -67,12 +76,12 @@ public static int Verbose (string tag, Java.Lang.Throwable tr, string msg)
6776

6877
public static int Verbose (string tag, Java.Lang.Throwable tr, string format, params object[] args)
6978
{
70-
return Verbose (tag, string.Format (format, args), tr);
79+
return Verbose (tag, string.Format (FormatProvider, format, args), tr);
7180
}
7281

7382
public static int Warn (string tag, string format, params object[] args)
7483
{
75-
return Warn (tag, string.Format (format, args));
84+
return Warn (tag, string.Format (FormatProvider, format, args));
7685
}
7786

7887
public static int Warn (string tag, Java.Lang.Throwable tr, string msg)
@@ -82,12 +91,12 @@ public static int Warn (string tag, Java.Lang.Throwable tr, string msg)
8291

8392
public static int Warn (string tag, Java.Lang.Throwable tr, string format, params object[] args)
8493
{
85-
return Warn (tag, string.Format (format, args), tr);
94+
return Warn (tag, string.Format (FormatProvider, format, args), tr);
8695
}
8796

8897
public static int Wtf (string tag, string format, params object[] args)
8998
{
90-
return Wtf (tag, string.Format (format, args));
99+
return Wtf (tag, string.Format (FormatProvider, format, args));
91100
}
92101

93102
public static int Wtf (string tag, Java.Lang.Throwable tr, string msg)
@@ -97,7 +106,7 @@ public static int Wtf (string tag, Java.Lang.Throwable tr, string msg)
97106

98107
public static int Wtf (string tag, Java.Lang.Throwable tr, string format, params object[] args)
99108
{
100-
return Wtf (tag, string.Format (format, args), tr);
109+
return Wtf (tag, string.Format (FormatProvider, format, args), tr);
101110
}
102111
}
103112
}

0 commit comments

Comments
 (0)