Address review feedback.

jpobst · jpobst · commit 11eacb7f12e4 · 2023-05-16T15:44:27.000-05:00
diff --git a/Directory.Build.props b/Directory.Build.props
@@ -3,14 +3,12 @@
   <!-- Note: MUST be imported *after* $(Configuration) is set! -->
   <PropertyGroup>
     <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <TreatWarningsAsErrors Condition=" '$(Configuration)' == 'Release' ">true</TreatWarningsAsErrors>
     <_OutputPath>$(MSBuildThisFileDirectory)bin\Build$(Configuration)\</_OutputPath>
     <GenerateAssemblyInfo>true</GenerateAssemblyInfo>
     <ProduceReferenceAssemblyInOutDir>true</ProduceReferenceAssemblyInOutDir>
     <DotNetTargetFrameworkVersion>7.0</DotNetTargetFrameworkVersion>
     <DotNetTargetFramework>net$(DotNetTargetFrameworkVersion)</DotNetTargetFramework>
-    <!-- These assemblies are used by Xamarin.Android.Build.Tasks which must be netstandard2.0
-         in order to be loaded into Visual Studio -->
-    <MSBuildRequiredTargetFramework>netstandard2.0</MSBuildRequiredTargetFramework>
   </PropertyGroup>
 
   <Import
@@ -69,19 +67,10 @@
   </PropertyGroup>
 
   <!--
-    When building on a bot w/ VS2019:
-
-      warning CS8032: An instance of analyzer System.Text.Json.SourceGeneration.JsonSourceGenerator cannot be created from
-
-    however, with `$(TreatWarningsAsErrors)`=True, this becomes an error,
-    meaning we can't build on VS2019.
-
-    Ignore CS8032 so that we can build on VS2019.
-    
     JniEnvironment.g.cs(34,8): error CS8981: The type name 'jobject' only contains lower-cased ascii characters. Such names may become reserved for the language.
     -->
   <PropertyGroup>
-    <NoWarn>$(NoWarn);CS8032;CS8981</NoWarn>
+    <NoWarn>$(NoWarn);CS8981</NoWarn>
   </PropertyGroup>
 
 </Project>
diff --git a/build-tools/automation/azure-pipelines.yaml b/build-tools/automation/azure-pipelines.yaml
@@ -21,7 +21,7 @@ variables:
   RunningOnCI: true
   Build.Configuration: Release
   MaxJdkVersion: 8
-  DotNetCoreVersion: 7.0.100-rc.1.22431.12
+  DotNetCoreVersion: 7.x
   DotNetTargetFramework: net7.0
   NetCoreTargetFrameworkPathSuffix: -$(DotNetTargetFramework)
   1ESWindowsPool: AzurePipelines-EO
diff --git a/build-tools/automation/templates/install-dependencies.yaml b/build-tools/automation/templates/install-dependencies.yaml
@@ -6,4 +6,3 @@ steps:
   displayName: Use .NET $(DotNetCoreVersion)
   inputs:
     version: $(DotNetCoreVersion)
-    includePreviewVersions: true
diff --git a/src/Java.Interop.Localization/Java.Interop.Localization.csproj b/src/Java.Interop.Localization/Java.Interop.Localization.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>$(MSBuildRequiredTargetFramework)</TargetFramework>
+    <TargetFramework>netstandard2.0</TargetFramework>
     <LangVersion>8.0</LangVersion>
     <Nullable>enable</Nullable>
     <DefineConstants>INTERNAL_NULLABLE_ATTRIBUTES</DefineConstants>
diff --git a/src/Java.Interop.Localization/Resources.Designer.cs b/src/Java.Interop.Localization/Resources.Designer.cs
diff --git a/src/Java.Interop.Localization/Resources.resx b/src/Java.Interop.Localization/Resources.resx
@@ -294,6 +294,11 @@ The following terms should not be translated: Metadata.xml.</comment>
     <value>Invalid namespace transform '{0}'</value>
     <comment>{0} - XML transform. (example: '&lt;ns-replace source="example" replacement="Example" /&gt;')</comment>
   </data>
