Skip to content

Commit 4c701ea

Browse files
knocteknocte
authored
Merge PR #842 from webwarrior-ws/asynchronous-funtion-names-v2
Implemented modes in AsynchronousFunctionNames rule. Contributes to #517
2 parents d04a873 + 82d9679 commit 4c701ea

File tree

9 files changed

+179
-49
lines changed

9 files changed

+179
-49
lines changed

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,15 @@ Add `Async` prefix (in case of `Async`) or suffix (in case of `Task`) to functio
3737
## Rule Settings
3838

3939
{
40-
"asynchronousFunctionNames": {
41-
"enabled": true
40+
"asynchronousFunctionNames": {
41+
"enabled": true,
42+
"config": {
43+
"mode": "OnlyPublicAPIsInLibraries"
44+
}
4245
}
4346
}
47+
48+
* *mode* - determines what functions are checked. Possible values: "OnlyPublicAPIsInLibraries", "AnyPublicAPIs", "AllAPIs".
49+
* "OnlyPublicAPIsInLibraries" only runs in projects that are likely library projects.
50+
* "AnyPublicAPIs" flags all public APIs.
51+
* "AllAPIs" also flags (gives violations about) internal, private, and nested APIs.

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,8 @@ For each function `FooAsync(): Task<'T>` create corresponding function `AsyncFoo
4747
}
4848
}
4949
}
50+
51+
* *mode* - determines what functions are checked. Possible values: "OnlyPublicAPIsInLibraries", "AnyPublicAPIs", "AllAPIs".
52+
* "OnlyPublicAPIsInLibraries" only runs in projects that are likely library projects.
53+
* "AnyPublicAPIs" flags all public APIs.
54+
* "AllAPIs" also flags (gives violations about) internal, private, and nested APIs.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ type Configuration =
566566
DiscourageStringInterpolationWithStringFormat:EnabledConfig option
567567
FavourNamedMembers:EnabledConfig option
568568
SynchronousFunctionNames:EnabledConfig option
569-
AsynchronousFunctionNames:EnabledConfig option
569+
AsynchronousFunctionNames:RuleConfig<AsynchronousFunctionNames.Config> option
570570
SimpleAsyncComplementaryHelpers:RuleConfig<SimpleAsyncComplementaryHelpers.Config> option }
571571
with
572572
// Method Zero is too big but can't be split into parts because it returns a record
@@ -903,7 +903,7 @@ let flattenConfig (config:Configuration) =
903903
config.DiscourageStringInterpolationWithStringFormat |> Option.bind (constructRuleIfEnabled DiscourageStringInterpolationWithStringFormat.rule)
904904
config.FavourNamedMembers |> Option.bind (constructRuleIfEnabled FavourNamedMembers.rule)
905905
config.SynchronousFunctionNames |> Option.bind (constructRuleIfEnabled SynchronousFunctionNames.rule)
906-
config.AsynchronousFunctionNames |> Option.bind (constructRuleIfEnabled AsynchronousFunctionNames.rule)
906+
config.AsynchronousFunctionNames |> Option.bind (constructRuleWithConfig AsynchronousFunctionNames.rule)
907907
config.SimpleAsyncComplementaryHelpers |> Option.bind (constructRuleWithConfig SimpleAsyncComplementaryHelpers.rule)
908908
|]
909909

‎src/FSharpLint.Core/Rules/Conventions/Naming/AsynchronousFunctionNames.fs‎

Lines changed: 58 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,16 @@ open FSharpLint.Framework
55
open FSharpLint.Framework.Suggestion
66
open FSharpLint.Framework.Ast
77
open FSharpLint.Framework.Rules
8+
open FSharpLint.Rules.Utilities.LibraryHeuristics
89
open Helper.Naming.Asynchronous
910
open FSharp.Compiler.Syntax
1011

11-
let runner (args: AstNodeRuleParams) =
12+
[<RequireQualifiedAccess>]
13+
type Config = {
14+
Mode: AsynchronousFunctionsMode
15+
}
16+
17+
let runner (config: Config) (args: AstNodeRuleParams) =
1218
let emitWarning range (newFunctionName: string) (returnTypeName: string) =
1319
Array.singleton
1420
{
@@ -18,53 +24,67 @@ let runner (args: AstNodeRuleParams) =
1824
TypeChecks = List.empty
1925
}
2026

21-
match args.AstNode with
22-
| AstNode.Binding (SynBinding (_, _, _, _, attributes, _, _, SynPat.LongIdent(funcIdent, _, _, _, (None | Some(SynAccess.Public _)), identRange), returnInfo, _, _, _, _))
23-
when not <| Helper.Naming.isAttribute "Obsolete" attributes ->
24-
let parents = args.GetParents args.NodeIndex
25-
let hasEnclosingFunctionOrMethod =
26-
parents
27-
|> List.exists
28-
(fun node ->
29-
match node with
30-
| AstNode.Binding (SynBinding (_, _, _, _, _, _, _, SynPat.LongIdent(_), _, _, _, _, _)) -> true
31-
| _ -> false)
27+
let isAccessibilityLevelApplicable (accessibility: Option<SynAccess>) =
28+
match accessibility with
29+
| None
30+
| Some(SynAccess.Public _) -> true
31+
| _ -> config.Mode = AllAPIs
32+
33+
let likelyhoodOfBeingInLibrary =
34+
match args.ProjectCheckInfo with
35+
| Some projectInfo -> howLikelyProjectIsLibrary projectInfo.ProjectContext.ProjectOptions.ProjectFileName
36+
| None -> Unlikely
37+
38+
if config.Mode = OnlyPublicAPIsInLibraries && likelyhoodOfBeingInLibrary <> Likely then
39+
Array.empty
40+
else
41+
match args.AstNode with
42+
| AstNode.Binding (SynBinding (_, _, _, _, attributes, _, _, SynPat.LongIdent(funcIdent, _, _, _, accessibility, identRange), returnInfo, _, _, _, _))
43+
when isAccessibilityLevelApplicable accessibility && not <| Helper.Naming.isAttribute "Obsolete" attributes ->
44+
let parents = args.GetParents args.NodeIndex
45+
let hasEnclosingFunctionOrMethod =
46+
parents
47+
|> List.exists
48+
(fun node ->
49+
match node with
50+
| AstNode.Binding (SynBinding (_, _, _, _, _, _, _, SynPat.LongIdent(_), _, _, _, _, _)) -> true
51+
| _ -> false)
3252

33-
if hasEnclosingFunctionOrMethod then
34-
Array.empty
35-
else
36-
match returnInfo with
37-
| Some ReturnsAsync ->
38-
match funcIdent with
39-
| HasAsyncPrefix _ ->
53+
if hasEnclosingFunctionOrMethod && config.Mode <> AllAPIs then
54+
Array.empty
55+
else
56+
match returnInfo with
57+
| Some ReturnsAsync ->
58+
match funcIdent with
59+
| HasAsyncPrefix _ ->
60+
Array.empty
61+
| HasAsyncSuffix name
62+
| HasNoAsyncPrefixOrSuffix name ->
63+
let nameWithAsync = asyncSuffixOrPrefix + name
64+
emitWarning identRange nameWithAsync "Async"
65+
| Some ReturnsTask ->
66+
match funcIdent with
67+
| HasAsyncSuffix _ ->
68+
Array.empty
69+
| HasAsyncPrefix name
70+
| HasNoAsyncPrefixOrSuffix name ->
71+
let nameWithAsync = name + asyncSuffixOrPrefix
72+
emitWarning identRange nameWithAsync "Task"
73+
| None ->
74+
// TODO: get type using typed tree in args.CheckInfo
4075
Array.empty
41-
| HasAsyncSuffix name
42-
| HasNoAsyncPrefixOrSuffix name ->
43-
let nameWithAsync = asyncSuffixOrPrefix + name
44-
emitWarning identRange nameWithAsync "Async"
45-
| Some ReturnsTask ->
46-
match funcIdent with
47-
| HasAsyncSuffix _ ->
76+
| _ ->
4877
Array.empty
49-
| HasAsyncPrefix name
50-
| HasNoAsyncPrefixOrSuffix name ->
51-
let nameWithAsync = name + asyncSuffixOrPrefix
52-
emitWarning identRange nameWithAsync "Task"
53-
| None ->
54-
// TODO: get type using typed tree in args.CheckInfo
55-
Array.empty
56-
| _ ->
57-
Array.empty
58-
| _ -> Array.empty
78+
| _ -> Array.empty
5979

60-
let rule =
80+
let rule config =
6181
AstNodeRule
6282
{
6383
Name = "AsynchronousFunctionNames"
6484
Identifier = Identifiers.AsynchronousFunctionNames
6585
RuleConfig =
6686
{
67-
AstNodeRuleConfig.Runner = runner
87+
AstNodeRuleConfig.Runner = runner config
6888
Cleanup = ignore
6989
}
7090
}

‎src/FSharpLint.Core/Rules/Conventions/Naming/SimpleAsyncComplementaryHelpers.fs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,6 @@ open Helper.Naming.Asynchronous
1010
open FSharp.Compiler.Syntax
1111
open FSharp.Compiler.Text
1212

13-
type AsynchronousFunctionsMode =
14-
| OnlyPublicAPIsInLibraries
15-
| AnyPublicAPIs
16-
| AllAPIs
17-
1813
[<RequireQualifiedAccess>]
1914
type Config = {
2015
Mode: AsynchronousFunctionsMode

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,11 @@ let getFunctionIdents (pattern:SynPat) =
466466
| _ -> Array.empty
467467

468468
module Asynchronous =
469+
type AsynchronousFunctionsMode =
470+
| OnlyPublicAPIsInLibraries
471+
| AnyPublicAPIs
472+
| AllAPIs
473+
469474
let asyncSuffixOrPrefix = "Async"
470475

471476
let (|HasAsyncPrefix|HasAsyncSuffix|HasNoAsyncPrefixOrSuffix|) (pattern: SynLongIdent) =

‎src/FSharpLint.Core/fsharplint.json‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,12 @@
349349
},
350350
"favourNamedMembers": { "enabled": false },
351351
"synchronousFunctionNames": { "enabled": true },
352-
"asynchronousFunctionNames": { "enabled": true },
352+
"asynchronousFunctionNames": {
353+
"enabled": true,
354+
"config": {
355+
"mode": "OnlyPublicAPIsInLibraries"
356+
}
357+
},
353358
"simpleAsyncComplementaryHelpers": {
354359
"enabled": false,
355360
"config": {

‎tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/AsynchronousFunctionNames.fs‎

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@ open NUnit.Framework
44

55
open FSharpLint.Rules
66
open FSharpLint.Core.Tests
7+
open FSharpLint.Rules.Helper.Naming.Asynchronous
78

89
[<TestFixture>]
910
type TestAsynchronousFunctionNames() =
10-
inherit TestAstNodeRuleBase.TestAstNodeRuleBase(AsynchronousFunctionNames.rule)
11+
inherit TestAstNodeRuleBase.TestAstNodeRuleBase(AsynchronousFunctionNames.rule { Mode = AnyPublicAPIs })
1112

1213
[<Test>]
1314
member this.``Function returning Async<'T> should give violations offering adding Async prefix``() =
@@ -173,3 +174,93 @@ type Foo() =
173174
"""
174175

175176
Assert.IsTrue this.NoErrorsExist
177+
178+
[<TestFixture>]
179+
type TestAsynchronousFunctionNamesAllAPIs() =
180+
inherit TestAstNodeRuleBase.TestAstNodeRuleBase(AsynchronousFunctionNames.rule { Mode = AllAPIs })
181+
182+
[<TestCase "private">]
183+
[<TestCase "internal">]
184+
member this.``Non-public function returning Async<'T> should give violations`` (accessibility: string) =
185+
this.Parse <|
186+
sprintf
187+
"""
188+
module Foo =
189+
let %s Bar(): Async<int> =
190+
async { return 1 }
191+
"""
192+
accessibility
193+
194+
Assert.IsTrue this.ErrorsExist
195+
StringAssert.Contains("AsyncBar", this.ErrorMsg)
196+
197+
[<TestCase "private">]
198+
[<TestCase "internal">]
199+
member this.``Non-public function returning Task<'T> should give violations`` (accessibility: string) =
200+
this.Parse <|
201+
sprintf
202+
"""
203+
module Foo =
204+
let %s Bar(): Task<int> =
205+
null
206+
"""
207+
accessibility
208+
209+
Assert.IsTrue this.ErrorsExist
210+
StringAssert.Contains("BarAsync", this.ErrorMsg)
211+
212+
[<Test>]
213+
member this.``Nested functions returning Async<'T> should give violations``() =
214+
this.Parse """
215+
module Foo =
216+
let Foo() =
217+
let Bar(): Async<int> =
218+
async { return 1 }
219+
()
220+
"""
221+
222+
Assert.IsTrue this.ErrorsExist
223+
StringAssert.Contains("AsyncBar", this.ErrorMsg)
224+
225+
[<Test>]
226+
member this.``Nested functions returning Task<'T> should give violations``() =
227+
this.Parse """
228+
module Foo =
229+
let Foo() =
230+
let Bar(): Task<int> =
231+
null
232+
()
233+
"""
234+
235+
Assert.IsTrue this.ErrorsExist
236+
StringAssert.Contains("BarAsync", this.ErrorMsg)
237+
238+
[<TestCase "private">]
239+
[<TestCase "internal">]
240+
member this.``Non-public method returning Async<'T> should give violations`` (accessibility: string) =
241+
this.Parse <|
242+
sprintf
243+
"""
244+
type Foo() =
245+
member %s this.Bar(): Async<int> =
246+
async { return 1 }
247+
"""
248+
accessibility
249+
250+
Assert.IsTrue this.ErrorsExist
251+
StringAssert.Contains("AsyncBar", this.ErrorMsg)
252+
253+
[<TestCase "private">]
254+
[<TestCase "internal">]
255+
member this.``Non-public method returning Task<'T> should give violations`` (accessibility: string) =
256+
this.Parse <|
257+
sprintf
258+
"""
259+
type Foo() =
260+
member %s this.Bar(): Task<int> =
261+
null
262+
"""
263+
accessibility
264+
265+
Assert.IsTrue this.ErrorsExist
266+
StringAssert.Contains("BarAsync", this.ErrorMsg)

‎tests/FSharpLint.Core.Tests/Rules/Conventions/Naming/SimpleAsyncComplementaryHelpers.fs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ open NUnit.Framework
55
open FSharpLint.Rules
66
open FSharpLint.Core.Tests
77
open FSharpLint.Rules.SimpleAsyncComplementaryHelpers
8+
open FSharpLint.Rules.Helper.Naming.Asynchronous
89

910
[<TestFixture>]
1011
type TestSimpleAsyncComplementaryHelpers() =

0 commit comments

Comments
 (0)