search

AArnott / CodeGeneration.Roslyn

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

[RFC] Attribute delivered as nuget contentFiles #150

Closed amis92 closed 5 years ago

amis92 commented 5 years ago

Recently I've been thinking about how to un-tfm BuildTime and Attributes packages.

Since the move to a separate netcoreapp-based tooling, we no longer require any specific type of framework to be compatible with (did we at any point in time?). As such, the TFM in BuildTime is only unnecessarily complicating things. That package should not declare any frameworks, and just deliver the msbuild files.

Attributes package theoretically does contain a dll used and referenced almost anywhere. But in reality, all it has is a single Attribute file, it's unique identity is not used anyway (just a simple Type.Name comparison, doesn't even account for the namespace):

https://github.com/AArnott/CodeGeneration.Roslyn/blob/2b937a442a1380eb4c65fa7da45077ba62a10b33/src/CodeGeneration.Roslyn.Engine/DocumentTransform.cs#L154-L162

This single file could actually be "inlined" as a compiled source file in a generator-attribute containing project. It'd remove dependency on a whole DLL, remove any framework checks, etc. For published generators, the attributes package could also use the same philosophy to remove the unnecessary package dependencies.

What are your feelings, @AArnott ?

AArnott commented 5 years ago

Not a fan. I've done it both ways a few times. Including source files into a project is problematic as different stylecop rules, compiler settings and other analyzers tend to make someone unsatisfied with the experience. Also as far as the assembly dependency, it never loads for attributes except when reflection queries for them. In fact we can easily prevent the attributes from surviving in the final assembly by attributing our attribute with a ConditionalAttribute like this: https://github.com/dotnet/corefx/blob/da3ba7ebe34771dde769b65dfb7f78fa55a5abb6/src/Common/src/CoreLib/System/Diagnostics/CodeAnalysis/SuppressMessageAttribute.cs#L23

That, and compiling our attributes assembly to netstandard1.0, should avoid all trouble.

amis92 commented 5 years ago

Agreed. Did not take into account, well, most of what you said. :)