Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New rule UnneededRecKeyword #652

Merged
merged 8 commits into from
Jan 8, 2024
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
1 change: 1 addition & 0 deletions docs/content/how-tos/rule-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,4 @@ The following rules can be specified for linting.
- [UnnestedFunctionNames (FL0080)](rules/FL0080.html)
- [NestedFunctionNames (FL0081)](rules/FL0081.html)
- [UsedUnderscorePrefixedElements (FL0082)](rules/FL0082.html)
- [UnneededRecKeyword (FL0083)](rules/FL0083.html)
29 changes: 29 additions & 0 deletions docs/content/how-tos/rules/FL0083.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
title: FL0083
category: how-to
hide_menu: true
---

# UnneededRecKeyword (FL0083)

*Introduced in `0.23.8`*

## Cause

Recursive function (function marked with a "rec" keyword) does not invoke itself.

## Rationale

Using "rec" keyword on a function that is not recursive is unnecessary.

## How To Fix

Update the function to invoke itself or remove "rec" keyword in case it doesn't need to invoke itself recursively.

## Rule Settings

{
"unneededRecKeyword": {
"enabled": true
}
}
5 changes: 5 additions & 0 deletions src/FSharpLint.Core/Application/Configuration.fs
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ type ConventionsConfig =
redundantNewKeyword:EnabledConfig option
favourStaticEmptyFields:EnabledConfig option
asyncExceptionWithoutReturn:EnabledConfig option
unneededRecKeyword:EnabledConfig option
nestedStatements:RuleConfig<NestedStatements.Config> option
cyclomaticComplexity:RuleConfig<CyclomaticComplexity.Config> option
reimplementsFunction:EnabledConfig option
Expand All @@ -333,6 +334,7 @@ with
this.favourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule) |> Option.toArray
this.favourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule) |> Option.toArray
this.asyncExceptionWithoutReturn |> Option.bind (constructRuleIfEnabled AsyncExceptionWithoutReturn.rule) |> Option.toArray
this.unneededRecKeyword |> Option.bind (constructRuleIfEnabled UnneededRecKeyword.rule) |> Option.toArray
this.nestedStatements |> Option.bind (constructRuleWithConfig NestedStatements.rule) |> Option.toArray
this.favourConsistentThis |> Option.bind (constructRuleWithConfig FavourConsistentThis.rule) |> Option.toArray
this.cyclomaticComplexity |> Option.bind (constructRuleWithConfig CyclomaticComplexity.rule) |> Option.toArray
Expand Down Expand Up @@ -406,6 +408,7 @@ type Configuration =
FavourReRaise:EnabledConfig option
FavourStaticEmptyFields:EnabledConfig option
AsyncExceptionWithoutReturn:EnabledConfig option
UnneededRecKeyword:EnabledConfig option
NestedStatements:RuleConfig<NestedStatements.Config> option
FavourConsistentThis:RuleConfig<FavourConsistentThis.Config> option
CyclomaticComplexity:RuleConfig<CyclomaticComplexity.Config> option
Expand Down Expand Up @@ -494,6 +497,7 @@ with
FavourReRaise = None
FavourStaticEmptyFields = None
AsyncExceptionWithoutReturn = None
UnneededRecKeyword = None
NestedStatements = None
FavourConsistentThis = None
CyclomaticComplexity = None
Expand Down Expand Up @@ -645,6 +649,7 @@ let flattenConfig (config:Configuration) =
config.FavourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule)
config.FavourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule)
config.AsyncExceptionWithoutReturn |> Option.bind (constructRuleIfEnabled AsyncExceptionWithoutReturn.rule)
config.UnneededRecKeyword |> Option.bind (constructRuleIfEnabled UnneededRecKeyword.rule)
config.NestedStatements |> Option.bind (constructRuleWithConfig NestedStatements.rule)
config.FavourConsistentThis |> Option.bind (constructRuleWithConfig FavourConsistentThis.rule)
config.CyclomaticComplexity |> Option.bind (constructRuleWithConfig CyclomaticComplexity.rule)
Expand Down
1 change: 1 addition & 0 deletions src/FSharpLint.Core/FSharpLint.Core.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
<Compile Include="Rules\Formatting\TypedItemSpacing.fs" />
<Compile Include="Rules\Formatting\TypePrefixing.fs" />
<Compile Include="Rules\Formatting\UnionDefinitionIndentation.fs" />
<Compile Include="Rules\Conventions\UnneededRecKeyword.fs" />
<Compile Include="Rules\Conventions\AsyncExceptionWithoutReturn.fs" />
<Compile Include="Rules\Conventions\FavourStaticEmptyFields.fs" />
<Compile Include="Rules\Conventions\RecursiveAsyncFunction.fs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ let rec identFromSimplePat = function
| SynSimplePat.Typed(p, _, _) -> identFromSimplePat p
| SynSimplePat.Attrib(_) -> None

