Skip to content

Commit 9ddeb16

Browse files
janusjanus
authored
Add new rule FavourStaticEmptyFields (#530)
It favours use of static empty fields.
1 parent 40a160a commit 9ddeb16

File tree

10 files changed

+262
-1
lines changed

10 files changed

+262
-1
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,4 +115,5 @@ The following rules can be specified for linting.
115115
- [FailwithBadUsage (FL0072)](rules/FL0072.html)
116116
- [FavourReRaise (FL0073)](rules/FL0073.html)
117117
- [FavourConsistentThis (FL0074)](rules/FL0074.html)
118-
- [AvoidTooShortNames (FL0075)](rules/FL0075.html)
118+
- [AvoidTooShortNames (FL0075)](rules/FL0075.html)
119+
- [FavourStaticEmptyFields (FL0076)](rules/FL0076.html)

‎docs/content/how-tos/rules/FL0076.md‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
title: FL0076
3+
category: how-to
4+
hide_menu: true
5+
---
6+
7+
# FavourStaticEmptyFields (FL0076)
8+
9+
*Introduced in `0.21.1`*
10+
11+
## Cause
12+
13+
Use of immediate string "", empty list [] or empty array [||].
14+
15+
## Rationale
16+
17+
Using static empty fields aids readibility.
18+
19+
## How To Fix
20+
21+
Use a static empty field such as String.Empty, List.Empty or Array.Empty.
22+
23+
## Rule Settings
24+
25+
{
26+
"favourStaticEmptyFields": { "enabled": false }
27+
}

‎src/FSharpLint.Core/Application/Configuration.fs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,7 @@ type ConventionsConfig =
308308
{ recursiveAsyncFunction:EnabledConfig option
309309
avoidTooShortNames:EnabledConfig option
310310
redundantNewKeyword:EnabledConfig option
311+
favourStaticEmptyFields:EnabledConfig option
311312
nestedStatements:RuleConfig<NestedStatements.Config> option
312313
cyclomaticComplexity:RuleConfig<CyclomaticComplexity.Config> option
313314
reimplementsFunction:EnabledConfig option
@@ -326,6 +327,7 @@ with
326327
this.avoidTooShortNames |> Option.bind (constructRuleIfEnabled AvoidTooShortNames.rule) |> Option.toArray
327328
this.redundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule) |> Option.toArray
328329
this.favourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule) |> Option.toArray
330+
this.favourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule) |> Option.toArray
329331
this.nestedStatements |> Option.bind (constructRuleWithConfig NestedStatements.rule) |> Option.toArray
330332
this.favourConsistentThis |> Option.bind (constructRuleWithConfig FavourConsistentThis.rule) |> Option.toArray
331333
this.cyclomaticComplexity |> Option.bind (constructRuleWithConfig CyclomaticComplexity.rule) |> Option.toArray
@@ -394,6 +396,7 @@ type Configuration =
394396
AvoidTooShortNames:EnabledConfig option
395397
RedundantNewKeyword:EnabledConfig option
396398
FavourReRaise:EnabledConfig option
399+
FavourStaticEmptyFields:EnabledConfig option
397400
NestedStatements:RuleConfig<NestedStatements.Config> option
398401
FavourConsistentThis:RuleConfig<FavourConsistentThis.Config> option
399402
CyclomaticComplexity:RuleConfig<CyclomaticComplexity.Config> option
@@ -477,6 +480,7 @@ with
477480
AvoidTooShortNames = None
478481
RedundantNewKeyword = None
479482
FavourReRaise = None
483+
FavourStaticEmptyFields = None
480484
NestedStatements = None
481485
FavourConsistentThis = None
482486
CyclomaticComplexity = None
@@ -623,6 +627,7 @@ let flattenConfig (config:Configuration) =
623627
config.AvoidTooShortNames |> Option.bind (constructRuleIfEnabled AvoidTooShortNames.rule)
624628
config.RedundantNewKeyword |> Option.bind (constructRuleIfEnabled RedundantNewKeyword.rule)
625629
config.FavourReRaise |> Option.bind (constructRuleIfEnabled FavourReRaise.rule)
630+
config.FavourStaticEmptyFields |> Option.bind (constructRuleIfEnabled FavourStaticEmptyFields.rule)
626631
config.NestedStatements |> Option.bind (constructRuleWithConfig NestedStatements.rule)
627632
config.FavourConsistentThis |> Option.bind (constructRuleWithConfig FavourConsistentThis.rule)
628633
config.CyclomaticComplexity |> Option.bind (constructRuleWithConfig CyclomaticComplexity.rule)

