New rule: Favour Nested Functions

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

@@ -131,3 +131,4 @@ The following rules can be specified for linting.
 - [IndexerAccessorStyleConsistency (FL0088)](rules/FL0088.html)
 - [FavourSingleton (FL0089)](rules/FL0089.html)
 - [NoAsyncRunSynchronouslyInLibrary (FL0090)](rules/FL0090.html)
+- [FavourNestedFunctions (FL0091)](rules/FL0091.html)

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

@@ -0,0 +1,31 @@
+---
+title: FL0091
+category: how-to
+hide_menu: true
+---
+
+# FavourNestedFunctions (FL0091)
+
+*Introduced in `0.26.12`*
+
+## Cause
+
+Prefer using local (nested) functions over private module-level functions.
+
+## Rationale
+
+With a nested function, one can clearly see from reading the code that there is only one consumer of the function.
+The code being this way becomes more streamlined.
+Code is also more concise because nested functions don't need accessibility modifiers.
+
+## How To Fix
+
+Move private function inside function that uses it.
+
+## Rule Settings
+
+    {
+        "FavourNestedFunctions": {
+            "enabled": false
+        }
+    }

src/FSharpLint.Console/Program.fs

@@ -90,16 +90,6 @@ let internal expandWildcard (pattern:string) =
     else
         List.empty
 
-let private parserProgress (output:Output.IOutput) = function
-    | Starting file ->
-        String.Format(Resources.GetString("ConsoleStartingFile"), file) |> output.WriteInfo
-    | ReachedEnd (_, warnings) ->
-        String.Format(Resources.GetString("ConsoleFinishedFile"), List.length warnings) |> output.WriteInfo
-    | Failed (file, parseException) ->
-        String.Format(Resources.GetString("ConsoleFailedToParseFile"), file) |> output.WriteError
-        output.WriteError
-            $"Exception Message:{Environment.NewLine}{parseException.Message}{Environment.NewLine}Exception Stack Trace:{Environment.NewLine}{parseException.StackTrace}{Environment.NewLine}"
-
 /// Checks if a string contains wildcard characters.
 let internal containsWildcard (target:string) =
     target.Contains("*") || target.Contains("?")
@@ -117,101 +107,111 @@ let internal inferFileType (target:string) =
     else
         FileType.Source
 
-let private lint
-    (lintArgs: ParseResults<LintArgs>)
-    (output: Output.IOutput)
-    (toolsPath:Ionide.ProjInfo.Types.ToolsPath)
-    : ExitCode =
-    let mutable exitCode = ExitCode.Success
-
-    let handleError (str:string) =
-        output.WriteError str
-        exitCode <- ExitCode.Failure
-
-    let handleLintResult = function
-        | LintResult.Success(warnings) ->
-            String.Format(Resources.GetString("ConsoleFinished"), List.length warnings)
-            |> output.WriteInfo
-            if not (List.isEmpty warnings) then
-                exitCode <- ExitCode.Failure
-        | LintResult.Failure(failure) ->
-            handleError failure.Description
-
-    let lintConfig = lintArgs.TryGetResult Lint_Config
-
-    let configParam =
-        match lintConfig with
-        | Some configPath -> FromFile configPath
-        | None -> Default
-
-    let lintParams =
-        {
-            CancellationToken = None
-            ReceivedWarning = Some output.WriteWarning
-            Configuration = configParam
-            ReportLinterProgress = Some (parserProgress output)
-        }
-
-    let target = lintArgs.GetResult Target
-    let fileType = lintArgs.TryGetResult File_Type |> Option.defaultValue (inferFileType target)
-
-    try
-        let lintResult =
-            match fileType with
-            | FileType.File -> Lint.asyncLintFile lintParams target |> Async.RunSynchronously
-            | FileType.Source -> Lint.asyncLintSource lintParams target |> Async.RunSynchronously
-            | FileType.Solution -> Lint.asyncLintSolution lintParams target toolsPath |> Async.RunSynchronously
-            | FileType.Wildcard ->
-                output.WriteInfo "Wildcard detected, but not recommended. Using a project (slnx/sln/fsproj) can detect more issues."
-                let files = expandWildcard target
-                if List.isEmpty files then
-                    output.WriteInfo $"No files matching pattern '%s{target}' were found."
-                    LintResult.Success List.empty
-                else
-                    output.WriteInfo $"Found %d{List.length files} file(s) matching pattern '%s{target}'."
-                    Lint.asyncLintFiles lintParams files |> Async.RunSynchronously
-            | FileType.Project
-            | _ -> Lint.asyncLintProject lintParams target toolsPath |> Async.RunSynchronously
-        handleLintResult lintResult
-    with
-    | exn ->
-        let target = if fileType = FileType.Source then "source" else target
-        handleError
-            $"Lint failed while analysing %s{target}.{Environment.NewLine}Failed with: %s{exn.Message}{Environment.NewLine}Stack trace: {exn.StackTrace}"
-
-    exitCode
-
-let private start (arguments:ParseResults<ToolArgs>) (toolsPath:Ionide.ProjInfo.Types.ToolsPath) =
-    let output =
-        match arguments.TryGetResult Format with
-        | Some OutputFormat.MSBuild -> Output.MSBuildOutput() :> Output.IOutput
-        | Some OutputFormat.Standard
-        | Some _
-        | None -> Output.StandardOutput() :> Output.IOutput
-
-    if arguments.Contains ToolArgs.Version then
-        let maybeVersion =
-            Assembly.GetExecutingAssembly().GetCustomAttributes false
-            |> Seq.tryPick (function | :? AssemblyInformationalVersionAttribute as aiva -> Some aiva.InformationalVersion | _ -> None)
-        match maybeVersion with
-        | Some version ->
-            output.WriteInfo $"Current version: {version}"
-            Environment.Exit 0
-        | None ->
-            failwith "Error: unable to get version"
-
-    match arguments.GetSubCommand() with
-    | Lint lintArgs ->
-        lint lintArgs output toolsPath
-    | _ ->
-        ExitCode.Failure
-
 /// Must be called only once per process.
 /// We're calling it globally so we can call main multiple times from our tests.
 let toolsPath = Ionide.ProjInfo.Init.init (DirectoryInfo <| Directory.GetCurrentDirectory())  None
 
 [<EntryPoint>]
 let main argv =