+  <data name="Generator_BG8A08" xml:space="preserve">
+    <value>Metadata.xml element '{0}' is missing the 'path' attribute.</value>
+    <comment>{0} - XML transform. (example: '&lt;remove-node path="/api/package[@name='javax.sql']"')
+The following terms should not be translated: Metadata.xml, path.</comment>
+  </data>
   <data name="Generator_BG8B00" xml:space="preserve">
     <value>Unknown generic argument constraint type '{0}' for member '{1}'.</value>
     <comment>{0} - .NET type name
diff --git a/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil.csproj b/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>$(MSBuildRequiredTargetFramework)</TargetFramework>
+    <TargetFramework>netstandard2.0</TargetFramework>
     <LangVersion>8.0</LangVersion>
     <Nullable>enable</Nullable>
     <DefineConstants>INTERNAL_NULLABLE_ATTRIBUTES</DefineConstants>
diff --git a/src/Java.Interop.Tools.Diagnostics/Java.Interop.Tools.Diagnostics.csproj b/src/Java.Interop.Tools.Diagnostics/Java.Interop.Tools.Diagnostics.csproj
@@ -1,7 +1,7 @@
 ﻿<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>$(MSBuildRequiredTargetFramework)</TargetFramework>
+    <TargetFramework>netstandard2.0</TargetFramework>
     <LangVersion>8.0</LangVersion>
     <Nullable>enable</Nullable>
     <DefineConstants>INTERNAL_NULLABLE_ATTRIBUTES</DefineConstants>
diff --git a/src/Java.Interop.Tools.Generator/Enumification/ConstantEntry.cs b/src/Java.Interop.Tools.Generator/Enumification/ConstantEntry.cs
@@ -148,18 +148,18 @@ public static ConstantEntry FromElement (XElement elem)
 			var entry = new ConstantEntry {
 				Action = ConstantAction.None,
 				ApiLevel = NamingConverter.ParseApiLevel (elem),
-				JavaSignature = elem.Parent.Parent.Attribute ("name").Value,
+				JavaSignature = elem.Parent?.Parent?.Attribute ("name")?.Value,
 				Value = elem.Attribute ("value")?.Value,
 			};
 
-			var java_package = elem.Parent.Parent.Attribute ("name").Value.Replace ('.', '/');
-			var java_type = elem.Parent.Attribute ("name").Value.Replace ('.', '$');
-			var java_member = elem.Attribute ("name").Value;
+			var java_package = elem.Parent?.Parent?.Attribute ("name")?.Value.Replace ('.', '/');
+			var java_type = elem.Parent?.Attribute ("name")?.Value.Replace ('.', '$');
+			var java_member = elem.Attribute ("name")?.Value;
 
 			entry.JavaSignature = $"{java_package}/{java_type}.{java_member}".TrimStart ('/');
 
 			// Interfaces get an "I:" prefix
-			if (elem.Parent.Name.LocalName == "interface")
+			if (elem.Parent?.Name.LocalName == "interface")
 				entry.JavaSignature = "I:" + entry.JavaSignature;
 
 			return entry;
diff --git a/src/Java.Interop.Tools.Generator/Enumification/ConstantsParser.cs b/src/Java.Interop.Tools.Generator/Enumification/ConstantsParser.cs
@@ -20,7 +20,7 @@ public static List<ConstantEntry> FromEnumMapCsv (TextReader reader)
 			var constants = new List<ConstantEntry> ();
 			var transient = false;
 
-			string s;
+			string? s;
 
 			// Read the enum csv file
 			while ((s = reader.ReadLine ()) != null) {
@@ -105,7 +105,7 @@ public class JavaSignatureComparer : IEqualityComparer<ConstantEntry>
 	{
 		public static JavaSignatureComparer Instance { get; } = new JavaSignatureComparer ();
 
-		public bool Equals (ConstantEntry x, ConstantEntry y) => x?.JavaSignature == y?.JavaSignature;
+		public bool Equals (ConstantEntry? x, ConstantEntry? y) => x?.JavaSignature == y?.JavaSignature;
 		public int GetHashCode (ConstantEntry obj) => 0;
 	}
 }
