Skip to content

Commit c9757b1

Browse files
committed
Rule to recommend if-else construct instead of match clause
It improves code readability. Fixes: #545
1 parent 2688e50 commit c9757b1

File tree

9 files changed

+128
-0
lines changed

9 files changed

+128
-0
lines changed

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
---
2+
title: FL0076
3+
category: how-to
4+
hide_menu: true
5+
---
6+
7+
# FavourBasicControlFlow (FL0076)
8+
9+
*Introduced in `0.21.3`*
10+
11+
## Cause
12+
13+
Use of match clause when it is used in basic control flow.
14+
15+
## Rationale
16+
17+
Match clauses are more adequate for complex pattern-matching scenarios.
18+
For simple comparisons, an if-then-else block makes terser code.
19+
20+
## How To Fix
21+
22+
Use if-else construct instead of match clause.
23+
24+
## Rule Settings
25+
26+
{
27+
"favourBasicControlFlow": {
28+
"enabled": false
29+
}
30+
}

src/FSharpLint.Core/Application/Configuration.fs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ type ConventionsConfig =
318318
numberOfItems:NumberOfItemsConfig option
319319
binding:BindingConfig option
320320
favourReRaise:EnabledConfig option
321+
favourBasicControlFlow:EnabledConfig option
321322
favourConsistentThis:RuleConfig<FavourConsistentThis.Config> option }
322323
with
323324
member this.Flatten() =
@@ -326,6 +327,7 @@ with
326327
this.avoidTooShortNames |> Option.bind (constructRuleIfEnabled AvoidTooShortNames.rule) |> Option.toArray
327328
this.redundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule) |> Option.toArray
328329
this.favourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule) |> Option.toArray
330+
this.favourBasicControlFlow |> Option.bind (constructRuleIfEnabled FavourBasicControlFlow.rule) |> Option.toArray
329331
this.nestedStatements |> Option.bind (constructRuleWithConfig NestedStatements.rule) |> Option.toArray
330332
this.favourConsistentThis |> Option.bind (constructRuleWithConfig FavourConsistentThis.rule) |> Option.toArray
331333
this.cyclomaticComplexity |> Option.bind (constructRuleWithConfig CyclomaticComplexity.rule) |> Option.toArray
@@ -394,6 +396,7 @@ type Configuration =
394396
AvoidTooShortNames:EnabledConfig option
395397
RedundantNewKeyword:EnabledConfig option
396398
FavourReRaise:EnabledConfig option
399+
FavourBasicControlFlow:EnabledConfig option
397400
NestedStatements:RuleConfig<NestedStatements.Config> option
398401
FavourConsistentThis:RuleConfig<FavourConsistentThis.Config> option
399402
CyclomaticComplexity:RuleConfig<CyclomaticComplexity.Config> option
@@ -477,6 +480,7 @@ with
477480
AvoidTooShortNames = None
478481
RedundantNewKeyword = None
479482
FavourReRaise = None
483+
FavourBasicControlFlow = None
480484
NestedStatements = None
481485
FavourConsistentThis = None
482486
CyclomaticComplexity = None
@@ -623,6 +627,7 @@ let flattenConfig (config:Configuration) =
623627
config.AvoidTooShortNames |> Option.bind (constructRuleIfEnabled AvoidTooShortNames.rule)
624628
config.RedundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule)
625629
config.FavourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule)
630+
config.FavourBasicControlFlow |> Option.bind (constructRuleIfEnabled FavourBasicControlFlow.rule)
626631
config.NestedStatements |> Option.bind (constructRuleWithConfig NestedStatements.rule)
627632
config.FavourConsistentThis |> Option.bind (constructRuleWithConfig FavourConsistentThis.rule)
628633
config.CyclomaticComplexity |> Option.bind (constructRuleWithConfig CyclomaticComplexity.rule)

