Skip to content

Commit 431829a

Browse files
authored
Merge PR #834 from webwarrior-ws/synchronous-function-names
Add SynchronousFunctionNames rule. Contributes to #517
2 parents e8a2d2b + 4f8f8f3 commit 431829a

File tree

10 files changed

+307
-1
lines changed

10 files changed

+307
-1
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,3 +135,4 @@ The following rules can be specified for linting.
135135
- [DisallowShadowing (FL0092)](rules/FL0092.html)
136136
- [DiscourageStringInterpolationWithStringFormat (FL0093)](rules/FL0093.html)
137137
- [FavourNamedMembers (FL0094)](rules/FL0094.html)
138+
- [SynchronousFunctionNames (FL0095)](rules/FL0095.html)
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
title: FL0095
3+
category: how-to
4+
hide_menu: true
5+
---
6+
7+
# SynchronousFunctionNames (FL0095)
8+
9+
*Introduced in `0.26.12`*
10+
11+
## Cause
12+
13+
Function that is not async (does not return `Async` or `Task`) has Async prefix or suffix.
14+
15+
## Rationale
16+
17+
By convention, `Async` prefix or suffix in function name means that the function is asyncronous.
18+
19+
Prefix is used for functions that return `Async` type, e.g.:
20+
```
21+
let AsyncFoo(): Async<int> =
22+
async { return 1 }
23+
```
24+
25+
Suffix is used for functions that return `System.Threading.Task` type, e.g.:
26+
```
27+
open System.Threading
28+
29+
let FooAsync(): Task<int> =
30+
task { return 1 }
31+
```
32+
33+
## How To Fix
34+
35+
Remove `Async` prefix or suffix from function name.
36+
37+
## Rule Settings
38+
39+
{
40+
"synchronousFunctionNames": {
41+
"enabled": true
42+
}
43+
}

src/FSharpLint.Core/Application/Configuration.fs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,8 @@ type Configuration =
564564
FavourNestedFunctions:EnabledConfig option
565565
DisallowShadowing:EnabledConfig option
566566
DiscourageStringInterpolationWithStringFormat:EnabledConfig option
567-
FavourNamedMembers:EnabledConfig option }
567+
FavourNamedMembers:EnabledConfig option
568+
SynchronousFunctionNames:EnabledConfig option }
568569
with
569570
// Method Zero is too big but can't be split into parts because it returns a record
570571
// and it requires all fields to be set.
@@ -674,6 +675,7 @@ with
674675
DisallowShadowing = None
675676
DiscourageStringInterpolationWithStringFormat = None
676677
FavourNamedMembers = None
678+
SynchronousFunctionNames = None
677679
}
678680

679681
// fsharplint:enable MaxLinesInMember
@@ -896,6 +898,7 @@ let flattenConfig (config:Configuration) =
896898
config.DisallowShadowing |> Option.bind (constructRuleIfEnabled DisallowShadowing.rule)
897899
config.DiscourageStringInterpolationWithStringFormat |> Option.bind (constructRuleIfEnabled DiscourageStringInterpolationWithStringFormat.rule)
898900
config.FavourNamedMembers |> Option.bind (constructRuleIfEnabled FavourNamedMembers.rule)
901+
config.SynchronousFunctionNames |> Option.bind (constructRuleIfEnabled SynchronousFunctionNames.rule)
899902
|]
900903

901904
let allEnabledRules = Array.choose id allPossibleRules

