[generator] Add managedOverride values none and reabstract.

[jpobst]

Fixes #981

Add support for the managedOverride = none value:

<attr path="//method[@name='example']" name="managedOverride">none</attr>

This value ensures that the member is not marked as virtual or override, useful for sealed classes.

public sealed class MyClass {
    public void DoThing () => ...
}

---

Additionally add support for the managedOverride = reabstract value:

<attr path="//method[@name='example']" name="managedOverride">reabstract</attr>

This only affects interfaces, and allows an interface member to be "reabstracted":

public partial interface IAnnotatedType {
    // Contains default interface method
    virtual IAnnotatedType? AnnotatedOwnerType => null;
}
public partial interface IAnnotatedArrayType : IAnnotatedType {
    // Contains *no* method body; re-abstracted
    abstract IAnnotatedType? IAnnotatedType.AnnotatedOwnerType {get;}
}

[jonpryor]

This change may be incomplete. Use-case: reabstract interface members:

diff --git a/src/Java.Base/Java.Base.csproj b/src/Java.Base/Java.Base.csproj
index b05f70d6..7980c93f 100644
--- a/src/Java.Base/Java.Base.csproj
+++ b/src/Java.Base/Java.Base.csproj
@@ -4,8 +4,7 @@
     <TargetFrameworks>$(DotNetTargetFramework)</TargetFrameworks>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <Nullable>enable</Nullable>
-    <!-- TODO: CS0108 is due to e.g. interfaces re-abstracting default interface methods -->
-    <NoWarn>$(NoWarn);8764;CS0108</NoWarn>
+    <NoWarn>$(NoWarn);8764</NoWarn>
     <Version>$(JICoreLibVersion)</Version>
   </PropertyGroup>
 
diff --git a/src/Java.Base/Transforms/Metadata.xml b/src/Java.Base/Transforms/Metadata.xml
index 21828b34..0dffb755 100644
--- a/src/Java.Base/Transforms/Metadata.xml
+++ b/src/Java.Base/Transforms/Metadata.xml
@@ -12,6 +12,15 @@
   <!-- warning CS0672: Member 'Enum.JavaFinalize()' overrides obsolete member 'Object.JavaFinalize()'. Add the Obsolete attribute to 'Enum.JavaFinalize()'. -->
   <attr path="/api/package[@name='java.lang']/class[@name='Enum']/method[@name='finalize' and count(parameter)=0]" name="deprecated">deprecated</attr>
 
+  <!-- warning CS0108: 'MEMBER' hides inherited member 'MEMBER'. Use the new keyword if hiding was intended. -->
+  <attr path="/api/package[@name='java.lang.reflect']/interface[
+        @name='AnnotatedParameterizedType'
+        or @name='AnnotatedArrayType'
+        or @name='AnnotatedTypeVariable'
+        or @name='AnnotatedWildcardType'
+      ]/method[@name='getAnnotatedOwnerType' and count(parameter)=0]"
+      name="managedOverride">reabstract</attr>
+
   <!-- AbstractStringBuilder is package-private; fixity fix -->
   <remove-node path="//api/package[@name='java.lang']/class[@name='AbstractStringBuilder']" />
 

After building, we still get CS0108 warnings:

obj/Debug-net7.0/mcw/Java.Lang.Reflect.IAnnotatedArrayType.cs(16,54): warning CS0108: 'IAnnotatedArrayType.AnnotatedOwnerType' hides inherited member 'IAnnotatedType.AnnotatedOwnerType'. Use the new keyword if hiding was intended.

"Offending" code:

	public partial interface IAnnotatedArrayType : global::Java.Lang.Reflect.IAnnotatedType {
		// Currently generated
		abstract global::Java.Lang.Reflect.IAnnotatedType? AnnotatedOwnerType {
			// Metadata.xml XPath method reference: path="/api/package[@name='java.lang.reflect']/interface[@name='AnnotatedArrayType']/method[@name='getAnnotatedOwnerType' and count(parameter)=0]"
			[global::Java.Interop.JniMethodSignature ("getAnnotatedOwnerType", "()Ljava/lang/reflect/AnnotatedType;")]
			get; 
		}
	}

