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

warnings for invalid #nowarn arguments #17870

Closed
Martin521 opened this issue Oct 11, 2024 · 2 comments · Fixed by #17871
Closed

warnings for invalid #nowarn arguments #17870

Martin521 opened this issue Oct 11, 2024 · 2 comments · Fixed by #17871

Comments

@Martin521
Copy link
Contributor

Martin521 commented Oct 11, 2024

Working on "scoped nowarn", I came across the following items.

  1. With SDK 8.0, a directive #nowarn "FS0020" creates a warning "Invalid warning number 'FS0020'".
    With the current compiler and "--langversion:8.0", there is no warning. (It got lost in Allow ParsedHashDirectives to take non string arguments #17206.)
    The warning should be brought back for "--langversion:8.0".

  2. I also propose to introduces (under the F# 9.0 language flag ParsedHashDirectiveArgumentNonQuotes) warnings for invalid nowarn arguments like in #nowarn "xyz". Specifically, for arguments that do not match the format (FS)?\d+ (for directives) or [A-Z]*\d+ (for compiler options).

  3. Independent of the language version, the range of the argument (rather than the whole directive) should be used for the warning, making it more precise.

Draft PR exists (#17871)

@KevinRansom
Copy link
Member

@Martin521

I believe the compiler does the right thing:

c:\temp>type C:\Users\kevinr\source\repos\ConsoleApp6\ConsoleApp6\Program.fs
//#nowarn "FS0020"

let something =
  2+2               // => FS0020: This expression should have type 'unit'
  "hello"

produces this output:

c:\temp>fsc C:\Users\kevinr\source\repos\ConsoleApp6\ConsoleApp6\Program.fs --langversion:9.0
Microsoft (R) F# Compiler version 12.9.100.0 for F# 9.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

C:\Users\kevinr\source\repos\ConsoleApp6\ConsoleApp6\Program.fs(4,3): warning FS0020: The result of this expression has type 'int' and is implicitly ignored. Consider using 'ignore' to discard this value explicitly, e.g. 'expr |> ignore', or 'let' to bind the result to a name, e.g. 'let result = expr'.

and

c:\temp>fsc C:\Users\kevinr\source\repos\ConsoleApp6\ConsoleApp6\Program.fs --langversion:8.0
Microsoft (R) F# Compiler version 12.9.100.0 for F# 9.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

C:\Users\kevinr\source\repos\ConsoleApp6\ConsoleApp6\Program.fs(4,3): warning FS0020: The result of this expression has type 'int' and is implicitly ignored. Consider using 'ignore' to discard this value explicitly, e.g. 'expr |> ignore', or 'let' to bind the result to a name, e.g. 'let result = expr'.

and this file:
c:\temp>type C:\Users\kevinr\source\repos\ConsoleApp6\ConsoleApp6\Program.fs
#nowarn "FS0020"

let something =
2+2 // => FS0020: This expression should have type 'unit'
"hello"

produces

c:\temp>fsc C:\Users\kevinr\source\repos\ConsoleApp6\ConsoleApp6\Program.fs --langversion:8.0
Microsoft (R) F# Compiler version 12.9.100.0 for F# 9.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

c:\temp>fsc C:\Users\kevinr\source\repos\ConsoleApp6\ConsoleApp6\Program.fs --langversion:9.0
Microsoft (R) F# Compiler version 12.9.100.0 for F# 9.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

c:\temp>

I note that the IDE does not seem to produce the correct out:
Image

@Martin521
Copy link
Contributor Author

Martin521 commented Oct 23, 2024

If you compile the second file under SDK 8.0.xxx (i.e. with the "real" F# 8.0 compiler), you get a FS0203 and a FS0020, like in the IDE.
As I understand the compatibility rules, the new compiler, with --langversion:8.0, should produce the same diagnostics.

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.

2 participants