[generator] Refactor logic for applying [Obsolete] attributes (#1024)

jpobst · web-flow · commit 440c05eef904 · 2022-08-11T16:20:07.000-04:00
Context: dotnet/android#7234 Refactor logic for applying `[Obsolete]` attributes into a single common method. This method will later be extended to add support for [`[ObsoletedOSPlatformAttribute]`][0]. Doing this piece first and separately allows us to verify that the refactor does not break anything, as the existing logic is tricky. A future PR will also remove the temporary hacks used to preserve stylistic compatibility with a `generator` refactor in 6bbb00a. [0]: dotnet/runtime#72970
diff --git a/tools/generator/SourceWriters/BoundClass.cs b/tools/generator/SourceWriters/BoundClass.cs
@@ -44,8 +44,7 @@ public BoundClass (ClassGen klass, CodeGenerationOptions opt, CodeGeneratorConte
 			klass.JavadocInfo?.AddJavadocs (Comments);
 			Comments.Add ($"// Metadata.xml XPath class reference: path=\"{klass.MetadataXPathReference}\"");
 
-			if (klass.IsDeprecated)
-				Attributes.Add (new ObsoleteAttr (klass.DeprecatedComment) { WriteAttributeSuffix = true });
+			SourceWriterExtensions.AddObsolete (Attributes, klass.DeprecatedComment, klass.IsDeprecated, writeAttributeSuffix: true);
 
 			SourceWriterExtensions.AddSupportedOSPlatform (Attributes, klass, opt);
 
diff --git a/tools/generator/SourceWriters/BoundConstructor.cs b/tools/generator/SourceWriters/BoundConstructor.cs
@@ -33,8 +33,7 @@ public BoundConstructor (ClassGen klass, Ctor constructor, bool useBase, CodeGen
 				Attributes.Add (new RegisterAttr (".ctor", constructor.JniSignature, string.Empty, additionalProperties: constructor.AdditionalAttributeString ()));
 			}
 
-			if (constructor.Deprecated != null)
-				Attributes.Add (new ObsoleteAttr (constructor.Deprecated.Replace ("\"", "\"\"")));
+			SourceWriterExtensions.AddObsolete (Attributes, constructor.Deprecated);
 
 			if (constructor.CustomAttributes != null)
 				Attributes.Add (new CustomAttr (constructor.CustomAttributes));
diff --git a/tools/generator/SourceWriters/BoundField.cs b/tools/generator/SourceWriters/BoundField.cs
@@ -30,8 +30,9 @@ public BoundField (GenBase type, Field field, CodeGenerationOptions opt)
 
 			if (field.IsEnumified)
 				Attributes.Add (new GeneratedEnumAttr ());
-			if (field.IsDeprecated)
-				Attributes.Add (new ObsoleteAttr (field.DeprecatedComment, field.IsDeprecatedError) { NoAtSign = true, WriteEmptyString = true });
+
+			SourceWriterExtensions.AddObsolete (Attributes, field.DeprecatedComment, field.IsDeprecated, noAtSign: true, writeEmptyString: true, 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,8 +47,7 @@ public BoundFieldAsProperty (GenBase type, Field field, CodeGenerationOptions op
 				Attributes.Add (new RegisterAttr (field.JavaName, additionalProperties: field.AdditionalAttributeString ()));
 			}
 
-			if (field.IsDeprecated)
-				Attributes.Add (new ObsoleteAttr (field.DeprecatedComment, field.IsDeprecatedError) { NoAtSign = true });
+			SourceWriterExtensions.AddObsolete (Attributes, field.DeprecatedComment, field.IsDeprecated, noAtSign: true, isError: field.IsDeprecatedError);
 
 			SetVisibility (field.Visibility);
 			UseExplicitPrivateKeyword = true;
diff --git a/tools/generator/SourceWriters/BoundInterface.cs b/tools/generator/SourceWriters/BoundInterface.cs
@@ -43,8 +43,7 @@ public BoundInterface (InterfaceGen iface, CodeGenerationOptions opt, CodeGenera
 			iface.JavadocInfo?.AddJavadocs (Comments);
 			Comments.Add ($"// Metadata.xml XPath interface reference: path=\"{iface.MetadataXPathReference}\"");
 
-			if (iface.IsDeprecated)
-				Attributes.Add (new ObsoleteAttr (iface.DeprecatedComment) { WriteAttributeSuffix = true, WriteEmptyString = true });
+			SourceWriterExtensions.AddObsolete (Attributes, iface.DeprecatedComment, iface.IsDeprecated, writeAttributeSuffix: true, writeEmptyString: true);
 
 			if (!iface.IsConstSugar (opt)) {
 				var signature = string.IsNullOrWhiteSpace (iface.Namespace)
diff --git a/tools/generator/SourceWriters/BoundInterfaceMethodDeclaration.cs b/tools/generator/SourceWriters/BoundInterfaceMethodDeclaration.cs
@@ -33,8 +33,9 @@ public BoundInterfaceMethodDeclaration (Method method, string adapter, CodeGener
 
 			if (method.DeclaringType.IsGeneratable)
 				Comments.Add ($"// Metadata.xml XPath method reference: path=\"{method.GetMetadataXPathReference (method.DeclaringType)}\"");
-			if (method.Deprecated != null)
-				Attributes.Add (new ObsoleteAttr (method.Deprecated.Replace ("\"", "\"\"")));
+
+			SourceWriterExtensions.AddObsolete (Attributes, method.Deprecated);
+
 			if (method.IsReturnEnumified)
 				Attributes.Add (new GeneratedEnumAttr (true));
 			if (method.IsInterfaceDefaultMethod)
diff --git a/tools/generator/SourceWriters/BoundMethod.cs b/tools/generator/SourceWriters/BoundMethod.cs
@@ -74,8 +74,7 @@ public BoundMethod (GenBase type, Method method, CodeGenerationOptions opt, bool
 			if (method.DeclaringType.IsGeneratable)
 				Comments.Add ($"// Metadata.xml XPath method reference: path=\"{method.GetMetadataXPathReference (method.DeclaringType)}\"");
 
-			if (method.Deprecated.HasValue ())
-				Attributes.Add (new ObsoleteAttr (method.Deprecated.Replace ("\"", "\"\"")));
+			SourceWriterExtensions.AddObsolete (Attributes, method.Deprecated);
 
 			if (method.IsReturnEnumified)
 				Attributes.Add (new GeneratedEnumAttr (true));
diff --git a/tools/generator/SourceWriters/BoundMethodAbstractDeclaration.cs b/tools/generator/SourceWriters/BoundMethodAbstractDeclaration.cs
@@ -53,8 +53,7 @@ public BoundMethodAbstractDeclaration (GenBase gen, Method method, CodeGeneratio
 			if (method.DeclaringType.IsGeneratable)
 				Comments.Add ($"// Metadata.xml XPath method reference: path=\"{method.GetMetadataXPathReference (method.DeclaringType)}\"");
 
-			if (method.Deprecated.HasValue ())
-				Attributes.Add (new ObsoleteAttr (method.Deprecated.Replace ("\"", "\"\"")));
+			SourceWriterExtensions.AddObsolete (Attributes, method.Deprecated);
 
 			SourceWriterExtensions.AddSupportedOSPlatform (Attributes, method, opt);
 
diff --git a/tools/generator/SourceWriters/BoundMethodExtensionStringOverload.cs b/tools/generator/SourceWriters/BoundMethodExtensionStringOverload.cs
@@ -26,8 +26,7 @@ public BoundMethodExtensionStringOverload (Method method, CodeGenerationOptions
 			SetVisibility (method.Visibility);
 			ReturnType = new TypeReferenceWriter (opt.GetTypeReferenceName (method.RetVal).Replace ("Java.Lang.ICharSequence", "string").Replace ("global::string", "string"));
 
-			if (method.Deprecated != null)
-				Attributes.Add (new ObsoleteAttr (method.Deprecated.Replace ("\"", "\"\"").Trim ()));
+			SourceWriterExtensions.AddObsolete (Attributes, method.Deprecated);
 
 			SourceWriterExtensions.AddSupportedOSPlatform (Attributes, method, opt);
 
diff --git a/tools/generator/SourceWriters/BoundMethodStringOverload.cs b/tools/generator/SourceWriters/BoundMethodStringOverload.cs
@@ -24,8 +24,7 @@ public BoundMethodStringOverload (Method method, CodeGenerationOptions opt)
 			SetVisibility (method.Visibility);
 			ReturnType = new TypeReferenceWriter (opt.GetTypeReferenceName (method.RetVal).Replace ("Java.Lang.ICharSequence", "string").Replace ("global::string", "string"));
 
-			if (method.Deprecated != null)
-				Attributes.Add (new ObsoleteAttr (method.Deprecated.Replace ("\"", "\"\"").Trim ()));
+			SourceWriterExtensions.AddObsolete (Attributes, method.Deprecated);
 
 			SourceWriterExtensions.AddSupportedOSPlatform (Attributes, method, opt);
 
diff --git a/tools/generator/SourceWriters/BoundProperty.cs b/tools/generator/SourceWriters/BoundProperty.cs
@@ -73,8 +73,10 @@ public BoundProperty (GenBase gen, Property property, CodeGenerationOptions opt,
 			}
 
 			// Unlike [Register], [Obsolete] cannot be put on property accessors, so we can apply them only under limited condition...
-			if (property.Getter.Deprecated != null && (property.Setter == null || property.Setter.Deprecated != null))
-				Attributes.Add (new ObsoleteAttr (property.Getter.Deprecated.Replace ("\"", "\"\"").Trim () + (property.Setter != null && property.Setter.Deprecated != property.Getter.Deprecated ? " " + property.Setter.Deprecated.Replace ("\"", "\"\"").Trim () : null)));
+			if (property.Getter.Deprecated != null && (property.Setter == null || property.Setter.Deprecated != null)) {
+				var message = property.Getter.Deprecated.Trim () + (property.Setter != null && property.Setter.Deprecated != property.Getter.Deprecated ? " " + property.Setter.Deprecated.Trim () : null);
+				SourceWriterExtensions.AddObsolete (Attributes, message);
+			}
 
 			SourceWriterExtensions.AddSupportedOSPlatform (Attributes, property.Getter, opt);
 
diff --git a/tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs b/tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs
@@ -301,6 +301,20 @@ 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)
+		{
+			// 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
+			});
+		}
+
 		public static void WriteMethodInvokerBody (CodeWriter writer, Method method, CodeGenerationOptions opt, string contextThis)
 		{
 			writer.WriteLine ($"if ({method.EscapedIdName} == IntPtr.Zero)");
diff --git a/tools/generator/SourceWriters/MethodCallback.cs b/tools/generator/SourceWriters/MethodCallback.cs
@@ -43,8 +43,7 @@ public MethodCallback (GenBase type, Method method, CodeGenerationOptions option
 			IsStatic = true;
 			IsPrivate = method.IsInterfaceDefaultMethod;
 
-			if (!string.IsNullOrWhiteSpace (method.Deprecated))
-				Attributes.Add (new ObsoleteAttr ());
+			SourceWriterExtensions.AddObsolete (Attributes, null, forceDeprecate: !string.IsNullOrWhiteSpace (method.Deprecated));
 
 			SourceWriterExtensions.AddSupportedOSPlatform (Attributes, method, opt);
 
@@ -136,8 +135,7 @@ public GetDelegateHandlerMethod (Method method, CodeGenerationOptions opt)
 			IsStatic = true;
 			IsPrivate = method.IsInterfaceDefaultMethod;
 
-			if (!string.IsNullOrWhiteSpace (method.Deprecated))
-				Attributes.Add (new ObsoleteAttr ());
+			SourceWriterExtensions.AddObsolete (Attributes, null, forceDeprecate: !string.IsNullOrWhiteSpace (method.Deprecated));
 
 			SourceWriterExtensions.AddSupportedOSPlatform (Attributes, method, opt);
 		}

Original file line number	Diff line number	Diff line change
`@@ -33,8 +33,7 @@ public BoundConstructor (ClassGen klass, Ctor constructor, bool useBase, CodeGen`
`33`	`33`	`Attributes.Add (new RegisterAttr (".ctor", constructor.JniSignature, string.Empty, additionalProperties: constructor.AdditionalAttributeString ()));`
`34`	`34`	`}`
`35`	`35`
`36`		`- if (constructor.Deprecated != null)`
`37`		`- Attributes.Add (new ObsoleteAttr (constructor.Deprecated.Replace ("\"", "\"\"")));`
	`36`	`+ SourceWriterExtensions.AddObsolete (Attributes, constructor.Deprecated);`
`38`	`37`
`39`	`38`	`if (constructor.CustomAttributes != null)`
`40`	`39`	`Attributes.Add (new CustomAttr (constructor.CustomAttributes));`
Original file line number	Diff line number	Diff line change
`@@ -47,8 +47,7 @@ public BoundFieldAsProperty (GenBase type, Field field, CodeGenerationOptions op`
`47`	`47`	`Attributes.Add (new RegisterAttr (field.JavaName, additionalProperties: field.AdditionalAttributeString ()));`
`48`	`48`	`}`
`49`	`49`
`50`		`- if (field.IsDeprecated)`
`51`		`- Attributes.Add (new ObsoleteAttr (field.DeprecatedComment, field.IsDeprecatedError) { NoAtSign = true });`
	`50`	`+ SourceWriterExtensions.AddObsolete (Attributes, field.DeprecatedComment, field.IsDeprecated, noAtSign: true, isError: field.IsDeprecatedError);`
`52`	`51`
`53`	`52`	`SetVisibility (field.Visibility);`
`54`	`53`	`UseExplicitPrivateKeyword = true;`