Skip to content

Commit f9af6eb

Browse files
knocteknocte
authored
Merge PR #673 from webwarrior-ws/fix-651
Add EnsureTailCallDiagnosticsInRecursiveFunctions rule.
2 parents 01c9bc9 + 77839b3 commit f9af6eb

File tree

11 files changed

+175
-28
lines changed

11 files changed

+175
-28
lines changed

‎docs/content/how-tos/rule-configuration.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,3 +125,4 @@ The following rules can be specified for linting.
125125
- [UsedUnderscorePrefixedElements (FL0082)](rules/FL0082.html)
126126
- [UnneededRecKeyword (FL0083)](rules/FL0083.html)
127127
- [FavourNonMutablePropertyInitialization (FL0084)](rules/FL0084.html)
128+
- [EnsureTailCallDiagnosticsInRecursiveFunctions (FL0085)](rules/FL0085.html)

‎docs/content/how-tos/rules/FL0085.md‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
title: FL0085
3+
category: how-to
4+
hide_menu: true
5+
---
6+
7+
# EnsureTailCallDiagnosticsInRecursiveFunctions (FL0085)
8+
9+
*Introduced in `0.24.2`*
10+
11+
## Cause
12+
13+
Recursive function is not marked as tail-recursive using `[<TailCall>]` attribute.
14+
15+
## Rationale
16+
17+
Tail-recursive functions have no risk of stack overflow and perform better.
18+
Adding `[<TailCall>]` attribute to recursive function makes compiler check that it's indeed tail-recursive and give warning otherwise.
19+
`<WarningsAsErrors>FS3569</WarningsAsErrors>` property in project file will make compiler treat this as warning as error.
20+
Requires .NET 8 or later.
21+
22+
## How To Fix
23+
24+
Add `[<TailCall>]` attribute to your function.
25+
26+
## Rule Settings
27+
28+
{
29+
"ensureTailCallDiagnosticsInRecursiveFunctions": {
30+
"enabled": true
31+
}
32+
}

‎src/FSharpLint.Core/Application/Configuration.fs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,8 @@ type ConventionsConfig =
329329
favourReRaise:EnabledConfig option
330330
favourConsistentThis:RuleConfig<FavourConsistentThis.Config> option
331331
suggestUseAutoProperty:EnabledConfig option
332-
usedUnderscorePrefixedElements:EnabledConfig option }
332+
usedUnderscorePrefixedElements:EnabledConfig option
333+
ensureTailCallDiagnosticsInRecursiveFunctions:EnabledConfig option}
333334
with
334335
member this.Flatten() =
335336
[|
@@ -354,6 +355,7 @@ with
354355
this.numberOfItems |> Option.map (fun config -> config.Flatten()) |> Option.toArray |> Array.concat
355356
this.binding |> Option.map (fun config -> config.Flatten()) |> Option.toArray |> Array.concat
356357
this.suggestUseAutoProperty |> Option.bind (constructRuleIfEnabled SuggestUseAutoProperty.rule) |> Option.toArray
358+
this.ensureTailCallDiagnosticsInRecursiveFunctions |> Option.bind (constructRuleIfEnabled EnsureTailCallDiagnosticsInRecursiveFunctions.rule) |> Option.toArray
357359
|] |> Array.concat
358360

359361
type TypographyConfig =
@@ -476,7 +478,8 @@ type Configuration =
476478
TrailingNewLineInFile:EnabledConfig option
477479
NoTabCharacters:EnabledConfig option
478480
NoPartialFunctions:RuleConfig<NoPartialFunctions.Config> option
479-
SuggestUseAutoProperty:EnabledConfig option }
481+
SuggestUseAutoProperty:EnabledConfig option
482+
EnsureTailCallDiagnosticsInRecursiveFunctions:EnabledConfig option }
480483
with
481484
static member Zero = {
482485
Global = None
@@ -567,6 +570,7 @@ with
567570
NoTabCharacters = None
568571
NoPartialFunctions = None
569572
SuggestUseAutoProperty = None
573+
EnsureTailCallDiagnosticsInRecursiveFunctions = None
570574
}
571575

