Warn when fixities are different from base

[brandonchinn178]

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}]

[brandonchinn178]

This is technically different behavior than the previous version: if debug is not enabled, the new version still relativizes the cabal file path, even though it's not printed.

I could create a version of logDebugM that takes a to-be-computed IO String message, but I don't think adding such a bespoke version is worth this relatively cheap call.

[brandonchinn178]

Yeah, this is a bit hacky and unprincipled, smuggling the config through the global environment. But I figured this was much neater than storing the config in R's environment and passing it through addFixityInfo.

I call this function immediately in all the entrypoints (main, ormolu, and for tests), so we should be good in principle. The only issue might be if someone imports from Ormolu's submodules + calls functions directly. I guess I could set a default logger config instead of erroring with "logger not configured yet".

[mrkkrp]

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

[brandonchinn178]

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 <#1069 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABUC7EUUCIMA5PHDHLUKOH3X7ASTJANCNFSM6AAAAAA4SYFNS4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[brandonchinn178]

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