AArnott / CodeGeneration.Roslyn

Assists in performing Roslyn-based code generation during a build.
Microsoft Public License
408 stars 59 forks source link

Adding more control to output generation #40

Closed Pzixel closed 6 years ago

Pzixel commented 7 years ago

Currently I encountered two problems that are caused by current design:

  1. Generated members always are nested in parent's namespace. Sometimes it's good, sometimes it's not required.
  2. Generated members always are placed in same assembly where attribute is declared. Again, sometimes it's good, sometimes it's not required.

I propose to add more customisation here. I think ICodeGenerator have to provide somehow information if it want inherit ancestor's usings/namespace or it doesn't and information about path where it want to be stored:

Task<GenerationResult> GenerateAsync(TransformationContext context, IProgress<Diagnostic> progress, CancellationToken cancellationToken);

public class GenerationResult
{
   SyntaxList<MemberDeclarationSyntax> GeneratedMembers { get; }
   string DestinationProjectDirectoryFullPath { get; }
   bool IsNestedInAncestor { get; }
}

It's just an idea, I don't really like how it looks now, but we may invent something better.

Use case: I have an attribute in a class in an assembly foo with multiple dependency, for example, ASP.Net. But I obviosly won't use it in my generator, so I don't want these dependencies. I can create assembly foo.generated without any dependency and store generation results there.

I can try to implement it if you agree that we need this feature and we can merge it in master. It's a bit more complicated than other changes and require some API rework, so I don't want to spend several days to get I cannot afford merging it, it completly ruins my design :)

AArnott commented 7 years ago

Thanks for checking first. I need to mull this over a bit before advising you to proceed. The most concern comes from creating an entirely new assembly for the generated. That is rather non-trivial because it turns out there are lots of concerns that spring up when you do that, like satellite assemblies, signing, added references, etc. I don't think I want to go there without at least one more use case from someone else to justify it. But I do appreciate you offering to do the work.

As for the ability to generate members in another namespace, that sounds like a bad idea in general, but for specific use cases may be perfectly legit (for example, if you're writing a generator specialized for your own purposes rather than for general reuse). I would accept a PR that makes this possible. And I like how your proposed design will allow for enhancements in the future in the generation result without breaking changes to existing generators (although this one itself will be a breaking change).

Pzixel commented 7 years ago

@AArnott completly different assembly may led to other multiple problems (such as how to reference it?). I think empty assembly for generated classes is quite OK. You can sign it, add versioning, and so on, using common flow. Implementing it as result of generation may be complicated and unwanted.

AArnott commented 7 years ago

If code generation produces an assembly the signing has to be determined up front. As does most other aspects of it like versioning. My answer for extra assembly is no.

Pzixel commented 7 years ago

@AArnott I agree, this is what i have written :) But ability to choose project where cs is injected is useful.

AArnott commented 7 years ago

Sorry, I misunderstood your argument then. So you want to generate code into another project (not the one that is currently building)? That sounds rather complicated on the CG.R side, and would require careful build scheduling as well, up-to-date checks, etc. I still feel like we need more than one user who needs it to justify the added complexity all around.

Pzixel commented 7 years ago

@AArnott in my case it's just practice: I want to generate some client for some API. This client is only using some http libraries while server side may contain tens of dependencies, such as performance counters for services/winapi calls/filesystem logging and so on... I want my client to be generated based on service's API, but really want it to be in an independent assembly. It works fine with WCF, where I can declare service contracts in one assembly and then implement them in this assembly with dependencies (and have my clients generated in this interface assembly), but it's not this cool with ASP.Net which doesn't have thing like routings on interfaces, so I have to work with actual classes.

I agree that it's complicated (a bit, because it's easily solved with customisable path to output. We may specify it in attribute and we're golden), but I just can't proceed further until it's done. So i'd better help you with implementing this rather than... I don't know... Implement my own mini-framework on top of ASP.Net to make attributes inherited from interfaces and move on WCF track :)

