Warnings for invalid #nowarn arguments (#17871)

Martin521 · 0101 · web-flow · commit 72cf286e0216 · 2024-11-05T14:57:25.000+01:00
* bringing #nowarn args warnings back

* warnings for invalid #nowarn arguments

* fix v8 case

* formatting

* release notes

* further v8 fixes

* fix duplicate prefix removal and add test for it

* another stupid error fixed and test added

* more fixes, tests

* formatting

* simplification and fixes.

* simplification

* removed regex compilation

* fixes as proposed in review

* avoiding Regex

* Using WarningDescription type to get warning number

* More descriptive variable names

Co-authored-by: Petr Pokorny &lt;petr@innit.cz&gt;

* More descriptive variable names - now correct

---------

Co-authored-by: Petr Pokorny &lt;petr@innit.cz&gt;
diff --git a/docs/release-notes/.FSharp.Compiler.Service/9.0.200.md b/docs/release-notes/.FSharp.Compiler.Service/9.0.200.md
@@ -28,5 +28,6 @@
 * 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 `inherit` objects error reporting. ([PR #17893](https://github.com/dotnet/fsharp/pull/17893))
+* 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
diff --git a/src/Compiler/Driver/CompilerConfig.fs b/src/Compiler/Driver/CompilerConfig.fs
@@ -22,6 +22,7 @@ open FSharp.Compiler.DiagnosticsLogger
 open FSharp.Compiler.Features
 open FSharp.Compiler.IO
 open FSharp.Compiler.CodeAnalysis
+open FSharp.Compiler.Syntax
 open FSharp.Compiler.Text
 open FSharp.Compiler.Text.Range
 open FSharp.Compiler.Xml
@@ -91,24 +92,61 @@ 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
+[<RequireQualifiedAccess>]
+type WarningNumberSource =
+    | CommandLineOption
+    | CompilerDirective
+
+[<RequireQualifiedAccess>]
+type WarningDescription =
+    | Int32 of int
+    | String of string
+    | Ident of Ident
+
+let GetWarningNumber (m, description: WarningDescription, langVersion: LanguageVersion, source: WarningNumberSource) =
+    let argFeature = LanguageFeature.ParsedHashDirectiveArgumentNonQuotes
+
+    let parse (numStr: string) =
+        let trimPrefix (s: string) =
+            if s.StartsWithOrdinal "FS" then s[2..] else s
+
+        let tryParseIntWithFailAction (s: string) (failAction: unit -> unit) =
+            match Int32.TryParse s with
+            | true, n -> Some n
+            | false, _ ->
+                failAction ()
+                None
+
+        let warnInvalid () =
+            warning (Error(FSComp.SR.buildInvalidWarningNumber numStr, m))
+
+        if source = WarningNumberSource.CommandLineOption then
+            tryParseIntWithFailAction (trimPrefix numStr) id
+        elif langVersion.SupportsFeature(argFeature) then
+            tryParseIntWithFailAction (trimPrefix numStr) warnInvalid
+        else
+            tryParseIntWithFailAction numStr id
 
-        if Char.IsDigit(warningNumber[0]) then
-            Some(int32 warningNumber)
+    match description with
+    | WarningDescription.Int32 n ->
+        if tryCheckLanguageFeatureAndRecover langVersion argFeature m then
+            Some n
+        else
+            None
+    | WarningDescription.String s ->
+        if
+            source = WarningNumberSource.CompilerDirective
+            && not (langVersion.SupportsFeature argFeature)
+            && s.StartsWithOrdinal "FS"
+        then
+            warning (Error(FSComp.SR.buildInvalidWarningNumber s, m))
+
+        parse s
+    | WarningDescription.Ident ident ->
+        if tryCheckLanguageFeatureAndRecover langVersion argFeature m then
+            parse ident.idText
         else
             None
-    with _ ->
-        warning (Error(FSComp.SR.buildInvalidWarningNumber warningNumber, m))
-        None
 
 let ComputeMakePathAbsolute implicitIncludeDir (path: string) =
     try
@@ -934,7 +972,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, WarningDescription.String s, tcConfigB.langVersion, WarningNumberSource.CommandLineOption) with
         | None -> ()
         | Some n ->
             // nowarn:62 turns on mlCompatibility, e.g. shows ML compat items in intellisense menus
@@ -949,7 +987,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, WarningDescription.String s, tcConfigB.langVersion, WarningNumberSource.CommandLineOption) with
         | None -> ()
         | Some n ->
             // warnon 62 turns on mlCompatibility, e.g. shows ML compat items in intellisense menus
diff --git a/src/Compiler/Driver/CompilerConfig.fsi b/src/Compiler/Driver/CompilerConfig.fsi
@@ -17,6 +17,7 @@ open FSharp.Compiler.Diagnostics
 open FSharp.Compiler.DiagnosticsLogger
 open FSharp.Compiler.Features
 open FSharp.Compiler.CodeAnalysis
+open FSharp.Compiler.Syntax
 open FSharp.Compiler.Text
 open FSharp.Compiler.BuildGraph
 
@@ -926,7 +927,20 @@ 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
+[<RequireQualifiedAccess>]
+type WarningNumberSource =
+    | CommandLineOption
+    | CompilerDirective
+
+[<RequireQualifiedAccess>]
+type WarningDescription =
+    | Int32 of int
+    | String of string
+    | Ident of Ident
+
+val GetWarningNumber:
+    m: range * description: WarningDescription * langVersion: LanguageVersion * source: WarningNumberSource ->
+        int option
 
 /// Get the name used for FSharp.Core
 val GetFSharpCoreLibraryName: unit -> string
