[build] Enable string operations globalization code analyzers. (#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

Author: jpobst

.editorconfig

@@ -332,9 +332,9 @@ dotnet_diagnostic.CA1200.severity = none # Avoid using cref tags with a prefix
 dotnet_diagnostic.CA1303.severity = none # Donotpassliteralsaslocalizedparameters
 dotnet_diagnostic.CA1304.severity = none # SpecifyCultureInfo
 dotnet_diagnostic.CA1305.severity = none # SpecifyIFormatProvider
-dotnet_diagnostic.CA1307.severity = none # SpecifyStringComparison
+#dotnet_diagnostic.CA1307.severity = none # Specify StringComparison - Controlled via Directory.Build.props
 dotnet_diagnostic.CA1308.severity = none # Normalizestringstouppercase
-dotnet_diagnostic.CA1309.severity = none # Useordinalstringcomparison
+#dotnet_diagnostic.CA1309.severity = none # Use ordinal stringcomparison - Controlled via Directory.Build.props
 dotnet_diagnostic.CA1401.severity = none # P/Invokesshouldnotbevisible
 dotnet_diagnostic.CA1417.severity = none # Donotuse'OutAttribute' onstringparametersforP/Invokes
 dotnet_diagnostic.CA1501.severity = none # Avoidexcessiveinheritance

Directory.Build.props

@@ -40,7 +40,7 @@
  <AppendTargetFrameworkToOutputPathCondition=" '$(AppendTargetFrameworkToOutputPath)' == '' ">False</AppendTargetFrameworkToOutputPath>
  <BaseIntermediateOutputPathCondition=" '$(BaseIntermediateOutputPath)' == '' ">obj\</BaseIntermediateOutputPath>
  </PropertyGroup>
- <PropertyGroupCondition=" '$(TargetFramework)' == 'net6.0' ">
+ <PropertyGroupCondition="( '$(TargetFramework)' != '' AND !$(TargetFramework.StartsWith('nets'))) AND (!$(TargetFramework.StartsWith('net4'))) "> 
  <JIBuildingForNetCoreApp>True</JIBuildingForNetCoreApp>
  </PropertyGroup>
  <PropertyGroupCondition=" '$(JIBuildingForNetCoreApp)' == 'True' ">
@@ -85,6 +85,15 @@
  <_RunJNIEnvGenCondition=" '$(JIBuildingForNetCoreApp)' != 'True' ">$(Runtime) "$(_JNIEnvGenPath)"</_RunJNIEnvGen>
  </PropertyGroup>
 
+<!-- The net6.0 versions of these are stricter and require overloads not available in .NET Framework, so start with just .NET Framework -->
+ <PropertyGroupCondition=" '$(JIBuildingForNetCoreApp)' != 'True' ">
+ <AnalysisMode>AllEnabledByDefault</AnalysisMode>
+ <WarningsAsErrors>$(WarningsAsErrors);CA1307;CA1309;CA1310</WarningsAsErrors>
+ </PropertyGroup>
+ <PropertyGroupCondition=" '$(JIBuildingForNetCoreApp)' == 'True' ">
+ <NoWarn>$(NoWarn);CA1307;CA1309;CA1310</NoWarn>
+ </PropertyGroup>
+
 <!-- Add Roslyn analyzers NuGet to all projects -->
  <ItemGroupCondition=" '$(DisableRoslynAnalyzers)' != 'True' ">
  <PackageReferenceInclude="Microsoft.CodeAnalysis.FxCopAnalyzers"Version="3.3.0">

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

@@ -8,6 +8,13 @@ steps:
 projects: Java.Interop.sln
 arguments: '-c $(Build.Configuration) -target:Prepare -p:MaxJdkVersion=$(MaxJdkVersion)'
 
+- task: DotNetCoreCLI@2
+displayName: Shut down existing build server
+inputs:
+command: custom
+custom: build-server
+arguments: shutdown
+
 - task: DotNetCoreCLI@2
 displayName: Build Solution
 inputs:

build-tools/jnienv-gen/Generator.cs

@@ -773,7 +773,7 @@ public override string[] VerifyParameter (HandleStyle style, string variable)
 {
 if(managed!="IntPtr")
 returnnewstring[0];
-varvariableName=variable.StartsWith("@")
+varvariableName=variable.StartsWith("@",StringComparison.Ordinal)
 ?variable.Substring(1)
 :variable;
 returnnew[]{
@@ -851,7 +851,7 @@ public override string[] GetMarshalToManagedStatements (HandleStyle style, strin
 
 publicoverridestring[]VerifyParameter(HandleStylestyle,stringvariable)
 {
-varvariableName=variable.StartsWith("@")
+varvariableName=variable.StartsWith("@",StringComparison.Ordinal)
 ?variable.Substring(1)
 :variable;
 returnnew[]{
@@ -940,7 +940,7 @@ public override string GetManagedToMarshalExpression (HandleStyle style, string
 
 publicoverridestring[]VerifyParameter(HandleStylestyle,stringvariable)
 {
-varvariableName=variable.StartsWith("@")
+varvariableName=variable.StartsWith("@",StringComparison.Ordinal)
 ?variable.Substring(1)
 :variable;
 switch(style){
@@ -1097,7 +1097,7 @@ public override string[] GetMarshalToManagedStatements (HandleStyle style, strin
 
 publicoverridestring[]VerifyParameter(HandleStylestyle,stringvariable)
 {
-varvariableName=variable.StartsWith("@")
+varvariableName=variable.StartsWith("@",StringComparison.Ordinal)
 ?variable.Substring(1)
 :variable;
 switch(style){

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

@@ -1,3 +1,4 @@
+usingSystem;
 usingSystem.Collections.Generic;
 usingSystem.IO;
 usingSystem.Linq;
@@ -24,7 +25,7 @@ public static List<ConstantEntry> FromEnumMapCsv (TextReader reader)
 // Read the enum csv file
 while((s=reader.ReadLine())!=null){
 // Skip empty lines and comments
-if(string.IsNullOrEmpty(s)||s.StartsWith("//"))
+if(string.IsNullOrEmpty(s)||s.StartsWith("//",StringComparison.Ordinal))
 continue;
 
 // Transient mode means remove the original field

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

@@ -693,7 +693,7 @@ public Signature (string name, string signature, string connector, string manage
 Name=name;
 
 varjnisig=signature;
-intcloser=jnisig.IndexOf(")");
+intcloser=jnisig.IndexOf(')');
 stringret=jnisig.Substring(closer+1);
 retval=JavaNativeTypeManager.Parse(ret).Type;
 stringjniparms=jnisig.Substring(1,closer-1);

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

@@ -112,10 +112,10 @@ static IEnumerable<XElement> GetParagraphs (ParseTreeNodeList children)
 intlen=0;
 intn=-1;
 
-if((n=s.IndexOf(UnixParagraph,i))>=0){
+if((n=s.IndexOf(UnixParagraph,i,StringComparison.Ordinal))>=0){
 len=UnixParagraph.Length;
 }
-elseif((n=s.IndexOf(DosParagraph,i))>=0){
+elseif((n=s.IndexOf(DosParagraph,i,StringComparison.Ordinal))>=0){
 len=DosParagraph.Length;
 }
 

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

@@ -58,8 +58,8 @@ public static JniTypeName Parse (string jniType)
 
 publicstaticIEnumerable<JniTypeName>FromSignature(stringsignature)
 {
-if(signature.StartsWith("(")){
-inte=signature.IndexOf(")");
+if(signature.StartsWith("(",StringComparison.Ordinal)){
+inte=signature.IndexOf(')');
 signature=signature.Substring(1,e>=0?e-1:signature.Length-1);
 }
 inti=0;
@@ -102,7 +102,7 @@ public static JniTypeName ReturnTypeFromSignature (string signature)
 case'J':
 returnnewJniTypeName{Type="long",IsKeyword=true};
 case'L':{
-vare=signature.IndexOf(";",index);
+vare=signature.IndexOf(';',index);
 if(e<=0)
 thrownewInvalidOperationException("Missing reference type after 'L' at index "+i+"in: "+signature);
 vars=index;

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

@@ -67,7 +67,7 @@ static Stream FixAnnotationXML (Stream s)
 // them up before loading with a validating XML parser.
 vardoc=newHtmlDocument();
 doc.Load(s);
-if(doc.DocumentNode.FirstChild.InnerHtml.StartsWith("<?xml"))
+if(doc.DocumentNode.FirstChild.InnerHtml.StartsWith("<?xml",StringComparison.Ordinal))
 doc.DocumentNode.FirstChild.Remove();
 
 varms=newMemoryStream();

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

@@ -129,7 +129,7 @@ XAttribute GetPlatform ()
 
 boolIsGeneratedName(stringparameterName)
 {
-returnparameterName.StartsWith("p")&&parameterName.Length>1&&Char.IsDigit(parameterName[1]);
+returnparameterName.StartsWith("p",StringComparison.Ordinal)&&parameterName.Length>1&&Char.IsDigit(parameterName[1]);
 }
 
 IEnumerable<ClassFile>GetDescendants(ClassFiletheClass,IList<ClassFile>classFiles)