DRAFT: Add new rule FavourNamedMembers

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

@@ -117,3 +117,4 @@ The following rules can be specified for linting.
 - [FavourConsistentThis (FL0074)](rules/FL0074.html)
 - [AvoidTooShortNames (FL0075)](rules/FL0075.html)
 - [FavourStaticEmptyFields (FL0076)](rules/FL0076.html)
+- [FavourNamedMembers (FL0077)](rules/FL0077.html)

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

@@ -0,0 +1,27 @@
+---
+title: FL0077
+category: how-to
+hide_menu: true
+---
+
+# FavourNamedMembers (FL0077)
+
+*Introduced in `0.21.1`*
+
+## Cause
+
+Rule to detect unnamed members in discriminated unions and pattern matching on them.
+
+## Rationale
+
+Using names aids readibility.
+
+## How To Fix
+
+Add names to the members of discriminated unions and their pattern matching cases.
+
+## Rule Settings
+
+    {
+        "favourNamedMembers": { "enabled": false }
+    }

src/FSharpLint.Core/Application/Configuration.fs

@@ -309,6 +309,7 @@ type ConventionsConfig =
       avoidTooShortNames:EnabledConfig option
       redundantNewKeyword:EnabledConfig option
       favourStaticEmptyFields:EnabledConfig option
+      favourNamedMembers:EnabledConfig option
       nestedStatements:RuleConfig<NestedStatements.Config> option
       cyclomaticComplexity:RuleConfig<CyclomaticComplexity.Config> option
       reimplementsFunction:EnabledConfig option
@@ -328,6 +329,7 @@ with
             this.redundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule) |> Option.toArray
             this.favourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule) |> Option.toArray
             this.favourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule) |> Option.toArray
+            this.favourNamedMembers |> Option.bind (constructRuleIfEnabled FavourNamedMembers.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
@@ -397,6 +399,7 @@ type Configuration =
       RedundantNewKeyword:EnabledConfig option
       FavourReRaise:EnabledConfig option
       FavourStaticEmptyFields:EnabledConfig option
+      FavourNamedMembers:EnabledConfig option
       NestedStatements:RuleConfig<NestedStatements.Config> option
       FavourConsistentThis:RuleConfig<FavourConsistentThis.Config> option
       CyclomaticComplexity:RuleConfig<CyclomaticComplexity.Config> option
@@ -481,6 +484,7 @@ with
         RedundantNewKeyword = None
         FavourReRaise = None
         FavourStaticEmptyFields = None
+        FavourNamedMembers = None
         NestedStatements = None
         FavourConsistentThis = None
         CyclomaticComplexity = None
@@ -628,6 +632,7 @@ let flattenConfig (config:Configuration) =
             config.RedundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule)
             config.FavourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule)
             config.FavourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule)