+    let lint
+        (lintArgs: ParseResults<LintArgs>)
+        (output: Output.IOutput)
+        (toolsPath:Ionide.ProjInfo.Types.ToolsPath)
+        : ExitCode =
+        let parserProgress (output:Output.IOutput) = function
+            | Starting file ->
+                String.Format(Resources.GetString("ConsoleStartingFile"), file) |> output.WriteInfo
+            | ReachedEnd (_, warnings) ->
+                String.Format(Resources.GetString("ConsoleFinishedFile"), List.length warnings) |> output.WriteInfo
+            | Failed (file, parseException) ->
+                String.Format(Resources.GetString("ConsoleFailedToParseFile"), file) |> output.WriteError
+                output.WriteError
+                    $"Exception Message:{Environment.NewLine}{parseException.Message}{Environment.NewLine}Exception Stack Trace:{Environment.NewLine}{parseException.StackTrace}{Environment.NewLine}"
+
+        let mutable exitCode = ExitCode.Success
+
+        let handleError (str:string) =
+            output.WriteError str
+            exitCode <- ExitCode.Failure
+
+        let handleLintResult = function
+            | LintResult.Success(warnings) ->
+                String.Format(Resources.GetString("ConsoleFinished"), List.length warnings)
+                |> output.WriteInfo
+                if not (List.isEmpty warnings) then
+                    exitCode <- ExitCode.Failure
+            | LintResult.Failure(failure) ->
+                handleError failure.Description
+
+        let lintConfig = lintArgs.TryGetResult Lint_Config
+
+        let configParam =
+            match lintConfig with
+            | Some configPath -> FromFile configPath
+            | None -> Default
+
+        let lintParams =
+            {
+                CancellationToken = None
+                ReceivedWarning = Some output.WriteWarning
+                Configuration = configParam
+                ReportLinterProgress = Some (parserProgress output)
+            }
+
+        let target = lintArgs.GetResult Target
+        let fileType = lintArgs.TryGetResult File_Type |> Option.defaultValue (inferFileType target)
+
+        try
+            let lintResult =
+                match fileType with
+                | FileType.File -> Lint.asyncLintFile lintParams target |> Async.RunSynchronously
+                | FileType.Source -> Lint.asyncLintSource lintParams target |> Async.RunSynchronously
+                | FileType.Solution -> Lint.asyncLintSolution lintParams target toolsPath |> Async.RunSynchronously
+                | FileType.Wildcard ->
+                    output.WriteInfo "Wildcard detected, but not recommended. Using a project (slnx/sln/fsproj) can detect more issues."
+                    let files = expandWildcard target
+                    if List.isEmpty files then
+                        output.WriteInfo $"No files matching pattern '%s{target}' were found."
+                        LintResult.Success List.empty
+                    else
+                        output.WriteInfo $"Found %d{List.length files} file(s) matching pattern '%s{target}'."
+                        Lint.asyncLintFiles lintParams files |> Async.RunSynchronously
+                | FileType.Project
+                | _ -> Lint.asyncLintProject lintParams target toolsPath |> Async.RunSynchronously
+            handleLintResult lintResult
+        with
+        | exn ->
+            let target = if fileType = FileType.Source then "source" else target
+            handleError
+                $"Lint failed while analysing %s{target}.{Environment.NewLine}Failed with: %s{exn.Message}{Environment.NewLine}Stack trace: {exn.StackTrace}"
+
+        exitCode
+
+    let start (arguments:ParseResults<ToolArgs>) (toolsPath:Ionide.ProjInfo.Types.ToolsPath) =
+        let output =
+            match arguments.TryGetResult Format with
+            | Some OutputFormat.MSBuild -> Output.MSBuildOutput() :> Output.IOutput
+            | Some OutputFormat.Standard
+            | Some _
+            | None -> Output.StandardOutput() :> Output.IOutput
+
+        if arguments.Contains ToolArgs.Version then
+            let maybeVersion =
+                Assembly.GetExecutingAssembly().GetCustomAttributes false
+                |> Seq.tryPick (function | :? AssemblyInformationalVersionAttribute as aiva -> Some aiva.InformationalVersion | _ -> None)
+            match maybeVersion with
+            | Some version ->
+                output.WriteInfo $"Current version: {version}"
+                Environment.Exit 0
+            | None ->
+                failwith "Error: unable to get version"
+
+        match arguments.GetSubCommand() with
+        | Lint lintArgs ->
+            lint lintArgs output toolsPath
+        | _ ->
+            ExitCode.Failure
+
     let errorHandler = ProcessExiter(colorizer = function
         | ErrorCode.HelpText -> None
         | _ -> Some ConsoleColor.Red)

