Add warnings to Microsoft.NET.Sdk.WindowsDesktop to improve UseWpf/UseWinforms and multi-targeting experience

[vatsan-madhavan]

Fixes the following:

• Fixes Add Page automatically, remove from None automatically #685: Remove **/*.xaml from None
• Fixes Cannot multi-target to netcoreapp < 3.0 and use WindowsDesktop SDK unconditionally #327: Cannot multi-target netcoreap < 3.0 and use WindowsDesktop SDK unconditionally.
• Fixes Support WPF and WinForms specific FrameworkReferences via Profiles #746: [Epic] Support WPF and WinForms specific FrameworkReferences a Profiles
  • Fixes Show error when neither UseWpf nor UseWindowsForms is set to true #867: Show error when neither UseWpf nor UseWindowsForms is set to true

Also cleans up the way in which we import Microsoft.WinFX.targets - UseLegacyPresentationBuildTasks has been broken for some time now and unusable.

[vatsan-madhavan]

/cc @davidwengier, @davkean

[dsplaisted]

It would be nice to have tests covering these fixes, for example that you can multitarget .NET Core 2.x and 3.0 using the WindowsDesktop SDK. Do you have a way of testing the WindowsDesktop SDK in this repo?

[vatsan-madhavan]

It would be nice to have tests covering these fixes, for example that you can multitarget .NET Core 2.x and 3.0 using the WindowsDesktop SDK. Do you have a way of testing the WindowsDesktop SDK in this repo?

Not yet. I'll sync with you next week to figure out how to seed this repo with some tests based off of dotnet/sdk.

For now, these are tested locally using a WPF app, WinForms app and a Console App, and using dotnet, msbuild and VS.

<!-- WPF project -->
<Project Sdk="Microsoft.NET.Sdk.WindowsDesktop">

  <PropertyGroup>
    <OutputType>WinExe</OutputType>
    <TargetFrameworks>netcoreapp3.0;net40;net45;net46;net452;net472;netcoreapp2.2;net35</TargetFrameworks>
    <UseWPF>true</UseWPF> <!-- change to false to test NETSDK1106 -->
    <UseWindowsForms>false</UseWindowsForms>
    <SuppressNETCoreSdkPreviewMessage>true</SuppressNETCoreSdkPreviewMessage>
  </PropertyGroup>

<PropertyGroup>
  <!--<MSBuildWarningsAsMessages>$(MSBuildWarningsAsMessages);NETSDK1096;NETSDK1105;NETSDK1107</MSBuildWarningsAsMessages>-->
  <!--<MSBuildWarningsAsMessages>$(MSBuildWarningsAsMessages);NETSDK1105</MSBuildWarningsAsMessages>-->
</PropertyGroup>

  <PropertyGroup>
    <FrameworkPathOverride Condition="$(TargetFramework.StartsWith('net3'))">$(MSBuildProgramFiles32)\Reference Assemblies\Microsoft\Framework\.NETFramework\v3.5\Profile\Client</FrameworkPathOverride>
  </PropertyGroup>
...
</Project>

<!-- WinForms project -->
<Project Sdk="Microsoft.NET.Sdk.WindowsDesktop">

  <PropertyGroup>
    <OutputType>WinExe</OutputType>
    <TargetFrameworks Condition="'$(MSBuildRuntimeType)'!='Core'">netcoreapp3.0;net30;net35;net40;net45;net472</TargetFrameworks>
	<TargetFrameworks Condition="'$(MSBuildRuntimeType)'=='Core'">netcoreapp3.0;net40;net45;net472</TargetFrameworks>
    <UseWindowsForms>true</UseWindowsForms>
  </PropertyGroup>
  
    <PropertyGroup>
    <FrameworkPathOverride Condition="$(TargetFramework.StartsWith('net3'))">$(MSBuildProgramFiles32)\Reference Assemblies\Microsoft\Framework\.NETFramework\v3.5\Profile\Client</FrameworkPathOverride>
  </PropertyGroup>
</Project>

<!-- Console project -->
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp3.0</TargetFramework>
    <UseWpf>true</UseWpf>
    <!--<MSBuildWarningsAsMessages>$(MSBuildWarningsAsMessages);NETSDK1096;NETSDK1105;NETSDK1107</MSBuildWarningsAsMessages>-->
  </PropertyGroup>
</Project>

[drewnoakes]

Would it be in scope to warn if property <UseWinForms> is defined? I'm sure lots of people would get tripped up on that.

[vatsan-madhavan]

Would it be in scope to warn if property <UseWinForms> is defined? I'm sure lots of people would get tripped up on that.

@drewnoakes, Are you suggesting that we should warn if <UseWinForms> is defined - which is an incorrect variant of UseWindowsForms? If this is what you have in mind, would you please open an issue and we can address this another PR?

