DRAFT: Add new rule AvoidTypeHintSuffixesInNames

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

@@ -0,0 +1,37 @@
+---
+title: FL0077
+category: how-to
+hide_menu: true
+---
+
+# AvoidTypeHintSuffixesInNames (FL0077)
+
+*Introduced in `0.21.2`*
+
+## Cause
+
+Use of type hints in names suffixes.
+
+## Rationale
+
+The similar practice that predates this was the Systems Hungarian notation dialect that consisted in adding a type-hint as a
+prefix for variable names. While still debatable, this practice is not being used anymore because of the redundancy that it
+brings (especially in statically typed languages); and using suffixes (instead of prefixes) is no different. This rule,
+however, will not flag variables/constants, just type members, because they are the ones that have higher chances to be
+affected by the biggest downside from this bad practice: possible inconsistency when the type is modified (and subsequent
+need to modify the member name besides the type, which increases the brittleness factor of the API, especially when referring
+to public members).
+
+## How To Fix
+
+Just remove the type hint suffix; example: a member named "NamesList" of type `List<string>` can simply be named "Names" (so
+that in the future could be changed to a Seq or Array without the need to modify the name, or the risk of its name becoming
+inconsistent).
+
+## Rule Settings
+
+    {
+        "AvoidTypeHintSuffixesInNames": {
+           "enabled": false
+        }
+    }

src/FSharpLint.Core/Application/Configuration.fs

@@ -319,6 +319,7 @@ type ConventionsConfig =
       numberOfItems:NumberOfItemsConfig option
       binding:BindingConfig option
       favourReRaise:EnabledConfig option
