warning for empty and non-matching warnon

dotnet · Aug 16, 2024 · 03790e2 · 03790e2
1 parent 4067a14
commit 03790e2
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 22 deletions.
diff --git a/src/Compiler/Driver/ParseAndCheckInputs.fs b/src/Compiler/Driver/ParseAndCheckInputs.fs
@@ -221,24 +221,20 @@ let GetScopedPragmas (langVersion: LanguageVersion) endPos hashDirectives =
         langVersion.SupportsFeature(LanguageFeature.ParsedHashDirectiveArgumentNonQuotes)
 
     let getWarnDirectiveInfos hd =
-        [
-            match hd with
-            | ParsedHashDirective(("nowarn" | "warnon") as hdIdent, args, m) ->
-                for arg in args do
-                    let warningNumber =
-                        match supportsNonStringArguments, arg 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)
-                        | _ -> None // emit warning?
-
-                    match warningNumber with
-                    | None -> ()
-                    | Some n -> (hdIdent, n), m
-            | _ -> ()
-        ]
+        match hd with
+        | ParsedHashDirective(("nowarn" | "warnon") as hdIdent, args, m) ->
+            let tryGetValidWarningNumber arg =
+                match supportsNonStringArguments, arg 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)
+                | _ -> None
+            let validWarningNumbers = args |> List.choose tryGetValidWarningNumber
+            if hdIdent = "warnon" && validWarningNumbers.IsEmpty then warning(Error(FSComp.SR.emptyWarnonDirective(), m))
+            validWarningNumbers |> List.map (fun n -> (hdIdent, n), m)
+        | _ -> []
 
     let processWarnDirectiveInfo (openPragmas, pragmas) ((hdIdent, n), m: range) =
         match hdIdent with
@@ -250,12 +246,12 @@ let GetScopedPragmas (langVersion: LanguageVersion) endPos hashDirectives =
         | "warnon" ->
             match Map.tryFind n openPragmas with
             | Some mm ->
-                let scope =
-                    Range.mkFileIndexRange m.FileIndex (mkPos mm.StartLine 0) (mkPos m.StartLine 0)
-
+                let scope = mkFileIndexRange m.FileIndex (mkPos mm.StartLine 0) (mkPos m.StartLine 0)
                 let pragma = ScopedPragma.WarningOff(scope, n)
                 Map.remove n openPragmas, pragma :: pragmas
-            | None -> openPragmas, pragmas // emit warning?
+            | None ->
+                warning (Error(FSComp.SR.nonMatchingWarnonDirective n, m))
+                openPragmas, pragmas
         | _ -> failwith "unexpected hdIdent in processWarnDirectiveInfo"
 
     let addOpenPragmas (openPragmas, pragmas) =

diff --git a/src/Compiler/FSComp.txt b/src/Compiler/FSComp.txt
@@ -1778,3 +1778,5 @@ featureEmptyBodiedComputationExpressions,"Support for computation expressions wi
 3870,parsExpectingUnionCaseField,"Expecting union case field"
 featureAllowAccessModifiersToAutoPropertiesGettersAndSetters,"Allow access modifiers to auto properties getters and setters"
 3871,tcAccessModifiersNotAllowedInSRTPConstraint,"Access modifiers cannot be applied to an SRTP constraint."
+3872,emptyWarnonDirective,"#warnon without warning numbers is ignored."
+3873,nonMatchingWarnonDirective,"Warning %d is ignored because there is no previous #nowarn directive for this warning."
diff --git a/tests/FSharp.Compiler.ComponentTests/CompilerDirectives/Nowarn.fs b/tests/FSharp.Compiler.ComponentTests/CompilerDirectives/Nowarn.fs
@@ -87,3 +87,39 @@ match None with None -> ()
             // Warning 25, Line 3, Col 7, Line 3, Col 11, matchNoneErrorMessage
             // Warning 25, Line 7, Col 7, Line 7, Col 11, matchNoneErrorMessage
         ]
+
+    let private sourceForWarnIfNoWarnonArgs = """
+module A
+#nowarn "25"
+match None with None -> ()
+#warnon
+match None with None -> ()
+    """
+
+    [<Fact>]
+    let ``warn if a warnon directive has no arguments`` () =
+        FSharp sourceForWarnIfNoWarnonArgs
+        |> withLangVersionPreview
+        |> compile
+        |> withDiagnostics [
+            Warning 3872, Line 5, Col 1, Line 5, Col 8, "#warnon without warning numbers is ignored."
+        ]
+
+    let private sourceForWarnIfNoMatchingWarnon = """
+module A
+#nowarn "25"
+match None with None -> ()
+#warnon "26"
+match None with None -> ()
+    """
+
+    [<Fact>]
+    let ``warn if a warnon directive has no previous matching nowarn directive`` () =
+        FSharp sourceForWarnIfNoMatchingWarnon
+        |> withLangVersionPreview
+        |> compile
+        |> withDiagnostics [
+            Warning 3873, Line 5, Col 1, Line 5, Col 13,
+                "Warning 26 is ignored because there is no previous #nowarn directive for this warning."
+        ]
+