Enhance msb4064 unexpected task attribute error

[BartoszKlonowski]

This pull request fixes #3922.

The MSB4064 error has been provided with additional information about the assembly such as it's version, full name, etc.

---

Changes provided within this commit contains:

• modify the MSB4064 string in resources,
• adjust it's usage in the code throwing this error,
• run the default translations by compilation

---

Results:

The example of MSB4064 printed is presented below:

[rainersigwald]

This looks great, but I have a vague recollection that it isn't enough. I'll try to figure out why I think that and get back to you. Hopefully I can convince myself that past-me was just wrong!

[BartoszKlonowski]

@rainersigwald Perhaps what you feel that is missing here is a path (on local disk) to the assembly?
Thing is, that in my first approach to this, I've tried to use:

_taskFactoryWrapper.TaskFactoryLoadedType.Assembly.AssemblyLocation

but when debugging it, this path has always been null.
So due to the fact that I couldn't find any better path-containing property/variable, I decided to not include the path in the error message, so there's no confusion in case of that path didn't really work for some user.
I've tried to avoid the possible situation, where the error message would just display the empty space instead of correct path.

[rainersigwald]

Perhaps what you feel that is missing here is a path (on local disk) to the assembly?
Thing is, that in my first approach to this, I've tried to use:

_taskFactoryWrapper.TaskFactoryLoadedType.Assembly.AssemblyLocation

but when debugging it, this path has always been null.
So due to the fact that I couldn't find any better path-containing property/variable, I decided to not include the path in the error message, so there's no confusion in case of that path didn't really work for some user.

This seems pretty plausible! It would definitely be nice to have the path (I'd strongly prefer that over assembly identity) but I agree with your tradeoffs, and this is better than nothing.

Is that path null when you try with the minimal repro project I (just) shared in #3922 (comment)?

[BartoszKlonowski]

@rainersigwald Answering your question: no, the path isn't null when reproducing the error with the example you've provided me with.
I've pushed the changes and updated the PR's description - please check.

[Forgind]

It looks like LoadedAssembly is null for the ValidateNonExistantParameter (sic) test. It looks artificial to me, but are there any circumstances when it would be executing a task without loading an assembly? If so, maybe add a second error possibility depending on whether that exists?

[cdmihai]

Looking through the code, LoadedType.LoadedAssembly is always checked for null, so apparently it can be null. My assumption is that tasks that are not loaded from assemblies have it set to null (like inline tasks for example). So you'll have to treat the null case somehow. Most basic is to revert to the old message if it's null. Another implementation is to revert to another message that writes out the task factory type, which hopefully should be enough to let the user guess where that task is coming from. The common case is to have assembly based tasks.

Regarding tests, you have to update the mocked types to have an assembly, probably over here:

msbuild/src/Build.UnitTests/BackEnd/TaskExecutionHost_Tests.cs

Lines 1159 to 1166 in 27981f1

	TypeLoader typeLoader = new TypeLoader(IsTaskFactoryClass);
	#if !FEATURE_ASSEMBLYLOADCONTEXT
	AssemblyLoadInfo loadInfo = AssemblyLoadInfo.Create(Assembly.GetAssembly(typeof(TaskBuilderTestTask.TaskBuilderTestTaskFactory)).FullName, null);
	#else
	AssemblyLoadInfo loadInfo = AssemblyLoadInfo.Create(typeof(TaskBuilderTestTask.TaskBuilderTestTaskFactory).GetTypeInfo().FullName, null);
	#endif
	LoadedType loadedType = new LoadedType(typeof(TaskBuilderTestTask.TaskBuilderTestTaskFactory), loadInfo);

Then you need to write two tests: one when the assembly is null, and one when the assembly is not null, and assert the proper error message in both.

[Forgind]

@BartoszKlonowski, status update?

[BartoszKlonowski]

@Forgind Thanks for asking, and sorry for such a long delay...
No update from my side - Unfortunatelly I have no time to work on this.
If you're asking because you'd like to take it, then let me know if you have any questions. Otherwise, I'll try to push that forward by the end of this week.
Let me know.

[Forgind]

No worries! If you didn't want to work on it anymore, I was willing to try to get it ship-shape, since I think it's pretty close. End of this week (or end of next week) is fine. Thanks!

[Forgind]

LGTM! Thank you!

[ladipro]

Looks great! One small nit inline.

[ladipro]

super-nit: In many other error messages mentioning UsingTask I see it wrapped in angle brackets making it clear that it refers to an XML element. Also, a comma before the last and would be more consistent with the style of the rest of this file.

[Forgind]

Thanks @BartoszKlonowski!