Merge PR #843 from webwarrior-ws/async-related-rules-types

If output type of function/method is not specified, get it from typed syntax tree.

This affects the following rules:
- SynchronousFunctionNames
- AsynchronousFunctionNames
- SimpleAsyncComplementaryHelpers

Contributes to #517

Author: knocte

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

@@ -7,6 +7,7 @@ open FSharpLint.Framework.Ast
 open FSharpLint.Framework.Rules
 open FSharpLint.Rules.Utilities.LibraryHeuristics
 open Helper.Naming.Asynchronous
+open Utilities.TypedTree
 open FSharp.Compiler.Syntax
 
 [<RequireQualifiedAccess>]
@@ -41,6 +42,24 @@ let runner (config: Config) (args: AstNodeRuleParams) =
         match args.AstNode with
         | AstNode.Binding (SynBinding (_, _, _, _, attributes, _, _, SynPat.LongIdent(funcIdent, _, _, _, accessibility, identRange), returnInfo, _, _, _, _))
             when isAccessibilityLevelApplicable accessibility && not <| Helper.Naming.isAttribute "Obsolete" attributes ->
+            let checkAsyncFunction () =
+                match funcIdent with
+                | HasAsyncPrefix _ ->
+                    Array.empty
+                | HasAsyncSuffix name 
+                | HasNoAsyncPrefixOrSuffix name ->
+                    let nameWithAsync = asyncSuffixOrPrefix + name
+                    emitWarning identRange nameWithAsync "Async"
+
+            let checkTaskFunction () =
+                match funcIdent with
+                | HasAsyncSuffix _ ->
+                    Array.empty
+                | HasAsyncPrefix name 
+                | HasNoAsyncPrefixOrSuffix name ->
+                    let nameWithAsync = name + asyncSuffixOrPrefix
+                    emitWarning identRange nameWithAsync "Task"
+            
             let parents = args.GetParents args.NodeIndex
             let hasEnclosingFunctionOrMethod =
                 parents
@@ -54,25 +73,20 @@ let runner (config: Config) (args: AstNodeRuleParams) =
                 Array.empty
             else
                 match returnInfo with
-                | Some ReturnsAsync ->
-                    match funcIdent with
-                    | HasAsyncPrefix _ ->
-                        Array.empty
-                    | HasAsyncSuffix name 
-                    | HasNoAsyncPrefixOrSuffix name ->
-                        let nameWithAsync = asyncSuffixOrPrefix + name
-                        emitWarning identRange nameWithAsync "Async"
-                | Some ReturnsTask ->
-                    match funcIdent with
-                    | HasAsyncSuffix _ ->
-                        Array.empty
-                    | HasAsyncPrefix name 
-                    | HasNoAsyncPrefixOrSuffix name ->
-                        let nameWithAsync = name + asyncSuffixOrPrefix
-                        emitWarning identRange nameWithAsync "Task"
+                | Some ReturnsAsync -> checkAsyncFunction()
+                | Some ReturnsTask -> checkTaskFunction()
                 | None -> 
-                    // TODO: get type using typed tree in args.CheckInfo
-                    Array.empty
+                    match args.CheckInfo with
+                    | Some checkInfo ->
+                        match getFunctionReturnType checkInfo args.Lines funcIdent with
+                        | Some returnType ->
+                            match returnType with
+                            | FSharpTypeAsync -> checkAsyncFunction()
+                            | FSharpTypeTask
+                            | FSharpTypeTaskNonGeneric -> checkTaskFunction()
+                            | FSharpTypeNonAsync -> Array.empty
+                        | None -> Array.empty
+                    | None -> Array.empty
                 | _ ->
                     Array.empty
         | _ -> Array.empty

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

@@ -9,15 +9,17 @@ open FSharpLint.Rules.Utilities.LibraryHeuristics
 open Helper.Naming.Asynchronous
 open FSharp.Compiler.Syntax
 open FSharp.Compiler.Text
+open Utilities.TypedTree
+open FSharp.Compiler.Symbols
 
 [<RequireQualifiedAccess>]
 type Config = {
     Mode: AsynchronousFunctionsMode
 }
 
 type private ReturnType =
