Add FavourAsKeyword rule (#709)

* FavourAsKeyword: add rule and config

* FavourAsKeyword: add docs

* FavourAsKeyword: add tests

* FavourAsKeyword: add rule to fsharplint.json

* FavourAsKeyword: refactor tests

* FavourAsKeyword: refactor error message text

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

@@ -126,3 +126,4 @@ The following rules can be specified for linting.
 - [UnneededRecKeyword (FL0083)](rules/FL0083.html)
 - [FavourNonMutablePropertyInitialization (FL0084)](rules/FL0084.html)
 - [EnsureTailCallDiagnosticsInRecursiveFunctions (FL0085)](rules/FL0085.html)
+- [FavourAsKeyword (FL0086)](rules/FL0086.html)

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

@@ -0,0 +1,29 @@
+---
+title: FL0086
+category: how-to
+hide_menu: true
+---
+
+# FavourAsKeyword (FL0086)
+
+*Introduced in `0.24.3`*
+
+## Cause
+
+A named pattern is used just to be compared in the guard against a constant expression e.g. `match something with | bar when bar = "baz" -> ()`
+
+## Rationale
+
+The named pattern can be changed to an as pattern that uses a constant pattern, improving the pattern matching exhaustiveness check
+
+## How To Fix
+
+Remove the guard and replace the named pattern with the as pattern using a constant pattern, e.g. change `match something with | bar when bar = "baz" -> ()` to `match something with | "baz" as bar -> ()`
+
+## Rule Settings
+
+    {
+        "favourAsKeyword": {
+            "enabled": true
+        }
+    }

src/FSharpLint.Core/Application/Configuration.fs

@@ -297,6 +297,7 @@ type BindingConfig =
       wildcardNamedWithAsPattern:EnabledConfig option
       uselessBinding:EnabledConfig option
       tupleOfWildcards:EnabledConfig option
+      favourAsKeyword:EnabledConfig option
       favourTypedIgnore:EnabledConfig option }
 with
     member this.Flatten() =
@@ -306,6 +307,7 @@ with
             this.wildcardNamedWithAsPattern |> Option.bind (constructRuleIfEnabled WildcardNamedWithAsPattern.rule) |> Option.toArray
             this.uselessBinding |> Option.bind (constructRuleIfEnabled UselessBinding.rule) |> Option.toArray
             this.tupleOfWildcards |> Option.bind (constructRuleIfEnabled TupleOfWildcards.rule) |> Option.toArray
+            this.favourAsKeyword |> Option.bind (constructRuleIfEnabled FavourAsKeyword.rule) |> Option.toArray
         |] |> Array.concat
 
 type ConventionsConfig =
@@ -479,7 +481,8 @@ type Configuration =
       NoTabCharacters:EnabledConfig option
       NoPartialFunctions:RuleConfig<NoPartialFunctions.Config> option
       SuggestUseAutoProperty:EnabledConfig option
-      EnsureTailCallDiagnosticsInRecursiveFunctions:EnabledConfig option }
+      EnsureTailCallDiagnosticsInRecursiveFunctions:EnabledConfig option
+      FavourAsKeyword:EnabledConfig option }
 with
     static member Zero = {
         Global = None
@@ -571,6 +574,7 @@ with
         NoPartialFunctions = None
         SuggestUseAutoProperty = None
         EnsureTailCallDiagnosticsInRecursiveFunctions = None
+        FavourAsKeyword = None
     }
 
 // fsharplint:enable RecordFieldNames
@@ -725,6 +729,7 @@ let flattenConfig (config:Configuration) =
             config.NoTabCharacters |> Option.bind (constructRuleIfEnabled NoTabCharacters.rule)
             config.NoPartialFunctions |> Option.bind (constructRuleWithConfig NoPartialFunctions.rule)
             config.EnsureTailCallDiagnosticsInRecursiveFunctions |> Option.bind (constructRuleIfEnabled EnsureTailCallDiagnosticsInRecursiveFunctions.rule)
+            config.FavourAsKeyword |> Option.bind (constructRuleIfEnabled FavourAsKeyword.rule)
         |] |> Array.choose id
 
     if config.NonPublicValuesNames.IsSome &&

src/FSharpLint.Core/FSharpLint.Core.fsproj

@@ -112,6 +112,7 @@
     <Compile Include="Rules\Conventions\Binding\WildcardNamedWithAsPattern.fs" />
     <Compile Include="Rules\Conventions\Binding\UselessBinding.fs" />
     <Compile Include="Rules\Conventions\Binding\TupleOfWildcards.fs" />
+    <Compile Include="Rules\Conventions\Binding\FavourAsKeyword.fs" />
     <Compile Include="Rules\Conventions\AvoidTooShortNames.fs" />
     <Compile Include="Rules\Typography\Indentation.fs" />
     <Compile Include="Rules\Typography\MaxCharactersOnLine.fs" />

src/FSharpLint.Core/Rules/Conventions/Binding/FavourAsKeyword.fs

