Skip to content

Commit d5ab47a

Browse files
committed
EnsureTailCallDiagnosticsInRecursiveFunctions: fix rule
Check all definitions in `let rec ... and ...` chain, not only the first one. Since it reuses code from UnneededRecKeyword, there are some changes in that rule's code as well.
1 parent f59467b commit d5ab47a

File tree

2 files changed

+54
-30
lines changed

2 files changed

+54
-30
lines changed

src/FSharpLint.Core/Rules/Conventions/EnsureTailCallDiagnosticsInRecursiveFunctions.fs

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,28 @@ let private emitWarning (func: UnneededRecKeyword.RecursiveFunctionInfo) =
2020

2121
let runner (args: AstNodeRuleParams) =
2222
match args.AstNode, args.CheckInfo with
23-
| UnneededRecKeyword.RecursiveFunction(func), Some checkInfo ->
24-
if UnneededRecKeyword.functionCallsItself checkInfo func then
25-
let hasTailCallAttribute =
26-
func.Attributes
27-
|> List.collect (fun attrs -> attrs.Attributes)
28-
|> List.exists
29-
(fun attr ->
30-
match attr.TypeName with
31-
| SynLongIdent([ident], _, _) ->
32-
ident.idText = "TailCall" || ident.idText = "TailCallAttribute"
33-
| _ -> false)
34-
if hasTailCallAttribute then
35-
Array.empty
36-
else
37-
emitWarning func |> Array.singleton
38-
else
39-
Array.empty
23+
| UnneededRecKeyword.RecursiveFunctions(funcs), Some checkInfo ->
24+
funcs
25+
|> List.choose
26+
(fun functionInfo ->
27+
if UnneededRecKeyword.functionIsCalledInOneOf checkInfo functionInfo funcs then
28+
let hasTailCallAttribute =
29+
functionInfo.Attributes
30+
|> List.collect (fun attrs -> attrs.Attributes)
31+
|> List.exists
32+
(fun attr ->
33+
match attr.TypeName with
34+
| SynLongIdent([ident], _, _) ->
35+
ident.idText = "TailCall" || ident.idText = "TailCallAttribute"
36+
| _ -> false)
37+
if hasTailCallAttribute then
38+
None
39+
else
40+
emitWarning functionInfo |> Some
41+
else
42+
None
43+
)
44+
|> List.toArray
4045
| _ -> Array.empty
4146

4247
let rule =

src/FSharpLint.Core/Rules/Conventions/UnneededRecKeyword.fs

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,33 @@ type internal RecursiveFunctionInfo =
1717
Attributes: SynAttributes
1818
}
1919

20-
let internal (|RecursiveFunction|_|) (astNode: AstNode) =
20+
let internal (|RecursiveFunctions|_|) (astNode: AstNode) =
2121
match astNode with
2222
| AstNode.ModuleDeclaration (SynModuleDecl.Let (true, bindings, _)) ->
23-
match bindings with
24-
| SynBinding (_, _, _, _, attributes, _, _, SynPat.LongIdent (SynLongIdent([ident], _, _), _, _, _, _, range), _, body, _, _, _) :: _ ->
25-
Some { Identifier = ident; Range = range; Body = body; Attributes = attributes }
26-
| _ -> None
23+
let recursiveBindings =
24+
bindings
25+
|> List.choose
26+
(fun binding ->
27+
match binding with
28+
| SynBinding (_, _, _, _, attributes, _, _, SynPat.LongIdent (SynLongIdent([ident], _, _), _, _, _, _, range), _, body, _, _, _) ->
29+
Some { Identifier = ident; Range = range; Body = body; Attributes = attributes }
30+
| _ -> None)
31+
match recursiveBindings with
32+
| [] -> None
33+
| _ -> Some recursiveBindings
2734
| _ -> None
2835

29-
let internal functionCallsItself (checkInfo: FSharpCheckFileResults) (func: RecursiveFunctionInfo) =
30-
let funcName = func.Identifier.idText
31-
checkInfo.GetAllUsesOfAllSymbolsInFile()
32-
|> Seq.exists (fun usage ->
33-
usage.Symbol.DisplayName = funcName
34-
&& ExpressionUtilities.rangeContainsOtherRange func.Body.Range usage.Range)
36+
let internal functionIsCalledInOneOf (checkInfo: FSharpCheckFileResults)
37+
(callee: RecursiveFunctionInfo)
38+
(callers: list<RecursiveFunctionInfo>) =
39+
let calleeName = callee.Identifier.idText
40+
callers
41+
|> List.exists
42+
(fun caller ->
43+
checkInfo.GetAllUsesOfAllSymbolsInFile()
44+
|> Seq.exists (fun usage ->
45+
usage.Symbol.DisplayName = calleeName
46+
&& ExpressionUtilities.rangeContainsOtherRange caller.Body.Range usage.Range))
3547

3648
let private emitWarning (func: RecursiveFunctionInfo) =
3749
{ Range = func.Range
@@ -45,8 +57,15 @@ let private emitWarning (func: RecursiveFunctionInfo) =
4557

4658
let runner (args: AstNodeRuleParams) =
4759
match args.AstNode, args.CheckInfo with
48-
| RecursiveFunction(func), Some checkInfo when not (functionCallsItself checkInfo func) ->
49-
emitWarning func |> Array.singleton
60+
| RecursiveFunctions(funcs), Some checkInfo ->
61+
funcs
62+
|> List.choose
63+
(fun functionInfo ->
64+
if not (functionIsCalledInOneOf checkInfo functionInfo funcs) then
65+
emitWarning functionInfo |> Some
66+
else
67+
None)
68+
|> List.toArray
5069
| _ -> Array.empty
5170

5271
let rule =

0 commit comments

Comments
 (0)