‎src/FSharpLint.Core/FSharpLint.Core.fsproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
<Compile Include="Rules\Formatting\TypedItemSpacing.fs" />
4343
<Compile Include="Rules\Formatting\TypePrefixing.fs" />
4444
<Compile Include="Rules\Formatting\UnionDefinitionIndentation.fs" />
45+
<Compile Include="Rules\Conventions\FavourStaticEmptyFields.fs" />
4546
<Compile Include="Rules\Conventions\RecursiveAsyncFunction.fs" />
4647
<Compile Include="Rules\Conventions\RedundantNewKeyword.fs" />
4748
<Compile Include="Rules\Conventions\NestedStatements.fs" />

‎src/FSharpLint.Core/Rules/Conventions/FavourStaticEmptyFields.fs‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
module FSharpLint.Rules.FavourStaticEmptyFields
2+
3+
open FSharpLint.Framework
4+
open FSharpLint.Framework.Suggestion
5+
open FSharp.Compiler.Syntax
6+
open FSharp.Compiler.Text
7+
open FSharpLint.Framework.Ast
8+
open FSharpLint.Framework.Rules
9+
10+
type private EmptyLiteralType =
11+
| EmptyStringLiteral
12+
| EmptyListLiteral
13+
| EmptyArrayLiteral
14+
15+
let private getEmptyLiteralType (str: string): EmptyLiteralType =
16+
if str.Length = 0 then
17+
EmptyLiteralType.EmptyStringLiteral
18+
elif str = "[]" then
19+
EmptyLiteralType.EmptyListLiteral
20+
else
21+
EmptyLiteralType.EmptyArrayLiteral
22+
23+
let private getStaticEmptyErrorMessage (range:FSharp.Compiler.Text.Range) (emptyLiteralType: EmptyLiteralType) =
24+
let errorMessageKey =
25+
match emptyLiteralType with
26+
| EmptyStringLiteral -> "RulesFavourStaticEmptyFieldsForString"
27+
| EmptyListLiteral -> "RulesFavourStaticEmptyFieldsForList"
28+
| EmptyArrayLiteral -> "RulesFavourStaticEmptyFieldsForArray"
29+
30+
let formatError errorName =
31+
Resources.GetString errorName
32+
33+
errorMessageKey |> formatError |> Array.singleton
34+
35+
let private generateError (range:FSharp.Compiler.Text.Range) (idText:string) (emptyLiteralType: EmptyLiteralType) =
36+
if idText = "" || idText = "[]" then
37+
getStaticEmptyErrorMessage range emptyLiteralType
38+
|> Array.map (fun message ->
39+
{ Range = range
40+
Message = message
41+
SuggestedFix = None
42+
TypeChecks = List.Empty })
43+
else
44+
Array.empty
45+
46+
let private runner (args: AstNodeRuleParams) =
47+
match args.AstNode with
48+
| AstNode.Expression expr ->
49+
match expr with
50+
| SynExpr.App (_, _, SynExpr.Ident failwithId, expression, range) ->
51+
match expression with
52+
| SynExpr.Const (SynConst.String (id, _, _), _) when id = "" ->
53+
(range, id, None, getEmptyLiteralType id)
54+
|> Array.singleton
55+
|> Array.collect (fun (range, idText, typeCheck, emptyLiteralType) ->
56+
let suggestions = generateError range idText emptyLiteralType
57+
suggestions |> Array.map (fun suggestion -> { suggestion with TypeChecks = Option.toList typeCheck }))
58+
| _ -> Array.empty
59+
| SynExpr.ArrayOrList (_, id, range) when "[]" = sprintf "%A" id || "[||]" = sprintf "%A" id ->
60+
(range, sprintf "%A" id, None, getEmptyLiteralType (sprintf "%A" id))
61+
|> Array.singleton
62+
|> Array.collect (fun (range, idText, typeCheck, emptyLiteralType) ->
63+
let suggestions = generateError range idText emptyLiteralType
64+
suggestions |> Array.map (fun suggestion -> { suggestion with TypeChecks = Option.toList typeCheck }))
65+
| SynExpr.Const (SynConst.String (id, _, range), _) when id = "" ->
66+
(range, id, None, getEmptyLiteralType id)
67+
|> Array.singleton
68+
|> Array.collect (fun (range, idText, typeCheck, emptyLiteralType) ->
69+
let suggestions = generateError range idText emptyLiteralType
70+
suggestions |> Array.map (fun suggestion -> { suggestion with TypeChecks = Option.toList typeCheck }))
71+
| _ -> Array.empty
72+
| Binding(SynBinding(_, _, _, _, _, _, _, _, _, expression, _, _)) ->
73+
match expression with
74+
| SynExpr.Const (SynConst.String (id, _, range), _) when id = "" ->
75+
(range, id, None, getEmptyLiteralType id)
76+
|> Array.singleton
77+
|> Array.collect (fun (range, idText, typeCheck, emptyLiteralType) ->
78+
let suggestions = generateError range idText emptyLiteralType
79+
suggestions |> Array.map (fun suggestion -> { suggestion with TypeChecks = Option.toList typeCheck }))
80+
| SynExpr.ArrayOrList (_, id, range) when "[]" = sprintf "%A" id || "[||]" = sprintf "%A" id ->
81+
(range, sprintf "%A" id, None, getEmptyLiteralType (sprintf "%A" id))
82+
|> Array.singleton
83+
|> Array.collect (fun (range, idText, typeCheck, emptyLiteralType) ->
84+
let suggestions = generateError range idText emptyLiteralType
85+
suggestions |> Array.map (fun suggestion -> { suggestion with TypeChecks = Option.toList typeCheck }))
86+
| _ -> Array.empty
87+
| _ -> Array.empty
88+
89+
90+
let rule =
91+
{ Name = "FavourStaticEmptyFields"
92+
Identifier = Identifiers.FavourStaticEmptyFields
93+
RuleConfig =
94+
{ AstNodeRuleConfig.Runner = runner
95+
Cleanup = ignore } }
96+
|> AstNodeRule

