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 #17871

Merged
merged 36 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
35a4995
bringing #nowarn args warnings back
Martin521 Oct 10, 2024
ba5e277
warnings for invalid #nowarn arguments
Martin521 Oct 10, 2024
f210bfd
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 10, 2024
9d521fa
fix v8 case
Martin521 Oct 11, 2024
a85a29c
formatting
Martin521 Oct 11, 2024
fba907a
release notes
Martin521 Oct 11, 2024
d555e88
further v8 fixes
Martin521 Oct 11, 2024
e564e51
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 11, 2024
d10b2b4
fix duplicate prefix removal and add test for it
Martin521 Oct 11, 2024
526b930
another stupid error fixed and test added
Martin521 Oct 11, 2024
6ae40f7
more fixes, tests
Martin521 Oct 11, 2024
87a95f1
formatting
Martin521 Oct 12, 2024
79ff74c
simplification and fixes.
Martin521 Oct 12, 2024
9f63a5f
simplification
Martin521 Oct 13, 2024
e69bd12
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 15, 2024
36b32c1
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 15, 2024
f4233e0
removed regex compilation
Martin521 Oct 16, 2024
0f17316
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 16, 2024
f4c237a
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 16, 2024
c576d14
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 17, 2024
75b640a
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 24, 2024
c15706a
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 24, 2024
4e00d85
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 24, 2024
9496bae
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 25, 2024
3ac01a3
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 28, 2024
8bd4e67
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 29, 2024
3bf23f6
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 30, 2024
5cc2997
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Oct 31, 2024
31eebf3
fixes as proposed in review
Martin521 Nov 1, 2024
86c372c
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Nov 1, 2024
0f4a215
avoiding Regex
Martin521 Nov 1, 2024
b3a1288
Using WarningDescription type to get warning number
Martin521 Nov 1, 2024
52c3bc8
More descriptive variable names
Martin521 Nov 4, 2024
4da9b36
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Nov 4, 2024
1225abc
More descriptive variable names - now correct
Martin521 Nov 4, 2024
12a5960
Merge remote-tracking branch 'upstream/main' into fix-missing-nowarn-…
Martin521 Nov 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.200.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,6 @@
* Better ranges for CE `use` error reporting. ([PR #17811](https://github.com/dotnet/fsharp/pull/17811))
* Better ranges for `inherit` error reporting. ([PR #17879](https://github.com/dotnet/fsharp/pull/17879))
* Better ranges for `inherit` `struct` error reporting. ([PR #17886](https://github.com/dotnet/fsharp/pull/17886))
* Better ranges for #nowarn error reporting; bring back #nowarn warnings for --langVersion:80; add warnings under feature flag ([PR #17871](https://github.com/dotnet/fsharp/pull/17871))

### Breaking Changes
46 changes: 30 additions & 16 deletions src/Compiler/Driver/CompilerConfig.fs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ open FSharp.Compiler.Text.Range
open FSharp.Compiler.Xml
open FSharp.Compiler.TypedTree
open FSharp.Compiler.BuildGraph
open System.Text.RegularExpressions

#if !NO_TYPEPROVIDERS
open FSharp.Core.CompilerServices
Expand Down Expand Up @@ -91,24 +92,37 @@ let ResolveFileUsingPaths (paths, m, fileName) =
let searchMessage = String.concat "\n " paths
raise (FileNameNotResolved(fileName, searchMessage, m))

let GetWarningNumber (m, warningNumber: string, prefixSupported) =
try
let warningNumber =
if warningNumber.StartsWithOrdinal "FS" then
if prefixSupported then
warningNumber.Substring 2
else
raise (new ArgumentException())
else
warningNumber
let GetWarningNumber (m, numStr: string, langVersion: LanguageVersion, isCompilerOption: bool) =
let argFeature = LanguageFeature.ParsedHashDirectiveArgumentNonQuotes
let errorInvalid = Error(FSComp.SR.buildInvalidWarningNumber numStr, m)
T-Gro marked this conversation as resolved.
Show resolved Hide resolved
let rxOptions = RegexOptions.CultureInvariant

let getNumber regexString failAction =
let mtch = Regex(regexString, rxOptions).Match(numStr)
T-Gro marked this conversation as resolved.
Show resolved Hide resolved

if Char.IsDigit(warningNumber[0]) then
Some(int32 warningNumber)
if mtch.Success then
Some(int mtch.Groups[2].Captures[0].Value)
else
failAction ()
None
with _ ->
warning (Error(FSComp.SR.buildInvalidWarningNumber warningNumber, m))

let matches regexString =
Regex(regexString, rxOptions).Match(numStr).Success

if isCompilerOption then
getNumber """^(FS)?(\d+)$""" (fun () ->
if not (matches """^([A-Z]+)(\d+)$""") then
warning errorInvalid)
elif langVersion.SupportsFeature(argFeature) then
getNumber """^("?)(?:(?:FS)?(\d+))\1$""" (fun () -> warning errorInvalid)
elif matches """^[^"].+$""" then
errorR (languageFeatureError langVersion argFeature m)
None
elif matches """^"FS.*"$""" then
warning errorInvalid
None
else
getNumber """^(")(\d+)"$""" (fun () -> ())

let ComputeMakePathAbsolute implicitIncludeDir (path: string) =
try
Expand Down Expand Up @@ -931,7 +945,7 @@ type TcConfigBuilder =
member tcConfigB.TurnWarningOff(m, s: string) =
use _ = UseBuildPhase BuildPhase.Parameter

match GetWarningNumber(m, s, tcConfigB.langVersion.SupportsFeature(LanguageFeature.ParsedHashDirectiveArgumentNonQuotes)) with
match GetWarningNumber(m, s, tcConfigB.langVersion, true) with
| None -> ()
| Some n ->
// nowarn:62 turns on mlCompatibility, e.g. shows ML compat items in intellisense menus
Expand All @@ -946,7 +960,7 @@ type TcConfigBuilder =
member tcConfigB.TurnWarningOn(m, s: string) =
use _ = UseBuildPhase BuildPhase.Parameter

match GetWarningNumber(m, s, tcConfigB.langVersion.SupportsFeature(LanguageFeature.ParsedHashDirectiveArgumentNonQuotes)) with
match GetWarningNumber(m, s, tcConfigB.langVersion, true) with
| None -> ()
| Some n ->
// warnon 62 turns on mlCompatibility, e.g. shows ML compat items in intellisense menus
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Driver/CompilerConfig.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,7 @@ val TryResolveFileUsingPaths: paths: string seq * m: range * fileName: string ->

val ResolveFileUsingPaths: paths: string seq * m: range * fileName: string -> string

val GetWarningNumber: m: range * warningNumber: string * prefixSupported: bool -> int option
val GetWarningNumber: m: range * numStr: string * langVersion: LanguageVersion * isCompilerOption: bool -> int option
T-Gro marked this conversation as resolved.
Show resolved Hide resolved

/// Get the name used for FSharp.Core
val GetFSharpCoreLibraryName: unit -> string
Expand Down
4 changes: 2 additions & 2 deletions src/Compiler/Driver/CompilerOptions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -832,15 +832,15 @@ let errorsAndWarningsFlags (tcConfigB: TcConfigBuilder) =
CompilerOption(
"nowarn",
tagWarnList,
OptionStringList(fun n -> tcConfigB.TurnWarningOff(rangeCmdArgs, trimFS n)),
OptionStringList(fun n -> tcConfigB.TurnWarningOff(rangeCmdArgs, n)),
None,
Some(FSComp.SR.optsNowarn ())
)

CompilerOption(
"warnon",
tagWarnList,
OptionStringList(fun n -> tcConfigB.TurnWarningOn(rangeCmdArgs, trimFS n)),
OptionStringList(fun n -> tcConfigB.TurnWarningOn(rangeCmdArgs, n)),
None,
Some(FSComp.SR.optsWarnOn ())
)
Expand Down
21 changes: 9 additions & 12 deletions src/Compiler/Driver/ParseAndCheckInputs.fs
Original file line number Diff line number Diff line change
Expand Up @@ -217,20 +217,15 @@ let PostParseModuleSpec (_i, defaultNamespace, isLastCompiland, fileName, intf)
SynModuleOrNamespaceSig(lid, isRecursive, kind, decls, xmlDoc, attributes, None, range, trivia)

let GetScopedPragmasForHashDirective hd (langVersion: LanguageVersion) =
let supportsNonStringArguments =
langVersion.SupportsFeature(LanguageFeature.ParsedHashDirectiveArgumentNonQuotes)

[
match hd with
| ParsedHashDirective("nowarn", numbers, m) ->
for s in numbers do
| ParsedHashDirective("nowarn", args, m) ->
for s in args do
let warningNumber =
match supportsNonStringArguments, s with
| _, ParsedHashDirectiveArgument.SourceIdentifier _ -> None
| true, ParsedHashDirectiveArgument.LongIdent _ -> None
| true, ParsedHashDirectiveArgument.Int32(n, _) -> GetWarningNumber(m, string n, true)
| true, ParsedHashDirectiveArgument.Ident(s, _) -> GetWarningNumber(m, s.idText, true)
| _, ParsedHashDirectiveArgument.String(s, _, _) -> GetWarningNumber(m, s, true)
match s with
| ParsedHashDirectiveArgument.Int32(n, m) -> GetWarningNumber(m, string n, langVersion, false)
T-Gro marked this conversation as resolved.
Show resolved Hide resolved
| ParsedHashDirectiveArgument.Ident(s, m) -> GetWarningNumber(m, s.idText, langVersion, false)
| ParsedHashDirectiveArgument.String(s, _, m) -> GetWarningNumber(m, $"\"{s}\"", langVersion, false)
T-Gro marked this conversation as resolved.
Show resolved Hide resolved
| _ -> None

match warningNumber with
Expand Down Expand Up @@ -912,7 +907,9 @@ let ProcessMetaCommandsFromInput
state

| ParsedHashDirective("nowarn", hashArguments, m) ->
let arguments = parsedHashDirectiveArguments hashArguments tcConfig.langVersion
let arguments =
parsedHashDirectiveArgumentsNoCheck hashArguments tcConfig.langVersion

List.fold (fun state d -> nowarnF state (m, d)) state arguments

| ParsedHashDirective(("reference" | "r") as c, [], m) ->
Expand Down
4 changes: 3 additions & 1 deletion src/Compiler/Interactive/fsi.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3844,7 +3844,9 @@ type FsiInteractionProcessor
istate, Completed None

| ParsedHashDirective("nowarn", nowarnArguments, m) ->
let numbers = (parsedHashDirectiveArguments nowarnArguments tcConfigB.langVersion)
let numbers =
(parsedHashDirectiveArgumentsNoCheck nowarnArguments tcConfigB.langVersion)

List.iter (fun (d: string) -> tcConfigB.TurnWarningOff(m, d)) numbers
istate, Completed None

Expand Down
10 changes: 10 additions & 0 deletions src/Compiler/SyntaxTree/SyntaxTreeOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,16 @@ let parsedHashDirectiveArguments (input: ParsedHashDirectiveArgument list) (lang
| false -> None)
input

let parsedHashDirectiveArgumentsNoCheck (input: ParsedHashDirectiveArgument list) (langVersion: LanguageVersion) =
T-Gro marked this conversation as resolved.
Show resolved Hide resolved
List.choose
(function
T-Gro marked this conversation as resolved.
Show resolved Hide resolved
| ParsedHashDirectiveArgument.String(s, _, _) -> Some s
| ParsedHashDirectiveArgument.SourceIdentifier(_, v, _) -> Some v
| ParsedHashDirectiveArgument.Int32(n, m) -> Some(string n)
| ParsedHashDirectiveArgument.Ident(ident, m) -> Some(ident.idText)
| ParsedHashDirectiveArgument.LongIdent(ident, m) -> Some(longIdentToString ident))
input

let parsedHashDirectiveStringArguments (input: ParsedHashDirectiveArgument list) (_langVersion: LanguageVersion) =
List.choose
(function
Expand Down
2 changes: 2 additions & 0 deletions src/Compiler/SyntaxTree/SyntaxTreeOps.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,8 @@ val synExprContainsError: inpExpr: SynExpr -> bool

val parsedHashDirectiveArguments: ParsedHashDirectiveArgument list -> LanguageVersion -> string list

val parsedHashDirectiveArgumentsNoCheck: ParsedHashDirectiveArgument list -> LanguageVersion -> string list

val parsedHashDirectiveStringArguments: ParsedHashDirectiveArgument list -> LanguageVersion -> string list

/// 'e1 && e2'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,20 @@ module NonStringArgs =
|> asExe
|> compile
|> shouldFail
|> withDiagnostics[
|> withDiagnostics [
if languageVersion = "8.0" then
(Warning 203, Line 6, Col 1, Line 6, Col 13, "Invalid warning number 'FS'")
(Error 3350, Line 3, Col 9, Line 3, Col 11, "Feature '# directives with non-quoted string arguments' is not available in F# 8.0. Please use language version 9.0 or greater.")
(Error 3350, Line 4, Col 9, Line 4, Col 15, "Feature '# directives with non-quoted string arguments' is not available in F# 8.0. Please use language version 9.0 or greater.")
(Error 3350, Line 5, Col 9, Line 5, Col 13, "Feature '# directives with non-quoted string arguments' is not available in F# 8.0. Please use language version 9.0 or greater.")
(Warning 203, Line 6, Col 9, Line 6, Col 13, """Invalid warning number '"FS"'""");
(Warning 203, Line 7, Col 9, Line 7, Col 17, """Invalid warning number '"FSBLAH"'""");
else
(Warning 203, Line 3, Col 1, Line 3, Col 11, "Invalid warning number 'FS'")
(Warning 203, Line 6, Col 1, Line 6, Col 13, "Invalid warning number 'FS'")
(Warning 203, Line 3, Col 9, Line 3, Col 11, "Invalid warning number 'FS'");
(Warning 203, Line 4, Col 9, Line 4, Col 15, "Invalid warning number 'FSBLAH'");
(Warning 203, Line 5, Col 9, Line 5, Col 13, "Invalid warning number 'ACME'");
(Warning 203, Line 6, Col 9, Line 6, Col 13, """Invalid warning number '"FS"'""");
(Warning 203, Line 7, Col 9, Line 7, Col 17, """Invalid warning number '"FSBLAH"'""");
(Warning 203, Line 8, Col 9, Line 8, Col 15, """Invalid warning number '"ACME"'""")
]


Expand All @@ -55,14 +60,20 @@ module NonStringArgs =
|> asExe
|> compile
|> shouldFail
|> withDiagnostics[
|> withDiagnostics [
if languageVersion = "8.0" then
(Warning 203, Line 2, Col 1, Line 9, Col 11, "Invalid warning number 'FS'")
(Error 3350, Line 4, Col 5, Line 4, Col 7, "Feature '# directives with non-quoted string arguments' is not available in F# 8.0. Please use language version 9.0 or greater.")
(Error 3350, Line 5, Col 5, Line 5, Col 11, "Feature '# directives with non-quoted string arguments' is not available in F# 8.0. Please use language version 9.0 or greater.")
(Error 3350, Line 6, Col 5, Line 6, Col 9, "Feature '# directives with non-quoted string arguments' is not available in F# 8.0. Please use language version 9.0 or greater.")
(Warning 203, Line 7, Col 5, Line 7, Col 9, """Invalid warning number '"FS"'""");
(Warning 203, Line 8, Col 5, Line 8, Col 13, """Invalid warning number '"FSBLAH"'""");
T-Gro marked this conversation as resolved.
Show resolved Hide resolved
else
(Warning 203, Line 2, Col 1, Line 9, Col 11, "Invalid warning number 'FS'")
(Warning 203, Line 4, Col 5, Line 4, Col 7, "Invalid warning number 'FS'");
(Warning 203, Line 5, Col 5, Line 5, Col 11, "Invalid warning number 'FSBLAH'");
(Warning 203, Line 6, Col 5, Line 6, Col 9, "Invalid warning number 'ACME'");
(Warning 203, Line 7, Col 5, Line 7, Col 9, """Invalid warning number '"FS"'""");
(Warning 203, Line 8, Col 5, Line 8, Col 13, """Invalid warning number '"FSBLAH"'""");
(Warning 203, Line 9, Col 5, Line 9, Col 11, """Invalid warning number '"ACME"'""")
]


Expand All @@ -81,12 +92,18 @@ module NonStringArgs =
|> shouldFail
|> withDiagnostics [
if languageVersion = "8.0" then
(Warning 203, Line 3, Col 1, Line 3, Col 44, "Invalid warning number 'FS'")
(Error 3350, Line 3, Col 9, Line 3, Col 11, "Feature '# directives with non-quoted string arguments' is not available in F# 8.0. Please use language version 9.0 or greater.")
(Error 3350, Line 3, Col 12, Line 3, Col 18, "Feature '# directives with non-quoted string arguments' is not available in F# 8.0. Please use language version 9.0 or greater.")
(Error 3350, Line 3, Col 19, Line 3, Col 23, "Feature '# directives with non-quoted string arguments' is not available in F# 8.0. Please use language version 9.0 or greater.")
(Warning 203, Line 3, Col 24, Line 3, Col 28, """Invalid warning number '"FS"'""");
(Warning 203, Line 3, Col 29, Line 3, Col 37, """Invalid warning number '"FSBLAH"'""");
else
(Warning 203, Line 3, Col 1, Line 3, Col 44, "Invalid warning number 'FS'")
(Warning 203, Line 3, Col 9, Line 3, Col 11, "Invalid warning number 'FS'");
T-Gro marked this conversation as resolved.
Show resolved Hide resolved
(Warning 203, Line 3, Col 12, Line 3, Col 18, "Invalid warning number 'FSBLAH'");
(Warning 203, Line 3, Col 19, Line 3, Col 23, "Invalid warning number 'ACME'");
(Warning 203, Line 3, Col 24, Line 3, Col 28, """Invalid warning number '"FS"'""");
(Warning 203, Line 3, Col 29, Line 3, Col 37, """Invalid warning number '"FSBLAH"'""");
(Warning 203, Line 3, Col 38, Line 3, Col 44, """Invalid warning number '"ACME"'""")
]


Expand Down Expand Up @@ -128,12 +145,34 @@ module DoBinding =
(Warning 1104, Line 5, Col 15, Line 5, Col 31, "Identifiers containing '@' are reserved for use in F# code generation")
(Error 3350, Line 2, Col 9, Line 2, Col 11, "Feature '# directives with non-quoted string arguments' is not available in F# 8.0. Please use language version 9.0 or greater.")
(Error 3350, Line 2, Col 12, Line 2, Col 18, "Feature '# directives with non-quoted string arguments' is not available in F# 8.0. Please use language version 9.0 or greater.")
(Warning 203, Line 2, Col 26, Line 2, Col 34, """Invalid warning number '"FS3221"'""")
T-Gro marked this conversation as resolved.
Show resolved Hide resolved
]
else
compileResult
|> shouldSucceed


[<InlineData("8.0")>]
[<InlineData("9.0")>]
[<Theory>]
let ``#nowarn - errors - compiler options`` (languageVersion) =

FSharp """
match None with None -> () // creates FS0025 - ignored due to flag
"" // creates FS0020 - ignored due to flag
""" // creates FS0988 - not ignored, different flag prefix
|> withLangVersion languageVersion
|> withOptions ["--nowarn:NU0988"; "--nowarn:FS25"; "--nowarn:20"; "--nowarn:FS"; "--nowarn:FSBLAH"]
|> asExe
|> compile
|> shouldFail
|> withDiagnostics [
(Warning 203, Line 0, Col 1, Line 0, Col 1, "Invalid warning number 'FS'")
(Warning 203, Line 0, Col 1, Line 0, Col 1, "Invalid warning number 'FSBLAH'")
(Warning 988, Line 3, Col 3, Line 3, Col 3, "Main module of program is empty: nothing will happen when it is run")
]


[<InlineData("8.0")>]
[<InlineData("9.0")>]
[<Theory>]
Expand Down Expand Up @@ -248,7 +287,7 @@ printfn "Hello, World"
|> asExe
|> compile
|> shouldFail
|> withDiagnostics[
|> withDiagnostics [
(Error 76, Line 2, Col 9, Line 2, Col 11, "This directive may only be used in F# script files (extensions .fsx or .fsscript). Either remove the directive, move this code to a script file or delimit the directive with '#if INTERACTIVE'/'#endif'.")
(Error 76, Line 3, Col 9, Line 3, Col 14, "This directive may only be used in F# script files (extensions .fsx or .fsscript). Either remove the directive, move this code to a script file or delimit the directive with '#if INTERACTIVE'/'#endif'.")
(Error 76, Line 4, Col 9, Line 4, Col 17, "This directive may only be used in F# script files (extensions .fsx or .fsscript). Either remove the directive, move this code to a script file or delimit the directive with '#if INTERACTIVE'/'#endif'.")
Expand Down
Loading