beakona / AutoInterface

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

An arbitrary aspect type #13

Closed StendProg closed 2 years ago

StendProg commented 2 years ago

Is it possible to specify an arbitrary aspect type instead of a specific interface? If any class does not explicitly implement the interface, but has all the methods of the interface, then the generator must produce correct code. It's easy to manually change the aspect type from interface to the required class, but the generator stops working and removes the generated code.

interface IPrintable
{
   void Print();
}

public partial class Person : IPrintable
{
   // Aspect type is PrinterV1 instead of IPrintalbe
   [BeaKona.AutoInterface]
   private readonly PrinterV1 aspect1;
}

// not implement IPrintable, but have Print() method
pablic class PrinterV1 
{
  public void Print();
}

I'm trying to achieve goals similar to #4. If you extract the interface manually using the IDE tools, then it seems to work, but the generator feature described above leaves it.

You have a very mature project, I have tried dozens of generators and they all produce errors if you deviate a little from the standard examples. Your generator passed the test, thanks for your work.

beakona commented 2 years ago

Wow, you described feature that would allow us to use some type (PrinterV1) as if it implements given interface even it does not (for whatever reason). This looks like some kind of elevated ad-hoc adapter pattern.

It is easy to implement; but in my opinion with one (minor) change to your example; interface type should be explicitly specified for field/property using attribute because in general case it cannot be safely inferred. Here is an example where aspect1 do not have relation with IDebuggable.

interface IPrintable
{
   void Print();
}

interface IDebuggable
{
   void Print();
}

public partial class Person : IDebuggable
{
   // Here we explicitly specify what interface we want to 'bind'
   [BeaKona.AutoInterface(typeof(IPrintable))]
   private readonly PrinterV1 aspect1;
}

// not implement IPrintable, but have Print() method
public class PrinterV1 
{
  public void Print();
}

//what should be generated
partial class Person : IPrintable
{
   void IPrintable.Print() => this.aspect1.Print();
}
StendProg commented 2 years ago

Thanks for the quick response. Yes, specifying the interface type in attribute is definitely a good addition. I thought about something like that, but I had no idea where it would be better to indicate it. At a minimum, this is an extra confirmation from the user that he understands what he is doing. And in case of non-compiled generated code, it should blame itself and not the generator.

beakona commented 2 years ago

I've created version v1.0.23 that should support an arbitrary interface, but I do not have time to test it thoroughly.

StendProg commented 2 years ago

Warning CS8785 Generator 'AutoInterfaceSourceGenerator' failed to create a source. This will not affect the output and compilation errors that may result. Exception type: "MissingMethodException", message: "Method not found: "Boolean Microsoft.CodeAnalysis.IParameterSymbol.get_IsNullChecked()".

Any idea how to fix that?

beakona commented 2 years ago

I've commented out offending check. This property is related to double bang (!!) operator which is introduced and then removed. I assumed that IsNullChecked would remain inside ISymbol even if it is not used later on. Interestingly, my compiler uses Microsoft.CodeAnalysis.dll v4.1.0 which does have that property.

StendProg commented 2 years ago

It looks like the problem was really on my side. I had .net 6 preview 3 installed. Anyway, your fresh fix now works for it too.

I ran into a new problem. Generics do not pass type method check against interface methods.

Error BK-AG04 Unable to cast from type 'ScottPlot.Plot' to 'ScottPlot.IPlot' or type 'ScottPlot.Plot' does not expose all members required by interface 'ScottPlot.IPlot'. This error appear on methods like this:

// typical
ScatterPlotList<T> AddScatterList<T>(Color? color = null, float lineWidth = 1, float markerSize = 5, string label = null, MarkerShape markerShape = MarkerShape.filledCircle, LineStyle lineStyle = LineStyle.Solid);

// hardest method that i can find
SignalPlotXYConst<TX, TY> PlotSignalXYConst<TX, TY>(TX[] xs, TY[] ys, Color? color = null, double lineWidth = 1, double markerSize = 5, string label = null, int? minRenderIndex = null, int? maxRenderIndex = null, LineStyle lineStyle = LineStyle.Solid, bool useParallel = true)
            where TX : struct, IComparable
            where TY : struct, IComparable;

With all this, the old generator based on Interface to Interface works fine on these methods. Looks like it's a new check problem.

  1. The simplest solution would be to add a simple flag that disables this check somewhere in the attribute:

    [BeaKona.AutoInterface(typeof(IPlot), false)]
  2. Handle generic methods correctly. In the second example, I tried to give the most difficult method, in my opinion, of those with which I have to deal.

I don't understand anything about generators and can't appreciate the complexity of the second solution. Any of the options will suit me.

beakona commented 2 years ago

Roslyn has several equality checks but none of them fit our needs (they all check parent/defining type). We need comparison by 'signature' so I wrote custom equality comparer which traverses through ISymbols and, unfortunately, made several mistakes. You encountered bug in generic parameter comparison where generic type T and other type T have same name, index and constraints but parent/defining types was not the same.

Here is the interface I used for testing:

public interface IArbitrary<T>
{
    T Length { get; }
    int? Method(int a, out int b, ref int c, dynamic d, params int[] p);

    SignalPlotXYConst<TXi, TYi?, T>? Method2<TXi, TYi>(TXi x, TYi[] ys, in int s)
       where TXi : struct, IComparable
       where TYi : struct, IComparable, ISome, ISome2;
}
StendProg commented 2 years ago

It works fine, but there is another problem.

On methods with rectangular arrays as parameters generator throws NullReferenceException.

// any of this methods give error
Heatmap AddHeatmap(double?[,] intensities, Colormap colormap = null, bool? lockScales = true);
Heatmap AddHeatmap(double[,] intensities, Colormap colormap = null, bool? lockScales = null);

I commented all methods with rectangular parameters and it looks like it's the last problem all other work fine.

beakona commented 2 years ago

I've fixed it.. in CLR arrays have lower bound and it's value is not zero but instead array holding LowerBounds is uninitialized..

StendProg commented 2 years ago

Thanks for your work. I couldn't find any additional errors on my test interface (200+ lines).