diff --git a/docs/content/how-tos/rule-configuration.md b/docs/content/how-tos/rule-configuration.md index 421beeabe..b0af74e66 100644 --- a/docs/content/how-tos/rule-configuration.md +++ b/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) diff --git a/docs/content/how-tos/rules/FL0089.md b/docs/content/how-tos/rules/FL0089.md new file mode 100644 index 000000000..054319575 --- /dev/null +++ b/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 + } + } diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index 1d4b62a49..44cc28a77 100644 --- a/src/FSharpLint.Core/Application/Configuration.fs +++ b/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 diff --git a/src/FSharpLint.Core/FSharpLint.Core.fsproj b/src/FSharpLint.Core/FSharpLint.Core.fsproj index 3188a59e7..86bb8d7e8 100644 --- a/src/FSharpLint.Core/FSharpLint.Core.fsproj +++ b/src/FSharpLint.Core/FSharpLint.Core.fsproj @@ -69,6 +69,7 @@ + diff --git a/src/FSharpLint.Core/Framework/ParseFile.fs b/src/FSharpLint.Core/Framework/ParseFile.fs index 6a5edcb56..b8e844401 100644 --- a/src/FSharpLint.Core/Framework/ParseFile.fs +++ b/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) diff --git a/src/FSharpLint.Core/Rules/Conventions/FavourSingleton.fs b/src/FSharpLint.Core/Rules/Conventions/FavourSingleton.fs new file mode 100644 index 000000000..1279c2efb --- /dev/null +++ b/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 } } diff --git a/src/FSharpLint.Core/Rules/Identifiers.fs b/src/FSharpLint.Core/Rules/Identifiers.fs index 7b2456824..910e968d6 100644 --- a/src/FSharpLint.Core/Rules/Identifiers.fs +++ b/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 diff --git a/src/FSharpLint.Core/Text.resx b/src/FSharpLint.Core/Text.resx index 97f8f804a..92b713b3b 100644 --- a/src/FSharpLint.Core/Text.resx +++ b/src/FSharpLint.Core/Text.resx @@ -387,4 +387,7 @@ Consider switching the indexer accessor to {0} style. + + Consider using List.singleton/Array.singleton instead of single member list/array. + diff --git a/src/FSharpLint.Core/fsharplint.json b/src/FSharpLint.Core/fsharplint.json index 4e37a666a..6077efb11 100644 --- a/src/FSharpLint.Core/fsharplint.json +++ b/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", diff --git a/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj b/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj index 319680687..b376875d1 100644 --- a/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj +++ b/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj @@ -48,6 +48,7 @@ + diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/FavourSingleton.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/FavourSingleton.fs new file mode 100644 index 000000000..3b4a9be85 --- /dev/null +++ b/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 + +[] +type TestConventionsFavourSingleton() = + inherit TestAstNodeRuleBase.TestAstNodeRuleBase(FavourSingleton.rule) + + [] + member this.ListWithManyItemsShouldNotProduceError() = + this.Parse """ +let foo = [ 10; 20 ]""" + + this.AssertNoWarnings() + + [] + member this.ListWithASingleConstantShouldProduceError() = + this.Parse """ +let foo = [ 10 ]""" + + Assert.IsTrue this.ErrorsExist + Assert.IsTrue(this.ErrorExistsAt(2, 12)) + + [] + member this.ListWithASingleIdentShouldProduceError() = + this.Parse """ +let bar = true +let foo = [ bar ]""" + + Assert.IsTrue this.ErrorsExist + Assert.IsTrue(this.ErrorExistsAt(3, 10)) + + [] + member this.ListWithMultipleIdentsShouldNotProduceError() = + this.Parse """ +let bar = true +let foo = [ bar; false; true ]""" + + this.AssertNoWarnings() + + [] + member this.ArrayWithManyItemsShouldNotProduceError() = + this.Parse """ +let foo = [| 10; 20 |]""" + + Assert.IsTrue this.NoErrorsExist + + [] + member this.ArrayWithASingleConstantShouldProduceError() = + this.Parse """ +let foo = [| 10 |]""" + + Assert.IsTrue this.ErrorsExist + Assert.IsTrue(this.ErrorExistsAt(2, 13)) + + [] + member this.ArrayWithASingleIdentShouldProduceError() = + this.Parse """ +let bar = true +let foo = [| bar |]""" + + Assert.IsTrue this.ErrorsExist + Assert.IsTrue(this.ErrorExistsAt(3, 10)) + + [] + member this.ArrayWithMultipleIdentsShouldNotProduceError() = + this.Parse """ +let bar = true +let foo = [| bar; false; true |]""" + + this.AssertNoWarnings() + + [] + member this.SingletonListWithMatchCaseShouldNotProduceError() = + this.Parse """ +let foo = List.empty +match foo with +| [x] -> printf x +| _ -> printf "baz" """ + + this.AssertNoWarnings() + + [] + member this.SingletonArrayWithMatchCaseShouldNotProduceError() = + this.Parse """ +let foo = Array.empty +match foo with +| [| x |] -> printf x +| _ -> printf "baz" """ + + this.AssertNoWarnings() diff --git a/tests/FSharpLint.Core.Tests/Rules/Hints/HintMatcher.fs b/tests/FSharpLint.Core.Tests/Rules/Hints/HintMatcher.fs index 311bd5c4b..05a5c9fe9 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Hints/HintMatcher.fs +++ b/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) \ No newline at end of file + Assert.AreEqual(expected, this.ApplyQuickFix source) + + [] + 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)