@@ -0,0 +1,58 @@
+module FSharpLint.Rules.FavourAsKeyword
+
+open System
+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 checkForNamedPatternEqualsConstant (args:AstNodeRuleParams) pattern whenExpr (range:Range) =
+    let patternIdent =
+        match pattern with
+        | SynPat.Named(SynIdent.SynIdent(ident, _), _, _, _) -> Some(ident.idText)
+        | _ -> None
+
+
+    match whenExpr with
+    | SynExpr.App(_, _, funcExpr, SynExpr.Const(_, constRange), _) ->
+        match funcExpr with
+        | SynExpr.App(_, _, ExpressionUtilities.Identifier([opIdent], _), SynExpr.Ident(ident), _)
+            when opIdent.idText = "op_Equality" && Option.contains ident.idText patternIdent ->
+
+            let fromRange = Range.mkRange "" range.Start constRange.End
+
+            let suggestedFix =
+                ExpressionUtilities.tryFindTextOfRange fromRange args.FileContent
+                |> Option.bind (fun text ->
+
+                    ExpressionUtilities.tryFindTextOfRange constRange args.FileContent
+                    |> Option.bind (fun constText ->
+
+                        lazy (Some { FromText = text; FromRange = fromRange; ToText = constText + " as " +  ident.idText})
+                        |> Some
+                    )
+
+                )
+
+            { Range = fromRange
+              Message = Resources.GetString("RulesFavourAsKeyword")
+              SuggestedFix = suggestedFix
+              TypeChecks = [] } |> Array.singleton
+
+        | _ -> Array.empty
+    | _ -> Array.empty
+
+let private runner (args:AstNodeRuleParams) =
+    match args.AstNode with
+    | AstNode.Match(SynMatchClause.SynMatchClause(pat, Some(whenExpr), _, range, _, _)) ->
+        checkForNamedPatternEqualsConstant args pat whenExpr range
+
+    | _ -> Array.empty
+
+let rule =
+    { Name = "FavourAsKeyword"
+      Identifier = Identifiers.FavourAsKeyword
+      RuleConfig = { AstNodeRuleConfig.Runner = runner; Cleanup = ignore } }
+    |> AstNodeRule

src/FSharpLint.Core/Rules/Identifiers.fs

@@ -90,3 +90,4 @@ let UsedUnderscorePrefixedElements = identifier 82
 let UnneededRecKeyword = identifier 83
 let FavourNonMutablePropertyInitialization = identifier 84
 let EnsureTailCallDiagnosticsInRecursiveFunctions = identifier 85
+let FavourAsKeyword = identifier 86

src/FSharpLint.Core/Text.resx

@@ -368,5 +368,8 @@
   </data>
   <data name="RulesEnsureTailCallDiagnosticsInRecursiveFunctions" xml:space="preserve">
     <value>The '{0}' function has a "rec" keyword, but no [&lt;TailCall&gt;] attribute. Consider adding [&lt;TailCall&gt;] attribute to the function and &lt;WarningsAsErrors&gt;FS3569&lt;/WarningsAsErrors&gt; property to project file (but only on .NET 8 and higher).</value>
-  </data>  
+  </data>
+  <data name="RulesFavourAsKeyword" xml:space="preserve">
+    <value>Prefer using the 'as' pattern to match a constant and bind it to a variable.</value>
+  </data>
 </root>

src/FSharpLint.Core/fsharplint.json

@@ -332,6 +332,7 @@
         }
     },
     "ensureTailCallDiagnosticsInRecursiveFunctions": { "enabled": false },
+    "favourAsKeyword": { "enabled": true },
     "hints": {
         "add": [
             "not (a =  b) ===> a <> b",

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

@@ -80,6 +80,7 @@
     <Compile Include="Rules\Binding\UselessBinding.fs" />
     <Compile Include="Rules\Binding\WildcardNamedWithAsPattern.fs" />
     <Compile Include="Rules\Binding\TupleOfWildcards.fs" />
+    <Compile Include="Rules\Binding\FavourAsKeyword.fs" />
     <Compile Include="Rules\Hints\HintMatcher.fs" />
   </ItemGroup>
 

tests/FSharpLint.Core.Tests/Rules/Binding/FavourAsKeyword.fs

@@ -0,0 +1,53 @@
+module FSharpLint.Core.Tests.Rules.Binding.FavourAsKeyword
+
+open NUnit.Framework
+open FSharpLint.Rules
+
+[<TestFixture>]
+type TestBindingFavourAsKeyword() =
+    inherit TestAstNodeRuleBase.TestAstNodeRuleBase(FavourAsKeyword.rule)
+
+    [<Test>]
+    member this.FavourAsKeywordShouldQuickFix() =
+        let source = """
+module Program
+
+match "" with
+| bar when bar = "baz" -> ()
+"""
+
+        this.Parse(source)
+
+        let expected = """
+module Program
+
+match "" with
+| "baz" as bar -> ()
+"""
+
+        Assert.AreEqual(expected, this.ApplyQuickFix source)
+
+
+    [<Test>]
+    member this.FavourAsKeywordShouldProduceError() =
+        this.Parse """
+module Program
+
+match "" with
+| bar when bar = "baz" -> ()
+"""
+
+        this.AssertErrorWithMessageExists("Prefer using the 'as' pattern to match a constant and bind it to a variable.")
+
+
+    [<Test>]
+    member this.FavourAsKeywordShouldNotProduceError() =
+        this.Parse """
+module Program
+
+match "" with
+| "baz" as bar -> ()
+"""
+
+        Assert.IsTrue(this.NoErrorsExist)
+