Skip to content

Commit 2dd4985

Browse files
authored
Correct bug with missing parens in fix of applied composition (#469)
1 parent 1cd07c6 commit 2dd4985

File tree

3 files changed

+59
-8
lines changed

3 files changed

+59
-8
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
## [Unreleased]
44

5+
Bug fixes:
6+
- `(f << g) a` (any applied composition with a single argument) was previously fixed to `f <| g a` without parens which could lead to compile errors with precedence, for example in the fixed code `f <| g a == y`
7+
58
## [2.1.14] - 2026-01-30
69

710
- Disabled `List.sum` and `List.product` simplifications on list reordering operations: `List.reverse`, `List.sort`, `List.sortBy`, `List.sortWith`.

src/Simplify.elm

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3354,14 +3354,16 @@ expressionVisitorHelp (Node expressionRange expression) config context =
33543354
}
33553355

33563356
Node parens (Expression.ParenthesizedExpression ((Node _ (Expression.OperatorApplication "<<" _ _ _)) as operationNode)) ->
3357-
pipingIntoCompositionChecks
3357+
callingCompositionChecks
33583358
{ commentRanges = context.commentRanges
33593359
, extractSourceCode = context.extractSourceCode
33603360
, direction = CallStyle.RightToLeft
33613361
}
33623362
(\() ->
33633363
if List.isEmpty argsAfterFirst then
3364-
removeBoundariesFix applied
3364+
Fix.insertAt expressionRange.start "("
3365+
:: Fix.insertAt expressionRange.end ")"
3366+
:: removeBoundariesFix applied
33653367

33663368
else
33673369
[ Fix.removeRange
@@ -3381,7 +3383,9 @@ expressionVisitorHelp (Node expressionRange expression) config context =
33813383
}
33823384
(\() ->
33833385
if List.isEmpty argsAfterFirst then
3384-
removeBoundariesFix applied
3386+
Fix.insertAt expressionRange.start "("
3387+
:: Fix.insertAt expressionRange.end ")"
3388+
:: removeBoundariesFix applied
33853389

33863390
else
33873391
[ Fix.removeRange
@@ -17783,7 +17787,7 @@ pipelineChecks :
1778317787
}
1778417788
-> Maybe (Error {})
1778517789
pipelineChecks checkInfo =
17786-
pipingIntoCompositionChecks checkInfo (always []) checkInfo.pipedInto
17790+
callingCompositionChecks checkInfo (always []) checkInfo.pipedInto
1778717791
|> onNothing
1778817792
(\() ->
1778917793
reversedCompositionChecks checkInfo (\() -> []) checkInfo.pipedInto
@@ -17852,7 +17856,7 @@ fullyAppliedLambdaInPipelineChecks checkInfo =
1785217856
Nothing
1785317857

1785417858

17855-
pipingIntoCompositionChecks :
17859+
callingCompositionChecks :
1785617860
{ context
1785717861
| commentRanges : List Range
1785817862
, extractSourceCode : Range -> String
@@ -17861,7 +17865,7 @@ pipingIntoCompositionChecks :
1786117865
-> (() -> List Fix)
1786217866
-> Node Expression
1786317867
-> Maybe (Error {})
17864-
pipingIntoCompositionChecks context fixesFromParent expressionNode =
17868+
callingCompositionChecks context fixesFromParent expressionNode =
1786517869
let
1786617870
targetAndReplacement : { opToFind : String, replacement : String }
1786717871
targetAndReplacement =

tests/Simplify/PipelineTest.elm

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ After: data |> fn1 |> fn2"""
469469
, under = ">>"
470470
}
471471
|> Review.Test.whenFixed """module A exposing (..)
472-
a = g <| f b
472+
a = (g <| f b)
473473
"""
474474
]
475475
, test "should reverse >> when used in a plain application call, and add parens if there are more arguments" <|
@@ -492,6 +492,28 @@ After: data |> fn1 |> fn2"""
492492
}
493493
|> Review.Test.whenFixed """module A exposing (..)
494494
a = (g <| f b) c
495+
"""
496+
]
497+
, test "should convert application from >> to parenthesized <| to avoid precedence issues" <|
498+
\() ->
499+
"""module A exposing (..)
500+
a = (f >> g) b == 0
501+
"""
502+
|> Review.Test.run ruleWithDefaults
503+
|> Review.Test.expectErrors
504+
[ Review.Test.error
505+
{ message = "Use <| instead of >>"
506+
, details =
507+
[ "Mixing chains of functions with >> in a direct function call with arguments positioned at the other end is confusing."
508+
, "To make it more idiomatic in Elm and generally easier to read, please use <| instead. You may need to remove some parentheses to do this."
509+
, """Here is an example:
510+
Before: data |> (fn1 >> fn2)
511+
After: data |> fn1 |> fn2"""
512+
]
513+
, under = ">>"
514+
}
515+
|> Review.Test.whenFixed """module A exposing (..)
516+
a = (g <| f b) == 0
495517
"""
496518
]
497519
, test "should convert from << to <| when used in a plain application call" <|
@@ -513,7 +535,29 @@ After: fn3 <| fn2 <| fn1 <| data"""
513535
, under = "<<"
514536
}
515537
|> Review.Test.whenFixed """module A exposing (..)
516-
a = g <| f b
538+
a = (g <| f b)
539+
"""
540+
]
541+
, test "should convert application from << to parenthesized <| to avoid precedence issues" <|
542+
\() ->
543+
"""module A exposing (..)
544+
a = (g << f) b == 0
545+
"""
546+
|> Review.Test.run ruleWithDefaults
547+
|> Review.Test.expectErrors
548+
[ Review.Test.error
549+
{ message = "Use <| instead of <<"
550+
, details =
551+
[ "Because of the precedence of operators, using << at this location is the same as using <|."
552+
, "To make it more idiomatic in Elm and generally easier to read, please use <| instead. You may need to remove some parentheses to do this."
553+
, """Here is an example:
554+
Before: (fn3 << fn2) <| fn1 <| data
555+
After: fn3 <| fn2 <| fn1 <| data"""
556+
]
557+
, under = "<<"
558+
}
559+
|> Review.Test.whenFixed """module A exposing (..)
560+
a = (g <| f b) == 0
517561
"""
518562
]
519563
, test "should convert from << to <| when used in a plain application call, and add parens if there are more arguments" <|

0 commit comments

Comments
 (0)