Merge PR #652 from Mersho/fix-650

New rule UnneededRecKeyword.

Fixes #650

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

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

docs/content/how-tos/rules/FL0083.md

@@ -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
+        }
+    }

src/FSharpLint.Core/Application/Configuration.fs

@@ -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
@@ -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
@@ -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
@@ -494,6 +497,7 @@ with
         FavourReRaise = None
         FavourStaticEmptyFields = None
         AsyncExceptionWithoutReturn = None
+        UnneededRecKeyword = None
         NestedStatements = None
         FavourConsistentThis = None
         CyclomaticComplexity = None
@@ -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)

src/FSharpLint.Core/FSharpLint.Core.fsproj

@@ -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" />

src/FSharpLint.Core/Rules/Conventions/Naming/NamingHelper.fs

@@ -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
 

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

@@ -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

src/FSharpLint.Core/Rules/Identifiers.fs

@@ -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

src/FSharpLint.Core/Text.resx

@@ -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>

src/FSharpLint.Core/fsharplint.json

@@ -292,6 +292,7 @@
     "suggestUseAutoProperty": { "enabled": false },
     "avoidTooShortNames": { "enabled": false },
     "asyncExceptionWithoutReturn": { "enabled": false },
+    "unneededRecKeyword": { "enabled": true },
     "indentation": {
         "enabled": false
     },

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

@@ -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" />

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

@@ -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