From 13735514c64905730f4ba0c27f25d23c102e7807 Mon Sep 17 00:00:00 2001 From: Aaron VonderHaar Date: Sat, 24 Jun 2017 14:41:28 -0700 Subject: [PATCH 1/4] WIP --- elm-format.cabal | 1 + src/ElmFormat/Render/Box.hs | 3 +- src/ElmFormat/Render/GroupBinops.hs | 28 +++++++++++++++++++ tests/run-tests.sh | 2 ++ .../transform/AddParensForAndOr.elm | 4 +++ .../transform/AddParensForAndOr.formatted.elm | 5 ++++ 6 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 src/ElmFormat/Render/GroupBinops.hs create mode 100644 tests/test-files/transform/AddParensForAndOr.elm create mode 100644 tests/test-files/transform/AddParensForAndOr.formatted.elm diff --git a/elm-format.cabal b/elm-format.cabal index 85caf2e7e..0fe62bee2 100644 --- a/elm-format.cabal +++ b/elm-format.cabal @@ -91,6 +91,7 @@ library ElmFormat.Filesystem ElmFormat.FileStore ElmFormat.Operation + ElmFormat.Render.GroupBinops ElmFormat.Version Flags Messages.Formatter.Format diff --git a/src/ElmFormat/Render/Box.hs b/src/ElmFormat/Render/Box.hs index 7aae8c950..c50965494 100644 --- a/src/ElmFormat/Render/Box.hs +++ b/src/ElmFormat/Render/Box.hs @@ -21,6 +21,7 @@ import Data.Maybe (fromMaybe, maybeToList) import qualified Data.Text as Text import qualified ElmFormat.Render.ElmStructure as ElmStructure import qualified ElmFormat.Render.Markdown +import qualified ElmFormat.Render.GroupBinops as GroupBinops import qualified ElmFormat.Version import qualified Parse.Parse as Parse import qualified Reporting.Annotation as RA @@ -945,7 +946,7 @@ expressionParens inner outer = formatExpression :: ElmVersion -> ExpressionContext -> AST.Expression.Expr -> Box formatExpression elmVersion context aexpr = - case RA.drop aexpr of + case RA.drop (GroupBinops.extractAnds aexpr) of AST.Expression.Literal lit -> formatLiteral lit diff --git a/src/ElmFormat/Render/GroupBinops.hs b/src/ElmFormat/Render/GroupBinops.hs new file mode 100644 index 000000000..1067c98a9 --- /dev/null +++ b/src/ElmFormat/Render/GroupBinops.hs @@ -0,0 +1,28 @@ +module ElmFormat.Render.GroupBinops (extractAnds) where + +import AST.V0_16 +import AST.Expression +import Reporting.Annotation +import qualified AST.Variable as Var + +extractAnds :: Expr -> Expr +extractAnds expr = + case expr of + A loc (Binops left ops multiline) -> + A loc $ done $ foldl step (left, [], []) ops + + _ -> + expr + + +type State = (Expr, [(Comments, Var.Ref, Comments, Expr)], [Expr]) + + +step :: State -> (Comments, Var.Ref, Comments, Expr) -> State +step (left, inners, outers) (pre, op, post, next) = + (left, inners, outers) + + +done :: State -> Expr' +done (left, inners, outers) = + Unit [] diff --git a/tests/run-tests.sh b/tests/run-tests.sh index 1813cfc56..dae4140f9 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -297,6 +297,8 @@ echo "# elm-format test suite" checkWaysToRun +checkTransformation 0.18 AddParensForAndOr.elm + checkGood 0.16 Simple.elm checkGood 0.16 AllSyntax/0.16/AllSyntax.elm checkGoodAllSyntax 0.16 Module diff --git a/tests/test-files/transform/AddParensForAndOr.elm b/tests/test-files/transform/AddParensForAndOr.elm new file mode 100644 index 000000000..0e5c2afc8 --- /dev/null +++ b/tests/test-files/transform/AddParensForAndOr.elm @@ -0,0 +1,4 @@ + + +addParentsToAnd = + 1 == 2 && 3 /= 4 && 5 < 6 && 7 > 8 && 9 <= 10 && 11 >= 12 diff --git a/tests/test-files/transform/AddParensForAndOr.formatted.elm b/tests/test-files/transform/AddParensForAndOr.formatted.elm new file mode 100644 index 000000000..211a320ed --- /dev/null +++ b/tests/test-files/transform/AddParensForAndOr.formatted.elm @@ -0,0 +1,5 @@ +module Main exposing (..) + + +addParentsToAnd = + (1 == 2) && (3 /= 4) && (5 < 6) && (7 > 8) && (9 <= 10) && (11 >= 12) From 9d473184d4aea2ac82249fab42b2f775fd5f0b4b Mon Sep 17 00:00:00 2001 From: Aaron VonderHaar Date: Mon, 3 Jul 2017 23:14:02 -0700 Subject: [PATCH 2/4] WIP: basic case for grouping && --- src/ElmFormat/Render/Box.hs | 4 +-- src/ElmFormat/Render/GroupBinops.hs | 46 +++++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/ElmFormat/Render/Box.hs b/src/ElmFormat/Render/Box.hs index c50965494..d3a043668 100644 --- a/src/ElmFormat/Render/Box.hs +++ b/src/ElmFormat/Render/Box.hs @@ -792,7 +792,7 @@ formatDefinition elmVersion name args comments expr = body = stack1 $ concat [ map formatComment comments - , [ formatExpression elmVersion SyntaxSeparated expr ] + , [ formatExpression elmVersion SyntaxSeparated (GroupBinops.extractAnds expr) ] -- TODO: use extractAnds for all expressions ] in ElmStructure.definition "=" True @@ -946,7 +946,7 @@ expressionParens inner outer = formatExpression :: ElmVersion -> ExpressionContext -> AST.Expression.Expr -> Box formatExpression elmVersion context aexpr = - case RA.drop (GroupBinops.extractAnds aexpr) of + case RA.drop aexpr of AST.Expression.Literal lit -> formatLiteral lit diff --git a/src/ElmFormat/Render/GroupBinops.hs b/src/ElmFormat/Render/GroupBinops.hs index 1067c98a9..a91333840 100644 --- a/src/ElmFormat/Render/GroupBinops.hs +++ b/src/ElmFormat/Render/GroupBinops.hs @@ -4,12 +4,16 @@ import AST.V0_16 import AST.Expression import Reporting.Annotation import qualified AST.Variable as Var +import qualified Reporting.Region as Region + +-- TODO: handle Ors also +-- TODO: ensure it only groups comparison operators? extractAnds :: Expr -> Expr extractAnds expr = case expr of A loc (Binops left ops multiline) -> - A loc $ done $ foldl step (left, [], []) ops + A loc $ done multiline $ foldl step (left, [], []) ops _ -> expr @@ -19,10 +23,40 @@ type State = (Expr, [(Comments, Var.Ref, Comments, Expr)], [Expr]) step :: State -> (Comments, Var.Ref, Comments, Expr) -> State -step (left, inners, outers) (pre, op, post, next) = - (left, inners, outers) +step (left, inners, conditions) (pre, op, post, next) = -- TODO: Handle comments + case op of + Var.OpRef (SymbolIdentifier maybeAnd) -> + if maybeAnd == "&&" then + ( next, [], (noRegion $ Binops left inners False) : conditions ) -- TODO: Handle multiline + else + ( left, (pre, op, post, next) : inners, conditions ) + + _ -> + ( left, (pre, op, post, next) : inners, conditions ) + + + +done :: Bool -> State -> Expr' +done multiline (left, inners, []) = (Binops left (reverse inners) multiline) +done multiline (left, inners, conditions) = + let + finalConditions = + reverse $ (noRegion $ Binops left (reverse inners) False) : conditions --TODO: use multiline -- TODO: reverse inners is not tested + + buildOp right = + ([], Var.OpRef (SymbolIdentifier "&&"), [], right) + in + case finalConditions of + first : r -> + -- first + (Binops first (fmap buildOp r) False) -- TODO: Handle multiline + + +nowhere :: Region.Position +nowhere = + Region.Position 0 0 -done :: State -> Expr' -done (left, inners, outers) = - Unit [] +noRegion :: a -> Located a +noRegion = + at nowhere nowhere From d4924d066082d2a5e01fd860c9af82da57a8289d Mon Sep 17 00:00:00 2001 From: Aaron VonderHaar Date: Thu, 6 Jul 2017 21:41:02 -0700 Subject: [PATCH 3/4] Fix multiline and double parens --- src/ElmFormat/Render/GroupBinops.hs | 14 +++++--- .../transform/AddParensForAndOr.elm | 35 ++++++++++++++++++- .../transform/AddParensForAndOr.formatted.elm | 35 ++++++++++++++++++- 3 files changed, 77 insertions(+), 7 deletions(-) diff --git a/src/ElmFormat/Render/GroupBinops.hs b/src/ElmFormat/Render/GroupBinops.hs index a91333840..3b6d0b55b 100644 --- a/src/ElmFormat/Render/GroupBinops.hs +++ b/src/ElmFormat/Render/GroupBinops.hs @@ -27,7 +27,7 @@ step (left, inners, conditions) (pre, op, post, next) = -- TODO: Handle comment case op of Var.OpRef (SymbolIdentifier maybeAnd) -> if maybeAnd == "&&" then - ( next, [], (noRegion $ Binops left inners False) : conditions ) -- TODO: Handle multiline + ( next, [], packageCondition (left, inners, conditions) ) else ( left, (pre, op, post, next) : inners, conditions ) @@ -35,21 +35,25 @@ step (left, inners, conditions) (pre, op, post, next) = -- TODO: Handle comment ( left, (pre, op, post, next) : inners, conditions ) +packageCondition :: State -> [Expr] +packageCondition (left, [], conditions) = left : conditions +packageCondition (left, inners, conditions) = + (noRegion $ Binops left (reverse inners) False) : conditions + done :: Bool -> State -> Expr' done multiline (left, inners, []) = (Binops left (reverse inners) multiline) -done multiline (left, inners, conditions) = +done multiline state = let finalConditions = - reverse $ (noRegion $ Binops left (reverse inners) False) : conditions --TODO: use multiline -- TODO: reverse inners is not tested + reverse $ packageCondition state buildOp right = ([], Var.OpRef (SymbolIdentifier "&&"), [], right) in case finalConditions of first : r -> - -- first - (Binops first (fmap buildOp r) False) -- TODO: Handle multiline + (Binops first (fmap buildOp r) multiline) nowhere :: Region.Position diff --git a/tests/test-files/transform/AddParensForAndOr.elm b/tests/test-files/transform/AddParensForAndOr.elm index 0e5c2afc8..36ab2cf63 100644 --- a/tests/test-files/transform/AddParensForAndOr.elm +++ b/tests/test-files/transform/AddParensForAndOr.elm @@ -1,4 +1,37 @@ -addParentsToAnd = +addParensToAnd = 1 == 2 && 3 /= 4 && 5 < 6 && 7 > 8 && 9 <= 10 && 11 >= 12 + +addParensToAndWithComplexConditions = + 1 == 2 + 3 && 4 /= 5 && 6 < 7 + 8 && 9 > 10 && 11 <= 12 && 13 >= 14 + 15 + +addParensToAndMultiline = + 1 + == 2 && 3 /= 4 && 5 < 6 && 7 > 8 && 9 <= 10 && 11 >= 12 + +makesGroupedExpressionsSingleLine = + 1 + == + 2 + + + 7 + * + 5 + - + 6 + + + -8 + && + 3 + /= + 4 + + +doesntAddParensAroundParens = + (1 == 2) && (3 /= 4) && 5/= 6 && (7 /= 8) + +keepsNewLinesInExistingParens = + (1 + == 2 + + 3 * 4 - 5 + 6) && 7 /= 8 diff --git a/tests/test-files/transform/AddParensForAndOr.formatted.elm b/tests/test-files/transform/AddParensForAndOr.formatted.elm index 211a320ed..5936c6a71 100644 --- a/tests/test-files/transform/AddParensForAndOr.formatted.elm +++ b/tests/test-files/transform/AddParensForAndOr.formatted.elm @@ -1,5 +1,38 @@ module Main exposing (..) -addParentsToAnd = +addParensToAnd = (1 == 2) && (3 /= 4) && (5 < 6) && (7 > 8) && (9 <= 10) && (11 >= 12) + + +addParensToAndWithComplexConditions = + (1 == 2 + 3) && (4 /= 5) && (6 < 7 + 8) && (9 > 10) && (11 <= 12) && (13 >= 14 + 15) + + +addParensToAndMultiline = + (1 == 2) + && (3 /= 4) + && (5 < 6) + && (7 > 8) + && (9 <= 10) + && (11 >= 12) + + +makesGroupedExpressionsSingleLine = + (1 == 2 + 7 * 5 - 6 + -8) + && (3 /= 4) + + +doesntAddParensAroundParens = + (1 == 2) && (3 /= 4) && (5 /= 6) && (7 /= 8) + + +keepsNewLinesInExistingParens = + (1 + == 2 + + 3 + * 4 + - 5 + + 6 + ) + && (7 /= 8) From 1bf50e4262bbc396631ef81915e8c3aa11852d52 Mon Sep 17 00:00:00 2001 From: Aaron VonderHaar Date: Thu, 6 Jul 2017 22:22:57 -0700 Subject: [PATCH 4/4] WIP: Handle pipes among && --- src/ElmFormat/Render/GroupBinops.hs | 26 ++++++++++++------- .../transform/AddParensForAndOr.elm | 6 +++++ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/ElmFormat/Render/GroupBinops.hs b/src/ElmFormat/Render/GroupBinops.hs index 3b6d0b55b..5b434263e 100644 --- a/src/ElmFormat/Render/GroupBinops.hs +++ b/src/ElmFormat/Render/GroupBinops.hs @@ -9,6 +9,7 @@ import qualified Reporting.Region as Region -- TODO: handle Ors also -- TODO: ensure it only groups comparison operators? +-- TODO: make it work for expression that aren't directly the body of a definition extractAnds :: Expr -> Expr extractAnds expr = case expr of @@ -19,15 +20,20 @@ extractAnds expr = expr -type State = (Expr, [(Comments, Var.Ref, Comments, Expr)], [Expr]) +type State = (Expr, [(Comments, Var.Ref, Comments, Expr)], [(Expr, Var.Ref)]) + +shiftReverse :: [(a, b)] -> a -> [(b, a)] -> (a, [(b, a)]) +shiftReverse [] last acc = (last, acc) +shiftReverse ((prev, op):rest) last acc = + shiftReverse rest prev ((op, last) : acc) step :: State -> (Comments, Var.Ref, Comments, Expr) -> State step (left, inners, conditions) (pre, op, post, next) = -- TODO: Handle comments case op of - Var.OpRef (SymbolIdentifier maybeAnd) -> - if maybeAnd == "&&" then - ( next, [], packageCondition (left, inners, conditions) ) + Var.OpRef (SymbolIdentifier opSymbol) -> + if opSymbol == "&&" || opSymbol == "<|" then + ( next, [], (Var.OpRef (SymbolIdentifier opSymbol), packageCondition left inners) : conditions ) else ( left, (pre, op, post, next) : inners, conditions ) @@ -35,18 +41,18 @@ step (left, inners, conditions) (pre, op, post, next) = -- TODO: Handle comment ( left, (pre, op, post, next) : inners, conditions ) -packageCondition :: State -> [Expr] -packageCondition (left, [], conditions) = left : conditions -packageCondition (left, inners, conditions) = - (noRegion $ Binops left (reverse inners) False) : conditions +packageCondition :: Expr -> [(Comments, Var.Ref, Comments, Expr)] -> Expr +packageCondition left [] = left +packageCondition left inners = + noRegion $ Binops left (reverse inners) False done :: Bool -> State -> Expr' done multiline (left, inners, []) = (Binops left (reverse inners) multiline) -done multiline state = +done multiline (left, inners, conditions) = let finalConditions = - reverse $ packageCondition state + reverse $ packageCondition left inners : conditions buildOp right = ([], Var.OpRef (SymbolIdentifier "&&"), [], right) diff --git a/tests/test-files/transform/AddParensForAndOr.elm b/tests/test-files/transform/AddParensForAndOr.elm index 36ab2cf63..0506231d3 100644 --- a/tests/test-files/transform/AddParensForAndOr.elm +++ b/tests/test-files/transform/AddParensForAndOr.elm @@ -35,3 +35,9 @@ keepsNewLinesInExistingParens = (1 == 2 + 3 * 4 - 5 + 6) && 7 /= 8 + +doesntGroupPipes f = + f <| True && False + +doesntGroupPipes2 f = + True && False |> f