beakona / AutoInterface

C# interface-to-member source generator
MIT License
76 stars 10 forks source link

could you please add templating? #1

Closed ignatandrei closed 3 years ago

ignatandrei commented 3 years ago

Your pattern is great for a decorator pattern. Could you add templating ( Scriban ? ) to customize how the code is generated ?

beakona commented 3 years ago

Could you give an example where to put template text for each member? Following approach does not seem right because there are two methods Print1 and Print2. Polluting interface also does not look like right approach??

public interface IPrintable
{
    void Print1();
    void Print2();
}

public partial class Person : IPrintable
{
    [BeaKona.AutoInterface(Body = "bla bla", BodyLanguage = "scriban")]
    private readonly IPrintable aspect1 = new PersonPrinterV1();
}
ignatandrei commented 3 years ago

A template will look like this

{ for method in ,,,} public {{interfaceName}} . {{method.Name}} ({{ method.Arguments }} ) { return this.{{method.Name}} ({{ method.Arguments }} ) } {end for}

beakona commented 3 years ago

Thanks! I was closed-minded and also had an error in logic. I could not see meaningfulness because body would be full of 'if-s' for each case defying it's purpose.

There are four kind of templates; for methods, properties, indexers and events.

What if we add support for multiple Attribute? First Attribute can customize some set of methods, other can customize all properties etc.. All other cases will use fallback C# source text generation (current implementation). Here is example:

public partial class Person : ITest
{
   [BeaKona.AutoInterface]
   [BeaKona.AutoInterfaceTemplate(Target=Targets.Method, Filter="MyMethod\d+" Language="scriban" Code=" body1 ")]
   [BeaKona.AutoInterfaceTemplate(Target=Targets.Property | Targets.Indexer, Language="scriban" Code=" body2 ")]
   private readonly ITest aspect1 = new Test();

   //[BeaKona.AutoInterface]
   //[BeaKona.AutoInterfaceTemplate(Target=Targets.Method, Code=" body3 ")]
   //private readonly ITest aspect2 = new Test();
}

Corner cases that I am aware of:

  1. Specializing AutoInterfaceTemplates that overlap will create compile time error.
  2. All AutoInterfaceTemplates will be grouped by same target interface. In the example above if you uncomment those three lines there will be total of three AutoInterfaceTemplate entries for target-interface ITest.

Here is 'templated' version of current/fallback generator for methods:

{{~for method in interface.methods~}}
{{method.IsAsync?"async":""}} {{method.ReturnType}} {{interface.Name}}.{{method.Name}}({{method.Arguments}})
{
   {{~if method.IsAsync~}}
      {{~for element in elements~}}
         var temp{{for.index}} = (({{interface.Name}})this.{element.Name}).{{method.Name}}({{method.Arguments}});
      {{~endfor~}}
      {{~for element in elements~}}
         {{for.last&&method.ReturnExpected?"return":""}} await temp{{for.index}};
      {{~endfor~}}
   {{~else~}}
      {{~for element in elements~}}
         {{for.last&&method.ReturnExpected?"return":""}} (({{interface.Name}})this.{element.Name}).{{method.Name}}({{method.Arguments}});
      {{~endfor~}}
   {{~end~}}
}
{{~endfor~}}

What do you think?

Also, what about security? Someone can create public project using this source generator and write ?malicious? template. Template will be executed during compile time (interpreted string).

ignatandrei commented 3 years ago

I think it will be great to have just one template instead of 4

beakona commented 3 years ago

Unfortunately, there are multiple topics in this thread. As I understood it is desirable to have one monolith template that specify all four sections: methods, properties, indexers and events?

If you 'miss' one section then code for that section will not be generated. For example, this interface may require code generation for methods and events:

interface ITest
{
   void MyMethod();
   event EventHandler Receive;
}

Notice how code for events are different than one for methods. I was proposing solution for gaining configurability yet not concerning for noisy parts. Here is full version of scriban template that should be pasted to every template in order to get default implementation: (it was never tried; treat it as pseudo example)

{{~for method in interface.methods~}}
{{method.IsAsync?"async":""}} {{method.ReturnType}} {{interface.Name}}.{{method.Name}}({{method.Arguments}})
{
   {{~if method.IsAsync~}}
      {{~for element in elements~}}
         var temp{{for.index}} = (({{interface.Name}})this.{{element.Name}}).{{method.Name}}({{method.Arguments}});
      {{~endfor~}}
      {{~for element in elements~}}
         {{for.last&&method.ReturnExpected?"return":""}} await temp{{for.index}};
      {{~endfor~}}
   {{~else~}}
      {{~for element in elements~}}
         {{for.last&&method.ReturnExpected?"return":""}} (({{interface.Name}})this.{{element.Name}}).{{method.Name}}({{method.Arguments}});
      {{~endfor~}}
   {{~end~}}
}
{{~endfor~}}

