dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.61k stars 1.04k forks source link

SDK should expose available WarningLevel values as MSBuild items #24570

Open tmeschter opened 2 years ago

tmeschter commented 2 years ago

Is your feature request related to a problem? Please describe.

The .NET Project System tries to provide the user all the available options for the WarningLevel property, along with a concise description of each, like so:

image

There are two problems with this:

  1. The Project System will always be playing catch up with the SDK as newer versions of the SDK come out. If the WarningLevel is set to a value the Project System doesn't know about we'll often end up showing nothing at all because we don't know how to interpret the value.
  2. If the user forces an older version of the SDK, the Project System may show them values that aren't meaningful. E.g., if the user forces the project to the .NET 5.0 SDK, we will still show "6" as an option because we don't know any better.

Describe the solution you'd like

The SDK should provide the set of meaningful values, perhaps as a set of items:

<ItemGroup>
  <AvailableWarningLevel Include="0" />
  <AvailableWarningLevel Include="1" />
  <AvailableWarningLevel Include="2" />
  <AvailableWarningLevel Include="3" />
  <AvailableWarningLevel Include="4" />
  <AvailableWarningLevel Include="5" />
  <AvailableWarningLevel Include="6" />
  <AvailableWarningLevel Include="7" />
  <AvailableWarningLevel Include="9999" />
</ItemGroup>

Or perhaps as a set of properties:

<PropertyGroup>
  <AvailableWarningLevels>0;1;2;3;4;5;6;7;9999</AvailableWarningLevels>
</PropertyGroup>

The Project System could then populate the dropdown using the values, avoiding the need for a change to VS every time a new warning level becomes available.

It would also be great if the SDK could provide the localized description of each warning level, but if that is difficult to do the Project System could continue to handle that part. The worse that would happen is there would be times when we don't have a description for a particular warning level.

dotnet-issue-labeler[bot] commented 2 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

baronfel commented 2 years ago

This seems good, but WarningLevel is a language-agnostic concept and so both the list and the descriptions would need to be keyed by language identifiers in some way. I'm also a huge fan of anything that puts data like this into the SDK to make the Project System (and any other consumers) more data-driven and less tightly bound :)

Nirmal4G commented 1 month ago

IMHO, putting too much data that's only needed in UI only context into MSBuild files could impact performance of the builds. Instead, putting this into Microsoft.Build.xsd and Microsoft.Build.CommonTypes.xsd or a separate file just for SDKs inside the SDKs (needs design) will work just fine.

@rainersigwald thoughts?

rainersigwald commented 1 month ago

The project systems today don't have any way to pull from the .xsd files and do have the ability to pull from items, so I don't think that would be a great direction @Nirmal4G.

Nirmal4G commented 1 month ago

I kinda knew that! But my real question to you is that instead of making the build slow by putting UI data into MSBuild props/targets, putting them into .xsd should give you that perf advantage, right? Separation of concerns comes into play here, provided, as you said, that the project system uses the definition files instead.

We already know the advantage that JSON Schema provides for JSON, XSD, XSLT for XML, etc... Terminal uses JSON Schema to fill up the valid values and suggested values. Both Project Systems could have used the XSD from the start. Alas! things didn't play out that way but that does not mean it shouldn't now.

KalleOlaviNiemitalo commented 1 month ago

Could these be items in a separate MSBuild file that the SDK imports only if the project system defines a property to indicate that it's needed? CI builds would then be unaffected, at least.

Or, have the SDK define a property that points to a file from which the project system can read this stuff.

MSBuild has similar-looking stuff in XamlRules. Does the project system use those; if so, how does it locate them?

Nirmal4G commented 1 month ago

Yes, with conditional imports, CI builds will be unaffected. But what about local dev loop? That's important too. What I'm looking for, is to completely separate UI only things from build only things but still have that integrated feeling. Then everybody wins!