From 440c05eef9044543e11d90075b54427c78287cf9 Mon Sep 17 00:00:00 2001 From: Jonathan Pobst Date: Thu, 11 Aug 2022 15:20:07 -0500 Subject: [PATCH] [generator] Refactor logic for applying [Obsolete] attributes (#1024) Context: https://github.com/xamarin/xamarin-android/issues/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 6bbb00aa. [0]: https://github.com/dotnet/runtime/issues/72970 --- tools/generator/SourceWriters/BoundClass.cs | 3 +-- tools/generator/SourceWriters/BoundConstructor.cs | 3 +-- tools/generator/SourceWriters/BoundField.cs | 5 +++-- .../SourceWriters/BoundFieldAsProperty.cs | 3 +-- tools/generator/SourceWriters/BoundInterface.cs | 3 +-- .../BoundInterfaceMethodDeclaration.cs | 5 +++-- tools/generator/SourceWriters/BoundMethod.cs | 3 +-- .../BoundMethodAbstractDeclaration.cs | 3 +-- .../BoundMethodExtensionStringOverload.cs | 3 +-- .../SourceWriters/BoundMethodStringOverload.cs | 3 +-- tools/generator/SourceWriters/BoundProperty.cs | 6 ++++-- .../Extensions/SourceWriterExtensions.cs | 14 ++++++++++++++ tools/generator/SourceWriters/MethodCallback.cs | 6 ++---- 13 files changed, 34 insertions(+), 26 deletions(-) diff --git a/tools/generator/SourceWriters/BoundClass.cs b/tools/generator/SourceWriters/BoundClass.cs index af63a36cc..2f2b6d24b 100644 --- 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 index 00ddc1884..ed7ad3ca4 100644 --- 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 index b5ef2b315..f97b68637 100644 --- 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 index 391dd3c9b..c7af39422 100644 --- 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 index 8081cc185..c582c257a 100644 --- 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 index 5e529e7e3..8d452ac34 100644 --- 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 index a6ddf53b9..806efe655 100644 --- 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 index 67191ac09..bbae48799 100644 --- 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 index 284bdb3df..dfae18351 100644 --- 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 index 463010012..80e47b452 100644 --- 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 index 7804d9bc1..41209eb2b 100644 --- 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 index 39241081b..abb4ac8c9 100644 --- a/tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs +++ b/tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs @@ -301,6 +301,20 @@ public static void AddSupportedOSPlatform (List attributes, Api } + public static void AddObsolete (List 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 index 7563369bb..c1107d3e5 100644 --- 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); }