diff --git a/src/Java.Interop.Tools.Generator/Enumification/MethodMapEntry.cs b/src/Java.Interop.Tools.Generator/Enumification/MethodMapEntry.cs
@@ -38,12 +38,12 @@ public static IEnumerable<MethodMapEntry> FromXml (XElement element)
 		static MethodMapEntry FromElement (XElement element, string parameterName)
 		{
 			var entry = new MethodMapEntry {
-				JavaPackage = element.Parent.Parent.XGetAttribute ("name")?.Replace ('.', '/'),
-				JavaType = element.Parent.XGetAttribute ("name")?.Replace ('.', '$'),
+				JavaPackage = element.Parent?.Parent?.XGetAttribute ("name")?.Replace ('.', '/'),
+				JavaType = element.Parent?.XGetAttribute ("name")?.Replace ('.', '$'),
 				JavaName = element.XGetAttribute ("name"),
 				ParameterName = parameterName,
 				ApiLevel = NamingConverter.ParseApiLevel (element),
-				IsInterface = element.Parent.Name == "interface"
+				IsInterface = element.Parent?.Name == "interface"
 			};
 
 			if (element.Name == "constructor")
diff --git a/src/Java.Interop.Tools.Generator/Enumification/MethodMapParser.cs b/src/Java.Interop.Tools.Generator/Enumification/MethodMapParser.cs
@@ -19,7 +19,7 @@ public static List<MethodMapEntry> FromMethodMapCsv (TextReader reader)
 		{
 			var entries = new List<MethodMapEntry> ();
 
-			string s;
+			string? s;
 
 			// Read the enum csv file
 			while ((s = reader.ReadLine ()) != null) {
diff --git a/src/Java.Interop.Tools.Generator/Java.Interop.Tools.Generator.csproj b/src/Java.Interop.Tools.Generator/Java.Interop.Tools.Generator.csproj
@@ -4,7 +4,6 @@
     <TargetFramework>$(DotNetTargetFramework)</TargetFramework>
     <LangVersion>8.0</LangVersion>
     <Nullable>enable</Nullable>
-    <DefineConstants>INTERNAL_NULLABLE_ATTRIBUTES</DefineConstants>
   </PropertyGroup>
 
   <Import Project="..\..\TargetFrameworkDependentValues.props" />
@@ -13,10 +12,6 @@
     <OutputPath>$(UtilityOutputFullPath)</OutputPath>
   </PropertyGroup>
 
-  <ItemGroup>
-    <Compile Include="..\utils\NullableAttributes.cs" />
-  </ItemGroup>
-
   <ItemGroup>
     <ProjectReference Include="..\Java.Interop.Localization\Java.Interop.Localization.csproj" />
   </ItemGroup>
diff --git a/src/Java.Interop.Tools.Generator/Metadata/FixupXmlDocument.cs b/src/Java.Interop.Tools.Generator/Metadata/FixupXmlDocument.cs
@@ -48,6 +48,11 @@ public void Apply (ApiXmlDocument apiDocument, string apiLevelString, int produc
 
 				prev_path = path;
 
+				if (path is null) {
+					Report.LogCodedWarning (0, Report.WarningNodeMissingPathAttribute, null, metaitem, metaitem.ToString ());
+					continue;
+				}
+
 				switch (metaitem.Name.LocalName) {
 				case "remove-node":
 					try {
@@ -105,9 +110,12 @@ public void Apply (ApiXmlDocument apiDocument, string apiLevelString, int produc
 					try {
 						var  attr_name = metaitem.XGetAttribute ("name");
 
-						if (string.IsNullOrEmpty (attr_name))
+						if (string.IsNullOrEmpty (attr_name)) {
 							// BG4307
 							Report.LogCodedError (Report.ErrorMissingAttrName, metaitem, path);
+							continue;
+						}
+
 						var nodes = attr_last_cache != null ? new XElement [] { attr_last_cache } : apiDocument.ApiDocument.XPathSelectElements (path);
 						var attr_matched = 0;
 
diff --git a/src/Java.Interop.Tools.Generator/Utilities/Report.cs b/src/Java.Interop.Tools.Generator/Utilities/Report.cs
@@ -71,6 +71,7 @@ public LocalizedMessage (int code, string value)
 		public static LocalizedMessage WarningMoveNodeMatchedNoNodes => new LocalizedMessage (0x8A05, Localization.Resources.Generator_BG8A00);
 		public static LocalizedMessage WarningRemoveAttrMatchedNoNodes => new LocalizedMessage (0x8A06, Localization.Resources.Generator_BG8A00);
 		public static LocalizedMessage WarningInvalidNamespaceTransform => new LocalizedMessage (0x8A07, Localization.Resources.Generator_BG8A07);
+		public static LocalizedMessage WarningNodeMissingPathAttribute => new LocalizedMessage (0x8A08, Localization.Resources.Generator_BG8A08);
 		public static LocalizedMessage WarningUnknownGenericConstraint => new LocalizedMessage (0x8B00, Localization.Resources.Generator_BG8B00);
 		public static LocalizedMessage WarningBaseInterfaceNotFound => new LocalizedMessage (0x8C00, Localization.Resources.Generator_BG8C00);
 		public static LocalizedMessage WarningBaseInterfaceInvalid => new LocalizedMessage (0x8C01, Localization.Resources.Generator_BG8C01);
diff --git a/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers.csproj b/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>$(MSBuildRequiredTargetFramework)</TargetFramework>
+    <TargetFramework>netstandard2.0</TargetFramework>
     <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
     <LangVersion>8.0</LangVersion>
     <Nullable>enable</Nullable>
diff --git a/src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocGrammar.HtmlBnfTerms.cs b/src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocGrammar.HtmlBnfTerms.cs
@@ -133,7 +133,7 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
 					| CreateStartElementIgnoreAttribute ("a", "id") + InlineDeclarations + CreateEndElement ("a", grammar, optional: true)
 					;
 				IgnorableElementDeclaration.AstConfig.NodeCreator = (context, parseNode) => {
-					var aElementValue = new XText (parseNode.ChildNodes [1].AstNode.ToString ());
+					var aElementValue = new XText (parseNode.ChildNodes [1].AstNode.ToString () ?? string.Empty);
 					parseNode.AstNode = aElementValue;
 				};
 
@@ -325,6 +325,9 @@ public override void Init (GrammarData grammarData)
 
 		public override Token? TryMatch (ParsingContext context, ISourceStream source)
 		{
+			if (_stopChars is null)
+				throw new InvalidOperationException ("Init must be called before TryMatch");
+
 			var stopIndex = source.Text.IndexOfAny (_stopChars, source.Location.Position);
 			if (stopIndex == source.Location.Position)
 				return null;
diff --git a/src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocGrammar.InlineTagsBnfTerms.cs b/src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocGrammar.InlineTagsBnfTerms.cs
@@ -35,7 +35,7 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
 
 				CodeDeclaration.Rule = grammar.ToTerm ("{@code") + InlineValue + "}";
 				CodeDeclaration.AstConfig.NodeCreator = (context, parseNode) => {
-					parseNode.AstNode = new XElement ("c", parseNode.ChildNodes [1].AstNode.ToString ().Trim ());
+					parseNode.AstNode = new XElement ("c", parseNode.ChildNodes [1].AstNode?.ToString ()?.Trim ());
 				};
 
 				DocRootDeclaration.Rule = grammar.ToTerm ("{@docRoot}");
@@ -69,7 +69,7 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
 					if (optionalBracketMatch.ChildNodes.Count > 0 && optionalBracketMatch.ChildNodes [0].Term.Name == "}") {
 						parseNode.AstNode = new XElement ("c", target);
 					} else {
-						parseNode.AstNode = new XText (target.ToString ());
+						parseNode.AstNode = new XText (target.ToString () ?? string.Empty);
 					}
 				};
 
@@ -78,13 +78,13 @@ internal void CreateRules (SourceJavadocToXmldocGrammar grammar)
 					// TODO: *everything*; {@link target label}, but target can contain spaces!
 					// Also need to convert to appropriate CREF value, use text for now.
 					var target = parseNode.ChildNodes [1].AstNode.ToString ();
-					parseNode.AstNode = new XText (target);
+					parseNode.AstNode = new XText (target ?? string.Empty);
 				};
 
 				LiteralDeclaration.Rule = grammar.ToTerm ("{@literal") + InlineValue + "}";
 				LiteralDeclaration.AstConfig.NodeCreator = (context, parseNode) => {
 					var content = parseNode.ChildNodes [1].AstNode.ToString ();
-					parseNode.AstNode = new XText (content);
+					parseNode.AstNode = new XText (content ?? string.Empty);
 				};
 
 				SeeDeclaration.Rule = grammar.ToTerm ("{@see") + InlineValue + "}";
diff --git a/src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocParser.cs b/src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/SourceJavadocToXmldocParser.cs
@@ -128,7 +128,7 @@ IEnumerable<XNode> CreateParseIterator (ParseTree parseTree)
 				yield return node;
 			}
 			else {
-				yield return new XCData (ast?.ToString ());
+				yield return new XCData (ast?.ToString () ?? string.Empty);
 			}
 		}
 
diff --git a/src/Java.Interop.Tools.JavaTypeSystem/Java.Interop.Tools.JavaTypeSystem.csproj b/src/Java.Interop.Tools.JavaTypeSystem/Java.Interop.Tools.JavaTypeSystem.csproj
@@ -3,7 +3,6 @@
   <PropertyGroup>
     <TargetFramework>$(DotNetTargetFramework)</TargetFramework>
     <Nullable>enable</Nullable>
-    <DefineConstants>INTERNAL_NULLABLE_ATTRIBUTES</DefineConstants>
     <LangVersion>8.0</LangVersion>
   </PropertyGroup>
 
@@ -15,10 +14,6 @@
 
   <Import Project="..\..\build-tools\scripts\cecil.projitems" />
 
-  <ItemGroup>
-    <Compile Include="..\utils\NullableAttributes.cs" />
-  </ItemGroup>
-
   <ItemGroup>
     <ProjectReference Include="..\..\src\Java.Interop.Tools.Cecil\Java.Interop.Tools.Cecil.csproj" />
   </ItemGroup>
diff --git a/src/Java.Interop.Tools.JavaTypeSystem/Utilities/JavaUnresolvableModel.cs b/src/Java.Interop.Tools.JavaTypeSystem/Utilities/JavaUnresolvableModel.cs
@@ -57,7 +57,7 @@ string GetUnresolvable ()
 			if (Unresolvable is JavaParameterModel p)
 				return p.DeclaringMethod.ToString ();
 
-			return Unresolvable.ToString ();
+			return Unresolvable.ToString () ?? string.Empty;
 		}
 
 		string GetReason ()
diff --git a/src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlLoaderExtensions.cs b/src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlLoaderExtensions.cs
@@ -33,7 +33,7 @@ public static void Load (this JavaApi api, XmlReader reader, bool isReferenceOnl
 					if (reader.NodeType != XmlNodeType.Element || reader.LocalName != "package")
 						throw XmlUtil.UnexpectedElementOrContent ("api", reader, "package");
 
-					var name = reader.GetAttribute ("name");
+					var name = reader.GetAttribute ("name") ?? throw new InvalidOperationException ("<package> element must contain a 'name' attribute");
 
 					if (!api.Packages.TryGetValue (name, out var pkg)) {
 						pkg = new JavaPackage (api);

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ public static List<ConstantEntry> FromEnumMapCsv (TextReader reader)`
`20`	`20`	`var constants = new List<ConstantEntry> ();`
`21`	`21`	`var transient = false;`
`22`	`22`
`23`		`- string s;`
	`23`	`+ string? s;`
`24`	`24`
`25`	`25`	`// Read the enum csv file`
`26`	`26`	`while ((s = reader.ReadLine ()) != null) {`
`@@ -105,7 +105,7 @@ public class JavaSignatureComparer : IEqualityComparer<ConstantEntry>`
`105`	`105`	`{`
`106`	`106`	`public static JavaSignatureComparer Instance { get; } = new JavaSignatureComparer ();`
`107`	`107`
`108`		`- public bool Equals (ConstantEntry x, ConstantEntry y) => x?.JavaSignature == y?.JavaSignature;`
	`108`	`+ public bool Equals (ConstantEntry? x, ConstantEntry? y) => x?.JavaSignature == y?.JavaSignature;`
`109`	`109`	`public int GetHashCode (ConstantEntry obj) => 0;`
`110`	`110`	`}`
`111`	`111`	`}`