Localize the text in the common target

[JaynieBai]

Fixes #10171

Context

MsBuild produces unlocalized messages for some specific validation cases that happen in scope of target.
The example of the task usage: https://github.com/YuliiaKovalova/msbuild/blob/be21253d85f7766356880d376e26aaa69c34c4cd/src/Tasks/Microsoft.Common.CurrentVersion.targets#L862

Changes Made

Refactor the existing messages with the recently added task allows to produce localized messages:
https://github.com/dotnet/msbuild/blob/79dff86b18613cfe3510b719ac28e8a8c3e7f96c/src/Tasks/MSBuildInternalMessage.cs , produced by with hardcoded text.

Testing

Add test cases for each new resource, except for the resource CommonSdk.PropertyWithTrailingSlash, which is tested manually as followed picture.

Notes

[JanKrivanek]

Looks good.

Though - I was not controllig in detail 1:1 equality of the literals in .targets with the new in resx files - I suppose they were copied over (with just applying format parameters)

[Nirmal4G]

Why not use CommonSdk instead of CommonTarget? It could be useful if you decide to do #1686?

Also, Message Code (like MSB3539) should be separately specified to ensure searchability in these files.

[Nirmal4G]

Code Style nit: Method parameter should start with lower case.

Other that that, it's looking good.

[Nirmal4G]

Message Code (like MSB3539) should be separately specified to ensure searchability in these files.

We only localize the text, so, it's not strictly required to put Code in. But I get the reason why. I can also argue that, when debugging an obscure build failure, you might need to locate the line in the targets where it spits out the message and start from there. Since, for almost all messages, the text log does not provide line number and file of the message task unless a high verbosity level is provided. So, you cannot locate the message task.

With Code specified in the targets, we can just do a text search on a preprocessed file, and we can go to that exact line. This saves a ton of time combing through binlog or imported files.

And if you want to indicate the Code mapping in RESX, why not add it in the comments? There's a reason that previous text only message tasks had a separate Code parameter instead of just putting them in the Text parameter.

[JaynieBai]

Message Code (like MSB3539) should be separately specified to ensure searchability in these files.

We only localize the text, so, it's not strictly required to put Code in. But I get the reason why. I can also argue that, when debugging an obscure build failure, you might need to locate the line in the targets where it spits out the message and start from there. Since, for almost all messages, the text log does not provide line number and file of the message task unless a high verbosity level is provided. So, you cannot locate the message task.

With Code specified in the targets, we can just do a text search on a preprocessed file, and we can go to that exact line. This saves a ton of time combing through binlog or imported files.

And if you want to indicate the Code mapping in RESX, why not add it in the comments? There's a reason that previous text only message tasks had a separate Code parameter instead of just putting them in the Text parameter.

Thanks for your explanation. I added that code in the comment.

[Nirmal4G]

I added that code in the comment.

Yeah, I meant in RESX Comments field but this works just as fine. However, it requires anyone who adds a message task to consistently place the Code in the comments, which is improbable. IMO, separating the Code from the resource string remains most effective.

Also, Code in the resource string seems to be optional even if the Severity is either Warning or Error. Since this is an internal task to MSBuild, can't we make the Code mandatory on those conditions? This would prevent some typos in the future and also force us to allocate Code for every warning or error.

[JaynieBai]

I added that code in the comment.

Yeah, I meant in RESX Comments field but this works just as fine. However, it requires anyone who adds a message task to consistently place the Code in the comments, which is improbable. IMO, separating the Code from the resource string remains most effective.

Also, Code in the resource string seems to be optional even if the Severity is either Warning or Error. Since this is an internal task to MSBuild, can't we make the Code mandatory on those conditions? This would prevent some typos in the future and also force us to allocate Code for every warning or error.

Yes, your suggestions are reasonable. Since some warning or error messages before don't have the code such as CommonSdk.InvalidConfigurationTextWhenBuildingInsideVisualStudio, CommonSdk.DeploymentUnpublishable . @JanKrivanek and @YuliiaKovalova Should we add the code for every message? If so, I can update that next.

[JanKrivanek]

@Nirmal4G thonks for nice suggestions - tracked as #11087 - as the change would require adjusting code violating this currently and is out of scope of this PR

[Nirmal4G]

@JanKrivanek No problem. As you said, these are just suggestions. And I thonk thank you (sorry, couldn't resist 😜) for considering it.

[JanKrivanek]

😆 appology - quick typing. I promise I gave more time into considering the suggestion than into to typing that response! 😉 (it's tracked after all)