Support loading parameters from properties for Type=Class.

[binki]

When authoring a Task in an assembly, task parameters are declared
simply by creating public properties. CodeTaskFactory instead requires
the user to manually declare all properties in a
element. This is tedious when using Type="Class" because the
parameters must be listed both as property declarations and in
. Also, this requirement increases maintenance when
using inheritence to extend or intercept an existing Task from another
assembly.

This change adds an AutoDetectParameters="True" option to the
element for Type="Class". When enabled, simple reflection is used to
calculate the parameters and any parameters provided via
are ignored.

Fixes #33.

[jeffkl]

This looks great. I am currently out of town on vacation with very little internet access. I’ll give it a full review when I return on April 25th. Sorry for the delay!

[binki]

@jeffkl Thanks for taking time out of your vacation to respond!

[jeffkl]

Okay I'm back. I'll preface this with an apology that I just happened to be on vacation when you submitted the issue and this pull request.

After thinking about it, I'm of the opinion that when a user specifies CodeTaskFactoryCodeType.Class, that they should not have to specify the parameters at all. I would say we have no AutoDetectParameters attribute and instead its just be automatic. As you've pointed out, at the moment users have to specify the parameters in XML and in code already. We might as well just use reflection and make it automatic. If a user specifies the parameters in XML, we could just ignore it or log a message saying they were ignored. This would have no breaking change because existing <UsingTask /> definitions already have all parameters defined and when a user is specifying a whole class they have their parameters defined. I think having to opt-in to this automatic behavior would be hard to discover and instead we could just make it opt-in if they specify a whole class.

What do you think?

[binki]

I think making it fully automatic should be fine.

Warning that they’re ignored if specified would be the best behavior because users will wonder why removing a parameter from that list fails to remove the parameter from the created task (using the <ParameterGroup/> to prevent public properties from being treated as parameters is the only use case I can imagine and I don’t know how that would even be useful). For people switching to/from the MSBuild .netfx built-in CodeTaskFactory and RoslynCodeTaskFactory, the warning may be annoying but will at least permit them to test this out without making additional changes.

This would need to be documented in a list of behavioral differences between RoslynCodeTaskFactory and .netfx CodeTaskFactory.

I can update my PR but it might take me a day to find the time to do so.

[jeffkl]

Okay no hurry. I'm in the process of porting my code task factory directly into MSBuild: dotnet/msbuild#3175

I'll port your change into that submissions as well after we've merged this. Thanks for the contribution!

[binki]

I’ve updated to have a warning an ignore any supplied <ParameterGroup/> when Type=Class. I’ve removed the README edits for now, not sure they make sense and probably at least doesn’t need to be part of this particular PR. Let me know if there’s anything else I can do to help get this functionality in :-).

[jeffkl]

Looks great, thanks!