We have the abstract introduced (yay), but the problem is that the property name needs to be explicitly specified; line 10 is AnnotatedOwnerType, but it needs to be IAnnotatedType.AnnotatedOwnerType to fix the CS0108:

	public partial interface IAnnotatedArrayType : global::Java.Lang.Reflect.IAnnotatedType {
		// Needs to generated
		abstract global::Java.Lang.Reflect.IAnnotatedType? IAnnotatedType.AnnotatedOwnerType {
			// Metadata.xml XPath method reference: path="/api/package[@name='java.lang.reflect']/interface[@name='AnnotatedArrayType']/method[@name='getAnnotatedOwnerType' and count(parameter)=0]"
			[global::Java.Interop.JniMethodSignature ("getAnnotatedOwnerType", "()Ljava/lang/reflect/AnnotatedType;")]
			get; 
		}
	}

I can "make the needed output happen" by adding an additional <attr/> to set propertyName:

  <attr path="/api/package[@name='java.lang.reflect']/interface[
        @name='AnnotatedParameterizedType'
        or @name='AnnotatedArrayType'
        or @name='AnnotatedTypeVariable'
        or @name='AnnotatedWildcardType'
      ]/method[@name='getAnnotatedOwnerType' and count(parameter)=0]"
      name="propertyName">IAnnotatedType.AnnotatedOwnerType</attr>

The question is whether we want to require two steps for this scenario -- set managedOverride and set propertyName -- or whether managedOverride=reabstract should implicitly make the member name explicitly qualified.

An added complication is that while I can workaround properties as above, I can't figure out how to work around method names:

obj/Debug-net7.0/mcw/Java.IO.ICloseable.cs(12,17): warning CS0108: 'ICloseable.Close()' hides inherited member 'IAutoCloseable.Close()'. Use the new keyword if hiding was intended.

Thus, I want to emit abstract void ICloseable.Close();. First, emit abstract:

  <attr path="/api/package[@name='java.io']/interface[
        @name='Closeable'
      ]/method[@name='close' and count(parameter)=0]"
      name="managedOverride">reabstract</attr>

but then I need an explicitly qualifiedICloseable.Close. If I try setting managedName:

  <attr path="/api/package[@name='java.io']/interface[
        @name='Closeable'
      ]/method[@name='close' and count(parameter)=0]"
      name="managedName">IAutoCloseable.Close</attr>

Then the . is replaced with _, resulting in the undesirable:

	public partial interface ICloseable : global::Java.Lang.IAutoCloseable {
		abstract void IAutoCloseable_Close ();
	}

Given that I can't fixup method names in this manner, I think that when managedOverride is reabstract, then interface member names should be automatically explicitly qualified. This would allow both methods & properties to be fixed.

[jpobst]

I think that when managedOverride is reabstract, then interface member names should be automatically explicitly qualified.

I'm not sure this is something we can do in an automated manner. I don't think we currently attempt to track what members implement which interface contracts. Also, I'm not sure we can do that.

For example, does this reabstract I1.DoThing () or I2.DoThing ()?

interface I1
{
    void DoThing () { }
}

interface I2
{
    void DoThing () { }
}

interface I3 : I1, I2
{
    abstract void DoThing ();
}

I'm also not sure using managedName is a good solution. In fact I was surprised it worked at all. It can still be mangled (as you saw), and we generate other pieces from it like connector names.

Perhaps we need an additional @explicitInterface that can be used to create an explicit name:

<attr path="/api/package[@name='java.io']/interface[@name='Closeable']/method[@name='close']" name="explicitInterface">
    IAutoCloseable
</attr>

[jonpryor]

@jpobst wrote:

In fact I was surprised it worked at all.

It might not have worked, actually. Remember that for my use case of Java.Base bindings, "marshal methods" are not emitted (bc5bcf4):

~~ TODO: Marshal Methods ~~
Marshal methods are currently skipped. Java-to-managed invocations
are not currently supported.

Commit 4787e01 implemented marshal methods for Java.Base by using the Java.Interop.Export infrastructure; generator still doesn't emit marshal methods for Java.Base.

Thus, Java.Base wouldn't currently hit a scenario where "managedName mangling" would be a problem, other than basic C# compilation.

Perhaps we need an additional @explicitInterface that can be used to create an explicit name:

That sounds like a good idea to me.

[jonpryor]

Given that we will need separate //interface/*/@explicitInterface metadata, I should return to the original question: does this PR need any additional changes?

I think the answer is No.