572576
// fsharplint:enable RecordFieldNames
@@ -720,6 +724,7 @@ let flattenConfig (config:Configuration) =
720724
config.TrailingNewLineInFile |> Option.bind (constructRuleIfEnabled TrailingNewLineInFile.rule)
721725
config.NoTabCharacters |> Option.bind (constructRuleIfEnabled NoTabCharacters.rule)
722726
config.NoPartialFunctions |> Option.bind (constructRuleWithConfig NoPartialFunctions.rule)
727+
config.EnsureTailCallDiagnosticsInRecursiveFunctions |> Option.bind (constructRuleIfEnabled EnsureTailCallDiagnosticsInRecursiveFunctions.rule)
723728
|] |> Array.choose id
724729

725730
if config.NonPublicValuesNames.IsSome &&

‎src/FSharpLint.Core/FSharpLint.Core.fsproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
<Compile Include="Rules\Conventions\SuggestUseAutoProperty.fs" />
5757
<Compile Include="Rules\Conventions\UsedUnderscorePrefixedElements.fs" />
5858
<Compile Include="Rules\Conventions\FavourNonMutablePropertyInitialization.fs" />
59+
<Compile Include="Rules\Conventions\EnsureTailCallDiagnosticsInRecursiveFunctions.fs" />
5960
<Compile Include="Rules\Conventions\RaiseWithTooManyArguments\RaiseWithTooManyArgumentsHelper.fs" />
6061
<Compile Include="Rules\Conventions\RaiseWithTooManyArguments\FailwithWithSingleArgument.fs" />
6162
<Compile Include="Rules\Conventions\RaiseWithTooManyArguments\RaiseWithSingleArgument.fs" />

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
module FSharpLint.Rules.EnsureTailCallDiagnosticsInRecursiveFunctions
2+
3+
open System
4+
5+
open FSharpLint.Framework
6+
open FSharpLint.Framework.Suggestion
7+
open FSharp.Compiler.Syntax
8+
open FSharpLint.Framework.Ast
9+
open FSharpLint.Framework.Rules
10+
11+
let private emitWarning (func: UnneededRecKeyword.RecursiveFunctionInfo) =
12+
{ Range = func.Range
13+
Message =
14+
String.Format(
15+
Resources.GetString "RulesEnsureTailCallDiagnosticsInRecursiveFunctions",
16+
func.Identifier.idText
17+
)
18+
SuggestedFix = None
19+
TypeChecks = list.Empty }
20+
21+
let runner (args: AstNodeRuleParams) =
22+
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
40+
| _ -> Array.empty
41+
42+
let rule =
43+
{ Name = "EnsureTailCallDiagnosticsInRecursiveFunctions"
44+
Identifier = Identifiers.EnsureTailCallDiagnosticsInRecursiveFunctions
45+
RuleConfig = { AstNodeRuleConfig.Runner = runner; Cleanup = ignore } }
46+
|> AstNodeRule

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

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,41 +2,51 @@
22

33
open System
44
open FSharp.Compiler.Syntax
5+
open FSharp.Compiler.CodeAnalysis
6+
open FSharp.Compiler.Text
57
open FSharpLint.Framework.Ast
68
open FSharpLint.Framework.Rules
79
open FSharpLint.Framework
810
open FSharpLint.Framework.Suggestion
911

10-
let runner (args: AstNodeRuleParams) =
11-
match args.AstNode, args.CheckInfo with
12-
| AstNode.ModuleDeclaration (SynModuleDecl.Let (isRecursive, bindings, letRange)), Some checkInfo when isRecursive ->
13-
match bindings with
14-
| SynBinding (_, _, _, _, _, _, _, SynPat.LongIdent (SynLongIdent([ident], _, _), _, _, _, _, range), _, _, _, _, _) :: _ ->
15-
let symbolUses = checkInfo.GetAllUsesOfAllSymbolsInFile()
16-
let funcName = ident.idText
12+
type internal RecursiveFunctionInfo =
13+
{
14+
Identifier: Ident
15+
Range: range
16+
Body: SynExpr
17+
Attributes: SynAttributes
18+
}
1719

