on master help sign in
Use mode instead of mean when calculating operator fixities

When deciding on the fixity of an operator, we assign a score based on some heuristics. When an operator is used multiple times in an operator chain, we used to calculate the mean of its scores to decide on the final fixity. However, this was prone to outliers. In an example like this: b & c .~ d & e %~ f g The first use of & is surrounded by other values, so we assign it a higher score (binds tighter). However the second occurence is at the beginning of a line, and this has a lower score. In the end, when we average those two values the result binds tighter than %~, which is not what we want. This PR replaces the average method with a mode operation instead, so it is less prone to outliers. This PR fixes this specific issue, and does not cause any change on other test cases, but it might cause some diffs in the wild.

commit0a8cfd62eff05b092a2b99754ed6f6fa3788132b authorUtku Demir <me@utdemir.com> dateSun 17 Nov 2019, 17:43 (7 years ago) parente91ced8
3 files changed+23−6
data/examples/declaration/value/function/operators-out.hs+5 data/examples/declaration/value/function/operators.hs+4 src/Ormolu/Printer/Operators.hs+14−6
data/examples/declaration/value/function/operators-out.hsnew file+5
@@ -0,0 +1,5 @@
1+a =
2+ b & c .~ d
3+ & e
4+ %~ f
5+ g
data/examples/declaration/value/function/operators.hsnew file+4
@@ -0,0 +1,4 @@
1+a =
2+ b & c .~ d
3+ & e %~ f
4+ g
src/Ormolu/Printer/Operators.hs+14−6
@@ -14,6 +14,7 @@ import BasicTypes (Fixity (..), SourceText (NoSourceText), compareFixity, defaul
1414import Data.Function (on)
1515import Data.List
1616import Data.Maybe (fromMaybe, mapMaybe)
17+import Data.Ord (Down (Down), comparing)
1718import GHC
1819import OccName (mkVarOcc)
1920import RdrName (mkRdrUnqual)
@@ -108,7 +109,7 @@ buildFixityMap getOpName opTree =
108109 . zip [0 ..]
109110 . groupBy (doubleWithinEps 0.00001 `on` snd)
110111 . (overrides ++)
111− . avgScores
112+ . modeScores
112113 $ score opTree
113114 where
114115 -- Add a special case for ($), since it is pretty unlikely for someone
@@ -141,18 +142,25 @@ buildFixityMap getOpName opTree =
141142 | otherwise =
142143 2 -- otherwise, assign a high score.
143144 return $ (opName, s) : score r
144− avgScores :: [(RdrName, Double)] -> [(RdrName, Double)]
145− avgScores =
145+ -- Pick the most common score per 'RdrName'.
146+ modeScores :: [(RdrName, Double)] -> [(RdrName, Double)]
147+ modeScores =
146148 sortOn snd
147149 . mapMaybe
148150 ( \case
149151 [] -> Nothing
150− xs@((n, _) : _) -> Just (n, avg $ map snd xs)
152+ xs@((n, _) : _) -> Just (n, mode $ map snd xs)
151153 )
152154 . groupBy ((==) `on` fst)
153155 . sort
154− avg :: [Double] -> Double
155− avg i = sum i / fromIntegral (length i)
156+ -- Return the most common number, leaning to the smaller
157+ -- one in case of a tie.
158+ mode :: [Double] -> Double
159+ mode =
160+ head
161+ . minimumBy (comparing (Down . length))
162+ . groupBy (doubleWithinEps 0.0001)
163+ . sort
156164 -- The start column of the rightmost operator.
157165 maxCol = go opTree
158166 where