From becbf688773d72f6bd0883d2799119a667b66f48 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Thu, 14 Aug 2025 11:28:52 +0200 Subject: [PATCH 1/4] Tests(FailwithBadUsage): expose failwithf false negative --- .../Rules/Conventions/FailwithBadUsage.fs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/FailwithBadUsage.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/FailwithBadUsage.fs index 511c7d7e8..203fb545f 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Conventions/FailwithBadUsage.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/FailwithBadUsage.fs @@ -110,6 +110,19 @@ with Assert.IsTrue this.ErrorsExist Assert.IsTrue(this.ErrorExistsAt(6, 4)) + [] + member this.FailwithfShouldNotSwallowExceptions2() = + this.Parse """ +try + foo() +with +| e -> + failwithf "bar %i" 42 +""" + + Assert.IsTrue this.ErrorsExist + Assert.IsTrue(this.ErrorExistsOnLine 6) + [] member this.FailwithWithfGoodArguments2() = this.Parse """ From 0853766850c6e9f3564fdf6ec83e1676fc8b40b6 Mon Sep 17 00:00:00 2001 From: webwarrior-ws Date: Thu, 14 Aug 2025 12:09:21 +0200 Subject: [PATCH 2/4] FailwithBadUsage: fixed case with failwithf application Fixed case when failwithf is used with arguments, like `failwithf "bar %i" 42`. --- .../Conventions/RaiseWithTooManyArguments/FailwithBadUsage.fs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/FSharpLint.Core/Rules/Conventions/RaiseWithTooManyArguments/FailwithBadUsage.fs b/src/FSharpLint.Core/Rules/Conventions/RaiseWithTooManyArguments/FailwithBadUsage.fs index 619a6cd4a..1ac1b8063 100644 --- a/src/FSharpLint.Core/Rules/Conventions/RaiseWithTooManyArguments/FailwithBadUsage.fs +++ b/src/FSharpLint.Core/Rules/Conventions/RaiseWithTooManyArguments/FailwithBadUsage.fs @@ -67,6 +67,8 @@ let private runner (args: AstNodeRuleParams) = let rec checkExpr node maybeIdentifier = match node with + | SynExpr.App (_, _, (SynExpr.App(_) as innerExpr), _, _) -> + checkExpr innerExpr maybeIdentifier | SynExpr.App (_, _, SynExpr.Ident failwithId, expression, range) when failwithId.idText = "failwith" || failwithId.idText = "failwithf" From 88d814c4e7d01cc427397a30700a9d9aa9f38915 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Thu, 14 Aug 2025 18:21:23 +0200 Subject: [PATCH 3/4] Tests(FailwithBadUsage): fix test name & add missing test The failwithf func was not being tested properly with an arg. --- .../Rules/Conventions/FailwithBadUsage.fs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/FailwithBadUsage.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/FailwithBadUsage.fs index 203fb545f..44712debe 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Conventions/FailwithBadUsage.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/FailwithBadUsage.fs @@ -124,7 +124,16 @@ with Assert.IsTrue(this.ErrorExistsOnLine 6) [] - member this.FailwithWithfGoodArguments2() = + member this.FailwithfWithGoodArguments() = + this.Parse """ +let foo () = + failwithf "foo %i" 42 +""" + + this.AssertNoWarnings() + + [] + member this.FailwithfWithGoodArguments2() = this.Parse """ let foo () = failwithf "foo" From 8c60856f6fb50d5871c40cb1e43dfe37f6ff8c2d Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Thu, 14 Aug 2025 18:34:31 +0200 Subject: [PATCH 4/4] Tests(FailwithBadUsage): change most err msgs This might likely fix the flaky tests that we started getting lately. Maybe they were exposed by some recent change that made them run in parallel [1]. [1] ca158b898da24984828d6e5286671078daf91deb Closes #763 --- .../Rules/Conventions/FailwithBadUsage.fs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/FailwithBadUsage.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/FailwithBadUsage.fs index 44712debe..fdec0ba3a 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Conventions/FailwithBadUsage.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/FailwithBadUsage.fs @@ -32,7 +32,7 @@ let bar () = member this.FailwithWithBadArgumentsEmptyMessage3() = this.Parse """ let foo () = - failwith "foo" + failwith "foo1" let bar () = failwith String.Empty """ @@ -44,9 +44,9 @@ let bar () = member this.FailwithWithGoodArguments() = this.Parse """ let foo () = - failwith "foo" + failwith "foo2" let bar () = - failwith "bar" + failwith "bar2" """ this.AssertNoWarnings() @@ -55,7 +55,7 @@ let bar () = member this.FailwithWithGoodArguments2() = this.Parse """ let foo () = - failwith "foo" + failwith "foo3" """ this.AssertNoWarnings() @@ -79,7 +79,7 @@ try foo() with | e -> - failwith "bar" + failwith "bar4" """ Assert.IsTrue this.ErrorsExist @@ -92,7 +92,7 @@ try foo() with | e -> - raise new Exception("bar",e) + raise new Exception("bar5", e) """ Assert.IsTrue this.NoErrorsExist @@ -104,7 +104,7 @@ try foo() with | e -> - failwithf "bar" + failwithf "bar6" """ Assert.IsTrue this.ErrorsExist @@ -117,7 +117,7 @@ try foo() with | e -> - failwithf "bar %i" 42 + failwithf "bar7 %i" 42 """ Assert.IsTrue this.ErrorsExist @@ -127,7 +127,7 @@ with member this.FailwithfWithGoodArguments() = this.Parse """ let foo () = - failwithf "foo %i" 42 + failwithf "foo8 %i" 42 """ this.AssertNoWarnings() @@ -136,7 +136,7 @@ let foo () = member this.FailwithfWithGoodArguments2() = this.Parse """ let foo () = - failwithf "foo" + failwithf "foo9" """ this.AssertNoWarnings()