‎src/FSharpLint.Core/Rules/Identifiers.fs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,4 @@ let FailwithBadUsage = identifier 72
8080
let FavourReRaise = identifier 73
8181
let FavourConsistentThis = identifier 74
8282
let AvoidTooShortNames = identifier 75
83+
let FavourStaticEmptyFields = identifier 76

‎src/FSharpLint.Core/Text.resx‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,4 +336,13 @@
336336
<data name="RulesFavourConsistentThis" xml:space="preserve">
337337
<value>Prefer using '{0}' consistently.</value>
338338
</data>
339+
<data name="RulesFavourStaticEmptyFieldsForString" xml:space="preserve">
340+
<value>Consider using 'String.Empty' instead.</value>
341+
</data>
342+
<data name="RulesFavourStaticEmptyFieldsForArray" xml:space="preserve">
343+
<value>Consider using 'Array.Empty' instead.</value>
344+
</data>
345+
<data name="RulesFavourStaticEmptyFieldsForList" xml:space="preserve">
346+
<value>Consider using 'List.Empty' instead.</value>
347+
</data>
339348
</root>

‎src/FSharpLint.Core/fsharplint.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,7 @@
266266
"tupleOfWildcards": { "enabled": true },
267267
"favourTypedIgnore": { "enabled": false },
268268
"favourReRaise": { "enabled": true },
269+
"favourStaticEmptyFields": { "enabled": false },
269270
"favourConsistentThis": {
270271
"enabled": false,
271272
"config": {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
<Compile Include="Rules\Formatting\TypedItemSpacing.fs" />
2727
<Compile Include="Rules\Formatting\UnionDefinitionIndentation.fs" />
2828
<Compile Include="Rules\Formatting\TypePrefixing.fs" />
29+
<Compile Include="Rules\Conventions\FavourStaticEmptyFields.fs" />
2930
<Compile Include="Rules\Conventions\RecursiveAsyncFunction.fs" />
3031
<Compile Include="Rules\Conventions\RedundantNewKeyword.fs" />
3132
<Compile Include="Rules\Conventions\RaiseWithTooManyArguments.fs" />

‎tests/FSharpLint.Core.Tests/Rules/Conventions/FavourStaticEmptyFields.fs‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
module FSharpLint.Core.Tests.Rules.Conventions.FavourStaticEmptyFields
2+
3+
open NUnit.Framework
4+
open FSharpLint.Rules
5+
open System
6+
7+
[<TestFixture>]
8+
type TestConventionsFavourStaticEmptyFields() =
9+
inherit TestAstNodeRuleBase.TestAstNodeRuleBase(FavourStaticEmptyFields.rule)
10+
11+
[<Test>]
12+
member this.FavourStaticEmptyFieldsShouldProduceError_1() =
13+
this.Parse "let foo = \"\""
14+
15+
Assert.IsTrue this.ErrorsExist
16+
17+
[<Test>]
18+
member this.FavourStaticEmptyFieldsShouldProduceError_2() =
19+
this.Parse "System.Console.WriteLine \"\""
20+
21+
Assert.IsTrue this.ErrorsExist
22+
23+
[<Test>]
24+
member this.FavourStaticEmptyFieldsShouldProduceError_3() =
25+
this.Parse "let aList = []"
26+
27+
Assert.IsTrue this.ErrorsExist
28+
29+
[<Test>]
30+
member this.FavourStaticEmptyFieldsShouldProduceError_4() =
31+
this.Parse "let aList = [ ]"
32+
33+
Assert.IsTrue this.ErrorsExist
34+
35+
[<Test>]
36+
member this.FavourStaticEmptyFieldsShouldProduceError_5() =
37+
this.Parse "System.Console.WriteLine([].Length)"
38+
39+
Assert.IsTrue this.ErrorsExist
40+
41+
[<Test>]
42+
member this.FavourStaticEmptyFieldsShouldProduceError_6() =
43+
this.Parse """
44+
let foo a =
45+
if a = 0 then
46+
"0"
47+
else
48+
"" """
49+
50+
Assert.IsTrue this.ErrorsExist
51+
52+
[<Test>]
53+
member this.FavourStaticEmptyFieldsShouldProduceError_7() =
54+
this.Parse "let anArray = [||]"
55+
56+
Assert.IsTrue this.ErrorsExist
57+
58+
[<Test>]
59+
member this.FavourStaticEmptyFieldsShouldProduceError_8() =
60+
this.Parse "let anArray = [| |]"
61+
62+
Assert.IsTrue this.ErrorsExist
63+
64+
[<Test>]
65+
member this.FavourStaticEmptyFieldsShouldProduceError_9() =
66+
this.Parse "System.Console.WriteLine([||].Length)"
67+
68+
Assert.IsTrue this.ErrorsExist
69+
70+
[<Test>]
71+
member this.FavourStaticEmptyFieldsShouldNotProduceError_1() =
72+
this.Parse "let bar = String.Empty"
73+
74+
Assert.IsTrue this.NoErrorsExist
75+
76+
[<Test>]
77+
member this.FavourStaticEmptyFieldsShouldNotProduceError_2() =
78+
this.Parse "let foo = \"My Name\""
79+
80+
Assert.IsTrue this.NoErrorsExist
81+
82+
[<Test>]
83+
member this.FavourStaticEmptyFieldsShouldNotProduceError_3() =
84+
this.Parse "System.Console.WriteLine System.String.Empty"
85+
86+
Assert.IsTrue this.NoErrorsExist
87+
88+
[<Test>]
89+
member this.FavourStaticEmptyFieldsShouldNotProduceError_4() =
90+
this.Parse "let aList = List.Empty"
91+
92+
Assert.IsTrue this.NoErrorsExist
93+
94+
[<Test>]
95+
member this.FavourStaticEmptyFieldsShouldNotProduceError_5() =
96+
this.Parse "System.Console.WriteLine List.Empty.Length"
97+
98+
Assert.IsTrue this.NoErrorsExist
99+
100+
[<Test>]
101+
member this.FavourStaticEmptyFieldsShouldNotProduceError_6() =
102+
this.Parse "let anArray = Array.empty"
103+
104+
Assert.IsTrue this.NoErrorsExist
105+
106+
[<Test>]
107+
member this.FavourStaticEmptyFieldsShouldNotProduceError_7() =
108+
this.Parse "System.Console.WriteLine Array.Empty.Length"
109+
110+
Assert.IsTrue this.NoErrorsExist
111+
112+
[<Test>]
113+
member this.FavourStaticEmptyFieldsShouldNotProduceError_8() =
114+
this.Parse """
115+
match foo with
116+
| [] -> true
117+
| head::_ -> false"""
118+
119+
Assert.IsTrue this.NoErrorsExist

0 commit comments

Comments
 (0)