tweag / ormolu

A formatter for Haskell source code
https://ormolu-live.tweag.io
Other
958 stars 83 forks source link

Extra space with negation operator #694

Closed brandon-leapyear closed 3 years ago

brandon-leapyear commented 3 years ago

Describe the bug

To Reproduce

f x = 1 / (1 + exp (-x))

gets reformatted as

f x = 1 / (1 + exp (- x))

Apparently GHC still treats this as equivalent, but it's very easy to mistakenly read it as a partially applied -

Expected behavior (-x) should kept as (-x)

Environment

Additional context Add any other context about the problem here.

mrkkrp commented 3 years ago

I don't consider this a bug. I wonder if we should keep this open.

brandon-leapyear commented 3 years ago

:sparkles: This is an old work account. Please reference @brandonchinn178 for all future communication :sparkles:


Sure, this could be considered a feature request. I think this is a worthwhile issue, though, because (- x) very much looks like a partial section, not a unary operator

mrkkrp commented 3 years ago

But (-x) and (- x) are parsed as identical constructs on the AST level, right?

brandon-leapyear commented 3 years ago

:sparkles: This is an old work account. Please reference @brandonchinn178 for all future communication :sparkles:


Yes, but the point of a formatter is to enforce consistent style for the sake of readability across a codebase. IMO, (- x) is much less readable than (-x), and the formatter should style in the more readable format

brandon-leapyear commented 3 years ago

:sparkles: This is an old work account. Please reference @brandonchinn178 for all future communication :sparkles:


Adding on to this,

f n = n-1

gets reformatted to

f n = n -1

Distinguishing between negation operator and minus would be nice

amesgen commented 3 years ago

The reason why ormolu adds spaces after - is due to #181: NegativeLiterals is not part of manualExts, so the expressions -1 and - 1 as parsed via ormolu without further options have differing ASTs (the literal -1 and the negated literal 1). Without NegativeLiterals, they would have the same AST. In all other places, the space after the - is not necessary.

In #734, I added NegativeLiterals to manualExts, and only add a space after - if we negate a literal and NegativeLiterals is actually enabled.

This means that

f x = 1 / (1 + exp (-x))

is a fixed point, and

f n = n-1

is formatted into

f n = n - 1

As a side note: GHC 9.0 added the new extension LexicalNegation which allows (- x) to be an operator section, such that

{-# LANGUAGE LexicalNegation #-}

add2AndSubtract3 :: Bifunctor f => f Int Int -> f Int Int
add2AndSubtract3 = bimap (+ 2) (- 3)

is a fixed point, and add2AndSubtract3 (1,2) == (3,-1). Without LexicalNegation, the code would be formatted to

add2AndSubtract3 :: Bifunctor f => f Int Int -> f Int Int
add2AndSubtract3 = bimap (+ 2) (-3)

which makes it easy to spot that something is wrong.