Skip to content

Commit 7068f4b

Browse files
authored
[build] Enable string operations globalization code analyzers. (dotnet#879)
Context: dotnet/android#6271 Context: https://developercommunity.visualstudio.com/t/XamarinAndroid-binding-compiling-proble/1477963 Context: https://www.learncroatian.eu/blog/the-croatian-letters If you attempt to build an Android Binding project on Windows within a Bosnian, Croatian, or Serbian locale, the build may break as the delegate type names created by 56955d9 may not be valid C#, e.g.: delegate IntPtr _JniMarshal_PP_Ljava/Lang/String; (IntPtr jnienv, IntPtr klass) It *should* be declaringn a `_JniMarshal_PP_L` type, *not* `_JniMarshal_PP_Ljava/Lang/String;`, the latter of which results in numerous C# errors: error CS1003: Syntax error, '(' expected error CS1001: Identifier expected error CS1001: Identifier expected error CS1003: Syntax error, ',' expected error CS1003: Syntax error, ',' expected error CS1001: Identifier expected error CS1026: ) expected The problem is caused by the interplay of two factors: 1. Commit 56955d9 uses the culture-sensitive [`string.StartsWith(string)`][0] method to determine if a JNI type name starts with `L`: if (jni_name.StartsWith ("L") || jni_name.StartsWith ("[")) return "L"; 2. In the `bs`, `hr`, and `sr` locales, the strings `Lj` and `lj` are treated as a single letter, *distinct from* `L` or `l`. In those locales, this expression is *false*, not true: "Ljava/lang/String;".StartsWith ("L") // false in bs, hr, sr; true everywhere else Additionally, this issue only arises when Java package names starting with `java` are used, e.g. `Ljava/lang/Object;`. Java types from packages that *don't* start with `java` don't encounter this bug. Fix this issue by enabling the [CA1307][1] and [CA1309][2] rules, previously disabled in commit ac914ce. These code analysis rules require the use of string methods which use the [`StringComparison`][3] enumeration, for which we then specify `StringComparison.Ordinal`, which is *not* culture-sensitive. One complication with enabling these rules is that the .NET 6+ version of these rules are stricter and require overloads that do not exist in .NET Framework or .NET Standard to fix the violations. Enabling these rules in `.editorconfig` affects all `$(TargetFrameworkMoniker)`s; we will instead use `Directory.Build.props` to only enable them for non-.NET 6+ builds. Finally, add a new `.yaml` step that shuts down the cached `dotnet` MSBuild processes between invocations, to fix the error that has been happening on Windows - NET Core: error MSB3027: Could not copy "obj\\Release\Java.Interop.BootstrapTasks.dll" to "D:\a\1\s\bin\BuildRelease\Java.Interop.BootstrapTasks.dll". Exceeded retry count of 10. Failed. [D:\a\1\s\build-tools\Java.Interop.BootstrapTasks\Java.Interop.BootstrapTasks.csproj] error MSB3021: Unable to copy file "obj\\Release\Java.Interop.BootstrapTasks.dll" to "D:\a\1\s\bin\BuildRelease\Java.Interop.BootstrapTasks.dll". The process cannot access the file 'D:\a\1\s\bin\BuildRelease\Java.Interop.BootstrapTasks.dll' because it is being used by another process. [D:\a\1\s\build-tools\Java.Interop.BootstrapTasks\Java.Interop.BootstrapTasks.csproj] [0]: https://docs.microsoft.com/en-us/dotnet/api/system.string.startswith?view=net-5.0#System_String_StartsWith_System_String_ [1]: https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1307 [2]: https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1309 [3]: https://docs.microsoft.com/en-us/dotnet/api/system.stringcomparison?view=net-5.0
1 parent 3e6a623 commit 7068f4b

File tree

45 files changed

+112
-94
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+112
-94
lines changed

.editorconfig

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,9 +332,9 @@ dotnet_diagnostic.CA1200.severity = none # Avoid using cref tags with a prefix
332332
dotnet_diagnostic.CA1303.severity = none # Do not pass literals as localized parameters
333333
dotnet_diagnostic.CA1304.severity = none # Specify CultureInfo
334334
dotnet_diagnostic.CA1305.severity = none # Specify IFormatProvider
335-
dotnet_diagnostic.CA1307.severity = none # Specify StringComparison
335+
#dotnet_diagnostic.CA1307.severity = none # Specify StringComparison - Controlled via Directory.Build.props
336336
dotnet_diagnostic.CA1308.severity = none # Normalize strings to uppercase
337-
dotnet_diagnostic.CA1309.severity = none # Use ordinal stringcomparison
337+
#dotnet_diagnostic.CA1309.severity = none # Use ordinal stringcomparison - Controlled via Directory.Build.props
338338
dotnet_diagnostic.CA1401.severity = none # P/Invokes should not be visible
339339
dotnet_diagnostic.CA1417.severity = none # Do not use 'OutAttribute' on string parameters for P/Invokes
340340
dotnet_diagnostic.CA1501.severity = none # Avoid excessive inheritance

Directory.Build.props

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
<AppendTargetFrameworkToOutputPath Condition=" '$(AppendTargetFrameworkToOutputPath)' == '' ">False</AppendTargetFrameworkToOutputPath>
4141
<BaseIntermediateOutputPath Condition=" '$(BaseIntermediateOutputPath)' == '' ">obj\</BaseIntermediateOutputPath>
4242
</PropertyGroup>
43-
<PropertyGroup Condition=" '$(TargetFramework)' == 'net6.0' ">
43+
<PropertyGroup Condition=" ( '$(TargetFramework)' != '' AND !$(TargetFramework.StartsWith('nets'))) AND (!$(TargetFramework.StartsWith('net4'))) ">
4444
<JIBuildingForNetCoreApp>True</JIBuildingForNetCoreApp>
4545
</PropertyGroup>
4646
<PropertyGroup Condition=" '$(JIBuildingForNetCoreApp)' == 'True' ">
@@ -85,6 +85,15 @@
8585
<_RunJNIEnvGen Condition=" '$(JIBuildingForNetCoreApp)' != 'True' ">$(Runtime) "$(_JNIEnvGenPath)"</_RunJNIEnvGen>
8686
</PropertyGroup>
8787

88+
<!-- The net6.0 versions of these are stricter and require overloads not available in .NET Framework, so start with just .NET Framework -->
89+
<PropertyGroup Condition=" '$(JIBuildingForNetCoreApp)' != 'True' ">
90+
<AnalysisMode>AllEnabledByDefault</AnalysisMode>
91+
<WarningsAsErrors>$(WarningsAsErrors);CA1307;CA1309;CA1310</WarningsAsErrors>
92+
</PropertyGroup>
93+
<PropertyGroup Condition=" '$(JIBuildingForNetCoreApp)' == 'True' ">
94+
<NoWarn>$(NoWarn);CA1307;CA1309;CA1310</NoWarn>
95+
</PropertyGroup>
96+
8897
<!-- Add Roslyn analyzers NuGet to all projects -->
8998
<ItemGroup Condition=" '$(DisableRoslynAnalyzers)' != 'True' ">
9099
<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="3.3.0">

build-tools/automation/templates/core-build.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ steps:
88
projects: Java.Interop.sln
99
arguments: '-c $(Build.Configuration) -target:Prepare -p:MaxJdkVersion=$(MaxJdkVersion)'
1010

11+
- task: DotNetCoreCLI@2
12+
displayName: Shut down existing build server
13+
inputs:
14+
command: custom
15+
custom: build-server
16+
arguments: shutdown
17+
1118
- task: DotNetCoreCLI@2
1219
displayName: Build Solution
1320
inputs:

build-tools/jnienv-gen/Generator.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,7 @@ public override string[] VerifyParameter (HandleStyle style, string variable)
773773
{
774774
if (managed != "IntPtr")
775775
return new string [0];
776-
var variableName = variable.StartsWith ("@")
776+
var variableName = variable.StartsWith ("@", StringComparison.Ordinal)
777777
? variable.Substring (1)
778778
: variable;
779779
return new[] {
@@ -851,7 +851,7 @@ public override string[] GetMarshalToManagedStatements (HandleStyle style, strin
851851

852852
public override string[] VerifyParameter (HandleStyle style, string variable)
853853
{
854-
var variableName = variable.StartsWith ("@")
854+
var variableName = variable.StartsWith ("@", StringComparison.Ordinal)
855855
? variable.Substring (1)
856856
: variable;
857857
return new[] {
@@ -940,7 +940,7 @@ public override string GetManagedToMarshalExpression (HandleStyle style, string
940940

941941
public override string[] VerifyParameter (HandleStyle style, string variable)
942942
{
943-
var variableName = variable.StartsWith ("@")
943+
var variableName = variable.StartsWith ("@", StringComparison.Ordinal)
944944
? variable.Substring (1)
945945
: variable;
946946
switch (style) {
@@ -1097,7 +1097,7 @@ public override string[] GetMarshalToManagedStatements (HandleStyle style, strin
10971097

10981098
public override string[] VerifyParameter (HandleStyle style, string variable)
10991099
{
1100-
var variableName = variable.StartsWith ("@")
1100+
var variableName = variable.StartsWith ("@", StringComparison.Ordinal)
11011101
? variable.Substring (1)
11021102
: variable;
11031103
switch (style) {

src/Java.Interop.Tools.Generator/Enumification/ConstantsParser.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Collections.Generic;
23
using System.IO;
34
using System.Linq;
@@ -24,7 +25,7 @@ public static List<ConstantEntry> FromEnumMapCsv (TextReader reader)
2425
// Read the enum csv file
2526
while ((s = reader.ReadLine ()) != null) {
2627
// Skip empty lines and comments
27-
if (string.IsNullOrEmpty (s) || s.StartsWith ("//"))
28+
if (string.IsNullOrEmpty (s) || s.StartsWith ("//", StringComparison.Ordinal))
2829
continue;
2930

3031
// Transient mode means remove the original field

src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/JavaCallableWrapperGenerator.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -693,7 +693,7 @@ public Signature (string name, string signature, string connector, string manage
693693
Name = name;
694694

695695
var jnisig = signature;
696-
int closer = jnisig.IndexOf (")");
696+
int closer = jnisig.IndexOf (')');
697697
string ret = jnisig.Substring (closer + 1);
698698
retval = JavaNativeTypeManager.Parse (ret).Type;
699699
string jniparms = jnisig.Substring (1, closer - 1);

src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocGrammar.HtmlBnfTerms.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,10 @@ static IEnumerable<XElement> GetParagraphs (ParseTreeNodeList children)
112112
int len = 0;
113113
int n = -1;
114114

115-
if ((n = s.IndexOf (UnixParagraph, i)) >= 0) {
115+
if ((n = s.IndexOf (UnixParagraph, i, StringComparison.Ordinal)) >= 0) {
116116
len = UnixParagraph.Length;
117117
}
118-
else if ((n = s.IndexOf (DosParagraph, i)) >= 0) {
118+
else if ((n = s.IndexOf (DosParagraph, i, StringComparison.Ordinal)) >= 0) {
119119
len = DosParagraph.Length;
120120
}
121121

src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ public static JniTypeName Parse (string jniType)
5858

5959
public static IEnumerable<JniTypeName> FromSignature (string signature)
6060
{
61-
if (signature.StartsWith ("(")) {
62-
int e = signature.IndexOf (")");
61+
if (signature.StartsWith ("(", StringComparison.Ordinal)) {
62+
int e = signature.IndexOf (')');
6363
signature = signature.Substring (1, e >= 0 ? e-1 : signature.Length-1);
6464
}
6565
int i = 0;
@@ -102,7 +102,7 @@ public static JniTypeName ReturnTypeFromSignature (string signature)
102102
case 'J':
103103
return new JniTypeName { Type = "long", IsKeyword = true };
104104
case 'L': {
105-
var e = signature.IndexOf (";", index);
105+
var e = signature.IndexOf (';', index);
106106
if (e <= 0)
107107
throw new InvalidOperationException ("Missing reference type after 'L' at index " + i + "in: " + signature);
108108
var s = index;

src/Xamarin.Android.Tools.AnnotationSupport/AndroidAnnotationsSupport.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ static Stream FixAnnotationXML (Stream s)
6767
// them up before loading with a validating XML parser.
6868
var doc = new HtmlDocument ();
6969
doc.Load (s);
70-
if (doc.DocumentNode.FirstChild.InnerHtml.StartsWith ("<?xml"))
70+
if (doc.DocumentNode.FirstChild.InnerHtml.StartsWith ("<?xml", StringComparison.Ordinal))
7171
doc.DocumentNode.FirstChild.Remove ();
7272

7373
var ms = new MemoryStream ();

src/Xamarin.Android.Tools.Bytecode/ClassPath.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ XAttribute GetPlatform ()
129129

130130
bool IsGeneratedName (string parameterName)
131131
{
132-
return parameterName.StartsWith ("p") && parameterName.Length > 1 && Char.IsDigit (parameterName [1]);
132+
return parameterName.StartsWith ("p", StringComparison.Ordinal) && parameterName.Length > 1 && Char.IsDigit (parameterName [1]);
133133
}
134134

135135
IEnumerable<ClassFile> GetDescendants (ClassFile theClass, IList<ClassFile> classFiles)

0 commit comments

Comments
 (0)