dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.83k stars 773 forks source link

Allow #nowarn to support the FS prefix on error codes to disable warnings #17209

Closed KevinRansom closed 2 weeks ago

KevinRansom commented 1 month ago

Execute the code in fsi:

let collection () = [
    yield "Hello"
    "And this"
    ]
collection()

And observe the warning:

      "And this"
  ----^^^^^^^^^^
stdin(3,5): warning FS3221: This expression returns a value of type 'string' but is implicitly discarded. Consider using 'let' to bind the result to a name, e.g. 'let result = expression'. If you intended to use the expression as a value in the sequence then use an explicit 'yield'.

As a keen developer you will observe the error ID FS3221 and say I know #nowarn "FS3221" will fix this right smartly.

If you subject this smart idea to reality by executing code similar to:

#nowarn "FS3221"
let collection () = [
    yield "Hello"
    "And this"
    ]
collection()

Reality will reward you with this - quite surprising - error message:

stdin(7,1): warning FS0203: Invalid warning number 'FS3221'

After some time reading around you might decide to try:

#nowarn "3221"
let collection () = [
    yield "Hello"
    "And this"
    ]
collection()

Which will do what you originally wanted.

Let's make FS# work correctly, you know it makes sense.

abelbraaksma commented 1 month ago

This is a related issue, but about the commandline, which, coincidentally, you fixed ;): https://github.com/dotnet/fsharp/pull/3631

Should this be part of the larger issue with #nowarn as covered here: https://github.com/fsharp/fslang-suggestions/issues/278?

In particular, see this comment, where the suggestion is to allow integers, instead of strings (which I've seen suggested before, btw): https://github.com/fsharp/fslang-suggestions/issues/278#issuecomment-429604143

I've been wanting to tackle this, esp since I've use cases for the warn-on/off scenario, and the #nowarn "FS1234" and #nowarn 1234 syntaxes should be trivial to tackle as well.

KevinRansom commented 1 month ago

@abelbraaksma this wasn't really intended to be a rework of the error/warning systems, it was mainly intended to allow ParsedHashDirectives to support argument types other than strings.

The motivation for that is to perhaps add functionality similar to:

#help SomeNamespace.SomeApi.Somethod

Conveniently it allows #nowarn to easily take int args and fsargs which gives me a concrete use case. And allows us to make the #nowarn and command line arguments consistent too.

As well as

#time on
and
#time off 
abelbraaksma commented 1 month ago

I understand, thanks for explaining. I felt this is as good as any an opportunity to formalize this part of the syntax, using your PR as a guide (there's nothing prescriptive in the spec about it). I tried to capture this and the other ideas in the lang suggestion here: https://github.com/fsharp/fslang-suggestions/issues/1368