src/FSharpLint.Core/FSharpLint.Core.fsproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
<Compile Include="Rules\Conventions\FunctionReimplementation\FunctionReimplementationHelper.fs" />
8181
<Compile Include="Rules\Conventions\FunctionReimplementation\ReimplementsFunction.fs" />
8282
<Compile Include="Rules\Conventions\FunctionReimplementation\CanBeReplacedWithComposition.fs" />
83+
<Compile Include="Rules\Conventions\Naming\SynchronousFunctionNames.fs" />
8384
<Compile Include="Rules\Conventions\Naming\NamingHelper.fs" />
8485
<Compile Include="Rules\Conventions\Naming\InterfaceNames.fs" />
8586
<Compile Include="Rules\Conventions\Naming\ExceptionNames.fs" />
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
module FSharpLint.Rules.SynchronousFunctionNames
2+
3+
open System
4+
open FSharpLint.Framework
5+
open FSharpLint.Framework.Suggestion
6+
open FSharpLint.Framework.Ast
7+
open FSharpLint.Framework.Rules
8+
open FSharp.Compiler.Syntax
9+
open FSharp.Compiler.Symbols
10+
11+
let asyncSuffixOrPrefix = "Async"
12+
13+
let (|HasAsyncPrefix|HasAsyncSuffix|HasNoAsyncPrefixOrSuffix|) (pattern: SynLongIdent) =
14+
match List.tryLast pattern.LongIdent with
15+
| Some name ->
16+
if name.idText.StartsWith(asyncSuffixOrPrefix, StringComparison.InvariantCultureIgnoreCase) then
17+
HasAsyncPrefix name.idText
18+
elif name.idText.EndsWith asyncSuffixOrPrefix then
19+
HasAsyncSuffix name.idText
20+
else
21+
HasNoAsyncPrefixOrSuffix name.idText
22+
| _ -> HasNoAsyncPrefixOrSuffix String.Empty
23+
24+
let (|ReturnsTask|ReturnsAsync|ReturnsNonAsync|) (returnInfo: SynBindingReturnInfo) =
25+
match returnInfo with
26+
| SynBindingReturnInfo(SynType.LongIdent(SynLongIdent(typeIdent, _, _)), _, _, _)
27+
| SynBindingReturnInfo(SynType.App(SynType.LongIdent(SynLongIdent(typeIdent, _, _)), _, _, _, _, _, _), _, _, _) ->
28+
match List.tryLast typeIdent with
29+
| Some ident when ident.idText = "Async" -> ReturnsAsync
30+
| Some ident when ident.idText = "Task" -> ReturnsTask
31+
| _ -> ReturnsNonAsync
32+
| _ -> ReturnsNonAsync
33+
34+
let runner (args: AstNodeRuleParams) =
35+
let emitWarning range (newFunctionName: string) =
36+
Array.singleton
37+
{
38+
Range = range
39+
Message = String.Format(Resources.GetString "RulesSynchronousFunctionNames", newFunctionName)
40+
SuggestedFix = None
41+
TypeChecks = List.empty
42+
}
43+
44+
match args.AstNode with
45+
| AstNode.Binding (SynBinding (_, _, _, _, _, _, _, SynPat.LongIdent(funcIdent, _, _, _, _, identRange), returnInfo, _, _, _, _)) ->
46+
match returnInfo with
47+
| Some ReturnsNonAsync ->
48+
match funcIdent with
49+
| HasAsyncPrefix name ->
50+
let startsWithLowercase = Char.IsLower name.[0]
51+
let nameWithoutAsync = name.Substring asyncSuffixOrPrefix.Length
52+
let suggestedName =
53+
if startsWithLowercase then
54+
sprintf "%c%s" (Char.ToLowerInvariant nameWithoutAsync.[0]) (nameWithoutAsync.Substring 1)
55+
else
56+
nameWithoutAsync
57+
emitWarning identRange suggestedName
58+
| HasAsyncSuffix name ->
59+
let nameWithoutAsync = name.Substring(0, name.Length - asyncSuffixOrPrefix.Length)
60+
emitWarning identRange nameWithoutAsync
61+
| HasNoAsyncPrefixOrSuffix _ -> Array.empty
62+
| None ->
63+
// TODO: get type using typed tree in args.CheckInfo
64+
Array.empty
65+
| _ ->
66+
Array.empty
67+
| _ -> Array.empty
68+
69+
let rule =
70+
AstNodeRule
71+
{
72+
Name = "SynchronousFunctionNames"
73+
Identifier = Identifiers.SynchronousFunctionNames
74+
RuleConfig =
75+
{
76+
AstNodeRuleConfig.Runner = runner
77+
Cleanup = ignore
78+
}
79+
}

