beakona / AutoInterface

C# interface-to-member source generator
MIT License
73 stars 9 forks source link

IsMemberImplemented check only targets explicit method implementation #9

Closed Dblm0 closed 2 years ago

Dblm0 commented 2 years ago

Publicly implemented interface member is being ignored because of targeting MethodKind.ExplicitInterfaceImplementation

Also, what was the original intention for interfaces being implemented only explicitly?

beakona commented 2 years ago

There is more than one view/expectation how "AutoInterface" mechanism should work. In other words, real question is what AutoInterface should solve. It is not hard to add few extra options and change default behavior but one need to be aware of them.

For me this project is exploration in the domain of language design. I view AutoInterface as mechanism (i.e. missing language feature) that introduces a form of composition-over-inheritance in halfway between composition and inheritance. I wanted hands-on project to try it whether is it usable as such or it introduces too much trouble.

Here is figurative/synthetic example that led me to the current solution:

interface IScoreAccount
{
   void Update();
}

interface IPlayerDrawable
{
   void Update();
}

class OurClass
{
   [Marker]
   IScoreAccount aspect1 = obj1;

   [Marker]
   IPlayerDrawable aspect2 = obj2;
}

OurClass is composed of two aspects and it also behaves as IScoreAccount and IPlayerDrawable... just as following:

class OurClass
{
   public IScoreAccount Aspect1 {get;}
   public IPlayerDrawable Aspect2 {get;}
}

but users do not have to manually dereference myclass.Aspect1.Update()

Here comes hard part..

Interfaces can have members with a same name.. In our example Update() should not be the same because we compose different aspects, but in some other context we can have property like int Count {get;} where interface members and public interface member should be the same.

  1. In the case of method Update(): public method Update() may exist and it does not have to relate to method from either interface. Customization is not possible without explicit interface members. Interference between public members and interfaces is unwanted.
  2. In the case of property int Count {get;}: thinking about explicit-interface is too much to ask

Choosing one somehow excludes the other. Actually, I was aware of that problem but decided to make more general solution and see how it unfolds.

As I understood, you favour case 2 to be default and if one needs to customize then he/she needs to opt-in

For example:

interface ITest
{
   int Count {get;}
}
class SomeCollection : ITest
{
   [Marker]
   private ITest aspect1 = obj;

   //public int Count => 1; uncomment this line to prevent autogeneration if public interface is preferred
   //int ITest.Count => this.Count; uncomment this line to prevent autogeneration if explicit interface is preferred
}

class OurClass
{
   //here we opt-in for more general case where public member
   //is being ignored and explicit interface member is the only option to customize behavior
   //default is opted-out
   [Marker(PleaseForwardEverything)] 
   IScoreAccount aspect1 = obj1;

   [Marker(PleaseForwardEverything)]
   IPlayerDrawable aspect2 = obj2;
}

What do you think, do this align with your expectations? Can you explain your use case?

Dblm0 commented 2 years ago

Now I see how general explicit implementation is. The simple thing is I haven't met or considered the situations of composing more than one aspect. So being comfortable implementing interfaces publicly, I wasn't aware of no relation between these cases.

My problem with explicit implementation could be easily solved on my side, as I faced some problems with reflection.

Given the wide hierarchy of interfaces i need to provide additional behavior which could be related to some interface method calls. So I came out with simple wrapper class, where some members are decorated and others are just forwarded. As interface hierarchy is complex, I had to forward 15+ members or something. I expect more of these wrappers and I thought, it would be convenient to have a language feature like delegate pattern in Kotlin. With pretty short research of existing source generators, I ended up here, expecting the same simple thing.

As for your example, I can't imagine right now, how to combine public and explicit implementations without directly marking them in the attribute declaration. Maybe, it would be better to stay on explicit solution, as it's general one.

beakona commented 2 years ago

Now I see that name AutoInterface is not well 'selected'. Here we have code reusability through:

  1. composition - wants aspects to be untangled
  2. delegation - everything is in public interface

I've changed Kotlin sample from your link to get colliding names as follows:

interface Base1 {
    val message: String
    fun print()
}

interface Base2 {
    val message: String
    fun print()
}

class Base1Impl(val x: Int) : Base1 {
    override val message = "Base1Impl: x = $x"
    override fun print() { println(message) }
}

class Base2Impl(val x: Int) : Base2 {
    override val message = "Base2Impl: x = $x"
    override fun print() { println(message) }
}

class Derived(b1: Base1, b2: Base2) : Base1 by b1, Base2 by b2 {
    override val message = "Message of Derived"
}

fun main() {
    val b1 = Base1Impl(10)
    val b2 = Base2Impl(10)
    val derived = Derived(b1, b2)
    derived.print()
    println(derived.message)
}

and it produced following error:

Class 'Derived' must override public open fun print(): Unit defined in Derived because it inherits many implementations of it

In other words, if things collide on public side you should think about it..

AutoInterface is equipped with templating by which you could hack things per class-member but in my opinion that should be used only in case of extreme desperation.

My proposal to cover everything we talked so far (and to communicate properly to users) would be to abandon AutoInterfaceAttribute and introduce new attribute for each use-case:

Names of both attributes are problematic; here are some variations: PublishInterfaceAttribute, AutoComposeAttribute, ForwardCallsToAttribute AutoDelegateAttribute, AutoImplementAttribute, DelegatedInterfaceAttribute

Also, can we make some useful interaction between AutoInterface source generation and user's reflection (as I understood, you call some members through reflection)?

felixhirt commented 2 years ago

This is all very interesting... I think it would be worthwhile to have separate attributes (or an enum option to pass into the attribute-ctor) to determine between public class members, protected class members and explicit interface implementations for the delegation calls

Dblm0 commented 2 years ago

Sorry for being silent for a long time. I've found a different way to solve my exact problem, using AOT. I think this source generator should remain as it is.