Merge PR #697 from Mersho/FalseNegativeAvoidSinglePipeOperator

Fix AvoidSinglePipeOperator false negatives (in all inner
expressions).

Author: knocte

src/FSharpLint.Core/Rules/Conventions/AvoidSinglePipeOperator.fs

@@ -17,7 +17,13 @@ let runner (args: AstNodeRuleParams) =
             TypeChecks = List.Empty
         } |> Array.singleton
 
-    let checkExpr (expr: SynExpr) =
+    let rec checkExpr (expr: SynExpr) (parentList: AstNode list): WarningDetails array =
+        let checkParentPiped (expr: AstNode) =
+            match expr with
+            | AstNode.Expression(SynExpr.App(_exprAtomicFlag, _isInfix, funcExpr, _argExpr, _range)) ->
+                checkExpr funcExpr [] |> Seq.isEmpty
+            | _ -> false
+
         match expr with
         | SynExpr.App(_exprAtomicFlag, _isInfix, funcExpr, argExpr, _range) ->
             match funcExpr with
@@ -29,7 +35,14 @@ let runner (args: AstNodeRuleParams) =
                     | SynExpr.IfThenElse _ ->
                         Array.empty
                     | _ ->
-                        errors ident.idRange
+                        let isParentPiped =
+                            match parentList with
+                            | head::_ -> checkParentPiped head
+                            | [] -> false
+                        if isParentPiped then
+                            Array.empty
+                        else
+                            errors ident.idRange
                 else
                     Array.empty
             | _ ->
@@ -39,14 +52,8 @@ let runner (args: AstNodeRuleParams) =
 
     let error =
         match args.AstNode with
-        | AstNode.Binding (SynBinding(_synAcc, _synBinding, _mustInline, _isMut, _synAttribs, _preXmlDoc, _synValData, _headPat, _synBindingRet, synExpr, _range, _debugPointAtBinding, _)) ->
-            match synExpr with
-            | SynExpr.App(_exprAtomicFlag, _isInfix, funcExpr, _argExpr, _range) ->
-                checkExpr funcExpr
-            | SynExpr.IfThenElse(_, SynExpr.App(_exprAtomicFlag, _isInfix, funcExpr, _argExpr, _range), _, _, _, _, _) ->
-                checkExpr funcExpr
-            | _ ->
-                Array.empty
+        | AstNode.Expression(SynExpr.App(_exprAtomicFlag, _isInfix, funcExpr, _argExpr, _range)) ->
+            checkExpr funcExpr (args.GetParents args.NodeIndex)
         | _ ->
             Array.empty
 

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

@@ -106,7 +106,7 @@ let foo param =
         Assert.True this.NoErrorsExist
 
     [<Test>]
-    member this.``Use pipe operator once on record``() =
+    member this.``Use pipe operator once on record within a if statement``() =
         this.Parse """
 type Person =
     {
@@ -123,7 +123,7 @@ let someFunc someParam =
         Assert.IsTrue this.ErrorsExist
 
     [<Test>]
-    member this.``Use pipe operator twice on record``() =
+    member this.``Use pipe operator twice on record within a if statement``() =
         this.Parse """
 type Person =
     {
@@ -140,3 +140,62 @@ let someFunc someParam =
 """
 
         Assert.IsTrue this.NoErrorsExist
+
+    [<Test>]
+    member this.``Use pipe operator once inside of an array``() =
+        this.Parse """
+let someFunc () =
+    [| "Foo" |> String.replicate 2 |]
+"""
+
+        Assert.IsTrue this.ErrorsExist
+
+    [<Test>]
+    member this.``Use pipe operator once on record within a nested if statement``() =
+        this.Parse """
+type Person =
+    {
+        FirstName: string
+    }
+
+let someFunc someParam barParam =
+    if someParam then
+        if barParam then
+            { FirstName = "Bar" } |> someOtherFunc
+        else
+            Array.empty
+    else
+        Array.empty
+"""
+
+        Assert.IsTrue this.ErrorsExist
+
+    [<Test>]
+    member this.``Use pipe operator once without binding``() =
+        this.Parse """
+module Foo
+
+-1.0 |> printf "%d"
+"""
+
+        Assert.IsTrue this.ErrorsExist
+
+    [<Test>]
+    member this.``Use pipe operator twice without binding``() =
+        this.Parse """
+module Foo
+
+-1.0 |> printf "%d" |> ignore
+"""
+
+        Assert.IsTrue this.NoErrorsExist
+
+    [<Test>]
+    member this.``Use pipe operator thrice without binding``() =
+        this.Parse """
+module Foo
+
+-1.0 |> printf "%d" |> ignore |> someOtherFunc
+"""
+
+        Assert.IsTrue this.NoErrorsExist