-
Notifications
You must be signed in to change notification settings - Fork 72
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #623 from Mersho/AsyncFuncFailWith-squashed
Rule that warns about forgetting "return" keyword when raising exceptions in async blocks. Fixes #597
- Loading branch information
Showing
10 changed files
with
242 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| --- | ||
| title: FL0078 | ||
| category: how-to | ||
| hide_menu: true | ||
| --- | ||
|
|
||
| # AsyncExceptionWithoutReturn (FL0078) | ||
|
|
||
| *Introduced in `0.21.6`* | ||
|
|
||
| ## Cause | ||
|
|
||
| Missing "return" keyword inside async blocks when throwing exceptions. | ||
|
|
||
| ## Rationale | ||
|
|
||
| When returning values or throwing exception in async blocks, the "return" keyword must be used. | ||
|
|
||
| ## How To Fix | ||
|
|
||
| Add "return" keyword to your raise/failwith/failwithf statment. | ||
|
|
||
| ## Rule Settings | ||
|
|
||
| { | ||
| "asyncExceptionWithoutReturn": { "enabled": false } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
73 changes: 73 additions & 0 deletions
73
src/FSharpLint.Core/Rules/Conventions/AsyncExceptionWithoutReturn.fs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| module FSharpLint.Rules.AsyncExceptionWithoutReturn | ||
|
|
||
| open FSharpLint.Framework | ||
| open FSharpLint.Framework.Suggestion | ||
| open FSharp.Compiler.Syntax | ||
| open FSharp.Compiler.Text | ||
| open FSharpLint.Framework.Ast | ||
| open FSharpLint.Framework.Rules | ||
|
|
||
| let rec checkExpression (expression: SynExpr) (range: range) = | ||
| match expression with | ||
| | SynExpr.Sequential (_, _, firstExpression, secondExpression, _) -> | ||
| let result = checkExpression firstExpression range | ||
| Array.append result (checkExpression secondExpression secondExpression.Range) | ||
| | SynExpr.Paren (innerExpression, _, _, range) -> checkExpression innerExpression range | ||
| | SynExpr.While (_, _, innerExpression, range) -> checkExpression innerExpression range | ||
| | SynExpr.For (_, _, _, _, _, innerExpression, range) -> checkExpression innerExpression range | ||
| | SynExpr.ForEach (_, _, _, _, _, innerExpression, range) -> checkExpression innerExpression range | ||
| | SynExpr.Match (_, _, clauses, range) -> | ||
| clauses | ||
| |> List.map (fun (SynMatchClause (_, _, clause, range, _)) -> checkExpression clause range) | ||
| |> List.toArray | ||
| |> Array.concat | ||
| | SynExpr.Do (innerExpression, range) -> checkExpression innerExpression range | ||
| | SynExpr.TryWith (tryExpression, tryRange, withCases, _, _, _, _) -> | ||
| withCases | ||
| |> List.map (fun (SynMatchClause (_, _, withCase, withRange, _)) -> checkExpression withCase withRange) | ||
| |> List.toArray | ||
| |> Array.concat | ||
| |> Array.append (checkExpression tryExpression tryRange) | ||
| | SynExpr.TryFinally (tryExpression, finallyExpr, range, _, _) -> | ||
| checkExpression finallyExpr range | ||
| |> Array.append (checkExpression tryExpression range) | ||
| | SynExpr.IfThenElse (_, thenExpr, elseExpr, _, _, _, range) -> | ||
| let checkThen = checkExpression thenExpr range | ||
|
|
||
| match elseExpr with | ||
| | Some elseExpression -> | ||
| checkThen | ||
| |> Array.append (checkExpression elseExpression range) | ||
| | None -> checkThen | ||
| | SynExpr.App (_, _, SynExpr.Ident failwithId, _, _) when | ||
| failwithId.idText = "failwith" | ||
| || failwithId.idText = "failwithf" | ||
| || failwithId.idText = "raise" | ||
| -> | ||
| { Range = range | ||
| Message = Resources.GetString "RulesAsyncExceptionWithoutReturn" | ||
| SuggestedFix = None | ||
| TypeChecks = List.Empty } | ||
| |> Array.singleton | ||
| | SynExpr.App (_, _, funcExpr, _, range) -> | ||
| checkExpression funcExpr range | ||
| | SynExpr.LetOrUse (_, _, _, body, range) -> | ||
| checkExpression body range | ||
| | _ -> Array.empty | ||
|
|
||
|
|
||
| let runner args = | ||
| match args.AstNode with | ||
| | AstNode.Expression | ||
| ( | ||
| SynExpr.App (_, _, (SynExpr.Ident compExprName), (SynExpr.CompExpr (_, _, innerExpression, _)), range) | ||
| ) when compExprName.idText = "async" -> checkExpression innerExpression range | ||
| | _ -> Array.empty | ||
|
|
||
| let rule = | ||
| { Name = "AsyncExceptionWithoutReturn" | ||
| Identifier = Identifiers.AsyncExceptionWithoutReturn | ||
| RuleConfig = | ||
| { AstNodeRuleConfig.Runner = runner | ||
| Cleanup = ignore } } | ||
| |> AstNodeRule |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
129 changes: 129 additions & 0 deletions
129
tests/FSharpLint.Core.Tests/Rules/Conventions/AsyncExceptionWithoutReturn.fs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| module FSharpLint.Core.Tests.Rules.Conventions.AsyncExceptionWithoutReturn | ||
|
|
||
| open NUnit.Framework | ||
| open FSharpLint.Framework.Rules | ||
| open FSharpLint.Rules | ||
|
|
||
| [<TestFixture>] | ||
| type TestAsyncExceptionWithoutReturn() = | ||
| inherit TestAstNodeRuleBase.TestAstNodeRuleBase(AsyncExceptionWithoutReturn.rule) | ||
|
|
||
| [<Test>] | ||
| member this.AsyncRaiseWithoutReturn() = | ||
| this.Parse(""" | ||
| namespace Program | ||
| let someAsyncFunction = | ||
| async { | ||
| raise (new System.Exception("An error occurred.")) | ||
| return true | ||
| }""") | ||
|
|
||
| Assert.IsTrue this.ErrorsExist | ||
|
|
||
| [<Test>] | ||
| member this.AsyncRaiseWithReturn() = | ||
| this.Parse(""" | ||
| namespace Program | ||
| let someAsyncFunction = | ||
| async { | ||
| return raise (new System.Exception("An error occurred.")) | ||
| }""") | ||
|
|
||
| this.AssertNoWarnings() | ||
|
|
||
| [<Test>] | ||
| member this.AsyncFailWithWithoutReturn() = | ||
| this.Parse(""" | ||
| namespace Program | ||
| let someAsyncFunction = | ||
| async { | ||
| failwith "An error occurred." | ||
| return true | ||
| }""") | ||
|
|
||
| Assert.IsTrue this.ErrorsExist | ||
|
|
||
| [<Test>] | ||
| member this.AsyncFailwithfWithoutReturn_1() = | ||
| this.Parse(""" | ||
| namespace Program | ||
| let someAsyncFunction = | ||
| async { | ||
| let errCode = 78 | ||
| failwithf "Dummy Error Message: %i" errCode | ||
| return true | ||
| }""") | ||
|
|
||
| Assert.IsTrue this.ErrorsExist | ||
|
|
||
| [<Test>] | ||
| member this.AsyncFailwithfWithoutReturn_2() = | ||
| this.Parse(""" | ||
| namespace Program | ||
| let someAsyncFunction = | ||
| async { | ||
| let errCode = 78 | ||
| failwithf "Dummy Error Message: %i" errCode | ||
| }""") | ||
|
|
||
| Assert.IsTrue this.ErrorsExist | ||
|
|
||
| [<Test>] | ||
| member this.AsyncFailwithWithReturn() = | ||
| this.Parse(""" | ||
| namespace Program | ||
| let someAsyncFunction = | ||
| async { | ||
| return failwith "An error occurred." | ||
| }""") | ||
|
|
||
| this.AssertNoWarnings() | ||
|
|
||
| [<Test>] | ||
| member this.AsyncFailwithfWithReturn() = | ||
| this.Parse(""" | ||
| namespace Program | ||
| let someAsyncFunction = | ||
| async { | ||
| let errCode = 78 | ||
| return failwithf "Dummy Error Message: %i" errCode | ||
| }""") | ||
|
|
||
| this.AssertNoWarnings() | ||
|
|
||
| [<Test>] | ||
| member this.AsyncRaiseWithReturnInnerExpression() = | ||
| this.Parse(""" | ||
| namespace Program | ||
| let someAsyncFunction = | ||
| async { | ||
| if 2 = 2 then | ||
| return raise (new System.Exception("An error occurred.")) | ||
| return true | ||
| }""") | ||
|
|
||
| this.AssertNoWarnings() | ||
|
|
||
| [<Test>] | ||
| member this.AsyncRaiseWithoutReturnInnerExpression() = | ||
| this.Parse(""" | ||
| namespace Program | ||
| let someAsyncFunction = | ||
| async { | ||
| if 2 = 2 then | ||
| raise (new System.Exception("An error occurred.")) | ||
| return true | ||
| }""") | ||
|
|
||
| Assert.IsTrue this.ErrorsExist |