Msbuild: Warning for leaked "private" variables

[rainersigwald]

MSBuild doesn't currently have any scoping concept for properties or items (adding one is #4074). As a stopgap, we could apply a heuristic, opt-in warning that some patterns are likely to be bad, with a suggested fix.

One possibility would be:

• If an item name is prefixed with an underscore, and
• It is not referenced in any target other than the one that defines it,

warn that it should be emptied before returning.

The goal here would be to reduce SDK authoring errors like dotnet/sdk#3027 where an item name gets inadvertently reused.

[cdmihai]

If an item name is prefixed with an underscore, and it is not referenced in any target other than the one that defines it, - This means that the set of variables marked as "pseudo-private" is relative to the import closure, meaning the same item / property from the SDK / common props/targets might be marked as pseudo-private in some evaluations (e.g. a dotnet new console evaluation), but not others (random project from random repo). For consistency this means you'd need to mine for private variables once, on some "golden standard" corpus, and then ship that json file or whatever with msbuild.

[rainersigwald]

My theory is: as long as it's deterministic for a single project it'd be ok if it changed with different SDK/package combinations. The idea being that SDK authors could test with "the simplest project that uses this SDK/package" and if instances of this are cleaned up there, you'd be in better-than-now shape, even if the error could be masked by adding some other package.

[davkean]

Emptying the field before returning? That seems like a bit of a cop out and more boilerplate MSBuild. I'd prefer a mechanism to be able to scope variables.