Include Dry::Monads[...] doesn't give access to class constants

[cllns]

Describe the bug

When using include Dry::Monads[:result], you can access the constructors Success(...) and Failure(...). But, you don't get the Success and Failure class constants themselves.

In a spec if you do expect(result).to be_a(Success), you get an uninitialized constant Success error. This is confusing because you do have access to the constructor which creates a Success, but not the class constant itself. Checking the type of a class can be considered a violation of duck typing but it's also a practical way to make sure you get the correct result type, especially in specs.

This happens with both include and extend.

There are a couple of workarounds:

• can still access them by their full name to get them, e.g. Dry::Monads::Result::Success.
• You can use expect(result).to be_success

For context, this came up here: https://discourse.hanamirb.org/t/trouble-testing-the-result-of-an-operation/1121

A benefit of including the entire class constants, is that it provides more flexibility in terms of creating result type, e.g. you can use Success[...] and Success.new(...), instead of just Success(...). This would have less encapsulation, but I believe the trade-off is worth it for improved developer experience.

Conversion vs constructor convention

As an aside, the conventions for String(...), Int(...), and Array(...) are as conversion methods which return the args if they're already the right type, rather than constructing a new object as a wrapper or copy. By using Sucess(...) and Failure(...) we're changing that convention to create a new object to wrap the arg regardless of the contents. Using those other syntax options for creating a new Result object are more conventional. We can't change that behavior at this point IMO, but we could change to recommending using Success[...] in our docs if we wanted to, or even Success.new(...) which is the most conventional. Happy to make this into a separate issue for discussion if you'd like.

To Reproduce

require "bundler/inline"

gemfile do
  source "https://rubygems.org"
  gem "dry-monads"
  gem "rspec"
end

require "dry/monads"

class Greeter
  include Dry::Monads[:result]
  # extend Dry::Monads[:result] # Uncommenting this doesn't fix it

  def call
    Success("Hello")
  end
end

require "rspec/autorun"

RSpec.describe Greeter do
  include Dry::Monads[:result]

  it "greets" do
    result = subject.call
    expect(result).to be_a(Success) # This is the only failure, illustrating this problem
    expect(result).to be_success
    expect(result.value!).to eq("Hello")
    expect(result).to eq(Success("Hello"))
  end
end

Expected behavior

I would expect to get Success and Failure classes included when I do include Dry::Monads[:result], not just their constructor methods.

My environment

• Affects my production application: No
• Ruby version: 3.4

[flash-gordon]

Actually, it has nothing to do with dry-monads, it's a peculiarity of how rspec works. Specs are actually written in a global lexical scope, there's no encompassing name around. When you ref Success in specs, it's been searched in Object. I don't know if there's a way or trick to make it work other than define Success as global constant.

[bkuhlmann]

💡 A refinement can be a nice solution. So for Dry Monads, you could do something like this:

# Implemntation
module Dry
  module Monads
    module Cast
      refine Kernel do
        def Success(object) = Result::Success object

        def Failure(object) = Result::Failure object
      end
    end
  end
end

# Usage
using Dry::Monads::Cast

Success "Pass"
Failure "Danger!"

I explain the above in a bit more detail here but is one of the super powers of using a refinement like this.

[cllns]

Thanks @flash-gordon, I forgot about that gotcha with RSpec using blocks.

And thanks @bkuhlmann, I was wondering if refinements could be used for this so good to know they can!

I think this can work too, to add access to the constants only within RSpec examples:

# frozen_string_literal: true

require "dry/monads"

RSpec.configure do |config|
  config.include Dry::Monads[:result]

  config.before(:all) do
    Success = Dry::Monads::Success
    Failure = Dry::Monads::Failure
  end
end

[flash-gordon]

Defining constants in specs can be problematic if you forgot to add a mixin somewhere in your code. Specs pass but in prod you'll have a missing constant error. Better define it on startup and call it a day.

[flash-gordon]

@bkuhlmann refinements help with methods but they don't solve the constant problem. FWIW you can add methods globally in tests with config.include Dry::Monads[:result]. What we need is file-scoped const_missing, something that can be achieved with

Object.extend(Module.new {
  def const_missing(name)
    if name.equal?(:Success) && caller_locations(1, 1).first.path.end_with?('_spec.rb')
      Dry::Monads::Result::Success
    else
      super
    end
  end
})

I actually like this approach. I think we can have an rspec extension in dry monads that does this. It shouldn't be costly since it doesn't unwind the whole stack trace.

I'll test it in my projects first.

[bkuhlmann]

Nikita: True. Refinements don't solve the constant problem but do ensure the global namespace isn't polluted. Use of config.include Dry::Monads[:result] is quite nice -- as Sean mentioned too -- but Alex makes a great point where this can lead to false positives in testing (which has definitely tripped me up in the past) where specs pass but code fails in production because you required monads in test but not in the actual implementation.

Use of const_missing? is a neat idea and like that you'll get an error if Dry Monads isn't required too.

[flash-gordon]

BTW requiring dry/monads should be cheap because it's loaded lazily with zeitwerk.

[flash-gordon]

You can test the working version in main:
https://github.com/dry-rb/dry-monads/blob/e4339f8b44cf2405305e4b7a5a92c10cdce3b7e1/CHANGELOG.md#added

I'll give it some time, a week or so, before making a release. I may want to add some features to it

[cllns]

Nice! Works for me in my app :)

I think the _spec.rb suffix is a reasonable approach. RSpec allows that to be configurable, but in practice I have never seen anyone change it. I could imagine someone calling their specs with a _test.rb suffix for historical reasons, but I'm not sure that imagined scenario is worth supporting.

As a possible different way to approach it, that could avoid the need for gem "debug_inspector" when using in helpers, and support spec files with any name, have you considered looking in the caller_locations for the substring /lib/rspec/core/example.rb? I think that approach could be adapted to work for both, though possibly slightly slower performance to iterate through caller_locations, but only in cases that'll lead to an error anyway. WDYT?

[flash-gordon]

@cllns

but only in cases that'll lead to an error anyway

This is not correct because it'll be called on every reference in specs, I don't define constants in const_missing because there's no place for them, I can't put them to Object, it'll ruin the point. Frankly speaking, I don't expect it taking any significant time either way but there's another concern.

I was in a hurry and didn't explain clearly why debug_inspector is a superior approach. It's a case like this (copy from specs):

    context "inline class" do
      let(:operation) do
        Class.new {
          def call(value)
            Success[value]
          end
        }.new
      end

      # we use debug_inspector to check if the error comes from a non-spec context
      it "raises an error when it comes from a non-spec context" do
        expect { operation.(1) }.to raise_error(NameError)
      end
    end

It should raise an error despite being referenced from a _spec.rb. Simple string analysis won't do in such a case, so I used debug_inspector as a more reliable solution.

I think the _spec.rb suffix is a reasonable approach. RSpec allows that to be configurable, but in practice I have never seen anyone change it. I could imagine someone calling their specs with a _test.rb suffix for historical reasons, but I'm not sure that imagined scenario is worth supporting.

I wouldn't bother. If there's a user of both dry-monads and such a peculiar config they'll let us know :)

[flash-gordon]

Nice! Works for me in my app :)

Thanks for testing!