src/FSharpLint.Core/Rules/Identifiers.fs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,4 @@ let FavourNestedFunctions = identifier 91
9999
let DisallowShadowing = identifier 92
100100
let DiscourageStringInterpolationWithStringFormat = identifier 93
101101
let FavourNamedMembers = identifier 94
102+
let SynchronousFunctionNames = identifier 95

src/FSharpLint.Core/Text.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,4 +408,7 @@
408408
<data name="RulesFavourNamedMembers" xml:space="preserve">
409409
<value>Use named discriminated union fields.</value>
410410
</data>
411+
<data name="RulesSynchronousFunctionNames" xml:space="preserve">
412+
<value>This function does not return Async or Task. Consider renaming it to {0}.</value>
413+
</data>
411414
</root>

src/FSharpLint.Core/fsharplint.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,7 @@
348348
"enabled": false
349349
},
350350
"favourNamedMembers": { "enabled": false },
351+
"synchronousFunctionNames": { "enabled": true },
351352
"hints": {
352353
"add": [
353354
"not (a = b) ===> a <> b",

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
<Compile Include="Rules\Conventions\FavourNestedFunctions.fs" />
5555
<Compile Include="Rules\Conventions\DisallowShadowing.fs" />
5656
<Compile Include="Rules\Conventions\FavourNamedMembers.fs" />
57+
<Compile Include="Rules\Conventions\Naming\SynchronousFunctionNames.fs" />
5758
<Compile Include="Rules\Conventions\Naming\NamingHelpers.fs" />
5859
<Compile Include="Rules\Conventions\Naming\InterfaceNames.fs" />
5960
<Compile Include="Rules\Conventions\Naming\ExceptionNames.fs" />
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
module FSharpLint.Core.Tests.Rules.Conventions.SynchronousFunctionNames
2+
3+
open NUnit.Framework
4+
5+
open FSharpLint.Rules
6+
open FSharpLint.Core.Tests
7+
8+
[<TestFixture>]
9+
type TestSynchronousFunctionNames() =
10+
inherit TestAstNodeRuleBase.TestAstNodeRuleBase(SynchronousFunctionNames.rule)
11+
12+
[<Test>]
13+
member this.``Non-asynchronous function named Async* should give violations offering removing Async prefix``() =
14+
this.Parse """
15+
module Foo =
16+
let AsyncBar(): int =
17+
1
18+
"""
19+
20+
Assert.IsTrue this.ErrorsExist
21+
StringAssert.Contains("Bar", this.ErrorMsg)
22+
23+
[<Test>]
24+
member this.``Non-asynchronous function named *Async should give violations offering removing Async suffix``() =
25+
this.Parse """
26+
module Foo =
27+
let BarAsync(): int =
28+
1
29+
"""
30+
31+
Assert.IsTrue this.ErrorsExist
32+
StringAssert.Contains("Bar", this.ErrorMsg)
33+
34+
[<Test>]
35+
member this.``Non-asynchronous nested function named *Async should give violations offering removing Async suffix``() =
36+
this.Parse """
37+
module Foo =
38+
let Bar(): int =
39+
let BazAsync(): int =
40+
0
41+
1
42+
"""
43+
44+
Assert.IsTrue this.ErrorsExist
45+
StringAssert.Contains("Baz", this.ErrorMsg)
46+
47+
[<Test>]
48+
member this.``Private non-asynchronous function named Async* should give violations offering removing Async suffix``() =
49+
this.Parse """
50+
module Foo =
51+
let private AsyncBar(): int =
52+
1
53+
"""
54+
55+
Assert.IsTrue this.ErrorsExist
56+
StringAssert.Contains("Bar", this.ErrorMsg)
57+
58+
[<Test>]
59+
member this.``Private non-asynchronous function named async* should give violations offering removing Async suffix``() =
60+
this.Parse """
61+
module Foo =
62+
let private asyncBar(): int =
63+
1
64+
"""
65+
66+
Assert.IsTrue this.ErrorsExist
67+
StringAssert.Contains("bar", this.ErrorMsg)
68+
69+
[<Test>]
70+
member this.``Internal non-asynchronous function named *Async should give violations offering removing Async suffix``() =
71+
this.Parse """
72+
module Foo =
73+
let internal AsyncBar(): int =
74+
1
75+
"""
76+
77+
Assert.IsTrue this.ErrorsExist
78+
StringAssert.Contains("Bar", this.ErrorMsg)
79+
80+
[<Test>]
81+
member this.``Async functions with Async prefix should give no violations``() =
82+
this.Parse """
83+
let AsyncFoo(): Async<int> =
84+
async { return 1 }
85+
let AsyncBar(): Async<unit> =
86+
async { return () }
87+
let AsyncBaz(): Async<unit> =
88+
async { do! Async.Sleep(1000) }
89+
"""
90+
91+
Assert.IsTrue this.NoErrorsExist
92+
93+
[<Test>]
94+
member this.``Functions that return Task with Async suffix should give no violations``() =
95+
this.Parse """
96+
let FooAsync(): Task =
97+
null
98+
let BarAsync(): Task<int> =
99+
null
100+
"""
101+
102+
Assert.IsTrue this.NoErrorsExist
103+
104+
105+
[<Test>]
106+
member this.``Non-asynchronous method named Async* should give violations offering removing Async prefix``() =
107+
this.Parse """
108+
type Foo() =
109+
member this.AsyncBar(): int =
110+
1
111+
"""
112+
113+
Assert.IsTrue this.ErrorsExist
114+
StringAssert.Contains("Bar", this.ErrorMsg)
115+
116+
[<Test>]
117+
member this.``Non-asynchronous method named *Async should give violations offering removing Async suffix``() =
118+
this.Parse """
119+
type Foo() =
120+
member this.BarAsync(): int =
121+
1
122+
"""
123+
124+
Assert.IsTrue this.ErrorsExist
125+
StringAssert.Contains("Bar", this.ErrorMsg)
126+
127+
[<Test>]
128+
member this.``Private non-asynchronous method named *Async should give violations offering removing Async suffix``() =
129+
this.Parse """
130+
type Foo() =
131+
member private this.AsyncBar(): int =
132+
1
133+
"""
134+
135+
Assert.IsTrue this.ErrorsExist
136+
StringAssert.Contains("Bar", this.ErrorMsg)
137+
138+
[<Test>]
139+
member this.``Internal non-asynchronous method named *Async should give violations offering removing Async suffix``() =
140+
this.Parse """
141+
type Foo() =
142+
member internal this.AsyncBar(): int =
143+
1
144+
"""
145+
146+
Assert.IsTrue this.ErrorsExist
147+
StringAssert.Contains("Bar", this.ErrorMsg)
148+
149+
[<Test>]
150+
member this.``Async methods with Async prefix should give no violations``() =
151+
this.Parse """
152+
type Foo() =
153+
member this.AsyncFoo(): Async<int> =
154+
async { return 1 }
155+
member this.AsyncBar(): Async<unit> =
156+
async { return () }
157+
member this.AsyncBaz(): Async<unit> =
158+
async { do! Async.Sleep(1000) }
159+
"""
160+
161+
Assert.IsTrue this.NoErrorsExist
162+
163+
[<Test>]
164+
member this.``Methods that return Task with Async suffix should give no violations``() =
165+
this.Parse """
166+
type Foo() =
167+
member this.FooAsync(): Task =
168+
null
169+
member this.BarAsync(): Task<int> =
170+
null
171+
"""
172+
173+
Assert.IsTrue this.NoErrorsExist

0 commit comments

Comments
 (0)