-    | Async of typeParam: Option<SynType> * isLowercase: bool
-    | Task of typeParam: Option<SynType>
+    | Async of typeParamString: Option<string> * isLowercase: bool
+    | Task of typeParamString: Option<string>
 
 type private Func =
     {
@@ -49,11 +51,6 @@ let runner (config: Config) (args: AstNodeRuleParams) =
             | Some text -> text
             | None -> failwithf "Invalid range: %A" func.Range
 
-        let tryGetTypeParamString (maybeTypeParam: Option<SynType>) =
-            match maybeTypeParam with
-            | Some typeParam -> ExpressionUtilities.tryFindTextOfRange typeParam.Range args.FileContent
-            | None -> None
-
         let message =
             match func.ReturnType with
             | Async (typeParam, isLowercase) -> 
@@ -69,7 +66,7 @@ let runner (config: Config) (args: AstNodeRuleParams) =
                     else
                         newFuncName
                 let typeDefString =
-                    match tryGetTypeParamString typeParam with
+                    match typeParam with
                     | Some "unit" -> ": Task"
                     | Some str -> $": Task<{str}>"
                     | None -> String.Empty
@@ -84,7 +81,7 @@ let runner (config: Config) (args: AstNodeRuleParams) =
             | Task typeParam ->
                 let newFuncName = funcDefinitionString.Replace(func.BaseName + asyncSuffixOrPrefix, asyncSuffixOrPrefix + func.BaseName)
                 let typeDefString =
-                    match tryGetTypeParamString typeParam with
+                    match typeParam with
                     | Some str -> $": Async<{str}>"
                     | None -> String.Empty
                 String.Format(
@@ -112,16 +109,36 @@ let runner (config: Config) (args: AstNodeRuleParams) =
             | None
             | Some(SynAccess.Public _) -> true
             | _ -> config.Mode = AllAPIs
+        
+        let tryGetReturnTypeParamDisplayName (displayContext: Option<FSharpDisplayContext>) (returnType: FSharpType) =
+            match tryGetLastGenericArg returnType with
+            | Some paramType ->
+                Some <| paramType.Format (displayContext |> Option.defaultValue FSharpDisplayContext.Empty)
+            | _ -> None
 
         let tryGetFunction (binding: SynBinding) =
             match binding with
-            | SynBinding(_, _, _, _, _, _, _, SynPat.LongIdent(funcIdent, _, _, argPats, accessibility, _), returnInfo, _, _, _, _)
+            | SynBinding(_, _, _, _, _, _, _, SynPat.LongIdent(funcIdent, _, _, argPats, accessibility, _), returnInfo, _, bindingRange, _, _)
                 when isAccessibilityLevelApplicable accessibility && not argPats.Patterns.IsEmpty ->
                 let returnTypeParam =
                     match returnInfo with
                     | Some(SynBindingReturnInfo(SynType.App(SynType.LongIdent(SynLongIdent _), _, [ typeParam ], _, _, _, _), _, _, _)) ->
-                        Some typeParam 
-                    | _ -> None
+                        ExpressionUtilities.tryFindTextOfRange typeParam.Range args.FileContent
+                    | _ ->
+                        match args.CheckInfo with
+                        | Some checkInfo ->
+                            match getFunctionReturnType checkInfo args.Lines funcIdent with
+                            | Some returnType ->
+                                match returnType with
+                                | FSharpTypeAsync
+                                | FSharpTypeTask ->
+                                    tryGetReturnTypeParamDisplayName
+                                        (checkInfo.GetDisplayContextForPos bindingRange.End)
+                                        returnType
+                                | FSharpTypeTaskNonGeneric -> Some "unit"
+                                | FSharpTypeNonAsync -> None
+                            | None -> None
+                        | None -> None
 
                 let funcArgs =
                     match argPats with

src/FSharpLint.Core/Rules/Conventions/Naming/SynchronousFunctionNames.fs

@@ -5,11 +5,9 @@ open FSharpLint.Framework
 open FSharpLint.Framework.Suggestion
 open FSharpLint.Framework.Ast
 open FSharpLint.Framework.Rules
-open Helper.Naming.Asynchronous
 open FSharp.Compiler.Syntax
-open FSharp.Compiler.Symbols
-
-let asyncSuffixOrPrefix = "Async"
+open Helper.Naming.Asynchronous
+open Utilities.TypedTree
 
 let runner (args: AstNodeRuleParams) =
     let emitWarning range (newFunctionName: string) =
@@ -21,28 +19,38 @@ let runner (args: AstNodeRuleParams) =
                 TypeChecks = List.empty
             }
 
+    let checkIdentifier (funcIdent: SynLongIdent) identRange =
+        match funcIdent with
+        | HasAsyncPrefix name ->
+            let startsWithLowercase = Char.IsLower name.[0]
+            let nameWithoutAsync = name.Substring asyncSuffixOrPrefix.Length
+            let suggestedName = 
+                if startsWithLowercase then
+                    sprintf "%c%s" (Char.ToLowerInvariant nameWithoutAsync.[0]) (nameWithoutAsync.Substring 1)
+                else
+                    nameWithoutAsync
+            emitWarning identRange suggestedName
+        | HasAsyncSuffix name ->
+            let nameWithoutAsync = name.Substring(0, name.Length - asyncSuffixOrPrefix.Length)
+            emitWarning identRange nameWithoutAsync
+        | HasNoAsyncPrefixOrSuffix _ -> Array.empty
+
     match args.AstNode with
     | AstNode.Binding (SynBinding (_, _, _, _, attributes, _, _, SynPat.LongIdent(funcIdent, _, _, _, _, identRange), returnInfo, _, _, _, _))
         when not <| Helper.Naming.isAttribute "Obsolete" attributes ->
         match returnInfo with
         | Some ReturnsNonAsync ->
-            match funcIdent with
-            | HasAsyncPrefix name ->
-                let startsWithLowercase = Char.IsLower name.[0]
-                let nameWithoutAsync = name.Substring asyncSuffixOrPrefix.Length
-                let suggestedName = 
-                    if startsWithLowercase then
-                        sprintf "%c%s" (Char.ToLowerInvariant nameWithoutAsync.[0]) (nameWithoutAsync.Substring 1)
-                    else
-                        nameWithoutAsync
-                emitWarning identRange suggestedName
-            | HasAsyncSuffix name ->
-                let nameWithoutAsync = name.Substring(0, name.Length - asyncSuffixOrPrefix.Length)
-                emitWarning identRange nameWithoutAsync
-            | HasNoAsyncPrefixOrSuffix _ -> Array.empty
+            checkIdentifier funcIdent identRange
         | None -> 
-            // TODO: get type using typed tree in args.CheckInfo
-            Array.empty
+            match args.CheckInfo with
+            | Some checkInfo ->
+                match getFunctionReturnType checkInfo args.Lines funcIdent with
+                | Some returnType ->
+                    match returnType with
+                    | FSharpTypeNonAsync -> checkIdentifier funcIdent identRange
+                    | _ -> Array.empty
+                | None -> Array.empty
+            | None -> Array.empty
         | _ ->
             Array.empty
     | _ -> Array.empty

src/FSharpLint.Core/Rules/NamingHelper.fs

@@ -476,7 +476,11 @@ module Asynchronous =
     let (|HasAsyncPrefix|HasAsyncSuffix|HasNoAsyncPrefixOrSuffix|) (pattern: SynLongIdent) =
         match List.tryLast pattern.LongIdent with
         | Some name ->
-            if name.idText.StartsWith(asyncSuffixOrPrefix, StringComparison.InvariantCultureIgnoreCase) then
+            if not (isNotDoubleBackTickedIdent name) then
+                // Consider any identifier surrounded by double backticks (usually used as test names)
+                // to not have Async prefix or suffix.
+                HasNoAsyncPrefixOrSuffix name.idText
+            elif name.idText.StartsWith(asyncSuffixOrPrefix, StringComparison.InvariantCultureIgnoreCase) then
                 HasAsyncPrefix name.idText
             elif name.idText.EndsWith asyncSuffixOrPrefix then
                 HasAsyncSuffix name.idText
@@ -493,3 +497,19 @@ module Asynchronous =
             | Some ident when ident.idText = "Task" -> ReturnsTask
             | _ -> ReturnsNonAsync
         | _ -> ReturnsNonAsync
+    
+    let (|FSharpTypeAsync|FSharpTypeTask|FSharpTypeTaskNonGeneric|FSharpTypeNonAsync|) (fSharpType: FSharpType) =
+        try
+            // BasicQualifiedName may throw InvalidOperationException for some types
+            // e.g. arrays, but not for Async<'T> or Task<'T>, so assume non-async type.
+            match fSharpType.BasicQualifiedName with
+            | "Microsoft.FSharp.Control.FSharpAsync`1" -> FSharpTypeAsync
+            | name when name.StartsWith "System.Threading.Tasks.Task" ->
+                if fSharpType.GenericArguments.Count > 0 then
+                    FSharpTypeTask
+                else
+                    FSharpTypeTaskNonGeneric
+            | _ -> FSharpTypeNonAsync
+        with
+        | :? InvalidOperationException ->
+            FSharpTypeNonAsync

src/FSharpLint.Core/Rules/Utilities.fs

@@ -1,5 +1,43 @@
 module FSharpLint.Rules.Utilities
 
+open FSharp.Compiler.CodeAnalysis
+open FSharp.Compiler.Symbols
+open FSharp.Compiler.Syntax
+
+module TypedTree =
+    let tryGetLastGenericArg (fSharpType: FSharpType) =
+        Seq.tryLast fSharpType.GenericArguments
+
+    [<TailCall>]
+    let rec private getReturnType (fSharpType: FSharpType) =
+        if fSharpType.IsFunctionType then
+            match tryGetLastGenericArg fSharpType with
+            | Some argType -> getReturnType argType
+            | None -> fSharpType
+        else
+            fSharpType
+
+    let getFunctionReturnType
+        (checkInfo: FSharpCheckFileResults)
+        (lines: array<string>)
+        (funcIdent: SynLongIdent) : Option<FSharpType> =
+        let maybeSymbolUse =
+            match List.tryLast funcIdent.LongIdent with
+            | Some ident ->
+                checkInfo.GetSymbolUseAtLocation(
+                    ident.idRange.EndLine,
+                    ident.idRange.EndColumn,
+                    lines.[ident.idRange.EndLine - 1],
+                    List.singleton ident.idText)
+            | None -> None
+        match maybeSymbolUse with
+        | Some symbolUse ->
+            match symbolUse.Symbol with
+            | :? FSharpMemberOrFunctionOrValue as func when func.IsFunction ->
+                Some <| getReturnType func.FullType
+            | _ -> None
+        | _ -> None
+
 module LibraryHeuristics =
     type LibraryHeuristicResultByProjectName =
         | Likely

tests/FSharpLint.Core.Tests/Rules/Conventions/AsyncExceptionWithoutReturn.fs

@@ -9,7 +9,7 @@ type TestAsyncExceptionWithoutReturn() =
     inherit FSharpLint.Core.Tests.TestAstNodeRuleBase.TestAstNodeRuleBase(AsyncExceptionWithoutReturn.rule)
 
     [<Test>]
-    member this.AsyncRaiseWithoutReturn() =
+    member this.RaiseInAsyncBlockWithoutReturn() =
         this.Parse("""
 module Program
 
@@ -22,7 +22,7 @@ let someAsyncFunction () =
         Assert.IsTrue this.ErrorsExist
 
     [<Test>]
-    member this.AsyncRaiseWithReturn() =
+    member this.RaiseInAsyncBlockWithReturn() =
         this.Parse("""
 module Program
 
@@ -34,7 +34,7 @@ let someAsyncFunction () =
         this.AssertNoWarnings()
 
     [<Test>]
-    member this.AsyncFailWithWithoutReturn() =
+    member this.FailWithInAsyncBlockWithoutReturn() =
         this.Parse("""
 module Program
 
@@ -47,7 +47,7 @@ let someAsyncFunction () =
         Assert.IsTrue this.ErrorsExist
 
     [<Test>]
-    member this.AsyncFailwithfWithoutReturn1() =
+    member this.FailwithfInAsyncBlockWithoutReturn1() =
         this.Parse("""
 module Program
 
@@ -61,7 +61,7 @@ let someAsyncFunction () =
         Assert.IsTrue this.ErrorsExist
 
     [<Test>]
-    member this.AsyncFailwithfWithoutReturn2() =
+    member this.FailwithfInAsyncBlockWithoutReturn2() =
         this.Parse("""
 module Program
 
@@ -74,7 +74,7 @@ let someAsyncFunction () =
         Assert.IsTrue this.ErrorsExist
 
     [<Test>]
-    member this.AsyncFailwithWithReturn() =
+    member this.FailwithInAsyncBlockWithReturn() =
         this.Parse("""
 module Program
 
@@ -86,7 +86,7 @@ let someAsyncFunction () =
         this.AssertNoWarnings()
 
     [<Test>]
-    member this.AsyncFailwithfWithReturn() =
+    member this.FailwithfInAsyncBlockWithReturn() =
         this.Parse("""
 module Program
 
@@ -99,7 +99,7 @@ let someAsyncFunction () =
         this.AssertNoWarnings()
 
     [<Test>]
-    member this.AsyncRaiseWithReturnInnerExpression() =
+    member this.RaiseInAsyncBlockWithReturnInnerExpression() =
         this.Parse("""
 module Program
 
@@ -114,7 +114,7 @@ let someAsyncFunction () =
         this.AssertNoWarnings()
 
     [<Test>]
-    member this.AsyncRaiseWithoutReturnInnerExpression() =
+    member this.RaiseInAsyncBlockWithoutReturnInnerExpression() =
         this.Parse("""
 module Program