Better error message for malformed item transformations

[cdmihai]

A definition of an item ThisItem

<ItemGroup>
    <ThisItem Include="@(AnotherItem->$(APath)\%(Filename)%(Extension))" />
</ItemGroup>

Followed by a usage of ThisItem of the form:

%(ThisItem.FullPath)

Fails with the error: error MSB4023: Cannot evaluate the item metadata "%(FullPath)". The item metadata "%(FullPath)" cannot be applied to the path "@(AnotherItem->c:\projects\msbuild\bin\Debug-NetCore\Windows_NT_Deployment_Test)". Illegal characters in path".

The error is currently caught when the malformed item is used, not when the malformed item is created. Instead, the error should be caught at item definition with a text mentioning the missing single quotes (correct definition is "@(AnotherItem->'$(APath)%(Filename)%(Extension)')")

[rainersigwald]

Note that the reason this isn't an error now is that MSBuild's model is permissive. When it determines that the item transformation isn't valid (it's either missing quotes--@(AnotherItem->'$(APath)\%(Filename)%(Extension)')--or it's missing an item function name after the ->), it falls back to interpreting what it can as literal text (@(AnotherItem->), property expansions ($(APath) → c:\projects\msbuild\bin\Debug-NetCore\Windows_NT_Deployment_Test), and shared metadata (%(Filename) and %(Extension) without an item qualifier, which both expand to the empty string).

There are even tests to enforce this behavior, such as ExpandItemVectorFunctionsInvalid2!

Because this permissiveness is a fairly core property of MSBuild, we can't take the approach I would really want (a parse error when considering the item in the first place). We also probably can't introduce a hard error for this situation, because it would be easy for many customers to have taken an inadvertent dependency on the current behavior. I think the best we could do is emit a warning.

[cdmihai]

I agree. In this case, for the sake of internal consistency, it would be better to have a warning when there's a missing item qualifier.