Fmap Gets Context Wrong in Nested Contexts

[turtlegrammar]

To reproduce:

(require '[cats.core :as cats])
(require '[cats.monad.either :as either])
(require '[cats.monad.maybe :as maybe])

(cats/mlet [x (either/right 1)
            z (either/right (maybe/from-maybe (cats/fmap #(do (prn "okay, here")
                                                              (inc %))
                                                         (maybe/just 1))
                                              0))]
           (either/right z))

;; => Right 1

The expected behavior: "okay, here" is printed, and the return is Right 2.
The actual behavior: the fmap never applies the function to Just 1, instead returning it as is.

I think this happens because fmap determines the context is Either, and Just 1 is not Right 1. See this example:

(require '[cats.context :as ctx])
(require '[cats.core :as cats])
(require '[cats.monad.either :as either])
(require '[cats.monad.maybe :as maybe])

(cats/mlet [x (either/right 1)]
           (either/right (ctx/infer (maybe/just 1))))
;; => Right #<Either>

I thought you could fix this by swapping the order of the first two cond clauses in context/infer -- first, check if v satisfies p/Contextual, and if so, get the context from v. Fall back to getting *context*. But, when I made this change, some tests started failing, and I don't have the time to figure out why.

[luminusian]

#188 seems related.

[sovelten]

Swapping the conds also solved my issue with cats using the wrong context. I can try to make a PR for this, but it would be nice to hear from the maintainers if this is the right thing to do.

[barconference]

@EricVM
I'm not a maintainer, but it would be great if you could make a PR.

[niwinz]

@EricVM +1 to the PR

[sovelten]

There are two tests that break when we try to infer from p/Contextual before using the dynamic context. Both of them are related to overriding the default context of p/Contextual with a dynamic context.

So, some of the possibilities are:

• Leave unchanged and never use nested contexts inside a dynamic context
• Prefer contexts inferred from p/Contextual and lose the ability to override default context

I find neither of these possibilities are good enough. I tried to circumvent this by defining a macro with-context-override when you need to override the context from p/Contextual #203