diff --git a/src/Compiler/Driver/CompilerOptions.fs b/src/Compiler/Driver/CompilerOptions.fs
@@ -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 ())
         )
diff --git a/src/Compiler/Driver/ParseAndCheckInputs.fs b/src/Compiler/Driver/ParseAndCheckInputs.fs
@@ -217,25 +217,23 @@ 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
-                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)
+        | ParsedHashDirective("nowarn", args, _) ->
+            for arg in args do
+                let rangeAndDescription =
+                    match arg with
+                    | ParsedHashDirectiveArgument.Int32(n, m) -> Some(m, WarningDescription.Int32 n)
+                    | ParsedHashDirectiveArgument.Ident(ident, m) -> Some(m, WarningDescription.Ident ident)
+                    | ParsedHashDirectiveArgument.String(s, _, m) -> Some(m, WarningDescription.String s)
                     | _ -> None
 
-                match warningNumber with
+                match rangeAndDescription with
                 | None -> ()
-                | Some n -> ScopedPragma.WarningOff(m, n)
+                | Some(m, description) ->
+                    match GetWarningNumber(m, description, langVersion, WarningNumberSource.CompilerDirective) with
+                    | None -> ()
+                    | Some n -> ScopedPragma.WarningOff(m, n)
         | _ -> ()
     ]
 
@@ -912,7 +910,8 @@ let ProcessMetaCommandsFromInput
                 state
 
             | ParsedHashDirective("nowarn", hashArguments, m) ->
-                let arguments = parsedHashDirectiveArguments hashArguments tcConfig.langVersion
+                let arguments = parsedHashDirectiveArgumentsNoCheck hashArguments
+
                 List.fold (fun state d -> nowarnF state (m, d)) state arguments
 
             | ParsedHashDirective(("reference" | "r") as c, [], m) ->
diff --git a/src/Compiler/Interactive/fsi.fs b/src/Compiler/Interactive/fsi.fs
@@ -3839,7 +3839,8 @@ type FsiInteractionProcessor
             istate, Completed None
 
         | ParsedHashDirective("nowarn", nowarnArguments, m) ->
-            let numbers = (parsedHashDirectiveArguments nowarnArguments tcConfigB.langVersion)
+            let numbers = (parsedHashDirectiveArgumentsNoCheck nowarnArguments)
+
             List.iter (fun (d: string) -> tcConfigB.TurnWarningOff(m, d)) numbers
             istate, Completed None
 
diff --git a/src/Compiler/SyntaxTree/SyntaxTreeOps.fs b/src/Compiler/SyntaxTree/SyntaxTreeOps.fs
@@ -1005,6 +1005,16 @@ let parsedHashDirectiveArguments (input: ParsedHashDirectiveArgument list) (lang
             | false -> None)
         input
 
+let parsedHashDirectiveArgumentsNoCheck (input: ParsedHashDirectiveArgument list) =
+    List.map
+        (function
+        | ParsedHashDirectiveArgument.String(s, _, _) -> s
+        | ParsedHashDirectiveArgument.SourceIdentifier(_, v, _) -> v
+        | ParsedHashDirectiveArgument.Int32(n, m) -> string n
+        | ParsedHashDirectiveArgument.Ident(ident, m) -> ident.idText
+        | ParsedHashDirectiveArgument.LongIdent(ident, m) -> longIdentToString ident)
+        input
+
 let parsedHashDirectiveStringArguments (input: ParsedHashDirectiveArgument list) (_langVersion: LanguageVersion) =
     List.choose
         (function
diff --git a/src/Compiler/SyntaxTree/SyntaxTreeOps.fsi b/src/Compiler/SyntaxTree/SyntaxTreeOps.fsi
@@ -322,6 +322,8 @@ val synExprContainsError: inpExpr: SynExpr -> bool
 
 val parsedHashDirectiveArguments: ParsedHashDirectiveArgument list -> LanguageVersion -> string list
 
+val parsedHashDirectiveArgumentsNoCheck: ParsedHashDirectiveArgument list -> string list
+
 val parsedHashDirectiveStringArguments: ParsedHashDirectiveArgument list -> LanguageVersion -> string list
 
 /// 'e1 && e2'
diff --git a/tests/FSharp.Compiler.ComponentTests/CompilerDirectives/NonStringArgs.fs b/tests/FSharp.Compiler.ComponentTests/CompilerDirectives/NonStringArgs.fs
@@ -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'")
             ]
 
 
@@ -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'");
             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'")
             ]
 
 
@@ -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'");
+                (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'")
             ]
 
 
@@ -128,12 +145,32 @@ 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'")
                 ]
         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 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>]
@@ -248,7 +285,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'.")
diff --git a/tests/service/data/SyntaxTree/ParsedHashDirective/TripleQuoteStringAsParsedHashDirectiveArgument.fs.bsl b/tests/service/data/SyntaxTree/ParsedHashDirective/TripleQuoteStringAsParsedHashDirectiveArgument.fs.bsl
@@ -2,7 +2,7 @@ ImplFile
   (ParsedImplFileInput
      ("/root/ParsedHashDirective/TripleQuoteStringAsParsedHashDirectiveArgument.fs",
       false, QualifiedNameOfFile TripleQuoteStringAsParsedHashDirectiveArgument,
-      [WarningOff ((2,0--2,16), 40)], [],
+      [WarningOff ((2,8--2,16), 40)], [],
       [SynModuleOrNamespace
          ([TripleQuoteStringAsParsedHashDirectiveArgument], false, AnonModule,
           [HashDirective
