tweag / ormolu

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

Warn when fixities are different from base #1069

Closed brandonchinn178 closed 5 months ago

brandonchinn178 commented 10 months ago

Resolves #1060

Tested with:

{-# LANGUAGE NoImplicitPrelude #-}

import MyCustomPrelude

main :: IO ()
main =
  putStrLn $
    "hello " ++ "world"

This shows the following with --debug:

*** WARNING *** Operator is possibly using the wrong fixity. Got: FixityApproximation {faDirection = Just InfixL, faMinPrecedence = 9, faMaxPrecedence = 9}, Fixities being shadowed: [FixityInfo {fiDirection = InfixR, fiPrecedence = 5},FixityInfo {fiDirection = InfixR, fiPrecedence = 5},FixityInfo {fiDirection = InfixR, fiPrecedence = 5},FixityInfo {fiDirection = InfixR, fiPrecedence = 5},FixityInfo {fiDirection = InfixR, fiPrecedence = 5}]
*** WARNING *** Operator is possibly using the wrong fixity. Got: FixityApproximation {faDirection = Just InfixL, faMinPrecedence = 9, faMaxPrecedence = 9}, Fixities being shadowed: [FixityInfo {fiDirection = InfixR, fiPrecedence = 0},FixityInfo {fiDirection = InfixR, fiPrecedence = 0},FixityInfo {fiDirection = InfixR, fiPrecedence = 0}]
mrkkrp commented 9 months ago

Could you add such a warning without introducing Ormolu.Logging?

brandonchinn178 commented 9 months ago

So without a logging framework, we would have to do the following:

  1. Add debug/quiet/log level to the environment of R
  2. Pass the log level through from main to where R is set up
  3. Pass log level to reassociateOpTree
  4. Pass log level to addFixityInfo
  5. Call traceM in addFixityInfo

This seemed rather annoying and not scalable, and I figured it'd be better to actually come up with a logging system that works in pure code.

I assume the biggest issue with this solution is the global ref? I think having a proper logging system would be better than the smattering of trace/putStrLn statements currently in the codebase. If the concern is with the global ref (which I agree is a bit code smelly), maybe I could store a LogContext in R and have the log functions take in the context?

Or maybe turn addFixityInfo into a StateT that outputs fixities different from base and print out the warnings when it gets back into the R monad?

On Thu, Oct 12, 2023, 10:32 AM Mark Karpov @.***> wrote:

Could you add such a warning without introducing Ormolu.Logging?

— Reply to this email directly, view it on GitHub https://github.com/tweag/ormolu/pull/1069#issuecomment-1760056345, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABUC7EUUCIMA5PHDHLUKOH3X7ASTJANCNFSM6AAAAAA4SYFNS4 . You are receiving this because you authored the thread.Message ID: @.***>

brandonchinn178 commented 9 months ago

@mrkkrp do you have any thoughts on the above approaches?