AArnott / CodeGeneration.Roslyn

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

DocumentTransform unit tests #79

Closed amis92 closed 6 years ago

amis92 commented 6 years ago

Validates bugs #29, #32 as "expected"

These are unit tests for DocumentTransform.TransformAsync which, for a given "document" string validate if it generates document equal to "expected" document string.

In the current state they all pass. I think that it might be a better approach to first create a "working as-is" environment, and then add bugfixes separately.


edit after additional commits, this PR now:

amis92 commented 6 years ago

I'll happily fix these bugs, although I'm wondering what behavior exactly would we expect; which of the trivia should we keep, and which should we drop?

IMHO, we should drop all except pragma, define if/else/endif; those should probably stay, at least for usings.

Second thing is, should namespace-nested usings also be dropped (as is), or should we leave them too?

AArnott commented 6 years ago

That all sounds good, with one response:

Second thing is, should namespace-nested usings also be dropped (as is), or should we leave them too?

I don't care too much either way. I would tend toward including them. But as long as the code compiles I don't care too much. :)

amis92 commented 6 years ago

After reconsidering the implications, I'm going to drop all trivia, including if/else directives, regions and pragmas. Because generation is expected to be run for each compilation, and a given compilation has a defined set of defines; and also because if-deactivated code parts are trivia nodes in Roslyn Syntax model.

AArnott commented 6 years ago

Does this actually fix #29 and #32 now (as opposed to writing tests to expect the prior behavior)?

amis92 commented 6 years ago

Yes, yes it does fix those two :) Glad to help.