Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
KevinRansom opened this issue May 21, 2024 · 3 comments · Fixed by #17206
Closed

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

KevinRansom opened this issue May 21, 2024 · 3 comments · Fixed by #17206

Comments

@KevinRansom
Copy link
Member

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.

@github-actions github-actions bot added this to the Backlog milestone May 21, 2024
@KevinRansom KevinRansom changed the title #nowarn does n't support the FS prefix on error codes to disable warning about #nowarn does not support the FS prefix on error codes to disable warning about May 21, 2024
@abelbraaksma
Copy link
Contributor

abelbraaksma commented May 22, 2024

This is a related issue, but about the commandline, which, coincidentally, you fixed ;): #3631

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

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

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 KevinRansom changed the title #nowarn does not support the FS prefix on error codes to disable warning about Allow #nowarn to support the FS prefix on error codes to disable warnings May 24, 2024
@KevinRansom
Copy link
Member Author

@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
Copy link
Contributor

abelbraaksma commented May 25, 2024

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: fsharp/fslang-suggestions#1368

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants