From 894f29a5353b177c56bc8a04eb99499305b6fd18 Mon Sep 17 00:00:00 2001 From: Retheesh Erathu Date: Mon, 20 Dec 2021 17:08:08 +0200 Subject: [PATCH 1/3] Add new rule C#-FriendlyAsyncOverload Fixes https://github.com/fsprojects/FSharpLint/issues/517 --- docs/content/how-tos/rule-configuration.md | 1 + docs/content/how-tos/rules/FL0075.md | 2 +- docs/content/how-tos/rules/FL0077.md | 29 +++++++ .../Application/Configuration.fs | 5 +- src/FSharpLint.Core/FSharpLint.Core.fsproj | 1 + .../CSharpFriendlyAsyncOverload.fs | 69 ++++++++++++++++ src/FSharpLint.Core/Rules/Identifiers.fs | 1 + src/FSharpLint.Core/Text.resx | 3 + src/FSharpLint.Core/fsharplint.json | 1 + .../FSharpLint.Core.Tests.fsproj | 1 + .../CSharpFriendlyAsyncOverload.fs | 82 +++++++++++++++++++ 11 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 docs/content/how-tos/rules/FL0077.md create mode 100644 src/FSharpLint.Core/Rules/Conventions/CSharpFriendlyAsyncOverload.fs create mode 100644 tests/FSharpLint.Core.Tests/Rules/Conventions/CSharpFriendlyAsyncOverload.fs diff --git a/docs/content/how-tos/rule-configuration.md b/docs/content/how-tos/rule-configuration.md index f53668bf9..67dfa7e64 100644 --- a/docs/content/how-tos/rule-configuration.md +++ b/docs/content/how-tos/rule-configuration.md @@ -117,3 +117,4 @@ The following rules can be specified for linting. - [FavourConsistentThis (FL0074)](rules/FL0074.html) - [AvoidTooShortNames (FL0075)](rules/FL0075.html) - [FavourStaticEmptyFields (FL0076)](rules/FL0076.html) +- [CSharpFriendlyAsyncOverload (FL0077)](rules/FL0077.html) diff --git a/docs/content/how-tos/rules/FL0075.md b/docs/content/how-tos/rules/FL0075.md index ce4d35d05..24da97d13 100644 --- a/docs/content/how-tos/rules/FL0075.md +++ b/docs/content/how-tos/rules/FL0075.md @@ -24,6 +24,6 @@ Use longer names for the flagged occurrences. { "avoidTooShortNames": { - "enabled": false + "enabled": false } } diff --git a/docs/content/how-tos/rules/FL0077.md b/docs/content/how-tos/rules/FL0077.md new file mode 100644 index 000000000..06cfea456 --- /dev/null +++ b/docs/content/how-tos/rules/FL0077.md @@ -0,0 +1,29 @@ +--- +title: FL0077 +category: how-to +hide_menu: true +--- + +# CSharpFriendlyAsyncOverload (FL0077) + +*Introduced in `0.21.1`* + +## Cause + +Rule to suggest adding C#-friendly async overloads. + +## Rationale + +Exposing public async APIs in a C#-friendly manner for better C# interoperability. + +## How To Fix + +Add an `Async`-suffixed version of the API that returns a `Task<'T>` + +## Rule Settings + + { + "csharpFriendlyAsyncOverload": { + "enabled": false + } + } diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index 7762fe4f7..076a75575 100644 --- a/src/FSharpLint.Core/Application/Configuration.fs +++ b/src/FSharpLint.Core/Application/Configuration.fs @@ -454,7 +454,8 @@ type Configuration = MaxLinesInFile:RuleConfig option TrailingNewLineInFile:EnabledConfig option NoTabCharacters:EnabledConfig option - NoPartialFunctions:RuleConfig option } + NoPartialFunctions:RuleConfig option + CSharpFriendlyAsyncOverload:EnabledConfig option } with static member Zero = { Global = None @@ -539,6 +540,7 @@ with TrailingNewLineInFile = None NoTabCharacters = None NoPartialFunctions = None + CSharpFriendlyAsyncOverload = None } // fsharplint:enable RecordFieldNames @@ -687,6 +689,7 @@ let flattenConfig (config:Configuration) = config.TrailingNewLineInFile |> Option.bind (constructRuleIfEnabled TrailingNewLineInFile.rule) config.NoTabCharacters |> Option.bind (constructRuleIfEnabled NoTabCharacters.rule) config.NoPartialFunctions |> Option.bind (constructRuleWithConfig NoPartialFunctions.rule) + config.CSharpFriendlyAsyncOverload |> Option.bind (constructRuleIfEnabled CSharpFriendlyAsyncOverload.rule) |] |> Array.choose id if config.NonPublicValuesNames.IsSome && diff --git a/src/FSharpLint.Core/FSharpLint.Core.fsproj b/src/FSharpLint.Core/FSharpLint.Core.fsproj index aa55c8d30..e53d2c144 100644 --- a/src/FSharpLint.Core/FSharpLint.Core.fsproj +++ b/src/FSharpLint.Core/FSharpLint.Core.fsproj @@ -50,6 +50,7 @@ + diff --git a/src/FSharpLint.Core/Rules/Conventions/CSharpFriendlyAsyncOverload.fs b/src/FSharpLint.Core/Rules/Conventions/CSharpFriendlyAsyncOverload.fs new file mode 100644 index 000000000..bc04a75f2 --- /dev/null +++ b/src/FSharpLint.Core/Rules/Conventions/CSharpFriendlyAsyncOverload.fs @@ -0,0 +1,69 @@ +module FSharpLint.Rules.CSharpFriendlyAsyncOverload + +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 + +type NodeDetails = { Ident: string; Range: range } + +let rec private getIdentFromSynPat = + function + | SynPat.LongIdent (longDotId = longDotId) -> + longDotId + |> ExpressionUtilities.longIdentWithDotsToString + |> Some + | SynPat.Typed (pat, _, _) -> getIdentFromSynPat pat + | _ -> None + +let runner (args: AstNodeRuleParams) = + let hasAsync (syntaxArray: array) nodeIndex fnIdent = + let rec hasAsync index = + if index >= syntaxArray.Length then + None + else + let node = syntaxArray.[index].Actual + match node with + | AstNode.Binding (SynBinding (_, _, _, _, _attributes, _, _, pattern, _, _, range, _)) -> + match getIdentFromSynPat pattern with + | Some ident when ident = fnIdent + "Async" -> + { Ident = fnIdent + Range = range } |> Some + | _ -> hasAsync (index + 1) + | _ -> hasAsync (index + 1) + + hasAsync nodeIndex + + match args.AstNode with + | AstNode.Binding (SynBinding (_, _, _, _, _, _, _, pattern, synInfo, _, range, _)) -> + match synInfo with + | Some (SynBindingReturnInfo (SynType.App(SynType.LongIdent(LongIdentWithDots(ident, _)), _, _, _, _, _, _), _, _)) -> + match ident with + | head::_ when head.idText = "Async" -> + let idents = getIdentFromSynPat pattern + match idents with + | Some ident when not (ident.EndsWith "Async") -> + match hasAsync args.SyntaxArray args.NodeIndex ident with + | Some _ -> Array.empty + | None -> + { Range = range + Message = String.Format(Resources.GetString "RulesCSharpFriendlyAsyncOverload", ident) + SuggestedFix = None + TypeChecks = List.Empty } + |> Array.singleton + | _ -> Array.empty + | _ -> Array.empty + | _ -> Array.empty + | _ -> Array.empty + + +let rule = + { Name = "CSharpFriendlyAsyncOverload" + Identifier = Identifiers.CSharpFriendlyAsyncOverload + RuleConfig = + { AstNodeRuleConfig.Runner = runner + Cleanup = ignore } } + |> AstNodeRule diff --git a/src/FSharpLint.Core/Rules/Identifiers.fs b/src/FSharpLint.Core/Rules/Identifiers.fs index 95fd12259..d4005f524 100644 --- a/src/FSharpLint.Core/Rules/Identifiers.fs +++ b/src/FSharpLint.Core/Rules/Identifiers.fs @@ -81,3 +81,4 @@ let FavourReRaise = identifier 73 let FavourConsistentThis = identifier 74 let AvoidTooShortNames = identifier 75 let FavourStaticEmptyFields = identifier 76 +let CSharpFriendlyAsyncOverload = identifier 77 diff --git a/src/FSharpLint.Core/Text.resx b/src/FSharpLint.Core/Text.resx index 1c90e1adc..53ab0ca1f 100644 --- a/src/FSharpLint.Core/Text.resx +++ b/src/FSharpLint.Core/Text.resx @@ -345,4 +345,7 @@ Consider using 'Array.empty' instead. + + Consider using a C#-friendly async overload for {0}. + diff --git a/src/FSharpLint.Core/fsharplint.json b/src/FSharpLint.Core/fsharplint.json index 06cf0a47e..0ff406a45 100644 --- a/src/FSharpLint.Core/fsharplint.json +++ b/src/FSharpLint.Core/fsharplint.json @@ -45,6 +45,7 @@ "invalidArgWithTwoArguments": { "enabled": true }, "failwithfWithArgumentsMatchingFormatString": { "enabled": true }, "failwithBadUsage": { "enabled": true }, + "csharpFriendlyAsyncOverload": { "enabled": false }, "maxLinesInLambdaFunction": { "enabled": false, "config": { diff --git a/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj b/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj index abc0ec7ca..1858864da 100644 --- a/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj +++ b/tests/FSharpLint.Core.Tests/FSharpLint.Core.Tests.fsproj @@ -39,6 +39,7 @@ + diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/CSharpFriendlyAsyncOverload.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/CSharpFriendlyAsyncOverload.fs new file mode 100644 index 000000000..7d8771a4d --- /dev/null +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/CSharpFriendlyAsyncOverload.fs @@ -0,0 +1,82 @@ +module FSharpLint.Core.Tests.Rules.Conventions.CSharpFriendlyAsyncOverload + +open NUnit.Framework +open FSharpLint.Rules + +[] +type TestConventionsCSharpFriendlyAsyncOverload() = + inherit TestAstNodeRuleBase.TestAstNodeRuleBase(CSharpFriendlyAsyncOverload.rule) + + [] + member this.``async function must suggest friendly implementation``() = + this.Parse(""" +module Foo = + let Bar(): Async = + Async.Sleep 5""") + + Assert.IsTrue(this.ErrorExistsAt(3, 8)) + + [] + member this.``async function with friendly implementation must not have errors``() = + this.Parse(""" +module Foo = + let Bar(): Async = + Async.Sleep 5 + let BarAsync(): Task = + Bar() |> Async.StartAsTask""") + + this.AssertNoWarnings() + + [] + member this.``non async function must not create warnings``() = + this.Parse(""" +module Foo = + let Bar() = + ()""") + + this.AssertNoWarnings() + + [] + member this.``async function must not have errors when not delcared immediately following the parent function``() = + this.Parse(""" +module Foo = + let Bar(): Async = + Async.Sleep 5 + let RandomFunction() = + () + let BarAsync(): Task = + Bar() |> Async.StartAsTask""") + + this.AssertNoWarnings() + + [] + member this.``multiple async functions must have errors``() = + this.Parse(""" +module Foo = + let Bar(): Async = + Async.Sleep 5 + let RandomFunction() = + () + let BarAsync(): Task = + Bar() |> Async.StartAsTask + let Foo(): Async = + Async.Sleep 10""") + + Assert.IsTrue(this.ErrorExistsAt(9, 8)) + + [] + member this.``multiple async functions must not have errors``() = + this.Parse(""" +module Foo = + let Bar(): Async = + Async.Sleep 5 + let RandomFunction() = + () + let BarAsync(): Task = + Bar() |> Async.StartAsTask + let Foo(): Async = + Async.Sleep 10 + let FooAsync(): Task = + Foo() |> Async.StartAsTask""") + + this.AssertNoWarnings() From 70d721fe1297f2e3595ffadc75cad5493cf71691 Mon Sep 17 00:00:00 2001 From: Parham Date: Sat, 29 Apr 2023 15:46:52 +0330 Subject: [PATCH 2/3] CSharpFriendlyAsyncOverload: add failing tests --- .../CSharpFriendlyAsyncOverload.fs | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/CSharpFriendlyAsyncOverload.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/CSharpFriendlyAsyncOverload.fs index 7d8771a4d..a38b48937 100644 --- a/tests/FSharpLint.Core.Tests/Rules/Conventions/CSharpFriendlyAsyncOverload.fs +++ b/tests/FSharpLint.Core.Tests/Rules/Conventions/CSharpFriendlyAsyncOverload.fs @@ -8,19 +8,40 @@ type TestConventionsCSharpFriendlyAsyncOverload() = inherit TestAstNodeRuleBase.TestAstNodeRuleBase(CSharpFriendlyAsyncOverload.rule) [] - member this.``async function must suggest friendly implementation``() = + member this.``async function should have the prefix Async``() = this.Parse(""" module Foo = let Bar(): Async = Async.Sleep 5""") Assert.IsTrue(this.ErrorExistsAt(3, 8)) + + [] + member this.``async function should have the prefix Async 2``() = + this.Parse(""" +module Foo = + let Bar(): Async = + Async.Sleep 5 + let BarAsync(): Task = + Bar() |> Async.StartAsTask""") + + Assert.IsTrue(this.ErrorExistsAt(3, 8)) + + + [] + member this.``async function must suggest friendly implementation``() = + this.Parse(""" +module Foo = + let AsyncBar(): Async = + Async.Sleep 5""") + + Assert.IsTrue(this.ErrorExistsAt(3, 8)) [] member this.``async function with friendly implementation must not have errors``() = this.Parse(""" module Foo = - let Bar(): Async = + let AsyncBar(): Async = Async.Sleep 5 let BarAsync(): Task = Bar() |> Async.StartAsTask""") @@ -40,7 +61,7 @@ module Foo = member this.``async function must not have errors when not delcared immediately following the parent function``() = this.Parse(""" module Foo = - let Bar(): Async = + let AsyncBar(): Async = Async.Sleep 5 let RandomFunction() = () @@ -53,13 +74,13 @@ module Foo = member this.``multiple async functions must have errors``() = this.Parse(""" module Foo = - let Bar(): Async = + let AsyncBar(): Async = Async.Sleep 5 let RandomFunction() = () let BarAsync(): Task = Bar() |> Async.StartAsTask - let Foo(): Async = + let AsyncFoo(): Async = Async.Sleep 10""") Assert.IsTrue(this.ErrorExistsAt(9, 8)) @@ -68,13 +89,13 @@ module Foo = member this.``multiple async functions must not have errors``() = this.Parse(""" module Foo = - let Bar(): Async = + let AsyncBar(): Async = Async.Sleep 5 let RandomFunction() = () let BarAsync(): Task = Bar() |> Async.StartAsTask - let Foo(): Async = + let AsyncFoo(): Async = Async.Sleep 10 let FooAsync(): Task = Foo() |> Async.StartAsTask""") From 0a6261e920bf2cb514e4c77ad3b8546a8ac39fe9 Mon Sep 17 00:00:00 2001 From: Parham Date: Sat, 29 Apr 2023 15:47:03 +0330 Subject: [PATCH 3/3] CSharpFriendlyAsyncOverload: fix failing tests --- .../CSharpFriendlyAsyncOverload.fs | 22 ++++++++++++------- src/FSharpLint.Core/Text.resx | 2 +- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/FSharpLint.Core/Rules/Conventions/CSharpFriendlyAsyncOverload.fs b/src/FSharpLint.Core/Rules/Conventions/CSharpFriendlyAsyncOverload.fs index bc04a75f2..e17fae7cc 100644 --- a/src/FSharpLint.Core/Rules/Conventions/CSharpFriendlyAsyncOverload.fs +++ b/src/FSharpLint.Core/Rules/Conventions/CSharpFriendlyAsyncOverload.fs @@ -18,6 +18,13 @@ let rec private getIdentFromSynPat = |> Some | SynPat.Typed (pat, _, _) -> getIdentFromSynPat pat | _ -> None + +let getErrorMessage (ident: string) (range: range) = + { Range = range + Message = String.Format(Resources.GetString "RulesCSharpFriendlyAsyncOverload", ident) + SuggestedFix = None + TypeChecks = List.Empty } + |> Array.singleton let runner (args: AstNodeRuleParams) = let hasAsync (syntaxArray: array) nodeIndex fnIdent = @@ -46,14 +53,13 @@ let runner (args: AstNodeRuleParams) = let idents = getIdentFromSynPat pattern match idents with | Some ident when not (ident.EndsWith "Async") -> - match hasAsync args.SyntaxArray args.NodeIndex ident with - | Some _ -> Array.empty - | None -> - { Range = range - Message = String.Format(Resources.GetString "RulesCSharpFriendlyAsyncOverload", ident) - SuggestedFix = None - TypeChecks = List.Empty } - |> Array.singleton + if (ident.StartsWith "Async") then + match hasAsync args.SyntaxArray args.NodeIndex (ident.Replace("Async", "", StringComparison.Ordinal)) with + | Some _ -> Array.empty + | None -> + getErrorMessage ident range + else + getErrorMessage ident range | _ -> Array.empty | _ -> Array.empty | _ -> Array.empty diff --git a/src/FSharpLint.Core/Text.resx b/src/FSharpLint.Core/Text.resx index 53ab0ca1f..4c87158fb 100644 --- a/src/FSharpLint.Core/Text.resx +++ b/src/FSharpLint.Core/Text.resx @@ -346,6 +346,6 @@ Consider using 'Array.empty' instead. - Consider using a C#-friendly async overload for {0}. + Async functions in F# should have the "Async" prefix and a C# friendly overload. Check these conventions for {0}.