+            config.FavourNamedMembers |> Option.bind (constructRuleIfEnabled FavourNamedMembers.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

@@ -43,6 +43,7 @@
     <Compile Include="Rules\Formatting\TypePrefixing.fs" />
     <Compile Include="Rules\Formatting\UnionDefinitionIndentation.fs" />
     <Compile Include="Rules\Conventions\FavourStaticEmptyFields.fs" />
+    <Compile Include="Rules\Conventions\FavourNamedMembers.fs" />
     <Compile Include="Rules\Conventions\RecursiveAsyncFunction.fs" />
     <Compile Include="Rules\Conventions\RedundantNewKeyword.fs" />
     <Compile Include="Rules\Conventions\NestedStatements.fs" />

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

@@ -0,0 +1,62 @@
+module FSharpLint.Rules.FavourNamedMembers
+
+open FSharpLint.Framework
+open FSharpLint.Framework.Suggestion
+open FSharp.Compiler.Syntax
+open FSharp.Compiler.Text
+open FSharpLint.Framework.Ast
+open FSharpLint.Framework.Rules
+open System
+
+let private checkIdentifierPart (identifier:Ident) (idText:string) =
+    let formatError errorName =
+        String.Format(Resources.GetString errorName, idText)
+
+    "RulesFavourNamedMembersError" |> formatError |> Array.singleton
+
+let private checkIdentifier (identifier:Ident) (idText:string) =
+    if idText.Length > 0 then
+        checkIdentifierPart identifier idText
+        |> Array.map (fun message ->
+            { Range = identifier.idRange
+              Message = message
+              SuggestedFix = None
+              TypeChecks = List.Empty })
+    else
+        Array.empty
+
+let private getUnnamedFields (fields: SynField list): bool =
+    let rec loop syncFieldArray =
+        match syncFieldArray with
+        | SynField(_, _,maybeName, _, _, _, _, _)::tail ->
+            match maybeName with
+            | Some _ -> 
+                loop tail
+            | None ->  true
+        | _ -> false
+    loop fields 
+
+let getIdentifiers args =
+    match args.AstNode with
+    | AstNode.UnionCase(SynUnionCase(_, identifier, SynUnionCaseKind.Fields fields, _, _, range)) ->
+        if getUnnamedFields fields then
+            (identifier, identifier.idText, None) |> Array.singleton
+        else
+            Array.empty
+    | AstNode.Match(SynMatchClause(SynPat.LongIdent(LongIdentWithDots (ids, _) ,_ ,_ ,SynArgPats.Pats(_),_,_), _, _, _ ,_)) ->        
+        match ids with
+        | head::_ -> (head, head.idText, None) |> Array.singleton
+        | _ -> Array.empty        
+    | _ -> Array.empty
+
+let runner (args:AstNodeRuleParams) =
+    getIdentifiers args
+    |> Array.collect (fun (identifier, idText, typeCheck) ->
+        let suggestions = checkIdentifier identifier idText
+        suggestions |> Array.map (fun suggestion -> { suggestion with TypeChecks = Option.toList typeCheck }))
+
+let rule =
+    { Name = "FavourNamedMembers"
+      Identifier = Identifiers.FavourNamedMembers
+      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 FavourNamedMembers = 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="RulesFavourNamedMembersError" xml:space="preserve">
+    <value>Prefer adding a name for the member.</value>
+  </data>
 </root>

src/FSharpLint.Core/fsharplint.json

@@ -267,6 +267,7 @@
     "favourTypedIgnore": { "enabled": false },
     "favourReRaise": { "enabled": true },
     "favourStaticEmptyFields": { "enabled": false },
+    "favourNamedMembers": { "enabled": true },
     "favourConsistentThis": {
         "enabled": false,
         "config": {

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

@@ -27,6 +27,7 @@
     <Compile Include="Rules\Formatting\UnionDefinitionIndentation.fs" />
     <Compile Include="Rules\Formatting\TypePrefixing.fs" />
     <Compile Include="Rules\Conventions\FavourStaticEmptyFields.fs" />
+    <Compile Include="Rules\Conventions\FavourNamedMembers.fs" />
     <Compile Include="Rules\Conventions\RecursiveAsyncFunction.fs" />
     <Compile Include="Rules\Conventions\RedundantNewKeyword.fs" />
     <Compile Include="Rules\Conventions\RaiseWithTooManyArguments.fs" />

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

@@ -0,0 +1,50 @@
+module FSharpLint.Core.Tests.Rules.Conventions.FavourNamedMembers
+
+open NUnit.Framework
+open FSharpLint.Rules
+open System
+
+[<TestFixture>]
+type TestConventionsFavourNamedMembers() =
+    inherit TestAstNodeRuleBase.TestAstNodeRuleBase(FavourNamedMembers.rule)
+
+    [<Test>]
+    member this.NamedMembersShouldNotProduceError_1() =
+        this.Parse """
+open System
+
+let examineData x =
+    match data with
+    | OnePartData(part1=p1) -> p1
+    | TwoPartData(part1=p1; part2=p2) -> p1 + p2"""
+
+        Assert.IsTrue this.NoErrorsExist
+
+    [<Test>]
+    member this.NamedMembersShouldNotProduceError_2() =
+        this.Parse """
+type Data =
+    | TwoParts of part1: string * part2: string
+    | OnePart of part0: string"""
+
+        Assert.IsTrue this.NoErrorsExist
+
+    [<Test>]
+    member this.UnnamedMembersSelfShouldProduceError_1() =
+        this.Parse """
+let examineData x =
+    match data with
+    | OnePart p1 -> p1
+    | TwoParts (p1, p2) -> p1 + p2"""
+
+        Assert.IsTrue this.ErrorsExist
+
+    [<Test>]
+    member this.UnnamedMembersSelfShouldProduceError_2() =
+        this.Parse """
+type Data =
+    | TwoParts of string * string
+    | OnePart of string"""
+
+        Assert.IsTrue this.ErrorsExist
+