We would need to add another warning string in the sdk - so we can't tag this change along here.

/cc @merriemcgaw

[RussKie]

if <UseWinForms> is defined - which is an incorrect variant of UseWindowsForms

I'd think UseWinForms would be a better (if not correct) setting to use. I speculate here, but based on my experience "WinForms" is significantly more recognisable than "Windows Forms"

[drewnoakes]

@vatsan-madhavan yes that's what I meant. Thanks for creating the issue.

@RussKie I agree (I filed issues against the new project dialog to make sure "winforms" actually returned any results) but in code the full name "WindowsForms" is used throughout. This is why I think an additional warning here would help a non-trivial number of people succeed here, especially since the current design of the project file around enable winforms/wpf is so difficult for us on the project system team to tool that customers are likely to need to write this XML by hand from memory.

[RussKie]

Thank you for clarifications.

I'd spin it from a customer perspective - how we call it in our codebase is an internal detail and immaterial outside the codebase.
What would make the most sense to our consumers?

[dsplaisted]

It's not about internal details of the code base. The namespace and assembly name is System.Windows.Forms, not something like System.WinForms. That is visible to customers. Project templates in VS, documentation, etc. all say Windows Forms instead of WinForms.

[stevenbrix]

It's not about internal details of the code base. The namespace and assembly name is System.Windows.Forms, not something like System.WinForms. That is visible to customers. Project templates in VS, documentation, etc. all say Windows Forms instead of WinForms.

Right, but no one calls WPF by it's actual name - Windows Presentation Foundation. Developers are lazy and don't want to type more then they have to. They are also opinionated though, and no answer is really more correct than the other one. As a broad generalization of our kind, I think brevity is a close second behind readability/understanding. Obviously something like <UseWF> would be a bad idea because it's not immediately clear what "WF" is. Likewise, it would be equally bad to have <UseWindowsPresentationFoundation> because it doesn't help the user to understand its purpose anymore than <UseWPF> does.

I do agree with your sentiment that everything should be consistent! The command to create a new WinForms app is dotnet new winforms, but in VS you create a new "Windows Forms App (.NET Core)":

Personally, I think most people are comfortable and familiar with the term "WinForms" and we should go with this everywhere. This would also make the VS experience more like WPF and say "WinForms App (.NET Core)" and then in the description say the actual name. But this is also just my opinion

[dsplaisted]

Good point about dotnet new winforms. It looks like we weren't consistent there.

[vatsan-madhavan]

Good point about dotnet new winforms. It looks like we weren't consistent there.

please don't change this to dotnet new windowsforms /cc @merriemcgaw

[OliaG]

I agree that the template dotnet new winforms should remain the same.

My personal preference would be in the project file as well since it is consistent with , clear (everyone knows what is WinForms, WindowsForms doesn't add any value), brief (users don't need to type more) and I'd also guesstimate the name "WinForms" is used more often than "Windows Forms". I'd make the correct property and prompt users if they are using . That being said I realize this would break existing WinForms Core apps, but if we want to make this change - the preview is the right time to do so.

@merriemcgaw, what do you think?

[vatsan-madhavan]

...and I'd also guesstimate the name "WinForms" is used more often than "Windows Forms". I'd make the correct property...
the preview is the right time to do so.

We are locking down tomorrow noon for preview 7.

A change like this requires coordinated changes in dotnet/sdk and dotnet/wpf. I recommend that we pursue an approach that augments the existing design by recognizing WinForms as a synonym for WindowsForms (assuming we want to pursue a change at all - I'll defer to the WinForms team for this decision). We can pursue such a change now or in P8 (because it would be a non-breaking change).

[davkean]

Lots of "gut feelings" in this thread. "Windows Forms" has been consistently through documentation, training, Visual Studio UI, templates, etc since the beginning of its existence. dotnet new winforms is the odd one out here. We should not change usage from "Windows Forms" -> "WinForms" (which would have a fairly significant impact across the product) without data/interviews showing that users are confused by this, and this PR is not the right place.

[drewnoakes]

Regardless of what's used, I maintain customers who hand edit their project file and use the incorrect version will appreciate guidance that they're not quite doing what's needed. Currently there's no feedback. This is not the pit of success.

[dsplaisted]

Strongly agree with David, but also agree that either a warning for using the wrong property name (or possibly just allowing either property name) is a good idea.

[davkean]

Yep, sorry didn't mean to say that shouldn't happen RE warning. Accepting both definitions I think would be a mistake, as there are already downstream consumers of this (our capabilities are already driven off this)

[RussKie]

Following your argument, we should use UseWindowsPresentationFoundation 😉

[vatsan-madhavan]

Please move the conversation over to dotnet/winforms#1226