mono / t4

T4 text templating engine
Other
388 stars 101 forks source link

Remove CompilerErrorCollection CodeDom dependency #95

Open mhutch opened 3 years ago

mhutch commented 3 years ago

Currently design-time templates (or their base classes, including the default one) are required have a property Errors of type CompilerErrorCollection. This is the sole reason they still need to reference System.CodeDom.dll.

Perhaps this could be replaced with a more modern/simple alternative - a type in the TextTemplating assembly itself, or a reporting method on a modernized host interface.

nathansoz commented 3 years ago

I started taking a look at this because I'm hitting issues incorporating Mono.TextTemplating into an MSBuild compiled task. CodeDom that is getting published with the task assembly ends up clashing somehow.

C:\scratch\testmsbuild\Test1\<redacted>.targets(45,5): error : One or more errors occurred. ([A]System.CodeDom.Compiler.CompilerErrorCollection cannot be cast to [B]System.CodeDom.Compiler.CompilerErrorCollection. Type A originates from 'System.CodeDom, Version=4.0.3.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' in the context 'Default' at location 'C:\Program Files\dotnet\sdk\3.1.402\System.CodeDom.dll'. Type B originates from 'System.CodeDom, Version=4.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' in the context 'Default' at location 'C:\Users\nsosnov\source\<redacted>\BuildTasks\bin\Debug\netstandard2.0\publish\System.CodeDom.dll'.) [C:\scratch\testmsbuild\Test1\Test.csproj]

The CompilerErrorCollection/CompilerError classes both have really simple public interfaces so I've forked T4 and implemented them. The tricky part here is that I assume that this interface shouldn't be broken. So there would need to be a new host interface (at a minimum) and then some branching logic for interacting with the various interfaces.

nathansoz commented 3 years ago

I went through and the CodeDom interfaces actually run a bit deeper than I thought. There are at least a few more POCO classes that are used. I'd be happy to work on a change for this but I'd like to understand what should be done about the interface I linked above. :)

mhutch commented 3 years ago

I meant removing the CodeDom dependency from the code that is generated from T4 files. That way projects that use preprocessed/runtime templates wouldn't need to depend on CodeDom.

Removing CodeDom from the Mono.TextTemplating engine would be a lot harder and I don't think it would be worth the effort.

For an example of running the templating engine in an MSBuild task, check out https://github.com/mono/t4/tree/build-targets/Mono.TextTemplating.Build

nathansoz commented 3 years ago

Oooo, that's exactly what I was looking for. Thanks!

mhutch commented 3 years ago

FWIW the MSBuild targets in that branch are mostly done and should be usable, I just haven't had time to finish them up. The only thing missing is incremental build i.e. they currently will always transform templates even if input files have not yet changed. Turns out incremental build is nontrivial and cannot be expressed via msbuild input/outputs as it depends on the content of the tt file.

bricelam commented 2 years ago

My two cents on this issue:

It's fine to just depend on System.CodeDom. Its CompilerError and CompilerErrorCollection types are too much a part of T4's public API:

It would be better if Mono.TextTemplating just maintained compatibility with these. (And necessary for issue #127.)

Also, System.CodeDom really isn't that big in the grand scheme of things--it's about the same size as something like System.Collections.Immutable.

T4's dependency on System.CodeDom is also ideal in a world of linkers where everything but these two types can just be trimmed out.

@mhutch Thoughts? Would you accept a PR that simply embraces System.CodeDom as a necessary evil of T4 and updates the APIs listed above to be compatible between VS and Mono.TextTemplating?

/cc @ajcvickers

bricelam commented 2 years ago

I meant removing the CodeDom dependency from the code that is generated from T4 files.

Ah, the scope is a bit narrower than I thought. But I think my argument is still valid. (PreprocessedTemplateClass).Errors should just be updated to public. The fact that Mono.TextTemplating generates it as protected makes working with preprocessed templates in both VS and Mono.TextTemplating impossible today.

mhutch commented 1 year ago

TBH, considering that the VS T4 impl hasn't been updated in years, I don't think maintaining API/ABI compatibility with it has much value. I'd prefer to improve usability and add features. However, I don't want to make breaking changes in a minor version of Mono.TextTemplating, and even making breaking changes in a major version isn't ideal.

I've been wondering whether perhaps we need a versioning model? Default to generating with the latest options, but have a compat model that allows generation using older patterns and host interfaces?