[Xamarin.Android.Tools.AndroidSdk] Fix NRT warnings (#206)

Fix NRT warnings that are showing up in CI build logs, e.g. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/AndroidAppManifest.cs(179,34): warning CS8604: Possible null reference argument for parameter 'attribute' in 'int? AndroidAppManifest.ParseSdkVersion(XAttribute attribute)'. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/AndroidAppManifest.cs(250,22): warning CS8600: Converting null literal or possible null value to non-nullable type. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/AndroidAppManifest.cs(252,6): warning CS8602: Dereference of a possibly null reference. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/AndroidAppManifest.cs(34,15): warning CS8601: Possible null reference assignment. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/AndroidAppManifest.cs(35,8): warning CS8602: Dereference of a possibly null reference. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/AndroidAppManifest.cs(38,18): warning CS8601: Possible null reference assignment. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/AndroidSdkInfo.cs(198,11): warning CS8603: Possible null reference return. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/AndroidVersion.cs(74,18): warning CS8600: Converting null literal or possible null value to non-nullable type. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/AndroidVersion.cs(74,27): warning CS8602: Dereference of a possibly null reference. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/AndroidVersion.cs(75,24): warning CS8604: Possible null reference argument for parameter 'element' in 'XElement.explicit operator int(XElement element)'. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/AndroidVersion.cs(76,18): warning CS8600: Converting null literal or possible null value to non-nullable type. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/AndroidVersion.cs(78,25): warning CS8604: Possible null reference argument for parameter 'element' in 'XElement.explicit operator bool(XElement element)'. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/AndroidVersion.cs(80,38): warning CS8602: Dereference of a possibly null reference. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs(403,23): warning CS8600: Converting null literal or possible null value to non-nullable type. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs(406,12): warning CS8602: Dereference of a possibly null reference. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs(52,10): warning CS8618: Non-nullable property 'JarPath' must contain a non-null value when exiting constructor. Consider declaring the property as nullable. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs(52,10): warning CS8618: Non-nullable property 'JavacPath' must contain a non-null value when exiting constructor. Consider declaring the property as nullable. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs(52,10): warning CS8618: Non-nullable property 'JavaPath' must contain a non-null value when exiting constructor. Consider declaring the property as nullable. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs(64,26): warning CS8601: Possible null reference assignment. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/Jdks/JdkLocations.MacOS.cs(23,29): warning CS8602: Dereference of a possibly null reference. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkUnix.cs(129,20): warning CS8602: Dereference of a possibly null reference. …/xamarin-android-tools/src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkUnix.cs(49,21): warning CS8602: Dereference of a possibly null reference. Fix these warnings. Rework the `JdkInfo` constructor so that the C# compiler can determine that `JarPath`, `JavacPath`, and `JavaPath` aren't `null`. Change the return type of `AndroidSdkUnix.GetUnixConfigFile()` and parameter type of `AndroidSdkUnix.SaveConfig()` to be `XElement` instead of `XDocument`, to remove warnings about `XDocument.Root` possibly being `null`.
dotnet · May 12, 2023 · 3b8c467 · 3b8c467
1 parent 7ef8ec5
commit 3b8c467
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 40 deletions.
diff --git a/src/Xamarin.Android.Tools.AndroidSdk/AndroidAppManifest.cs b/src/Xamarin.Android.Tools.AndroidSdk/AndroidAppManifest.cs
@@ -31,16 +31,20 @@ public class AndroidAppManifest
 				throw new ArgumentNullException (nameof (doc));
 			this.versions   = versions;
 			this.doc = doc;
-			manifest = doc.Root;
-			if (manifest.Name != "manifest")
+
+			if (doc.Root is null || doc.Root.Name != "manifest")
 				throw new ArgumentException ("App manifest does not have 'manifest' root element", nameof (doc));
 
-			application = manifest.Element ("application");
-			if (application == null)
+			manifest = doc.Root;
+
+			if (manifest.Element ("application") is XElement app)
+				application = app;
+			else
 				manifest.Add (application = new XElement ("application"));
 
-			usesSdk = manifest.Element ("uses-sdk");
-			if (usesSdk == null)
+			if (manifest.Element ("uses-sdk") is XElement uses)
+				usesSdk = uses;
+			else
 				manifest.Add (usesSdk = new XElement ("uses-sdk"));
 		}
 
