[generator] Remove [Obsolete] style compatibility hacks. (#1025)

`generator` was inconsistent with the style in which it generated `[Obsolete]` attributes, emitting variations such as: [Obsolete] [Obsolete ("")] [Obsolete (@"")] [ObsoleteAttribute] [global::System.Obsolete] We wrote a temporary compatibility layer to preserve these variations in commit 440c05e. This allowed us to do a source code level text compare against a large corpus of generated code in `Mono.Android`, AndroidX, and GooglePlayServices, to ensure there were no changes. Now that we are passed this phase, we can standardize on: [global::System.Obsolete] [global::System.Obsolete (@"message")]
dotnet · Aug 12, 2022 · 6d1ae4e · 6d1ae4e
1 parent 440c05e
commit 6d1ae4e
Show file tree

Hide file tree

Showing 20 changed files with 40 additions and 51 deletions.
diff --git a/...rator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1-NRT/WriteInterface.txt b/...rator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1-NRT/WriteInterface.txt
@@ -20,7 +20,7 @@ public abstract class MyInterface : Java.Lang.Object {
 }
 
 [Register ("java/code/IMyInterface", DoNotGenerateAcw=true)]
-[global::System.Obsolete ("Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
+[global::System.Obsolete (@"Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
 public abstract class MyInterfaceConsts : MyInterface {
 	private MyInterfaceConsts ()
 	{

diff --git a/...t-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/ObsoleteInterfaceAlternativeClass.txt b/...t-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/ObsoleteInterfaceAlternativeClass.txt
@@ -1,24 +1,24 @@
 [Register ("com/xamarin/android/Parent", DoNotGenerateAcw=true)]
-[global::System.Obsolete ("Use the 'Com.Xamarin.Android.IParent' type. This class will be removed in a future release.")]
+[global::System.Obsolete (@"Use the 'Com.Xamarin.Android.IParent' type. This class will be removed in a future release.")]
 public abstract class Parent : Java.Lang.Object {
 	internal Parent ()
 	{
 	}
 
 	// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='ACCEPT_HANDOVER']"
 	[Register ("ACCEPT_HANDOVER")]
-	[Obsolete ("Use 'Com.Xamarin.Android.IParent.AcceptHandover'. This class will be removed in a future release.")]
+	[global::System.Obsolete (@"Use 'Com.Xamarin.Android.IParent.AcceptHandover'. This class will be removed in a future release.")]
 	public const string AcceptHandover = (string) "android.permission.ACCEPT_HANDOVER";
 
 	// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='ALREADY_OBSOLETE']"
 	[Register ("ALREADY_OBSOLETE")]
-	[Obsolete ("deprecated")]
+	[global::System.Obsolete (@"deprecated")]
 	public const string AlreadyObsolete = (string) "android.permission.ACCEPT_HANDOVER";
 
 
 	// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='API_NAME']"
 	[Register ("API_NAME")]
-	[Obsolete ("Use 'Com.Xamarin.Android.IParent.ApiName'. This class will be removed in a future release.")]
+	[global::System.Obsolete (@"Use 'Com.Xamarin.Android.IParent.ApiName'. This class will be removed in a future release.")]
 	public static string ApiName {
 		get {
 			const string __id = "API_NAME.Ljava/lang/String;";
@@ -29,7 +29,7 @@ public abstract class Parent : Java.Lang.Object {
 	}
 
 	// Metadata.xml XPath method reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/method[@name='comparing' and count(parameter)=0]"
-	[Obsolete (@"Use 'Com.Xamarin.Android.IParent.Comparing'. This class will be removed in a future release.")]
+	[global::System.Obsolete (@"Use 'Com.Xamarin.Android.IParent.Comparing'. This class will be removed in a future release.")]
 	[Register ("comparing", "()I", "")]
 	public static unsafe int Comparing ()
 	{
@@ -42,7 +42,7 @@ public abstract class Parent : Java.Lang.Object {
 	}
 
 	// Metadata.xml XPath method reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/method[@name='comparingOld' and count(parameter)=0]"
-	[Obsolete (@"deprecated")]
+	[global::System.Obsolete (@"deprecated")]
 	[Register ("comparingOld", "()I", "")]
 	public static unsafe int ComparingOld ()
 	{
@@ -69,7 +69,7 @@ public partial interface IParent : IJavaObject, IJavaPeerable {
 
 	// Metadata.xml XPath field reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/field[@name='ALREADY_OBSOLETE']"
 	[Register ("ALREADY_OBSOLETE")]
-	[Obsolete ("deprecated")]
+	[global::System.Obsolete (@"deprecated")]
 	public const string AlreadyObsolete = (string) "android.permission.ACCEPT_HANDOVER";
 
 
@@ -97,7 +97,7 @@ public partial interface IParent : IJavaObject, IJavaPeerable {
 	}
 
 	// Metadata.xml XPath method reference: path="/api/package[@name='com.xamarin.android']/interface[@name='Parent']/method[@name='comparingOld' and count(parameter)=0]"
-	[Obsolete (@"deprecated")]
+	[global::System.Obsolete (@"deprecated")]
 	[Register ("comparingOld", "()I", "")]
 	public static unsafe int ComparingOld ()
 	{

diff --git a/...generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterface.txt b/...generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterface.txt
@@ -20,7 +20,7 @@ public abstract class MyInterface : Java.Lang.Object {
 }
 
 [Register ("java/code/IMyInterface", DoNotGenerateAcw=true)]
-[global::System.Obsolete ("Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
+[global::System.Obsolete (@"Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
 public abstract class MyInterfaceConsts : MyInterface {
 	private MyInterfaceConsts ()
 	{

diff --git a/...it-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceFieldAsDimProperty.txt b/...it-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteInterfaceFieldAsDimProperty.txt
@@ -33,7 +33,7 @@ public abstract class MyInterface : Java.Lang.Object {
 }
 
 [Register ("com/xamarin/android/MyInterface", DoNotGenerateAcw=true)]
-[global::System.Obsolete ("Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
+[global::System.Obsolete (@"Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
 public abstract class MyInterfaceConsts : MyInterface {
 	private MyInterfaceConsts ()
 	{

diff --git a/...sts/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteStaticInterfaceMethod.txt b/...sts/Unit-Tests/CodeGeneratorExpectedResults/XAJavaInterop1/WriteStaticInterfaceMethod.txt
@@ -20,7 +20,7 @@ public abstract class MyInterface : Java.Lang.Object {
 }
 
 [Register ("java/code/IMyInterface", DoNotGenerateAcw=true)]
-[global::System.Obsolete ("Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
+[global::System.Obsolete (@"Use the 'MyInterface' type. This type will be removed in a future release.", error: true)]
 public abstract class MyInterfaceConsts : MyInterface {
 	private MyInterfaceConsts ()
 	{

diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs
@@ -528,7 +528,7 @@ public void ObsoleteBoundMethodAbstractDeclaration ()
 			generator.Context.ContextTypes.Pop ();
 
 			// Ensure [Obsolete] was written
-			Assert.True (writer.ToString ().Contains ("[Obsolete (@\"This is so old!\")]"), writer.ToString ());
+			Assert.True (writer.ToString ().Contains ("[global::System.Obsolete (@\"This is so old!\")]"), writer.ToString ());
 		}
 
 		[Test]

diff --git a/tests/generator-Tests/expected.ji/Constructors/Xamarin.Test.SomeObject.cs b/tests/generator-Tests/expected.ji/Constructors/Xamarin.Test.SomeObject.cs
@@ -20,7 +20,7 @@ protected SomeObject (ref JniObjectReference reference, JniObjectReferenceOption
 		}
 
 		// Metadata.xml XPath constructor reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/constructor[@name='SomeObject' and count(parameter)=0]"
-		[Obsolete (@"deprecated")]
+		[global::System.Obsolete (@"deprecated")]
 		public unsafe SomeObject () : base (ref *InvalidJniObjectReference, JniObjectReferenceOptions.None)
 		{
 			const string __id = "()V";

diff --git a/tests/generator-Tests/expected.ji/NormalMethods/Xamarin.Test.SomeObject.cs b/tests/generator-Tests/expected.ji/NormalMethods/Xamarin.Test.SomeObject.cs
@@ -126,7 +126,7 @@ public virtual unsafe void VoidMethodWithParams (string astring, int anint, glob
 		}
 
 		// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/method[@name='ObsoleteMethod' and count(parameter)=0]"
-		[Obsolete (@"Deprecated please use IntegerMethod instead")]
+		[global::System.Obsolete (@"Deprecated please use IntegerMethod instead")]
 		public virtual unsafe int ObsoleteMethod ()
 		{
 			const string __id = "ObsoleteMethod.()I";

diff --git a/tests/generator-Tests/expected.ji/StaticMethods/Xamarin.Test.SomeObject.cs b/tests/generator-Tests/expected.ji/StaticMethods/Xamarin.Test.SomeObject.cs
@@ -44,7 +44,7 @@ public static unsafe string MethodAsString ()
 		}
 
 		// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/method[@name='Obsoletemethod' and count(parameter)=0]"
-		[Obsolete (@"Deprecated please use methodAsString")]
+		[global::System.Obsolete (@"Deprecated please use methodAsString")]
 		[global::Java.Interop.JniMethodSignature ("Obsoletemethod", "()Ljava/lang/String;")]
 		public static unsafe string Obsoletemethod ()
 		{

diff --git a/tests/generator-Tests/expected.xaji/Constructors/Xamarin.Test.SomeObject.cs b/tests/generator-Tests/expected.xaji/Constructors/Xamarin.Test.SomeObject.cs
@@ -38,7 +38,7 @@ protected SomeObject (IntPtr javaReference, JniHandleOwnership transfer) : base
 
 		// Metadata.xml XPath constructor reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/constructor[@name='SomeObject' and count(parameter)=0]"
 		[Register (".ctor", "()V", "")]
-		[Obsolete (@"deprecated")]
+		[global::System.Obsolete (@"deprecated")]
 		public unsafe SomeObject () : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
 		{
 			const string __id = "()V";

diff --git a/tests/generator-Tests/expected.xaji/NormalMethods/Xamarin.Test.SomeObject.cs b/tests/generator-Tests/expected.xaji/NormalMethods/Xamarin.Test.SomeObject.cs
@@ -269,15 +269,15 @@ public virtual unsafe void VoidMethodWithParams (string astring, int anint, glob
 
 		static Delegate cb_ObsoleteMethod;
 #pragma warning disable 0169
-		[Obsolete]
+		[global::System.Obsolete]
 		static Delegate GetObsoleteMethodHandler ()
 		{
 			if (cb_ObsoleteMethod == null)
 				cb_ObsoleteMethod = JNINativeWrapper.CreateDelegate ((_JniMarshal_PP_I) n_ObsoleteMethod);
 			return cb_ObsoleteMethod;
 		}
 
-		[Obsolete]
+		[global::System.Obsolete]
 		static int n_ObsoleteMethod (IntPtr jnienv, IntPtr native__this)
 		{
 			var __this = global::Java.Lang.Object.GetObject<global::Xamarin.Test.SomeObject> (jnienv, native__this, JniHandleOwnership.DoNotTransfer);
@@ -286,7 +286,7 @@ static int n_ObsoleteMethod (IntPtr jnienv, IntPtr native__this)
 #pragma warning restore 0169
 
 		// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/method[@name='ObsoleteMethod' and count(parameter)=0]"
-		[Obsolete (@"Deprecated please use IntegerMethod instead")]
+		[global::System.Obsolete (@"Deprecated please use IntegerMethod instead")]
 		[Register ("ObsoleteMethod", "()I", "GetObsoleteMethodHandler")]
 		public virtual unsafe int ObsoleteMethod ()
 		{

diff --git a/tests/generator-Tests/expected.xaji/StaticMethods/Xamarin.Test.SomeObject.cs b/tests/generator-Tests/expected.xaji/StaticMethods/Xamarin.Test.SomeObject.cs
@@ -61,7 +61,7 @@ public static unsafe string MethodAsString ()
 		}
 
 		// Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/class[@name='SomeObject']/method[@name='Obsoletemethod' and count(parameter)=0]"
-		[Obsolete (@"Deprecated please use methodAsString")]
+		[global::System.Obsolete (@"Deprecated please use methodAsString")]
 		[Register ("Obsoletemethod", "()Ljava/lang/String;", "")]
 		public static unsafe string Obsoletemethod ()
 		{

diff --git a/tests/generator-Tests/expected.xaji/TestInterface/Test.ME.ITestInterface.cs b/tests/generator-Tests/expected.xaji/TestInterface/Test.ME.ITestInterface.cs
@@ -32,7 +32,7 @@ internal TestInterface ()
 	}
 
 	[Register ("test/me/TestInterface", DoNotGenerateAcw=true)]
-	[global::System.Obsolete ("Use the 'TestInterface' type. This type will be removed in a future release.", error: true)]
+	[global::System.Obsolete (@"Use the 'TestInterface' type. This type will be removed in a future release.", error: true)]
 	public abstract class TestInterfaceConsts : TestInterface {
 		private TestInterfaceConsts ()
 		{

diff --git a/tools/generator/SourceWriters/Attributes/ObsoleteAttr.cs b/tools/generator/SourceWriters/Attributes/ObsoleteAttr.cs
@@ -11,10 +11,6 @@ public class ObsoleteAttr : AttributeWriter
 	{
 		public string Message { get; set; }
 		public bool IsError { get; set; }
-		public bool NoAtSign { get; set; }		// TODO: Temporary to match unit tests
-		public bool WriteEmptyString { get; set; }      // TODO: Temporary to match unit tests
-		public bool WriteAttributeSuffix { get; set; }  // TODO: Temporary to match unit tests
-		public bool WriteGlobal { get; set; }           // TODO: Temporary to match unit tests
 
 		public ObsoleteAttr (string message = null, bool isError = false)
 		{
@@ -24,22 +20,20 @@ public ObsoleteAttr (string message = null, bool isError = false)
 
 		public override void WriteAttribute (CodeWriter writer)
 		{
-			var attr_name = WriteAttributeSuffix ? "ObsoleteAttribute" : "Obsolete";
+			var parts = new List<string> ();
 
-			if (WriteGlobal)
-				attr_name = "global::System." + attr_name;
-
-			if (Message is null && !WriteEmptyString && !IsError) {
-				writer.WriteLine ($"[{attr_name}]");
-				return;
-			}
-
-			writer.Write ($"[{attr_name} ({(NoAtSign ? "" : "@")}\"{Message}\"");
+			if (Message != null)
+				parts.Add ($"@\"{Message}\"");
 
 			if (IsError)
-				writer.Write (", error: true");
+				parts.Add ("error: true");
+
+			var content = string.Join (", ", parts.ToArray ());
 
-			writer.WriteLine (")]");
+			if (content.HasValue ())
+				writer.WriteLine ($"[global::System.Obsolete ({content})]");
+			else
+				writer.WriteLine ("[global::System.Obsolete]");
 		}
 	}
 }
diff --git a/tools/generator/SourceWriters/BoundClass.cs b/tools/generator/SourceWriters/BoundClass.cs
@@ -44,7 +44,7 @@ public BoundClass (ClassGen klass, CodeGenerationOptions opt, CodeGeneratorConte
 			klass.JavadocInfo?.AddJavadocs (Comments);
 			Comments.Add ($"// Metadata.xml XPath class reference: path=\"{klass.MetadataXPathReference}\"");
 
-			SourceWriterExtensions.AddObsolete (Attributes, klass.DeprecatedComment, klass.IsDeprecated, writeAttributeSuffix: true);
+			SourceWriterExtensions.AddObsolete (Attributes, klass.DeprecatedComment, klass.IsDeprecated);
 
 			SourceWriterExtensions.AddSupportedOSPlatform (Attributes, klass, opt);
 

diff --git a/tools/generator/SourceWriters/BoundField.cs b/tools/generator/SourceWriters/BoundField.cs
@@ -31,7 +31,7 @@ public BoundField (GenBase type, Field field, CodeGenerationOptions opt)
 			if (field.IsEnumified)
 				Attributes.Add (new GeneratedEnumAttr ());
 
-			SourceWriterExtensions.AddObsolete (Attributes, field.DeprecatedComment, field.IsDeprecated, noAtSign: true, writeEmptyString: true, isError: field.IsDeprecatedError);
+			SourceWriterExtensions.AddObsolete (Attributes, field.DeprecatedComment, field.IsDeprecated, isError: field.IsDeprecatedError);
 
 			if (field.Annotation.HasValue ())
 				Attributes.Add (new CustomAttr (field.Annotation));

diff --git a/tools/generator/SourceWriters/BoundFieldAsProperty.cs b/tools/generator/SourceWriters/BoundFieldAsProperty.cs
@@ -47,7 +47,7 @@ public BoundFieldAsProperty (GenBase type, Field field, CodeGenerationOptions op
 				Attributes.Add (new RegisterAttr (field.JavaName, additionalProperties: field.AdditionalAttributeString ()));
 			}
 
-			SourceWriterExtensions.AddObsolete (Attributes, field.DeprecatedComment, field.IsDeprecated, noAtSign: true, isError: field.IsDeprecatedError);
+			SourceWriterExtensions.AddObsolete (Attributes, field.DeprecatedComment, field.IsDeprecated, isError: field.IsDeprecatedError);
 
 			SetVisibility (field.Visibility);
 			UseExplicitPrivateKeyword = true;

diff --git a/tools/generator/SourceWriters/BoundInterface.cs b/tools/generator/SourceWriters/BoundInterface.cs
@@ -43,7 +43,7 @@ public BoundInterface (InterfaceGen iface, CodeGenerationOptions opt, CodeGenera
 			iface.JavadocInfo?.AddJavadocs (Comments);
 			Comments.Add ($"// Metadata.xml XPath interface reference: path=\"{iface.MetadataXPathReference}\"");
 
-			SourceWriterExtensions.AddObsolete (Attributes, iface.DeprecatedComment, iface.IsDeprecated, writeAttributeSuffix: true, writeEmptyString: true);
+			SourceWriterExtensions.AddObsolete (Attributes, iface.DeprecatedComment, iface.IsDeprecated);
 
 			if (!iface.IsConstSugar (opt)) {
 				var signature = string.IsNullOrWhiteSpace (iface.Namespace)

diff --git a/tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs b/tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs
@@ -301,18 +301,13 @@ public static void AddSupportedOSPlatform (List<AttributeWriter> attributes, Api
 
 		}
 
-		public static void AddObsolete (List<AttributeWriter> attributes, string message, bool forceDeprecate = false, bool isError = false, bool noAtSign = false, bool writeEmptyString = false, bool writeAttributeSuffix = false, bool writeGlobal = false)
+		public static void AddObsolete (List<AttributeWriter> attributes, string message, bool forceDeprecate = false, bool isError = false)
 		{
 			// Bail if we're not obsolete
 			if ((!forceDeprecate && !message.HasValue ()) || message == "not deprecated")
 				return;
 
-			attributes.Add (new ObsoleteAttr (message: message?.Replace ("\"", "\"\"").Trim (), isError: isError) {
-				NoAtSign = noAtSign,
-				WriteAttributeSuffix = writeAttributeSuffix,
-				WriteEmptyString = writeEmptyString,
-				WriteGlobal = writeGlobal
-			});
+			attributes.Add (new ObsoleteAttr (message: message?.Replace ("\"", "\"\"").Trim (), isError: isError));
 		}
 
 		public static void WriteMethodInvokerBody (CodeWriter writer, Method method, CodeGenerationOptions opt, string contextThis)

diff --git a/tools/generator/SourceWriters/InterfaceMemberAlternativeClass.cs b/tools/generator/SourceWriters/InterfaceMemberAlternativeClass.cs
@@ -42,7 +42,7 @@ public InterfaceMemberAlternativeClass (InterfaceGen iface, CodeGenerationOption
 			Attributes.Add (new RegisterAttr (iface.RawJniName, noAcw: true, additionalProperties: iface.AdditionalAttributeString ()) { AcwLast = true });
 
 			if (should_obsolete)
-				Attributes.Add (new ObsoleteAttr ($"Use the '{iface.FullName}' type. This class will be removed in a future release.") { WriteGlobal = true, NoAtSign = true });
+				SourceWriterExtensions.AddObsolete (Attributes, $"Use the '{iface.FullName}' type. This class will be removed in a future release.");
 
 			Constructors.Add (new ConstructorWriter { Name = Name, IsInternal = true });
 
@@ -180,7 +180,7 @@ public InterfaceConstsForwardClass (InterfaceGen iface)
 			IsAbstract = true;
 
 			Attributes.Add (new RegisterAttr (iface.RawJniName, noAcw: true, additionalProperties: iface.AdditionalAttributeString ()));
-			Attributes.Add (new ObsoleteAttr ($"Use the '{iface.Name.Substring (1)}' type. This type will be removed in a future release.", true) { NoAtSign = true, WriteGlobal = true });
+			SourceWriterExtensions.AddObsolete (Attributes, $"Use the '{iface.Name.Substring (1)}' type. This type will be removed in a future release.", isError: true);
 
 			Constructors.Add (new ConstructorWriter {
 				Name = Name,