src/FSharpLint.Core/Application/Configuration.fs

@@ -385,7 +385,8 @@ type ConventionsConfig =
       favourConsistentThis:RuleConfig<FavourConsistentThis.Config> option
       suggestUseAutoProperty:EnabledConfig option
       usedUnderscorePrefixedElements:EnabledConfig option
-      ensureTailCallDiagnosticsInRecursiveFunctions:EnabledConfig option}
+      ensureTailCallDiagnosticsInRecursiveFunctions:EnabledConfig option
+      favourNestedFunctions:EnabledConfig option }
 with
     member this.Flatten() =
         Array.concat
@@ -413,6 +414,7 @@ with
                 this.suggestUseAutoProperty |> Option.bind (constructRuleIfEnabled SuggestUseAutoProperty.rule) |> Option.toArray
                 this.ensureTailCallDiagnosticsInRecursiveFunctions |> Option.bind (constructRuleIfEnabled EnsureTailCallDiagnosticsInRecursiveFunctions.rule) |> Option.toArray
                 this.indexerAccessorStyleConsistency |> Option.bind (constructRuleWithConfig IndexerAccessorStyleConsistency.rule) |> Option.toArray
+                this.favourNestedFunctions |> Option.bind (constructRuleIfEnabled FavourNestedFunctions.rule) |> Option.toArray
             |]
 
 [<Obsolete(ObsoleteMsg, ObsoleteWarnTreatAsError)>]
@@ -437,8 +439,6 @@ with
 
 // </Deprecated>
 
-let private getOrEmptyList hints = Option.defaultValue Array.empty hints
-
 type HintConfig = {
     add:string [] option
     ignore:string [] option
@@ -555,7 +555,8 @@ type Configuration =
       FavourAsKeyword:EnabledConfig option 
       InterpolatedStringWithNoSubstitution:EnabledConfig option
       FavourSingleton:EnabledConfig option
-      NoAsyncRunSynchronouslyInLibrary:EnabledConfig option}
+      NoAsyncRunSynchronouslyInLibrary:EnabledConfig option
+      FavourNestedFunctions:EnabledConfig option }
 with
     static member Zero = {
         Global = None
@@ -658,6 +659,7 @@ with
         InterpolatedStringWithNoSubstitution = None
         FavourSingleton = None
         NoAsyncRunSynchronouslyInLibrary = None
+        FavourNestedFunctions = None
     }
 
 // fsharplint:enable RecordFieldNames
@@ -708,19 +710,6 @@ let getGlobalConfig (globalConfig:GlobalConfig option) =
         Rules.GlobalRuleConfig.numIndentationSpaces = globalConfig.numIndentationSpaces |> Option.defaultValue Rules.GlobalRuleConfig.Default.numIndentationSpaces
     }) |> Option.defaultValue Rules.GlobalRuleConfig.Default
 