let rec isNested args nodeIndex =
let isNested args nodeIndex =
let parent = args.SyntaxArray.[nodeIndex].ParentIndex
let actual = args.SyntaxArray.[parent].Actual

Expand Down
48 changes: 48 additions & 0 deletions src/FSharpLint.Core/Rules/Conventions/UnneededRecKeyword.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
module FSharpLint.Rules.UnneededRecKeyword

open System
open FSharp.Compiler.Syntax
open FSharpLint.Framework.Ast
open FSharpLint.Framework.Rules
open FSharpLint.Framework
open FSharpLint.Framework.Suggestion

let runner (args: AstNodeRuleParams) =
match args.AstNode, args.CheckInfo with
| AstNode.ModuleDeclaration (SynModuleDecl.Let (isRecursive, bindings, letRange)), Some checkInfo when isRecursive ->
match bindings with
| SynBinding (_, _, _, _, _, _, _, SynPat.LongIdent (LongIdentWithDots([ident], _), _, _, _, _, range), _, _, _, _) :: _ ->
let symbolUses = checkInfo.GetAllUsesOfAllSymbolsInFile()
let funcName = ident.idText

let functionCalls =
symbolUses
|> Seq.filter (fun usage ->
usage.Symbol.DisplayName = funcName
&& usage.Range.StartLine >= letRange.StartLine
&& usage.Range.EndLine <= letRange.EndLine)


if (functionCalls |> Seq.length) <= 1 then
{ Range = range
Message =
String.Format(
Resources.GetString "RulesUnneededRecKeyword",
funcName
)
SuggestedFix = None
TypeChecks = list.Empty }
|> Array.singleton
else
Array.empty
| _ -> Array.empty

| _ -> Array.empty

let rule =
{ Name = "UnneededRecKeyword"
Identifier = Identifiers.UnneededRecKeyword
RuleConfig =
{ AstNodeRuleConfig.Runner = runner
Cleanup = ignore } }
|> AstNodeRule
3 changes: 2 additions & 1 deletion src/FSharpLint.Core/Rules/Identifiers.fs
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,5 @@ let AsyncExceptionWithoutReturn = identifier 78
let SuggestUseAutoProperty = identifier 79
let UnnestedFunctionNames = identifier 80
let NestedFunctionNames = identifier 81
let UsedUnderscorePrefixedElements = identifier 82
let UsedUnderscorePrefixedElements = identifier 82
let UnneededRecKeyword = identifier 83
3 changes: 3 additions & 0 deletions src/FSharpLint.Core/Text.resx
Original file line number Diff line number Diff line change
Expand Up @@ -357,4 +357,7 @@
<data name="RulesSuggestUseAutoProperty" xml:space="preserve">
<value>Consider using auto-properties via the 'val' keyword.</value>
</data>
<data name="RulesUnneededRecKeyword" xml:space="preserve">
<value>The '{0}' function has a "rec" keyword, but it is not really recursive, consider removing it.</value>
</data>
</root>
1 change: 1 addition & 0 deletions src/FSharpLint.Core/fsharplint.json
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@
"suggestUseAutoProperty": { "enabled": false },
"avoidTooShortNames": { "enabled": false },
"asyncExceptionWithoutReturn": { "enabled": false },
"unneededRecKeyword": { "enabled": true },
"indentation": {
"enabled": false
},
Expand Down
1 change: 1 addition & 0 deletions tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
<Compile Include="Rules\Formatting\TypedItemSpacing.fs" />
<Compile Include="Rules\Formatting\UnionDefinitionIndentation.fs" />
<Compile Include="Rules\Formatting\TypePrefixing.fs" />
<Compile Include="Rules\Conventions\UnneededRecKeyword.fs" />
<Compile Include="Rules\Conventions\AsyncExceptionWithoutReturn.fs" />
<Compile Include="Rules\Conventions\FavourStaticEmptyFields.fs" />
<Compile Include="Rules\Conventions\RecursiveAsyncFunction.fs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
module FSharpLint.Core.Tests.Rules.Conventions.UnneededRecKeyword

open NUnit.Framework
open FSharpLint.Framework.Rules
open FSharpLint.Rules

[<TestFixture>]
type TestConventionsUnneededRecKeyword() =
inherit TestAstNodeRuleBase.TestAstNodeRuleBase(UnneededRecKeyword.rule)

[<Test>]
member this.UnneededRecKeywordShouldNotProduceError() =
this.Parse """
let rec Foo () =
if someParam then
Foo()
else
()"""

Assert.IsTrue this.NoErrorsExist

[<Test>]
member this.UnneededRecKeywordShouldProduceError_1() =
this.Parse """
let rec Foo someParam =
()"""

Assert.IsTrue this.ErrorsExist

[<Test>]
member this.UnneededRecKeywordShouldProduceError_2() =
this.Parse """
let rec Foo someParam =
()

[<EntryPoint>]
let main args =
let Foo () =
()

Foo()
0"""

Assert.IsTrue this.ErrorsExist