AArnott commented 7 years ago

Interesting scenario. I just happened to notice this earlier today. Have you seen it? Might that fill your need? https://github.com/paulcbetts/refit#refit-the-automatic-type-safe-rest-library-for-net-core-xamarin-and-net

Pzixel commented 7 years ago

@AArnott it looks more like my old version based on reflection and runtime generation. But my approach is different a bit. However, you are right that my pet project is targeting the same issue. But it's beside the point :) They are fine for same reason why my old RemoteClient was fine - it gets generated in dynamic assembly without dependencies and can easily be referenced from anywhere. However, with Roslyn we have to place .cs in project at compile time, and this is why we are using it: we may want to look on generated code, modify it in some other tricky way or whatever. We just have to place it in different project, and generation tool can easily read this intent from attributes and react accordinly.

About my needs: I don't need it right now so I can wait until I write it the way I suppose being better.

AArnott commented 7 years ago

If we do support it in CG.R, I think the attributes must appear where the code will be injected. Could you work with that design? For example, in the 'other' project, you add:

[assembly: GenerateCodeForWebService(uriOrAssemblyName)]

And that triggers your code generation, which loads your assembly to look at the webapi calls and then generates code in this project?

Pzixel commented 7 years ago

@AArnott interesting idea. Now it's my time to to mull this over a bit :)

Pzixel commented 7 years ago

@AArnott well, now I think that it may work, but not really elegant. Because if we change interface and rebuild it, dependend project won't know about it. So for example we have a project A, A.Clients and B. B is referencing A.Clients. Now we rebuild project A. Interface gets updated but now we are getting erros in A.Clients that clients do not implement required interfaces. Now we have to rebuild A.Clients and finally get proper erros from B that now have to update usages of service.

And thinking logically, these clients are artifatcts of generating project A. They have to be stored in project A.Clients, but it's another story. In a nutshell generated classes belongs to A even if they are stored somewhere else. So they have to be generated when A is generated.

Your way is better when we just can't modify source code of these interfaces, but in this case we don't need this generation tool, it's useful when interface is changing frequently, and in this case it's frozen for centuries.

AArnott commented 7 years ago

If A.Clients.dll contains the generated code, I think you should have an A.Clients.csproj responsible for generating that code. And folks consuming the generated code in other assemblies should reference A.Client.csproj. Then if A ever changes, when you rebuild a consuming project, A.Clients also gets rebuilt and the interface updated. Why doesn't that solve the problem?

Pzixel commented 7 years ago

@AArnott if we can force A.Clients to rebuild then it's ok. But i'm not sure if it's possible via targets.

AArnott commented 7 years ago

If a consuming project references A.Clients.csproj, msbuild will build it. The incremental build nature of A.Clients.csproj can be modified fairly easily if necessary to ensure that when A.dll changes, A.clients.csproj builds a new dll.

Pzixel commented 7 years ago

Sounds reasonable.

amis92 commented 6 years ago

80 resolved the first part at least (controlling namespace/type ancestry of generated code), possibly closing this issue.

AArnott commented 6 years ago

I'm going to close this based on the part that is fulfilled. I think the rest of it won't get done since generating code in another assembly besides the one that applies the generator attributes is pretty far outside the scope of this project.

Pzixel commented 6 years ago

@AArnott well, I see that that would be weird. But this requirement was because we could have build-time dependency only, e.g. build some code based on server libraries without requiring them at runtime. For example, if we are generating some REST client based on server description we don't want to have any dependencies at all. But we need to have some dependency to make it possible to reflect it.

That probably could be solved via private assets though I'm not sure.

AArnott commented 6 years ago

Yes, a PrivateAssets="all" attribute on your reference makes it a build-time only dependency (assuming you don't then create a runtime dependency within your code on it). If you want to generate code in one project based on another project, that simply seems like another effort other than what this CodeGeneration.Roslyn project's scope is.