Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
45 changes: 32 additions & 13 deletions src/FSharpLint.Core/Rules/Conventions/UnneededRecKeyword.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading