Add new rule FavourSingleton

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

@@ -129,3 +129,4 @@ The following rules can be specified for linting.
 - [FavourAsKeyword (FL0086)](rules/FL0086.html)
 - [InterpolatedStringWithNoSubstitution (FL0087)](rules/FL0087.html)
 - [IndexerAccessorStyleConsistency (FL0088)](rules/FL0088.html)
+- [FavourSingleton (FL0089)](rules/FL0089.html)

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

@@ -0,0 +1,32 @@
+---
+title: FL0089
+category: how-to
+hide_menu: true
+---
+
+# FavourSingleton (FL0089)
+
+*Introduced in `0.26.10`*
+
+## Cause
+
+Rule to detect usage of lists or arrays with only one item.
+
+## Rationale
+
+`List.singleton foo`/`Array.singleton foo` is more readable and explicit than `[foo]`/`[|foo|]`.
+Especially if we take in account that F# newbies may not yet be familiar with the difference between
+inline array vs list instantiation; and square brackets with only one element could potentially be
+confused with an indexing accessor.
+
+## How To Fix
+
+Replace all occurrences of single member lists/arrays with a call to List.singleton/Array.singleton.
+
+## Rule Settings
+
+    {
+        "favourSingleton": {
+            "enabled": false
+        }
+    }

src/FSharpLint.Core/Application/Configuration.fs

@@ -513,7 +513,8 @@ type Configuration =
       SuggestUseAutoProperty:EnabledConfig option
       EnsureTailCallDiagnosticsInRecursiveFunctions:EnabledConfig option
       FavourAsKeyword:EnabledConfig option 
-      InterpolatedStringWithNoSubstitution:EnabledConfig option }
+      InterpolatedStringWithNoSubstitution:EnabledConfig option
+      FavourSingleton:EnabledConfig option }
 with
     static member Zero = {
         Global = None
@@ -610,6 +611,7 @@ with
         EnsureTailCallDiagnosticsInRecursiveFunctions = None
         FavourAsKeyword = None
         InterpolatedStringWithNoSubstitution = None
+        FavourSingleton = None
     }
 
 // fsharplint:enable RecordFieldNames
@@ -807,6 +809,7 @@ let flattenConfig (config:Configuration) =
                 config.EnsureTailCallDiagnosticsInRecursiveFunctions |> Option.bind (constructRuleIfEnabled EnsureTailCallDiagnosticsInRecursiveFunctions.rule)
                 config.FavourAsKeyword |> Option.bind (constructRuleIfEnabled FavourAsKeyword.rule)
                 config.InterpolatedStringWithNoSubstitution |> Option.bind (constructRuleIfEnabled InterpolatedStringWithNoSubstitution.rule)
+                config.FavourSingleton |> Option.bind (constructRuleIfEnabled FavourSingleton.rule)
             |]
 
     findDeprecation config deprecatedAllRules allRules

src/FSharpLint.Core/FSharpLint.Core.fsproj

@@ -69,6 +69,7 @@
     <Compile Include="Rules\Smells\RaiseWithTooManyArguments\InvalidArgWithTwoArguments.fs" />
     <Compile Include="Rules\Smells\RaiseWithTooManyArguments\FailwithfWithArgumentsMatchingFormatString.fs" />
     <Compile Include="Rules\Conventions\FailwithBadUsage.fs" />
+    <Compile Include="Rules\Conventions\FavourSingleton.fs" />
     <Compile Include="Rules\Conventions\SourceLength\SourceLengthHelper.fs" />
     <Compile Include="Rules\Conventions\SourceLength\MaxLinesInLambdaFunction.fs" />
     <Compile Include="Rules\Conventions\SourceLength\MaxLinesInMatchLambdaFunction.fs" />

src/FSharpLint.Core/Framework/ParseFile.fs

@@ -57,7 +57,7 @@ module ParseFile =
     let getProjectOptionsFromScript (checker:FSharpChecker) file (source:string) = async {
         let sourceText = SourceText.ofString source
         let assumeDotNetFramework = false
-        let otherOpts = [| "--targetprofile:netstandard" |]
+        let otherOpts = Array.singleton "--targetprofile:netstandard"
 
         let! options, _diagnostics =
             checker.GetProjectOptionsFromScript(file, sourceText, assumeDotNetFramework = assumeDotNetFramework, useSdkRefs = not assumeDotNetFramework, otherFlags = otherOpts)

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

@@ -0,0 +1,37 @@
+module FSharpLint.Rules.FavourSingleton
+
+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 runner args =
+    let generateViolation range =
+        let msg = Resources.GetString "RulesFavourSingleton"
+        Array.singleton
+            { Range = range
+              Message = msg
+              SuggestedFix = None
+              TypeChecks = List.Empty }
+    match args.AstNode with
+    | AstNode.Binding(SynBinding(_, _, _, _, _, _, _, _, _, expression, _, _, _)) ->
+        match expression with
+        | SynExpr.ArrayOrListComputed(_isArray, innerExpr, range) ->
+            match innerExpr with
+            | SynExpr.Const(_, range) ->
+              generateViolation range
+            | SynExpr.Ident _ ->
+              generateViolation range
+            | _ -> Array.empty
+        | _ -> Array.empty
+    | _ -> Array.empty
+let rule =
+    AstNodeRule
+        { Name = "FavourSingleton"
+          Identifier = Identifiers.FavourSingleton
+          RuleConfig =
+              { AstNodeRuleConfig.Runner = runner
+                Cleanup = ignore } }

src/FSharpLint.Core/Rules/Identifiers.fs

@@ -93,3 +93,4 @@ let EnsureTailCallDiagnosticsInRecursiveFunctions = identifier 85
 let FavourAsKeyword = identifier 86
 let InterpolatedStringWithNoSubstitution = identifier 87
 let IndexerAccessorStyleConsistency = identifier 88
+let FavourSingleton = identifier 89

src/FSharpLint.Core/Text.resx

@@ -387,4 +387,7 @@
   <data name="RulesIndexerAccessorStyleConsistency" xml:space="preserve">
     <value>Consider switching the indexer accessor to {0} style.</value>
   </data>
+  <data name="RulesFavourSingleton" xml:space="preserve">
+    <value>Consider using List.singleton/Array.singleton instead of single member list/array.</value>
+  </data>
 </root>

src/FSharpLint.Core/fsharplint.json

@@ -340,6 +340,7 @@
             "style": "OCaml"
         }
     },
+    "favourSingleton": { "enabled": false },
     "hints": {
         "add": [
             "not (a =  b) ===> a <> b",
@@ -450,7 +451,7 @@
             "pattern: x::[] ===> [x]",
 
             "x @ [] ===> x",
-            "[x] @ y ===> x::y",
+            "(List.singleton x) @ y ===> x :: y",
 
             "List.isEmpty [] ===> true",
             "Array.isEmpty [||] ===> true",

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

@@ -48,6 +48,7 @@
     <Compile Include="Rules\Conventions\FavourNonMutablePropertyInitialization.fs" />
     <Compile Include="Rules\Conventions\EnsureTailCallDiagnosticsInRecursiveFunctions.fs" />
     <Compile Include="Rules\Conventions\IndexerAccessorStyleConsistency.fs" />
+    <Compile Include="Rules\Conventions\FavourSingleton.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/FavourSingleton.fs

@@ -0,0 +1,94 @@
+module FSharpLint.Core.Tests.Rules.Conventions.FavourSingleton
+
+open System
+open NUnit.Framework
+open FSharpLint.Rules
+open FSharpLint.Core.Tests
+
+[<TestFixture>]
+type TestConventionsFavourSingleton() =
+    inherit TestAstNodeRuleBase.TestAstNodeRuleBase(FavourSingleton.rule)
+
+    [<Test>]
+    member this.ListWithManyItemsShouldNotProduceError() =
+        this.Parse """
+let foo = [ 10; 20 ]"""
+
+        this.AssertNoWarnings()
+
+    [<Test>]
+    member this.ListWithASingleConstantShouldProduceError() =
+        this.Parse """
+let foo = [ 10 ]"""
+
+        Assert.IsTrue this.ErrorsExist
+        Assert.IsTrue(this.ErrorExistsAt(2, 12))
+
+    [<Test>]
+    member this.ListWithASingleIdentShouldProduceError() =
+        this.Parse """
+let bar = true
+let foo = [ bar ]"""
+
+        Assert.IsTrue this.ErrorsExist
+        Assert.IsTrue(this.ErrorExistsAt(3, 10))
+
+    [<Test>]
+    member this.ListWithMultipleIdentsShouldNotProduceError() =
+        this.Parse """
+let bar = true
+let foo = [ bar; false; true ]"""
+
+        this.AssertNoWarnings()
+
+    [<Test>]
+    member this.ArrayWithManyItemsShouldNotProduceError() =
+        this.Parse """
+let foo = [| 10; 20 |]"""
+
+        Assert.IsTrue this.NoErrorsExist
+
+    [<Test>]
+    member this.ArrayWithASingleConstantShouldProduceError() =
+        this.Parse """
+let foo = [| 10 |]"""
+
+        Assert.IsTrue this.ErrorsExist
+        Assert.IsTrue(this.ErrorExistsAt(2, 13))
+
+    [<Test>]
+    member this.ArrayWithASingleIdentShouldProduceError() =
+        this.Parse """
+let bar = true
+let foo = [| bar |]"""
+
+        Assert.IsTrue this.ErrorsExist
+        Assert.IsTrue(this.ErrorExistsAt(3, 10))
+
+    [<Test>]
+    member this.ArrayWithMultipleIdentsShouldNotProduceError() =
+        this.Parse """
+let bar = true
+let foo = [| bar; false; true |]"""
+
+        this.AssertNoWarnings()
+
+    [<Test>]
+    member this.SingletonListWithMatchCaseShouldNotProduceError() =
+        this.Parse """
+let foo = List.empty
+match foo with
+| [x] -> printf x
+| _ -> printf "baz" """
+
+        this.AssertNoWarnings()
+
+    [<Test>]
+    member this.SingletonArrayWithMatchCaseShouldNotProduceError() =
+        this.Parse """
+let foo = Array.empty
+match foo with
+| [| x |] -> printf x
+| _ -> printf "baz" """
+
+        this.AssertNoWarnings()

tests/FSharpLint.Core.Tests/Rules/Hints/HintMatcher.fs

@@ -953,4 +953,14 @@ let x y =
 
         this.SetConfig(["List.map f (List.map g x) ===> List.map (g >> f) x"])
         this.Parse(source)
-        Assert.AreEqual(expected, this.ApplyQuickFix source)
+        Assert.AreEqual(expected, this.ApplyQuickFix source)
+
+    [<Test>]
+    member this.``List append of singleton item can be replaced with :: operator``() =
+        let source = """(List.singleton head) @ tail"""
+
+        let expected = """head :: tail"""
+
+        this.SetConfig(["(List.singleton x) @ y ===> x :: y"])
+        this.Parse(source)
+        Assert.AreEqual(expected, this.ApplyQuickFix source)