DRAFT: Rule to recommend if-else construct instead of match clause

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

@@ -0,0 +1,30 @@
+---
+title: FL0077
+category: how-to
+hide_menu: true
+---
+
+# FavourBasicControlFlow (FL0077)
+
+*Introduced in `0.21.3`*
+
+## Cause
+
+Use of match clause when it is used in basic control flow.
+
+## Rationale
+
+Match clauses are more adequate for complex pattern-matching scenarios.
+For simple comparisons, an if-then-else block makes terser code.
+
+## How To Fix
+
+Use if-else construct instead of match clause.
+
+## Rule Settings
+
+    {
+        "favourBasicControlFlow": {
+            "enabled": false
+        }
+    }

src/FSharpLint.Core/Application/Configuration.fs

@@ -319,6 +319,7 @@ type ConventionsConfig =
       numberOfItems:NumberOfItemsConfig option
       binding:BindingConfig option
       favourReRaise:EnabledConfig option
+      favourBasicControlFlow:EnabledConfig option
       favourConsistentThis:RuleConfig<FavourConsistentThis.Config> option }
 with
     member this.Flatten() =
@@ -327,6 +328,7 @@ with
             this.avoidTooShortNames |> Option.bind (constructRuleIfEnabled AvoidTooShortNames.rule) |> Option.toArray           
             this.redundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule) |> Option.toArray
             this.favourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule) |> Option.toArray
+            this.favourBasicControlFlow |> Option.bind (constructRuleIfEnabled FavourBasicControlFlow.rule) |> Option.toArray
             this.favourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule) |> Option.toArray
             this.nestedStatements |> Option.bind (constructRuleWithConfig NestedStatements.rule) |> Option.toArray
             this.favourConsistentThis |> Option.bind (constructRuleWithConfig FavourConsistentThis.rule) |> Option.toArray
@@ -396,6 +398,7 @@ type Configuration =
       AvoidTooShortNames:EnabledConfig option
       RedundantNewKeyword:EnabledConfig option
       FavourReRaise:EnabledConfig option
+      FavourBasicControlFlow:EnabledConfig option
       FavourStaticEmptyFields:EnabledConfig option
       NestedStatements:RuleConfig<NestedStatements.Config> option
       FavourConsistentThis:RuleConfig<FavourConsistentThis.Config> option
@@ -480,6 +483,7 @@ with
         AvoidTooShortNames = None
         RedundantNewKeyword = None
         FavourReRaise = None
+        FavourBasicControlFlow = None
         FavourStaticEmptyFields = None
         NestedStatements = None
         FavourConsistentThis = None
@@ -627,6 +631,7 @@ let flattenConfig (config:Configuration) =
             config.AvoidTooShortNames |> Option.bind (constructRuleIfEnabled AvoidTooShortNames.rule)
             config.RedundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule)
             config.FavourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule)
+            config.FavourBasicControlFlow |> Option.bind (constructRuleIfEnabled FavourBasicControlFlow.rule)
             config.FavourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule)
             config.NestedStatements |> Option.bind (constructRuleWithConfig NestedStatements.rule)
             config.FavourConsistentThis |> Option.bind (constructRuleWithConfig FavourConsistentThis.rule)

src/FSharpLint.Core/FSharpLint.Core.fsproj

@@ -50,6 +50,7 @@
     <Compile Include="Rules\Conventions\CyclomaticComplexity.fs" />
     <Compile Include="Rules\Conventions\FavourReRaise.fs" />
     <Compile Include="Rules\Conventions\FavourConsistentThis.fs" />
+    <Compile Include="Rules\Conventions\FavourBasicControlFlow.fs" />
     <Compile Include="Rules\Conventions\RaiseWithTooManyArguments\RaiseWithTooManyArgumentsHelper.fs" />
     <Compile Include="Rules\Conventions\RaiseWithTooManyArguments\FailwithWithSingleArgument.fs" />
     <Compile Include="Rules\Conventions\RaiseWithTooManyArguments\RaiseWithSingleArgument.fs" />

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

