[src] Transient strings must always be disposed. (#24894)

Fix a couple of places where transient strings weren't disposed.

And to make sure this doesn't happen again, add a new Roslyn analyzer (RBI0042) that
reports an error when transient strings are declared as local variables without the
'using' keyword.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: rolfbjarne

src/CoreGraphics/CGFont.cs

@@ -125,7 +125,7 @@ protected internal override void Release ()
 			// and have a unit tests to make sure this behavior does not change over time
 			if (name is null)
 				return null;
-			var nameHandle = new TransientCFString (name);
+			using var nameHandle = new TransientCFString (name);
 			return Create (CGFontCreateWithFontName (nameHandle));
 		}
 

src/Network/NWEthernetChannel.cs

@@ -113,7 +113,7 @@ public void Send (ReadOnlySpan<byte> content, ushort vlanTag, string remoteAddre
 				unsafe {
 					delegate* unmanaged<IntPtr, IntPtr, void> trampoline = &TrampolineSendCompletion;
 					using var block = new BlockLiteral (trampoline, callback, typeof (NWEthernetChannel), nameof (TrampolineSendCompletion));
-					var remoteAddressStr = new TransientString (remoteAddress);
+					using var remoteAddressStr = new TransientString (remoteAddress);
 					nw_ethernet_channel_send (GetCheckedHandle (), dispatchData.GetHandle (), vlanTag, remoteAddressStr, &block);
 				}
 			}

src/ObjCRuntime/Protocol.cs

@@ -89,7 +89,7 @@ public static IntPtr GetHandle (string name)
 
 		internal static IntPtr objc_getProtocol (string? name)
 		{
-			var namePtr = new TransientString (name);
+			using var namePtr = new TransientString (name);
 			return objc_getProtocol (namePtr);
 		}
 

src/rgen/Microsoft.Macios.Bindings.Analyzer/AnalyzerReleases.Unshipped.md

@@ -23,3 +23,4 @@
 | RBI0017 | Usage    | Error    | Field used with the wrong flag.                                                      |
 | RBI0018 | Usage    | Error    | The export attribute must have a nonnull selector.                                   |
 | RBI0019 | Usage    | Error    | The selector string cannot contain any whitespace characters.                        |
+| RBI0042 | Usage    | Error    | Transient disposable type not declared with 'using'.                                 |

src/rgen/Microsoft.Macios.Bindings.Analyzer/Resources.Designer.cs

@@ -560,4 +560,17 @@
         <value>Protocol constructor overlap:</value>
     </data>
 
+    <!-- RBI0042 -->
+    
+    <data name="RBI0042Description" xml:space="preserve">
+        <value>Transient types (TransientString, TransientCFString, TransientCFObject) allocate native memory and must be disposed. Use the 'using' keyword to ensure proper cleanup.</value>
+    </data>
+    <data name="RBI0042MessageFormat" xml:space="preserve">
+        <value>Variable '{0}' of type '{1}' must be declared with the 'using' keyword to ensure proper disposal of native resources</value>
+        <comment>{0} is the name of the variable, {1} is the type name.</comment>
+    </data>
+    <data name="RBI0042Title" xml:space="preserve">
+        <value>Transient disposable type not declared with 'using'</value>
+    </data>
+    
 </root>

src/rgen/Microsoft.Macios.Bindings.Analyzer/Resources.resx

@@ -631,4 +631,20 @@ public static class RgenDiagnostics {
 		description: new LocalizableResourceString (nameof (Resources.RBI0041Description), Resources.ResourceManager,
 			typeof (Resources))
 	);
+
+	/// <summary>
+	/// Diagnostic descriptor for when a transient disposable type (TransientString, TransientCFString, TransientCFObject)
+	/// is not declared with the 'using' keyword.
+	/// </summary>
+	internal static readonly DiagnosticDescriptor RBI0042 = new (
+		"RBI0042",
+		new LocalizableResourceString (nameof (Resources.RBI0042Title), Resources.ResourceManager, typeof (Resources)),
+		new LocalizableResourceString (nameof (Resources.RBI0042MessageFormat), Resources.ResourceManager,
+			typeof (Resources)),
+		"Usage",
+		DiagnosticSeverity.Error,
+		isEnabledByDefault: true,
+		description: new LocalizableResourceString (nameof (Resources.RBI0042Description), Resources.ResourceManager,
+			typeof (Resources))
+	);
 }

src/rgen/Microsoft.Macios.Bindings.Analyzer/RgenDiagnostics.cs

