Skip to content

Commit

Permalink
[generator] Refactor logic for applying [Obsolete] attributes (#1024)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jpobst authored Aug 11, 2022
1 parent 9b1d3ab commit 440c05e
Show file tree
Hide file tree
Showing 13 changed files with 34 additions and 26 deletions.
3 changes: 1 addition & 2 deletions tools/generator/SourceWriters/BoundClass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
3 changes: 1 addition & 2 deletions tools/generator/SourceWriters/BoundConstructor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
5 changes: 3 additions & 2 deletions tools/generator/SourceWriters/BoundField.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
3 changes: 1 addition & 2 deletions tools/generator/SourceWriters/BoundFieldAsProperty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions tools/generator/SourceWriters/BoundInterface.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions tools/generator/SourceWriters/BoundMethod.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
3 changes: 1 addition & 2 deletions tools/generator/SourceWriters/BoundMethodStringOverload.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
6 changes: 4 additions & 2 deletions tools/generator/SourceWriters/BoundProperty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
14 changes: 14 additions & 0 deletions tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)");
Expand Down
6 changes: 2 additions & 4 deletions tools/generator/SourceWriters/MethodCallback.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit 440c05e

Please sign in to comment.