[Bug]: GetType can still be called as property function, due to case-sensitive comparison

[KalleOlaviNiemitalo]

[Bug]: GetType can still be called as property function, due to case-sensitive comparison

[f-alizada]

If there is a possibility of low usage, shall we put the change behind the changewave?

[rainersigwald]

Nice find! I can't believe that match is not case sensitive in the first place.

[KalleOlaviNiemitalo]

Will the fix be considered a breaking change again?

[rainersigwald]

I'm inclined to say no--because "we already updated the documentation to say you can't do that" but it's definitely a concern. @baronfel do you have thoughts?

[KalleOlaviNiemitalo]

Another possible fix would be to block access to System.Type references in general, rather than just GetType().

[baronfel]

I'm inclined to say 'no' for the same reason - the justification for the breaking change notes that there is no broadly-known usage of GetType any longer, so I think the impact of this would be quite low. If we get any significant user feedback that it is in use we can of course add breaking change notices quite quickly and out-of-band of the MSBuild code itself.

[f-alizada]

I see :) We have the MSBUILDENABLEALLPROPERTYFUNCTIONS, so the need of changewave is lowered.
Had spent some time understanding why it is not reproducible in debug mode :

msbuild/src/Framework/LazyFormattedBuildEventArgs.cs

Line 183 in 28f488a

if (param != null && param.ToString() == param.GetType().FullName)

[KalleOlaviNiemitalo]

Does that InvalidOperationException also trigger for param == "System.String"? If so, that seems misguided.

"System.String".ToString() == "System.String".GetType().FullName

[f-alizada]

@KalleOlaviNiemitalo it was exactly the case.

[KalleOlaviNiemitalo]

Oh, I assumed you had param == typeof(RuntimeType), thus typeof(RuntimeType).ToString() == typeof(RuntimeType).GetType().FullName.

[KalleOlaviNiemitalo]

I imagine && !(param is string) would get rid of the false "Invalid type for message formatting argument" error.

[f-alizada]

To be honest, I was thinking to leave it as it is, since it is in debug mode + the fix is prepared.
I'm not 100% sure that this fix should be applicble only for string, however did not investigate details, there could be cases when ToString method overriden and returns the value == param.GetType().FullName, since the arguments is an array of object object[] args.