@@ -0,0 +1,39 @@
+module FSharpLint.Rules.FavourBasicControlFlow
+
+open FSharpLint.Framework
+open FSharpLint.Framework.Suggestion
+open FSharp.Compiler.Syntax
+open FSharp.Compiler.Text
+open FSharpLint.Framework.Ast
+open FSharpLint.Framework.Rules
+
+let private isBasicControlFlow (synMatchClauses: List<SynMatchClause>) =
+    let isFirstClauseTarget firstClause =
+        match firstClause with
+        | SynMatchClause(SynPat.Named(_, _, _, _, _), _, _, _, _)
+        | SynMatchClause(SynPat.Const( _, _), _, _, _, _)
+        | SynMatchClause(SynPat.LongIdent(LongIdentWithDots _, _, _, SynArgPats.Pats [], _, _), _, _, _, _) -> true
+        | _ -> false
+
+    let isLastClauseTarget lastClause =
+        match lastClause with
+        | SynMatchClause(SynPat.Wild _, None, _, _, _) -> true
+        | _ -> false
+
+    synMatchClauses.Length = 2 && isFirstClauseTarget synMatchClauses.[0] && isLastClauseTarget synMatchClauses.[1]
+
+let runner args =
+    match args.AstNode with
+    | AstNode.Expression(SynExpr.Match (_, _, synMatchClauses, range)) when isBasicControlFlow synMatchClauses ->
+        { Range = range
+          Message = Resources.GetString "FavourBasicControlFlow"
+          SuggestedFix = None
+          TypeChecks = List.empty }
+        |> Array.singleton
+    | _ -> Array.empty
+
+let rule =
+    { Name = "FavourBasicControlFlow"
+      Identifier = Identifiers.FavourBasicControlFlow
+      RuleConfig = { AstNodeRuleConfig.Runner = runner; Cleanup = ignore } }
+    |> AstNodeRule

src/FSharpLint.Core/Rules/Identifiers.fs

@@ -81,3 +81,4 @@ let FavourReRaise = identifier 73
 let FavourConsistentThis = identifier 74
 let AvoidTooShortNames = identifier 75
 let FavourStaticEmptyFields = identifier 76
+let FavourBasicControlFlow = identifier 77

src/FSharpLint.Core/Text.resx

@@ -345,4 +345,7 @@
   <data name="RulesFavourStaticEmptyFieldsForArray" xml:space="preserve">
     <value>Consider using 'Array.empty' instead.</value>
   </data>
+  <data name="FavourBasicControlFlow" xml:space="preserve">
+    <value>Rather use simpler if-else construct instead of match clause.</value>
+  </data>
 </root>

src/FSharpLint.Core/fsharplint.json

@@ -274,6 +274,7 @@
         }
     },
     "avoidTooShortNames": { "enabled": false },
+    "favourBasicControlFlow": { "enabled": false },
     "indentation": {
         "enabled": false
     },

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

@@ -38,6 +38,7 @@
     <Compile Include="Rules\Conventions\NoPartialFunctions.fs" />
     <Compile Include="Rules\Conventions\FavourReRaise.fs" />
     <Compile Include="Rules\Conventions\FavourConsistentThis.fs" />
+    <Compile Include="Rules\Conventions\FavourBasicControlFlow.fs" />
     <Compile Include="Rules\Conventions\AvoidTooShortNames.fs" />
     <Compile Include="Rules\Conventions\Naming\NamingHelpers.fs" />
     <Compile Include="Rules\Conventions\Naming\InterfaceNames.fs" />

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

@@ -0,0 +1,47 @@
+module FSharpLint.Core.Tests.Rules.Conventions.FavourBasicControlFlow
+
+open NUnit.Framework
+open FSharpLint.Rules
+
+[<TestFixture>]
+type TestConventionsFavourFavourBasicControlFlow() =
+    inherit TestAstNodeRuleBase.TestAstNodeRuleBase(FavourBasicControlFlow.rule)
+
+
+    [<Test>]
+    member this.MoreThanTwoClausesShouldNotProduceError() =
+        this.Parse """
+match foo with
+| bar -> ()
+| baz -> ()
+| _ -> () """
+
+        Assert.IsTrue this.NoErrorsExist
+
+
+    [<Test>]
+    member this.TwoClausesWithoutWildcardShouldNotProduceError() =
+        this.Parse """
+match foo with
+| bar -> ()
+| baz -> () """
+
+        Assert.IsTrue this.NoErrorsExist
+
+    [<Test>]
+    member this.ThePresenceOfDUShouldNotProduceError() =
+        this.Parse """
+match foo with
+| Bar baz -> ()
+| _ -> () """
+
+        Assert.IsTrue this.NoErrorsExist
+
+    [<Test>]
+    member this.TwoClausesWithWildcardShouldProduceError() =
+        this.Parse """
+match foo with
+| bar -> ()
+| _ -> () """
+
+        Assert.IsTrue this.ErrorsExist