Add TargetFramework(s) unexpected check #11285
Conversation
| string? frameworks = null; | ||
| string? framework = null; | ||
| // This is not SDK style project | ||
| if ((!context.Data.EvaluatedProperties.TryGetValue(PropertyNames.UsingMicrosoftNETSdk, out string? usingSdkStr) || |
There was a problem hiding this comment.
If I'm reading your conditions correctly here, I think this would fire on a C++/CLI vcxproj targeting .NET 8--but shouldn't.
There was a problem hiding this comment.
We can detect import of Microsoft.Cpp.props to handle this.
But together with this comment: #11285 (comment) - this leads to more importnat question: do you feel this is fundamentaly wrong and would you suggest different way of detecting non-SDK project?
There was a problem hiding this comment.
I wonder if we should restrict it to .csproj? Or check for any import called CSharp.targets? though I guess it should apply to vbproj and fsproj too . . .
There was a problem hiding this comment.
As brainstormed offline - I updated the check to detect ProjectCapability and if the project is sdk-style or c++/cli
Co-authored-by: Rainer Sigwald <[email protected]>
Co-authored-by: Rainer Sigwald <[email protected]>
|
|
||
| // We want to avoid repeated checking of a same project (as it might be evaluated multiple times) | ||
| // for this reason we use a hashset with already seen projects. | ||
| if (!_projectsSeen.Add(context.Data.ProjectFilePath)) |
There was a problem hiding this comment.
is having another dictionary with all the project paths in it worth it memory-wise to avoid looping over ProjectCapability? Which has worse perf I wonder?
This is fine, it makes sense to do this kind of check, though I wonder if we should somehow make it cheaper for individual checks to do this sort of thing.
| "'TargetFramework' nor 'TargetFrameworks' property should not be specified in projects not using .NET SDK." | ||
|
|
||
| 'TargetFramework' or 'TargetFrameworks' control the project output targets in modern .NET SDK projects. The older SDK-less projects interprets different properties for similar mechanism (like 'TargetFrameworkVersion') and the 'TargetFramework' or 'TargetFrameworks' are silently ignored. | ||
|
|
||
| Make sure the Target Framework is specified appropriately for your project. |
There was a problem hiding this comment.
Update this with the new error text please
Fixes #10635
Context
TargetFramework(s) are honored only in SDK-style projects - emit diagnostic if the property is used in legacy style project
Testing
Targetted test