Merge PR #696 from Mersho/AvoidSinglePipeOperatorFalseNegative

Fix AvoidSinglePipeOperator false negative.
fsprojects · Feb 12, 2024 · 08ceae7 · 08ceae7
2 parents 8a01dc4 + 0b25137
commit 08ceae7
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 18 deletions.
diff --git a/src/FSharpLint.Core/Rules/Conventions/AvoidSinglePipeOperator.fs b/src/FSharpLint.Core/Rules/Conventions/AvoidSinglePipeOperator.fs
@@ -16,30 +16,35 @@ let runner (args: AstNodeRuleParams) =
             SuggestedFix = None
             TypeChecks = List.Empty
         } |> Array.singleton
+
+    let checkExpr (expr: SynExpr) =
+        match expr with
+        | SynExpr.App(_exprAtomicFlag, _isInfix, funcExpr, argExpr, _range) ->
+            match funcExpr with
+            | ExpressionUtilities.Identifier([ ident ], _) ->
+                if ident.idText = "op_PipeRight" then
+                    match argExpr with
+                    | SynExpr.App(_exprAtomicFlag, _isInfix, _funcExpr, _argExpr, _range) ->
+                        Array.empty
+                    | SynExpr.IfThenElse _ ->
+                        Array.empty
+                    | _ ->
+                        errors ident.idRange
+                else
+                    Array.empty
+            | _ ->
+                Array.empty
+        | _ ->
+            Array.empty
 
     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) ->
-                match funcExpr with
-                | SynExpr.App(_exprAtomicFlag, _isInfix, funcExpr, argExpr, _range) ->
-                    match funcExpr with
-                    | ExpressionUtilities.Identifier([ ident ], _) ->
-                        if ident.idText = "op_PipeRight" then
-                            match argExpr with
-                            | SynExpr.App(_exprAtomicFlag, _isInfix, _funcExpr, _argExpr, _range) ->
-                                Array.empty
-                            | SynExpr.IfThenElse _ ->
-                                Array.empty
-                            | _ ->
-                                errors ident.idRange
-                        else
-                            Array.empty
-                    | _ ->
-                        Array.empty
-                | _ ->
-                    Array.empty
+                checkExpr funcExpr
+            | SynExpr.IfThenElse(_, SynExpr.App(_exprAtomicFlag, _isInfix, funcExpr, _argExpr, _range), _, _, _, _, _) ->
+                checkExpr funcExpr
             | _ ->
                 Array.empty
         | _ ->

diff --git a/tests/FSharpLint.Core.Tests/Rules/Conventions/AvoidSinglePipeOperator.fs b/tests/FSharpLint.Core.Tests/Rules/Conventions/AvoidSinglePipeOperator.fs
@@ -104,3 +104,39 @@ let foo param =
 """
 
         Assert.True this.NoErrorsExist
+
+    [<Test>]
+    member this.``Use pipe operator once on record``() =
+        this.Parse """
+type Person =
+    {
+        FirstName: string
+    }
+
+let someFunc someParam =
+    if someParam then
+        { FirstName = "Bar" } |> someOtherFunc
+    else
+        Array.empty
+"""
+
+        Assert.IsTrue this.ErrorsExist
+
+    [<Test>]
+    member this.``Use pipe operator twice on record``() =
+        this.Parse """
+type Person =
+    {
+        FirstName: string
+    }
+
+let someFunc someParam =
+    if someParam then
+        { FirstName = "Bar" }
+        |> someOtherFunc
+        |> yetAnotherFunc
+    else
+        Array.empty
+"""
+
+        Assert.IsTrue this.NoErrorsExist