{{~for property in interface.properties~}}
{{property.Type}} {{interface.Name}}.{{property.Name}}
{
   get
   {
      return (({{interface.Name}})this.{{elements[0].Name}}).{{property.Name}};
   }
   set
   {
      {{~for element in elements~}}
         (({{interface.Name}})this.{{element.Name}}).{{property.Name}} = value;
      {{~endfor~}}
   }
}
{{~endfor~}}

{{~for indexer in interface.indexers~}}
{{indexer.Type}} {{interface.Name}} this[{{indexer.Arguments}}]
{
   get
   {
      return (({{interface.Name}})this.{{elements[0].Name}})[{{method.Arguments}}];
   }
   set
   {
      {{~for element in elements~}}
         (({{interface.Name}})this.{{element.Name}})[{{method.Arguments}}] = value;
      {{~endfor~}}
   }
}
{{~endfor~}}

{{~for event in interface.events~}}
{{event.Type}} {{interface.Name}}.{{event.Name}}
{
   add
   {
      {{~for element in elements~}}
          (({{interface.Name}})this.{{element.Name}}).{{event.Name}} += value;
      {{~endfor~}}
   }
   remove
   {
      {{~for element in elements~}}
          (({{interface.Name}})this.{{element.Name}}).{{event.Name}} -= value;
      {{~endfor~}}
   }
}
{{~endfor~}}
ignatandrei commented 3 years ago

it is ok. Just have this template as a starting point and each developer can modify his. It is better rather than have 4 differents templates to look at.

ignatandrei commented 3 years ago

So, when this will be ready ? ;-)

beakona commented 3 years ago

I've made first version. See if it meets your expectations/requirements..

ignatandrei commented 3 years ago

trying

ignatandrei commented 3 years ago

Did you punlish on Nuget ? ;-)

beakona commented 3 years ago

I was hoping you will download github repository directly..

beakona commented 3 years ago

Now I made nuget push but not sure whether nuget path internals are valid. I am writing this because source generator have to have reference to foreign libraries (scriban) that will be loaded/dependent at runtime (your compiletime), but not at execution (your runtime). I hope I set paths right...

beakona commented 3 years ago

1.0.9-preview is now listed as nuget preview package

ignatandrei commented 3 years ago

It works . Thanks!

ignatandrei commented 3 years ago

https://github.com/ignatandrei/RSCG_Examples/tree/main/DP_Decorator

I will also blog about it. Thanks ! ( and maybe put a 1.0.10? ;-) )

beakona commented 3 years ago

Ok, in next few minutes I will publish 1.0.10.. Thanks!

beakona commented 3 years ago

I've visited your blog. It's great and useful! I have also looked at your DP_Decorator solution. Here is outline of version that I was proposing:

public partial class MilkDecorator : ICoffee
{
   [BeaKona.AutoInterface]
   [BeaKona.AutoInterfaceTemplate(Target=Targets.Property, Filter="Description", Language="scriban", Code=DescriptionTemplate)]
   [BeaKona.AutoInterfaceTemplate(Target=Targets.Property, Filter="Price", Language="scriban", Code=PriceTemplate)]
   private readonly ICoffee coffee;

   private const string DescriptionTemplate = @"
var name = this.GetType().Name.Replace(""Decorator"","""");
return {{expression}} + "" with "" + name;
";

   private const string PriceTemplate = @"return {{expression}} + this.DecoratorPrice;";

   public int DecoratorPrice { get; set; } = 1;
   public MilkDecorator(ICoffee coffee)
   {
       this.coffee = coffee;
   }
}
ignatandrei commented 3 years ago

Yes, it is much simpler . I will put as a alternate solution ( and thanks for the kind words)

beakona commented 3 years ago
  1. I did not implemented it (yet).
  2. I find your approach somehow more systematic because my approach is partial and it's easier to miss something.
  3. Currently in our both cases strings are 'baked' into assembly and visible in (user's) runtime. Here is solution to that: AutoInterface should be able to import template file (maybe mytemplate.scriban file) which will not be emitted in assembly and also easier to edit because it does not have to be encoded as C# string. See: https://devblogs.microsoft.com/dotnet/new-c-source-generator-samples/#user-content-csv-generator-usage
beakona commented 3 years ago

Also, here are SourceGenerator features Access Analyzer Config properties and Consume MSBuild properties and metadata that somehow may be used.

ignatandrei commented 3 years ago

Yes. The approach with a template file is much easy. I have an example with https://github.com/ignatandrei/AOP_With_Roslyn/tree/master/SkinnyControllers or https://github.com/ignatandrei/AOP_With_Roslyn/tree/master/AOPMethods

beakona commented 3 years ago

Thanks. I will extend current version to take named parameter TemplateFileName that will search 'AdditionalFiles' and 'Assembly's ManifestResourceStream' (as fallback) for a template body.

beakona commented 3 years ago

Version 1.0.11 can load template from analyzer additional file and also we have partial templates.