dotnet
F# diagnostics: create consistent WarningLevel behavior
I propose to fix F# diagnostics so that the WarningLevel option does what the compiler reference says (see below) and thus aligns with the C# concept of WarningLevel
Every C# diagnostic CSxxxx has a well-defined warning level.
Errors have level 0, warnings have level 1 to 3 and informational warnings have level 4.
When building with <WarningLevel>n</WarningLevel> or --warn:n, all diagnostics up to level n are shown.
For F#, the compiler reference says
--warn:warning-level
Sets a warning level (0 to 5). The default level is 3. Each warning is given a level based on its severity. Level 5 gives more, but less severe, warnings than level 1.
This compiler option is equivalent to the C# compiler option of the same name. For more information, see /warn (C# Compiler Options).
However, the levels are not documented. Playing with the warning level (or diving deep into the compiler), you find
- Level 0: errors
- Level 1: info (informational warnings)
- Level 2: non-opt-in warnings
- Level 5: a few of the "opt-in" warnings
- No level (never shown): the remaining "opt-in" warnings
I propose to change this to
- Level 0: errors
- Level 1: warnings
- Level 3: info
- Level 6: opt-in warnings
While 4 for Info would be closer to C#, the above choice is for backwards compatibility.
Also for compatibility reasons, the proposal uses level 6 for opt-in warnings. (Note that C# uses levels above 4 for "warnings waves", which, I believe, we don't need. EDIT: see discussion below.)
All the nowarn/warnon/warnaserror functionality will of course continue to be supported.
Pros and Cons
The new warning levels are more consistent and can easily be documented. They align with the SDK and C# concept of "Warning level". They will simplify the compiler code and make it more maintainable. (That's actually why I bring it up ;-))
Compatibility issues:
For WarningLevel 2 and lower, info warnings are no longer shown.
Hypothetically, for WarningLevel 6 and higher, opt-in warnings may appear that were not shown previously.
(I found no <WarningLevel>6</WarningLevel> in any fsproj file on github.)
Hey - warning level is a property that auto-increments for C# with each major version of the SDK. So the 8.x SDKs send a WarningLevel of 8 to the compiler for example. This lets diagnostics be created at a level that is below the currently WarningLevel max that will 'light up' automatically as the tooling advances.
A lot of this inference is actually handled in the SDK, so if the F# targets are hard-coding the behavior it should be simple to un-hardcode it and let the SDK take it from there.
Hmm, I understand how this makes sense for the "warning waves", but does it mean there is no longer a way to do what the WarningLevel documentation says?
The WarningLevel option specifies the warning level for the compiler to display.
<WarningLevel>3</WarningLevel>
The element value is the warning level you want displayed for the compilation: Lower numbers show only high severity warnings. Higher numbers show more warnings. The value must be zero or a positive integer
Those original values are special cased in the SDKs MSBuild inference logic, yes - but from 5 onwards the meaning I described (and the docs you linked to themselves link to) takes over.
Or is that MSBuild logic in the language specific targets, and we could keep the traditional logic for F#?
Yeah, the logic I'm speaking of is in a file that is currently only imported for C#/VB projects: https://github.com/dotnet/sdk/blob/05a82501a985078cf1d538b91da5cd1ad2f10373/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets#L1438
My main question is that there is this existing set of semantics that seem to be pretty well aligned with what you want - gradually-increasing strictness - and F# would have to do very little work to start piggybacking on this train, so why do additional work?
The problem are the F# "opt-in warnings". They are not shown unless enabled by warnon. Most of them, currently, have no level (are never checked against WarningLevel). To fit them into the scheme in a backwards-compatible way, I wanted to give them level 6.
Something like warning waves would only be a next step, on top of a (then) consistent set of base levels.
From a back-compat perspective, you could start this system with F# 10/.NET 10 and have Warning Level 10 be the first one that includes all of the current opt-in warnings.
I do like the idea of aligning this with SDK, but there are some warnings which we would likely never want to be turned ON by default for EVERYONE - they are too opinionated and obtrusive.
(e.g. warning about struct defensive copies)
Also, in the history, some PRs for warnings were only accepted on the premise of being optional, but enabling them for everyone (does not really matter if 1,2,3.. years in the future) is a different story - under that premise, they would likely never be accepted at all.
I was actually wondering if we should not turn all these "default-off" warnings into infos. Advantages:
- This fits better their nature,
- They can still be turned on in the same way as now,
- It would enable simpler and more consistent implementation (see PhasedDiagnostics.IsEnabled),
- It would enable the SDK mechanism that @baronfel pointed out.
I would volunteer to implement this if it makes sense for everyone.
I don't like flooding people's projects with infos that come from "opinionated" diagnostics. There is a reason they are opt in - they are not a good fit for everyone.
When we have an infrastructure for custom analyzers, I would happily encourage to move all the opinionated ones to custom analyzers, leaving only the warnings/infos where we can firmly say this is a good diagnostic for every kind of F# user. We just don't have the analyzer setup yet :(.
I'd like to come back then to my original proposal. That way there is no flooding, but more consistency with the F# documentation and with the C# behavior.