@@ -0,0 +1,57 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using static Microsoft.Macios.Generator.RgenDiagnostics;
+
+namespace Microsoft.Macios.Bindings.Analyzer;
+
+/// <summary>
+/// Analyzer to ensure that transient disposable types (TransientString, TransientCFString, TransientCFObject)
+/// are always declared with the 'using' keyword to guarantee proper disposal of native resources.
+/// </summary>
+[DiagnosticAnalyzer (LanguageNames.CSharp)]
+public class TransientDisposableAnalyzer : DiagnosticAnalyzer {
+
+	static readonly ImmutableHashSet<string> transientTypeFullNames = ImmutableHashSet.Create (
+		"ObjCRuntime.TransientString",
+		"ObjCRuntime.TransientCFString",
+		"ObjCRuntime.TransientCFObject"
+	);
+
+	public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create (RBI0042);
+
+	public override void Initialize (AnalysisContext context)
+	{
+		context.ConfigureGeneratedCodeAnalysis (GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
+		context.EnableConcurrentExecution ();
+		context.RegisterSyntaxNodeAction (AnalyzeNode, SyntaxKind.LocalDeclarationStatement);
+	}
+
+	void AnalyzeNode (SyntaxNodeAnalysisContext context)
+	{
+		if (context.Node is not LocalDeclarationStatementSyntax localDeclaration)
+			return;
+
+		// If the declaration already has 'using', it's fine.
+		if (localDeclaration.UsingKeyword != default)
+			return;
+
+		foreach (var variable in localDeclaration.Declaration.Variables) {
+			var symbol = context.SemanticModel.GetDeclaredSymbol (variable);
+			if (symbol is not ILocalSymbol localSymbol)
+				continue;
+
+			var typeDisplayName = localSymbol.Type.ToDisplayString ();
+			if (!transientTypeFullNames.Contains (typeDisplayName))
+				continue;
+
+			var diagnostic = Diagnostic.Create (RBI0042, variable.GetLocation (), variable.Identifier.Text, localSymbol.Type.Name);
+			context.ReportDiagnostic (diagnostic);
+		}
+	}
+}

src/rgen/Microsoft.Macios.Bindings.Analyzer/TransientDisposableAnalyzer.cs

@@ -0,0 +1,204 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+using System.Collections;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Xamarin.Tests;
+using Xamarin.Utils;
+using Xunit;
+
+namespace Microsoft.Macios.Bindings.Analyzer.Tests;
+
+public class TransientDisposableAnalyzerTests : BaseGeneratorWithAnalyzerTestClass {
+	class ErrorTestCases : IEnumerable<object []> {
+		public IEnumerator<object []> GetEnumerator ()
+		{
+			// TransientString without using
+			yield return [
+				"""
+				using System;
+
+				namespace ObjCRuntime {
+					struct TransientString : IDisposable {
+						public void Dispose () { }
+					}
+				}
+
+				class Test
+				{
+					void Method ()
+					{
+						var s = new ObjCRuntime.TransientString ();
+					}
+				}
+				"""];
+
+			// TransientCFString without using
+			yield return [
+				"""
+				using System;
+
+				namespace ObjCRuntime {
+					ref struct TransientCFString {
+						public void Dispose () { }
+					}
+				}
+
+				class Test
+				{
+					void Method ()
+					{
+						ObjCRuntime.TransientCFString s = new ObjCRuntime.TransientCFString ();
+					}
+				}
+				"""];
+
+			// TransientCFObject without using
+			yield return [
+				"""
+				using System;
+
+				namespace ObjCRuntime {
+					ref struct TransientCFObject {
+						public void Dispose () { }
+					}
+				}
+
+				class Test
+				{
+					void Method ()
+					{
+						var obj = new ObjCRuntime.TransientCFObject ();
+					}
+				}
+				"""];
+		}
+
+		IEnumerator IEnumerable.GetEnumerator () => GetEnumerator ();
+	}
+
+	class NoErrorTestCases : IEnumerable<object []> {
+		public IEnumerator<object []> GetEnumerator ()
+		{
+			// TransientString with using var
+			yield return [
+				"""
+				using System;
+
+				namespace ObjCRuntime {
+					struct TransientString : IDisposable {
+						public void Dispose () { }
+					}
+				}
+
+				class Test
+				{
+					void Method ()
+					{
+						using var s = new ObjCRuntime.TransientString ();
+					}
+				}
+				"""];
+
+			// TransientCFString with using var
+			yield return [
+				"""
+				using System;
+
+				namespace ObjCRuntime {
+					ref struct TransientCFString {
+						public void Dispose () { }
+					}
+				}
+
+				class Test
+				{
+					void Method ()
+					{
+						using var s = new ObjCRuntime.TransientCFString ();
+					}
+				}
+				"""];
+
+			// TransientCFObject with using var
+			yield return [
+				"""
+				using System;
+
+				namespace ObjCRuntime {
+					ref struct TransientCFObject {
+						public void Dispose () { }
+					}
+				}
+
+				class Test
+				{
+					void Method ()
+					{
+						using var obj = new ObjCRuntime.TransientCFObject ();
+					}
+				}
+				"""];
+
+			// Non-transient type without using is fine
+			yield return [
+				"""
+				using System;
+
+				struct SomeOtherStruct : IDisposable {
+					public void Dispose () { }
+				}
+
+				class Test
+				{
+					void Method ()
+					{
+						var s = new SomeOtherStruct ();
+					}
+				}
+				"""];
+
+			// User-defined TransientString outside ObjCRuntime namespace should not trigger
+			yield return [
+				"""
+				using System;
+
+				struct TransientString : IDisposable {
+					public void Dispose () { }
+				}
+
+				class Test
+				{
+					void Method ()
+					{
+						var s = new TransientString ();
+					}
+				}
+				"""];
+		}
+
+		IEnumerator IEnumerable.GetEnumerator () => GetEnumerator ();
+	}
+
+	[Theory]
+	[AllSupportedPlatformsClassData<ErrorTestCases>]
+	public async Task TransientWithoutUsingTests (ApplePlatform platform, string inputText)
+	{
+		var (compilation, _) = CreateCompilation (platform, sources: inputText);
+		var diagnostics = await RunAnalyzer (new TransientDisposableAnalyzer (), compilation);
+		var analyzerDiagnostics = diagnostics.Where (d => d.Id == "RBI0042").ToArray ();
+		Assert.True (analyzerDiagnostics.Length != 0);
+	}
+
+	[Theory]
+	[AllSupportedPlatformsClassData<NoErrorTestCases>]
+	public async Task TransientWithUsingTests (ApplePlatform platform, string inputText)
+	{
+		var (compilation, _) = CreateCompilation (platform, sources: inputText);
+		var diagnostics = await RunAnalyzer (new TransientDisposableAnalyzer (), compilation);
+		var analyzerDiagnostics = diagnostics.Where (d => d.Id == "RBI0042").ToArray ();
+		Assert.Empty (analyzerDiagnostics);
+	}
+}