18-
let functionCalls =
19-
symbolUses
20-
|> Seq.filter (fun usage ->
21-
usage.Symbol.DisplayName = funcName
22-
&& usage.Range.StartLine >= letRange.StartLine
23-
&& usage.Range.EndLine <= letRange.EndLine)
20+
let internal (|RecursiveFunction|_|) (astNode: AstNode) =
21+
match astNode with
22+
| 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
27+
| _ -> None
2428

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)
2535

26-
if (functionCalls |> Seq.length) <= 1 then
27-
{ Range = range
28-
Message =
29-
String.Format(
30-
Resources.GetString "RulesUnneededRecKeyword",
31-
funcName
32-
)
33-
SuggestedFix = None
34-
TypeChecks = list.Empty }
35-
|> Array.singleton
36-
else
37-
Array.empty
38-
| _ -> Array.empty
36+
let private emitWarning (func: RecursiveFunctionInfo) =
37+
{ Range = func.Range
38+
Message =
39+
String.Format(
40+
Resources.GetString "RulesUnneededRecKeyword",
41+
func.Identifier.idText
42+
)
43+
SuggestedFix = None
44+
TypeChecks = list.Empty }
3945

46+
let runner (args: AstNodeRuleParams) =
47+
match args.AstNode, args.CheckInfo with
48+
| RecursiveFunction(func), Some checkInfo when not (functionCallsItself checkInfo func) ->
49+
emitWarning func |> Array.singleton
4050
| _ -> Array.empty
4151

4252
let rule =

‎src/FSharpLint.Core/Rules/Identifiers.fs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,4 @@ let NestedFunctionNames = identifier 81
8989
let UsedUnderscorePrefixedElements = identifier 82
9090
let UnneededRecKeyword = identifier 83
9191
let FavourNonMutablePropertyInitialization = identifier 84
92+
let EnsureTailCallDiagnosticsInRecursiveFunctions = identifier 85

‎src/FSharpLint.Core/Text.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,4 +366,7 @@
366366
<data name="RulesFavourNonMutablePropertyInitializationError" xml:space="preserve">
367367
<value>Consider using non-mutable property initialization.</value>
368368
</data>
369+
<data name="RulesEnsureTailCallDiagnosticsInRecursiveFunctions" xml:space="preserve">
370+
<value>The '{0}' function has a "rec" keyword, but no [&lt;TailCall&gt;] attribute. Consider adding [&lt;TailCall&gt;] attribute to the function and &lt;WarningsAsErrors&gt;FS3569&lt;/WarningsAsErrors&gt; property to project file (but only on .NET 8 and higher).</value>
371+
</data>
369372
</root>

‎src/FSharpLint.Core/fsharplint.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,7 @@
331331
"additionalPartials": []
332332
}
333333
},
334+
"ensureTailCallDiagnosticsInRecursiveFunctions": { "enabled": false },
334335
"hints": {
335336
"add": [
336337
"not (a = b) ===> a <> b",

‎tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
<Compile Include="Rules\Conventions\SuggestUseAutoProperty.fs" />
4646
<Compile Include="Rules\Conventions\UsedUnderscorePrefixedElements.fs" />
4747
<Compile Include="Rules\Conventions\FavourNonMutablePropertyInitialization.fs" />
48+
<Compile Include="Rules\Conventions\EnsureTailCallDiagnosticsInRecursiveFunctions.fs" />
4849
<Compile Include="Rules\Conventions\Naming\NamingHelpers.fs" />
4950
<Compile Include="Rules\Conventions\Naming\InterfaceNames.fs" />
5051
<Compile Include="Rules\Conventions\Naming\ExceptionNames.fs" />

0 commit comments

Comments
 (0)