@@ -185,7 +189,7 @@ public int? TargetSdkVersion {
 			set { usesSdk.SetAttributeValue (aNS + "targetSdkVersion", value == null ? null : value.ToString ()); }
 		}
 
-		int? ParseSdkVersion (XAttribute attribute)
+		int? ParseSdkVersion (XAttribute? attribute)
 		{
 			var version = (string?) attribute;
 			if (version == null || string.IsNullOrEmpty (version))
@@ -247,9 +251,9 @@ void AddAndroidPermissions (IEnumerable<string> permissions)
 					lastPerm = el;
 				}
 			} else {
-				var parentNode = (XNode) manifest.Element ("application") ?? manifest.LastNode;
+				var parentNode = (XNode?) manifest.Element ("application") ?? manifest.LastNode;
 				foreach (var el in newElements)
-					parentNode.AddBeforeSelf (el);
+					parentNode!.AddBeforeSelf (el);
 			}
 		}
 

diff --git a/src/Xamarin.Android.Tools.AndroidSdk/AndroidSdkInfo.cs b/src/Xamarin.Android.Tools.AndroidSdk/AndroidSdkInfo.cs
@@ -193,7 +193,7 @@ public static void DetectAndSetPreferredJavaSdkPathToLatest (Action<TraceLevel,
 			sdk.SetPreferredJavaSdkPath (latestJdk.HomePath);
 		}
 
-		public string TryGetCommandLineToolsPath ()
+		public string? TryGetCommandLineToolsPath ()
 		{
 			return GetCommandLineToolsPaths ("latest").FirstOrDefault ();
 		}

diff --git a/src/Xamarin.Android.Tools.AndroidSdk/AndroidVersion.cs b/src/Xamarin.Android.Tools.AndroidSdk/AndroidVersion.cs
@@ -71,11 +71,11 @@ public static AndroidVersion Load (string uri)
 		// </AndroidApiInfo>
 		static AndroidVersion Load (XDocument doc)
 		{
-			var id      = (string) doc.Root.Element ("Id");
-			var level   = (int) doc.Root.Element ("Level");
-			var name    = (string) doc.Root.Element ("Name");
-			var version = (string) doc.Root.Element ("Version");
-			var stable  = (bool) doc.Root.Element ("Stable");
+			var id      = (string?) doc.Root?.Element ("Id") ?? throw new InvalidOperationException ("Missing Id element");
+			var level   = (int?) doc.Root?.Element ("Level") ?? throw new InvalidOperationException ("Missing Level element");
+			var name    = (string?) doc.Root?.Element ("Name") ?? throw new InvalidOperationException ("Missing Name element");
+			var version = (string?) doc.Root?.Element ("Version") ?? throw new InvalidOperationException ("Missing Version element");
+			var stable  = (bool?) doc.Root?.Element ("Stable") ?? throw new InvalidOperationException ("Missing Stable element");
 
 			return new AndroidVersion (level, version.TrimStart ('v'), name, id, stable);
 		}

diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs
@@ -61,18 +61,15 @@ public JdkInfo (string homePath, string? locator = null, Action<TraceLevel, stri
 			this.logger         = logger ?? AndroidSdkInfo.DefaultConsoleLogger;
 
 			var binPath         = Path.Combine (HomePath, "bin");
-			JarPath             = ProcessUtils.FindExecutablesInDirectory (binPath, "jar").FirstOrDefault ();
-			JavaPath            = ProcessUtils.FindExecutablesInDirectory (binPath, "java").FirstOrDefault ();
-			JavacPath           = ProcessUtils.FindExecutablesInDirectory (binPath, "javac").FirstOrDefault ();
+			JarPath             = RequireExecutableInDirectory (binPath, "jar");
+			JavaPath            = RequireExecutableInDirectory (binPath, "java");
+			JavacPath           = RequireExecutableInDirectory (binPath, "javac");
 
 			string? jdkJvmPath  = GetJdkJvmPath ();
 
-			ValidateFile ("jar",    JarPath);
-			ValidateFile ("java",   JavaPath);
-			ValidateFile ("javac",  JavacPath);
 			ValidateFile ("jvm",    jdkJvmPath);
 
-			JdkJvmPath          = jdkJvmPath!;
+			JdkJvmPath          = jdkJvmPath;
 
 			var includes        = new List<string> ();
 			var jdkInclude      = Path.Combine (HomePath, "include");
@@ -153,12 +150,21 @@ static IEnumerable<string> FindLibrariesInDirectory (string dir, string libraryN
 			return Directory.EnumerateFiles (dir, library, SearchOption.AllDirectories);
 		}
 
-		void ValidateFile (string name, string? path)
+		void ValidateFile (string name, [NotNull]string? path)
 		{
 			if (path == null || !File.Exists (path))
 				throw new ArgumentException ($"Could not find required file `{name}` within `{HomePath}`; is this a valid JDK?", "homePath");
 		}
 
+		string RequireExecutableInDirectory (string binPath, string fileName)
+		{
+			var file = ProcessUtils.FindExecutablesInDirectory (binPath, fileName).FirstOrDefault ();
+
+			ValidateFile (fileName, file);
+
+			return file;
+		}
+
 		static  Regex   NonDigitMatcher     = new Regex (@"[^\d]", RegexOptions.Compiled | RegexOptions.CultureInvariant);
 
 		Version? GetJavaVersion ()
@@ -400,12 +406,12 @@ static IEnumerable<string> GetLibexecJdkPaths (Action<TraceLevel, string> logger
 				yield break;
 			}
 			foreach (var info in plist.Elements ("array").Elements ("dict")) {
-				var JVMHomePath = (XNode) info.Elements ("key").FirstOrDefault (e => e.Value == "JVMHomePath");
+				var JVMHomePath = (XNode?) info.Elements ("key").FirstOrDefault (e => e.Value == "JVMHomePath");
 				if (JVMHomePath == null)
 					continue;
-				while (JVMHomePath.NextNode.NodeType != XmlNodeType.Element)
-					JVMHomePath = JVMHomePath.NextNode;
-				var strElement  = (XElement) JVMHomePath.NextNode;
+				while (JVMHomePath.NextNode!.NodeType != XmlNodeType.Element)
+					JVMHomePath = JVMHomePath.NextNode!;
+				var strElement  = (XElement) JVMHomePath.NextNode!;
 				var path        = strElement.Value;
 				yield return path;
 			}

diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Jdks/JdkLocations.MacOS.cs b/src/Xamarin.Android.Tools.AndroidSdk/Jdks/JdkLocations.MacOS.cs
@@ -20,7 +20,8 @@ static IEnumerable<JdkInfo> GetUnixPreferredJdks (Action<TraceLevel, string> log
 		static IEnumerable<string> GetUnixConfiguredJdkPaths (Action<TraceLevel, string> logger)
 		{
 			var config = AndroidSdkUnix.GetUnixConfigFile (logger);
-			foreach (var java_sdk in config.Root.Elements ("java-sdk")) {
+
+			foreach (var java_sdk in config.Elements ("java-sdk")) {
 				var path    = (string?) java_sdk.Attribute ("path");
 				if (path != null && !string.IsNullOrEmpty (path)) {
 					yield return path;

diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkUnix.cs b/src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkUnix.cs
@@ -46,7 +46,7 @@ public override string NdkHostPlatform64Bit {
 		public override string? PreferedAndroidSdkPath {
 			get {
 				var config_file = GetUnixConfigFile (Logger);
-				var androidEl = config_file.Root.Element ("android-sdk");
+				var androidEl = config_file.Element ("android-sdk");
 
 				if (androidEl != null) {
 					var path = (string?)androidEl.Attribute ("path");
@@ -61,7 +61,7 @@ public override string? PreferedAndroidSdkPath {
 		public override string? PreferedAndroidNdkPath {
 			get {
 				var config_file = GetUnixConfigFile (Logger);
-				var androidEl = config_file.Root.Element ("android-ndk");
+				var androidEl = config_file.Element ("android-ndk");
 
 				if (androidEl != null) {
 					var path = (string?)androidEl.Attribute ("path");
@@ -76,7 +76,7 @@ public override string? PreferedAndroidNdkPath {
 		public override string? PreferedJavaSdkPath {
 			get {
 				var config_file = GetUnixConfigFile (Logger);
-				var javaEl = config_file.Root.Element ("java-sdk");
+				var javaEl = config_file.Element ("java-sdk");
 
 				if (javaEl != null) {
 					var path = (string?)javaEl.Attribute ("path");
@@ -126,11 +126,12 @@ public override void SetPreferredAndroidSdkPath (string? path)
 			path = NullIfEmpty (path);
 
 			var doc = GetUnixConfigFile (Logger);
-			var androidEl = doc.Root.Element ("android-sdk");
+
+			var androidEl = doc.Element ("android-sdk");
 
 			if (androidEl == null) {
 				androidEl = new XElement ("android-sdk");
-				doc.Root.Add (androidEl);
+				doc.Add (androidEl);
 			}
 
 			androidEl.SetAttributeValue ("path", path);
@@ -142,11 +143,12 @@ public override void SetPreferredJavaSdkPath (string? path)
 			path = NullIfEmpty (path);
 
 			var doc = GetUnixConfigFile (Logger);
-			var javaEl = doc.Root.Element ("java-sdk");
+
+			var javaEl = doc.Element ("java-sdk");
 
 			if (javaEl == null) {
 				javaEl = new XElement ("java-sdk");
-				doc.Root.Add (javaEl);
+				doc.Add (javaEl);
 			}
 
 			javaEl.SetAttributeValue ("path", path);
@@ -158,18 +160,19 @@ public override void SetPreferredAndroidNdkPath (string? path)
 			path = NullIfEmpty (path);
 
 			var doc = GetUnixConfigFile (Logger);
-			var androidEl = doc.Root.Element ("android-ndk");
+
+			var androidEl = doc.Element ("android-ndk");
 
 			if (androidEl == null) {
 				androidEl = new XElement ("android-ndk");
-				doc.Root.Add (androidEl);
+				doc.Add (androidEl);
 			}
 
 			androidEl.SetAttributeValue ("path", path);
 			SaveConfig (doc);
 		}
 
-		void SaveConfig (XDocument doc)
+		void SaveConfig (XElement doc)
 		{
 			string cfg = UnixConfigPath;
 			List <string>? created = null;
@@ -229,7 +232,7 @@ private static string UnixConfigPath {
 			}
 		}
 
-		internal static XDocument GetUnixConfigFile (Action<TraceLevel, string> logger)
+		internal static XElement GetUnixConfigFile (Action<TraceLevel, string> logger)
 		{
 			var file = UnixConfigPath;
 			XDocument? doc = null;
@@ -254,7 +257,7 @@ internal static XDocument GetUnixConfigFile (Action<TraceLevel, string> logger)
 			if (doc == null || doc.Root == null) {
 				doc = new XDocument (new XElement ("monodroid"));
 			}
-			return doc;
+			return doc.Root!;
 		}
 
 		void FixOwnership (List<string>? paths)