-let private parseHints (hints:string []) =
-    let parseHint hint =
-        match FParsec.CharParsers.run HintParser.phint hint with
-        | FParsec.CharParsers.Success(hint, _, _) -> hint
-        | FParsec.CharParsers.Failure(error, _, _) ->
-            raise <| ConfigurationException $"Failed to parse hint: {hint}{Environment.NewLine}{error}"
-
-    hints
-    |> Array.filter (System.String.IsNullOrWhiteSpace >> not)
-    |> Array.map parseHint
-    |> Array.toList
-    |> MergeSyntaxTrees.mergeHints
-
 let findDeprecation config deprecatedAllRules allRules =
     if config.NonPublicValuesNames.IsSome &&
         (config.PrivateValuesNames.IsSome || config.InternalValuesNames.IsSome) then
@@ -756,6 +745,21 @@ let findDeprecation config deprecatedAllRules allRules =
 
 // fsharplint:disable MaxLinesInFunction
 let flattenConfig (config:Configuration) =
+    let parseHints (hints:string []) =
+        let parseHint hint =
+            match FParsec.CharParsers.run HintParser.phint hint with
+            | FParsec.CharParsers.Success(hint, _, _) -> hint
+            | FParsec.CharParsers.Failure(error, _, _) ->
+                raise <| ConfigurationException $"Failed to parse hint: {hint}{Environment.NewLine}{error}"
+
+        hints
+        |> Array.filter (System.String.IsNullOrWhiteSpace >> not)
+        |> Array.map parseHint
+        |> Array.toList
+        |> MergeSyntaxTrees.mergeHints
+
+    let getOrEmptyList hints = Option.defaultValue Array.empty hints
+
     let deprecatedAllRules =
         Array.concat
             [|
@@ -862,6 +866,7 @@ let flattenConfig (config:Configuration) =
                 config.InterpolatedStringWithNoSubstitution |> Option.bind (constructRuleIfEnabled InterpolatedStringWithNoSubstitution.rule)
                 config.FavourSingleton |> Option.bind (constructRuleIfEnabled FavourSingleton.rule)
                 config.NoAsyncRunSynchronouslyInLibrary |> Option.bind (constructRuleIfEnabled NoAsyncRunSynchronouslyInLibrary.rule)
+                config.FavourNestedFunctions |> Option.bind (constructRuleIfEnabled FavourNestedFunctions.rule)
             |]
 
     findDeprecation config deprecatedAllRules allRules

src/FSharpLint.Core/FSharpLint.Core.fsproj

@@ -60,6 +60,7 @@
     <Compile Include="Rules\Conventions\EnsureTailCallDiagnosticsInRecursiveFunctions.fs" />
     <Compile Include="Rules\Conventions\FailwithBadUsage.fs" />
     <Compile Include="Rules\Conventions\FavourSingleton.fs" />
+    <Compile Include="Rules\Conventions\FavourNestedFunctions.fs" />
     <Compile Include="Rules\Conventions\SourceLength\SourceLengthHelper.fs" />
     <Compile Include="Rules\Conventions\SourceLength\MaxLinesInLambdaFunction.fs" />
     <Compile Include="Rules\Conventions\SourceLength\MaxLinesInMatchLambdaFunction.fs" />

src/FSharpLint.Core/Framework/Suppression.fs

@@ -15,20 +15,20 @@ type SuppressionInfo =
 /// Specifies the suppressions for an individual line.
 type LineSuppression = { Line:int; Suppressions:SuppressionInfo list }
 
-/// Extracts rule names from a whitespace separated string of rule names.
-let private extractRules (rules:Set<String>) (str:string) =
-    let (splitOnWhitespace:char[]) = null
-    let entries =
-        str.Split(splitOnWhitespace, StringSplitOptions.RemoveEmptyEntries)
-        |> Seq.map (fun entry -> entry.ToLowerInvariant())
-        |> Seq.filter (fun entry -> rules.Contains(entry))
-        |> Set.ofSeq
-
-    // If no rules set, then all rules are applied.
-    if Seq.isEmpty entries then rules else entries
-
 /// Parses a given file to find lines containing rule suppressions.
 let parseSuppressionInfo (rules:Set<String>) (lines:string list) =
+    /// Extracts rule names from a whitespace separated string of rule names.
+    let extractRules (rules:Set<String>) (str:string) =
+        let (splitOnWhitespace:char[]) = null
+        let entries =
+            str.Split(splitOnWhitespace, StringSplitOptions.RemoveEmptyEntries)
+            |> Seq.map (fun entry -> entry.ToLowerInvariant())
+            |> Seq.filter (fun entry -> rules.Contains(entry))
+            |> Set.ofSeq
+
+        // If no rules set, then all rules are applied.
+        if Seq.isEmpty entries then rules else entries
+
     let rules = Set.map (fun (rule: String) -> rule.ToLowerInvariant()) rules
 
     let choose lineNum line =