From f59467b66945a26ef13d93f0c388aab938a3a077 Mon Sep 17 00:00:00 2001 From: webwarrior-ws Date: Thu, 31 Jul 2025 10:42:38 +0200 Subject: [PATCH 1/2] Tests: add tests for rule 85 For mutually-recursive functions (defined with `let rec` and `and`). --- ...TailCallDiagnosticsInRecursiveFunctions.fs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/EnsureTailCallDiagnosticsInRecursiveFunctions.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/EnsureTailCallDiagnosticsInRecursiveFunctions.fs index 9bcca1510..004880985 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Conventions/EnsureTailCallDiagnosticsInRecursiveFunctions.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/EnsureTailCallDiagnosticsInRecursiveFunctions.fs @@ -44,3 +44,33 @@ module Bar = """ Assert.IsTrue this.NoErrorsExist + + [] + member this.``Should error when functions are mutually recursive, but one of them has no [] attribute``() = + this.Parse """ +[] +let rec Foo someParam = + if someParam then + Foo false + else + Bar() +and Bar () = + Foo true +""" + + Assert.IsTrue <| this.ErrorExistsAt(8, 4) + + [] + member this.``Should not error when functions are mutually recursive, and both of them have [] attribute``() = + this.Parse """ +[] +let rec Foo someParam = + if someParam then + Foo false + else + Bar() +and [] Bar () = + Foo true +""" + + Assert.IsTrue this.NoErrorsExist From d5ab47a6ea93714437becc6a5fa66b6f81b3da2e Mon Sep 17 00:00:00 2001 From: webwarrior-ws Date: Thu, 31 Jul 2025 11:20:00 +0200 Subject: [PATCH 2/2] 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. --- ...TailCallDiagnosticsInRecursiveFunctions.fs | 39 +++++++++------- .../Rules/Conventions/UnneededRecKeyword.fs | 45 +++++++++++++------ 2 files changed, 54 insertions(+), 30 deletions(-) diff --git a/src/FSharpLint.Core/Rules/Conventions/EnsureTailCallDiagnosticsInRecursiveFunctions.fs b/src/FSharpLint.Core/Rules/Conventions/EnsureTailCallDiagnosticsInRecursiveFunctions.fs index 64a17a5be..4b6e832d1 100644 --- a/src/FSharpLint.Core/Rules/Conventions/EnsureTailCallDiagnosticsInRecursiveFunctions.fs +++ b/src/FSharpLint.Core/Rules/Conventions/EnsureTailCallDiagnosticsInRecursiveFunctions.fs @@ -20,23 +20,28 @@ let private emitWarning (func: UnneededRecKeyword.RecursiveFunctionInfo) = let runner (args: AstNodeRuleParams) = match args.AstNode, args.CheckInfo with - | UnneededRecKeyword.RecursiveFunction(func), Some checkInfo -> - if UnneededRecKeyword.functionCallsItself checkInfo func then - let hasTailCallAttribute = - func.Attributes - |> List.collect (fun attrs -> attrs.Attributes) - |> List.exists - (fun attr -> - match attr.TypeName with - | SynLongIdent([ident], _, _) -> - ident.idText = "TailCall" || ident.idText = "TailCallAttribute" - | _ -> false) - if hasTailCallAttribute then - Array.empty - else - emitWarning func |> Array.singleton - else - Array.empty + | UnneededRecKeyword.RecursiveFunctions(funcs), Some checkInfo -> + funcs + |> List.choose + (fun functionInfo -> + if UnneededRecKeyword.functionIsCalledInOneOf checkInfo functionInfo funcs then + let hasTailCallAttribute = + functionInfo.Attributes + |> List.collect (fun attrs -> attrs.Attributes) + |> List.exists + (fun attr -> + match attr.TypeName with + | SynLongIdent([ident], _, _) -> + ident.idText = "TailCall" || ident.idText = "TailCallAttribute" + | _ -> false) + if hasTailCallAttribute then + None + else + emitWarning functionInfo |> Some + else + None + ) + |> List.toArray | _ -> Array.empty let rule = diff --git a/src/FSharpLint.Core/Rules/Conventions/UnneededRecKeyword.fs b/src/FSharpLint.Core/Rules/Conventions/UnneededRecKeyword.fs index d302888f8..29b7fb823 100644 --- a/src/FSharpLint.Core/Rules/Conventions/UnneededRecKeyword.fs +++ b/src/FSharpLint.Core/Rules/Conventions/UnneededRecKeyword.fs @@ -17,21 +17,33 @@ type internal RecursiveFunctionInfo = Attributes: SynAttributes } -let internal (|RecursiveFunction|_|) (astNode: AstNode) = +let internal (|RecursiveFunctions|_|) (astNode: AstNode) = match astNode with | AstNode.ModuleDeclaration (SynModuleDecl.Let (true, bindings, _)) -> - match bindings with - | SynBinding (_, _, _, _, attributes, _, _, SynPat.LongIdent (SynLongIdent([ident], _, _), _, _, _, _, range), _, body, _, _, _) :: _ -> - Some { Identifier = ident; Range = range; Body = body; Attributes = attributes } - | _ -> None + let recursiveBindings = + bindings + |> List.choose + (fun binding -> + match binding with + | SynBinding (_, _, _, _, attributes, _, _, SynPat.LongIdent (SynLongIdent([ident], _, _), _, _, _, _, range), _, body, _, _, _) -> + Some { Identifier = ident; Range = range; Body = body; Attributes = attributes } + | _ -> None) + match recursiveBindings with + | [] -> None + | _ -> Some recursiveBindings | _ -> None -let internal functionCallsItself (checkInfo: FSharpCheckFileResults) (func: RecursiveFunctionInfo) = - let funcName = func.Identifier.idText - checkInfo.GetAllUsesOfAllSymbolsInFile() - |> Seq.exists (fun usage -> - usage.Symbol.DisplayName = funcName - && ExpressionUtilities.rangeContainsOtherRange func.Body.Range usage.Range) +let internal functionIsCalledInOneOf (checkInfo: FSharpCheckFileResults) + (callee: RecursiveFunctionInfo) + (callers: list) = + let calleeName = callee.Identifier.idText + callers + |> List.exists + (fun caller -> + checkInfo.GetAllUsesOfAllSymbolsInFile() + |> Seq.exists (fun usage -> + usage.Symbol.DisplayName = calleeName + && ExpressionUtilities.rangeContainsOtherRange caller.Body.Range usage.Range)) let private emitWarning (func: RecursiveFunctionInfo) = { Range = func.Range @@ -45,8 +57,15 @@ let private emitWarning (func: RecursiveFunctionInfo) = let runner (args: AstNodeRuleParams) = match args.AstNode, args.CheckInfo with - | RecursiveFunction(func), Some checkInfo when not (functionCallsItself checkInfo func) -> - emitWarning func |> Array.singleton + | RecursiveFunctions(funcs), Some checkInfo -> + funcs + |> List.choose + (fun functionInfo -> + if not (functionIsCalledInOneOf checkInfo functionInfo funcs) then + emitWarning functionInfo |> Some + else + None) + |> List.toArray | _ -> Array.empty let rule =