src/FSharpLint.Core/FSharpLint.Core.fsproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
<Compile Include="Rules\Conventions\CyclomaticComplexity.fs" />
5050
<Compile Include="Rules\Conventions\FavourReRaise.fs" />
5151
<Compile Include="Rules\Conventions\FavourConsistentThis.fs" />
52+
<Compile Include="Rules\Conventions\FavourBasicControlFlow.fs" />
5253
<Compile Include="Rules\Conventions\RaiseWithTooManyArguments\RaiseWithTooManyArgumentsHelper.fs" />
5354
<Compile Include="Rules\Conventions\RaiseWithTooManyArguments\FailwithWithSingleArgument.fs" />
5455
<Compile Include="Rules\Conventions\RaiseWithTooManyArguments\RaiseWithSingleArgument.fs" />
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
module FSharpLint.Rules.FavourBasicControlFlow
2+
3+
open FSharpLint.Framework
4+
open FSharpLint.Framework.Suggestion
5+
open FSharp.Compiler.Syntax
6+
open FSharp.Compiler.Text
7+
open FSharpLint.Framework.Ast
8+
open FSharpLint.Framework.Rules
9+
10+
let private isBasicControlFlow (synMatchClauses: List<SynMatchClause>) =
11+
let isFirstClauseTarget firstClause =
12+
match firstClause with
13+
| SynMatchClause(SynPat.Named(_, _, _, _, _), _, _, _, _)
14+
| SynMatchClause(SynPat.Const( _, _), _, _, _, _)
15+
| SynMatchClause(SynPat.LongIdent(LongIdentWithDots _, _, _, SynArgPats.Pats [], _, _), _, _, _, _) -> true
16+
| _ -> false
17+
18+
let isLastClauseTarget lastClause =
19+
match lastClause with
20+
| SynMatchClause(SynPat.Wild _, None, _, _, _) -> true
21+
| _ -> false
22+
23+
synMatchClauses.Length = 2 && isFirstClauseTarget synMatchClauses.[0] && isLastClauseTarget synMatchClauses.[1]
24+
25+
let runner args =
26+
match args.AstNode with
27+
| AstNode.Expression(SynExpr.Match (_, _, synMatchClauses, range)) when isBasicControlFlow synMatchClauses ->
28+
{ Range = range
29+
Message = Resources.GetString "FavourBasicControlFlow"
30+
SuggestedFix = None
31+
TypeChecks = List.empty }
32+
|> Array.singleton
33+
| _ -> Array.empty
34+
35+
let rule =
36+
{ Name = "FavourBasicControlFlow"
37+
Identifier = Identifiers.FavourBasicControlFlow
38+
RuleConfig = { AstNodeRuleConfig.Runner = runner; Cleanup = ignore } }
39+
|> AstNodeRule

src/FSharpLint.Core/Rules/Identifiers.fs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,4 @@ let FailwithBadUsage = identifier 72
8080
let FavourReRaise = identifier 73
8181
let FavourConsistentThis = identifier 74
8282
let AvoidTooShortNames = identifier 75
83+
let FavourBasicControlFlow = identifier 76

src/FSharpLint.Core/Text.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,4 +336,7 @@
336336
<data name="RulesFavourConsistentThis" xml:space="preserve">
337337
<value>Prefer using '{0}' consistently.</value>
338338
</data>
339+
<data name="FavourBasicControlFlow" xml:space="preserve">
340+
<value>Rather use simpler if-else construct instead of match clause.</value>
341+
</data>
339342
</root>

src/FSharpLint.Core/fsharplint.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@
273273
}
274274
},
275275
"avoidTooShortNames": { "enabled": false },
276+
"favourBasicControlFlow": { "enabled": false },
276277
"indentation": {
277278
"enabled": false
278279
},

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
<Compile Include="Rules\Conventions\NoPartialFunctions.fs" />
3838
<Compile Include="Rules\Conventions\FavourReRaise.fs" />
3939
<Compile Include="Rules\Conventions\FavourConsistentThis.fs" />
40+
<Compile Include="Rules\Conventions\FavourBasicControlFlow.fs" />
4041
<Compile Include="Rules\Conventions\AvoidTooShortNames.fs" />
4142
<Compile Include="Rules\Conventions\Naming\NamingHelpers.fs" />
4243
<Compile Include="Rules\Conventions\Naming\InterfaceNames.fs" />
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
module FSharpLint.Core.Tests.Rules.Conventions.FavourBasicControlFlow
2+
3+
open NUnit.Framework
4+
open FSharpLint.Rules
5+
6+
[<TestFixture>]
7+
type TestConventionsFavourFavourBasicControlFlow() =
8+
inherit TestAstNodeRuleBase.TestAstNodeRuleBase(FavourBasicControlFlow.rule)
9+
10+
11+
[<Test>]
12+
member this.MoreThanTwoClausesShouldNotProduceError() =
13+
this.Parse """
14+
match foo with
15+
| bar -> ()
16+
| baz -> ()
17+
| _ -> () """
18+
19+
Assert.IsTrue this.NoErrorsExist
20+
21+
22+
[<Test>]
23+
member this.TwoClausesWithoutWildcardShouldNotProduceError() =
24+
this.Parse """
25+
match foo with
26+
| bar -> ()
27+
| baz -> () """
28+
29+
Assert.IsTrue this.NoErrorsExist
30+
31+
[<Test>]
32+
member this.ThePresenceOfDUShouldNotProduceError() =
33+
this.Parse """
34+
match foo with
35+
| Bar baz -> ()
36+
| _ -> () """
37+
38+
Assert.IsTrue this.NoErrorsExist
39+
40+
[<Test>]
41+
member this.TwoClausesWithWildcardShouldProduceError() =
42+
this.Parse """
43+
match foo with
44+
| bar -> ()
45+
| _ -> () """
46+
47+
Assert.IsTrue this.ErrorsExist

0 commit comments

Comments
 (0)