Skip to content

Correct bug with missing parens in fix of applied composition #469

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jfmengels merged 2 commits into from
Feb 2, 2026
Merged

Correct bug with missing parens in fix of applied composition #469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
266ed53
correct bug with missing parens in fixing applied composition
lue-bird Feb 2, 2026
caf93a9
correct test names
lue-bird Feb 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## [Unreleased]

Bug fixes:
- `(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`

## [2.1.14] - 2026-01-30

- Disabled `List.sum` and `List.product` simplifications on list reordering operations: `List.reverse`, `List.sort`, `List.sortBy`, `List.sortWith`.
Expand Down
16 changes: 10 additions & 6 deletions src/Simplify.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3354,14 +3354,16 @@ expressionVisitorHelp (Node expressionRange expression) config context =
}

Node parens (Expression.ParenthesizedExpression ((Node _ (Expression.OperatorApplication "<<" _ _ _)) as operationNode)) ->
pipingIntoCompositionChecks
callingCompositionChecks
{ commentRanges = context.commentRanges
, extractSourceCode = context.extractSourceCode
, direction = CallStyle.RightToLeft
}
(\() ->
if List.isEmpty argsAfterFirst then
removeBoundariesFix applied
Fix.insertAt expressionRange.start "("
:: Fix.insertAt expressionRange.end ")"
:: removeBoundariesFix applied

else
[ Fix.removeRange
Expand All @@ -3381,7 +3383,9 @@ expressionVisitorHelp (Node expressionRange expression) config context =
}
(\() ->
if List.isEmpty argsAfterFirst then
removeBoundariesFix applied
Fix.insertAt expressionRange.start "("
:: Fix.insertAt expressionRange.end ")"
:: removeBoundariesFix applied

else
[ Fix.removeRange
Expand Down Expand Up @@ -17783,7 +17787,7 @@ pipelineChecks :
}
-> Maybe (Error {})
pipelineChecks checkInfo =
pipingIntoCompositionChecks checkInfo (always []) checkInfo.pipedInto
callingCompositionChecks checkInfo (always []) checkInfo.pipedInto
|> onNothing
(\() ->
reversedCompositionChecks checkInfo (\() -> []) checkInfo.pipedInto
Expand Down Expand Up @@ -17852,7 +17856,7 @@ fullyAppliedLambdaInPipelineChecks checkInfo =
Nothing


pipingIntoCompositionChecks :
callingCompositionChecks :
{ context
| commentRanges : List Range
, extractSourceCode : Range -> String
Expand All @@ -17861,7 +17865,7 @@ pipingIntoCompositionChecks :
-> (() -> List Fix)
-> Node Expression
-> Maybe (Error {})
pipingIntoCompositionChecks context fixesFromParent expressionNode =
callingCompositionChecks context fixesFromParent expressionNode =
let
targetAndReplacement : { opToFind : String, replacement : String }
targetAndReplacement =
Expand Down
48 changes: 46 additions & 2 deletions tests/Simplify/PipelineTest.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ After: data |> fn1 |> fn2"""
, under = ">>"
}
|> Review.Test.whenFixed """module A exposing (..)
a = g <| f b
a = (g <| f b)
"""
]
, test "should reverse >> when used in a plain application call, and add parens if there are more arguments" <|
Expand All @@ -492,6 +492,28 @@ After: data |> fn1 |> fn2"""
}
|> Review.Test.whenFixed """module A exposing (..)
a = (g <| f b) c
"""
]
, test "should convert application from >> to parenthesized <| to avoid precedence issues" <|
\() ->
"""module A exposing (..)
a = (f >> g) b == 0
"""
|> Review.Test.run ruleWithDefaults
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Use <| instead of >>"
, details =
[ "Mixing chains of functions with >> in a direct function call with arguments positioned at the other end is confusing."
, "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."
, """Here is an example:
Before: data |> (fn1 >> fn2)
After: data |> fn1 |> fn2"""
]
, under = ">>"
}
|> Review.Test.whenFixed """module A exposing (..)
a = (g <| f b) == 0
"""
]
, test "should convert from << to <| when used in a plain application call" <|
Expand All @@ -513,7 +535,29 @@ After: fn3 <| fn2 <| fn1 <| data"""
, under = "<<"
}
|> Review.Test.whenFixed """module A exposing (..)
a = g <| f b
a = (g <| f b)
"""
]
, test "should convert application from << to parenthesized <| to avoid precedence issues" <|
\() ->
"""module A exposing (..)
a = (g << f) b == 0
"""
|> Review.Test.run ruleWithDefaults
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Use <| instead of <<"
, details =
[ "Because of the precedence of operators, using << at this location is the same as using <|."
, "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."
, """Here is an example:
Before: (fn3 << fn2) <| fn1 <| data
After: fn3 <| fn2 <| fn1 <| data"""
]
, under = "<<"
}
|> Review.Test.whenFixed """module A exposing (..)
a = (g <| f b) == 0
"""
]
, test "should convert from << to <| when used in a plain application call, and add parens if there are more arguments" <|
Expand Down
Loading