Merge PR #757 from webwarrior-ws/rule85-fix

Improve EnsureTailCallDiagnosticsInRecursiveFunctions rule.

Author: knocte

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 =

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<RecursiveFunctionInfo>) =
+    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 =

tests/FSharpLint.Core.Tests/Rules/Conventions/EnsureTailCallDiagnosticsInRecursiveFunctions.fs

@@ -44,3 +44,33 @@ module Bar =
 """
 
         Assert.IsTrue this.NoErrorsExist
+
+    [<Test>]
+    member this.``Should error when functions are mutually recursive, but one of them has no [<TailCall>] attribute``() =
+        this.Parse """
+[<TailCall>]
+let rec Foo someParam =
+    if someParam then
+        Foo false
+    else
+        Bar()
+and Bar () =
+    Foo true
+"""
+        
+        Assert.IsTrue <| this.ErrorExistsAt(8, 4)
+
+    [<Test>]
+    member this.``Should not error when functions are mutually recursive, and both of them have [<TailCall>] attribute``() =
+        this.Parse """
+[<TailCall>]
+let rec Foo someParam =
+    if someParam then
+        Foo false
+    else
+        Bar()
+and [<TailCall>] Bar () =
+    Foo true
+"""
+        
+        Assert.IsTrue this.NoErrorsExist