[Feature Request]: Make error message more helpful when TaskItem.ItemSpec is not set

[MangelMaxime]

Summary

Hello,

Improve error message if ItemSpec is missing on a TaskItem

Background and Motivation

When creating a custom MSBuild task, it took me several hours to figure out why I was getting this error:

error MSB4028: The "EasyBuild.PackageReleaseNotes.Tasks.GetCurrentReleaseTask" task's outputs could not be retrieved from the "CurrentRelease" parameter. Parameter "includeEscaped" cannot have zero length.

The reason was that I didn't set the TaskItem.ItemSpec value.

Proposed Feature

Improve the error message to mention that ItemSpec should be set.

The "EasyBuild.PackageReleaseNotes.Tasks.GetCurrentReleaseTask" task's outputs could not be retrieved from the "CurrentRelease" parameter. Please set the value of ItemSpec, in your task.

Alternative Designs

Another solution would be to change the constructor of TaskItem to take a mandatory ItemSpec value, but I suspect it would be too big of a breaking change.

[baronfel]

I think this could be solvable by introducing a more specific error message here - right now we have a generic 'could not bind property' message, but for something as important as the Identity/ItemSpec it makes sense to have a more directed message.

[rainersigwald]

@MangelMaxime can you confirm that you called new TaskItem() (the parameterless constructor)?

We should document that that is not ever intended to be called in normal code, it's just there as a trap :(

[MangelMaxime]

@rainersigwald

This is indeed what I am doing:

this.CurrentRelease <- TaskItem()
this.CurrentRelease.ItemSpec <- version.Version.ToString()
this.CurrentRelease.SetMetadata("Version", version.Version.ToString())

Based on your message, I suppose TaskItem have other constructors, but I didn't think to check for them. Because it didn't occurred to me that a "trap constructor" would exist 😅

[rainersigwald]

Yeah that's completely fair!

I'd recommend using TaskItem(string itemSpec, IDictionary itemMetadata), or new TaskItem(string) and then set metadata.

. . . now we should make the docs say the same. And maybe even hide that "so this type is COM-createable" ctor from IntelliSense or something so it's less tempting?