+      avoidTypeHintSuffixesInNames: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.avoidTypeHintSuffixesInNames |> Option.bind (constructRuleIfEnabled AvoidTypeHintSuffixesInNames.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
+      AvoidTypeHintSuffixesInNames: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
+        AvoidTypeHintSuffixesInNames = 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.AvoidTypeHintSuffixesInNames |> Option.bind (constructRuleIfEnabled AvoidTypeHintSuffixesInNames.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\AvoidTypeHintSuffixesInNames.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/AvoidTypeHintSuffixesInNames.fs

@@ -0,0 +1,93 @@
+module FSharpLint.Rules.AvoidTypeHintSuffixesInNames
+
+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 ruleName: string = "AvoidTypeHintSuffixesInNames"
+let discouragedMemberSuffixes: List<string> = ["Lst"; "List"; "Array"; "Opt"; "Str"]
+
+let private hasTypeHint (identifier: string) =
+    let likelySuffixes =
+        discouragedMemberSuffixes
+        |> List.filter (fun text -> not (identifier.Equals text))
+    likelySuffixes |> List.exists identifier.EndsWith
+
+let private generateError (range: range) =
+    { Range = range
+      Message = Resources.GetString ruleName
+      SuggestedFix = None
+      TypeChecks = List.Empty }
+    |> Array.singleton
+
+let typeHintSuffixesInRecordFields (fields: List<SynField>) (range: range) =
+    let rec traverse recordFields =
+        match recordFields with
+        | SynField(_, _, maybeVal, _, _, _, _, _)::rest ->
+            match maybeVal with
+            | Some field ->
+                if hasTypeHint field.idText then
+                    generateError range
+                else
+                    traverse rest
+            | None ->
+                traverse rest
+        | [] -> Array.empty
+
+    traverse fields
+
+let typeHintSuffixesInUnionFields (fields: List<SynUnionCase>) (range: range) =
+    let rec traverse unionCases =
+        match unionCases with
+        | SynUnionCase(_, ident, _, _, _, _)::rest ->
+            if hasTypeHint ident.idText then
+                generateError range
+            else
+                traverse rest
+        | [] -> Array.empty
+
+    traverse fields
+
+let typeHintSuffixesInProperties (members: List<SynMemberDefn>) (range: range) =
+    let rec traverse memberDefinitions =
+        match memberDefinitions with
+        | SynMemberDefn.AutoProperty(_, _, ident, _, _, _, _, _, _expression, _, _)::rest ->
+            if hasTypeHint ident.idText then
+                generateError range
+            else
+                traverse rest
+        | SynMemberDefn.Member(SynBinding(_, _, _, _, _, _, _, pattern, _, _expression, _, _), _)::rest ->
+            match pattern with
+            | SynPat.LongIdent(LongIdentWithDots(ident, _), _, _, _, _, _) ->
+                if hasTypeHint ident.Tail.Head.idText then
+                    generateError range
+                else
+                    traverse rest
+            | _ -> traverse rest
+        | _::rest -> traverse rest
+        | [] -> Array.empty
+
+    traverse members
+
+let runner args =
+    match args.AstNode with
+    | TypeDefinition(SynTypeDefn(_, typeRepresentation, _members, _implicitCtor, range)) ->
+        match typeRepresentation with
+        | SynTypeDefnRepr.Simple(SynTypeDefnSimpleRepr.Record(_, fields, _), _) ->
+            typeHintSuffixesInRecordFields fields range
+        | SynTypeDefnRepr.Simple(SynTypeDefnSimpleRepr.Union(_, fields, _), _) ->
+            typeHintSuffixesInUnionFields fields range
+        | SynTypeDefnRepr.ObjectModel(_, members, _) ->
+            typeHintSuffixesInProperties members range
+        | _ -> Array.empty
+    | _ -> Array.empty
+
+let rule =
+    { Name = ruleName
+      Identifier = Identifiers.AvoidTypeHintSuffixesInNames
+      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 AvoidTypeHintSuffixesInNames = 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="AvoidTypeHintSuffixesInNames" xml:space="preserve">
+    <value>Consider removing from the name the suffix that gives a hint about its type.</value>
+  </data>
 </root>

src/FSharpLint.Core/fsharplint.json

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

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

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

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

@@ -0,0 +1,79 @@
+module FSharpLint.Core.Tests.Rules.Conventions.AvoidTypeHintSuffixesInNames
+
+open NUnit.Framework
+open FSharpLint.Rules
+open System
+
+[<TestFixture>]
+type TestConventionsAvoidTypeHintSuffixesInNames() =
+    inherit TestAstNodeRuleBase.TestAstNodeRuleBase(AvoidTypeHintSuffixesInNames.rule)
+
+    [<Test>]
+    member this.``Lint flags record member(s) with type hints``() =
+        this.Parse """
+module Person =
+    type FullName = { FirstName: string; SurnamesList: List<string> } """
+
+        Assert.IsTrue this.ErrorsExist
+
+    [<Test>]
+    member this.``Lint flags discriminated union member(s) with type hints``() =
+        this.Parse """
+type Tree =
+    | Scalar
+    | NodeList of int * Tree * Tree """
+
+        Assert.IsTrue this.ErrorsExist
+
+    [<Test>]
+    member this.``Lint flags autoproperty with type hints``() =
+        this.Parse """
+type MyClass() =
+    let random  = new System.Random()
+    member val RandomOpt = (random.Next() |> Some) with get, set """
+
+        Assert.IsTrue this.ErrorsExist
+
+    [<Test>]
+    member this.``Lint flags property with type hints``() =
+        this.Parse """
+type MyClass() =
+   let mutable someString = "someString"
+   member this.SomeStr with get() = someString and set(v : string) = someString <- v """
+
+        Assert.IsTrue this.ErrorsExist
+
+    [<Test>]
+    member this.``Lint does not flag discriminated union member(s) without type hints``() =
+        this.Parse """
+type Tree =
+    | Tip
+    | Node of int * Tree * Tree """
+
+        Assert.IsTrue this.NoErrorsExist
+
+    [<Test>]
+    member this.``Lint only flags type hints when they are suffixes``() =
+        this.Parse """
+type Tree =
+    | Opt
+    | Node of int * Tree * Tree """
+
+        Assert.IsTrue this.NoErrorsExist
+
+    [<Test>]
+    member this.``Lint does not flag record member(s) without type hints``() =
+        this.Parse """
+module Person =
+    type FullName = { First: string; Last: string } """
+
+        Assert.IsTrue this.NoErrorsExist
+
+    [<Test>]
+    member this.``Lint does not flag variable(s) even if they have type hints``() =
+        this.Parse """
+let getNothing () =
+    let valOpt = 90
+    valOpt """
+
+        Assert.IsTrue this.NoErrorsExist