I first raised this as an issue over on Github, before I read the rules. Oops, sorry 
In short, the way Do wraps methods replaces whatever modifier they had with whatever the default is (probably public).
As you can see from this example, I can call a previously-private method directly, without resorting to send chicanery.
I did some digging, and have been able to fix this behaviour in Do::All. In that context, we can use Module.private_instance_methods & Module.protected_instance_methods to determine what visibility each method has, and explicitly provide that visibility to the wrapped methods.
Like this: (from all.rb)
# @api private
module InstanceMixin
# @api private
def extended(object)
super
wrapper = ::Module.new
object.singleton_class.prepend(wrapper)
object.define_singleton_method(:singleton_method_added) do |method|
super(method)
next if method.equal?(:singleton_method_added)
visibility = if private_method_defined?(method, true)
:private
elsif protected_method_defined?(method, true)
:protected
else
:public
end
Do.wrap_method(wrapper, method, visibility)
end
object.singleton_class.public_instance_methods(false).each do |m|
Do.wrap_method(wrapper, m, :public)
end
object.singleton_class.protected_instance_methods(false).each do |m|
Do.wrap_method(wrapper, m, :protected)
end
object.singleton_class.private_instance_methods(false).each do |m|
Do.wrap_method(wrapper, m, :private)
end
end
end
Do.for has an additional challenge: for it to work properly, you need to specify which methods to wrap ahead of time - and that means we donāt know what visibility they will have once theyāre actually provided.
The only way I can think of to resolve that dilemma is to extend the API for Do.for to accept both a list of methods (that will be treated as public, per current behaviour), and keywords for explicit visibility:
# do.rb - L86
def for(*public_methods, **methods_with_visibility)
mod = ::Module.new do
public_methods.each do |method_name|
Do.wrap_method(self, method_name, nil)
end
methods_with_visibility.each do |visibility, methods|
methods.each do |method_name|
Do.wrap_method(self, method_name, visibility)
end
end
end
::Module.new do
singleton_class.send(:define_method, :included) do |base|
base.prepend(mod)
end
end
end
Usage:
class Example
include Dry::Monads[:result]
include Dry::Monads::Do.for(:call, :internal_method) # [1] current behaviour
include Dry::Monads::Do.for(:call, private: [:internal_method]) # [2] proposed API
def call(value)
result = yield internal_method(value)
end
private
def internal_method(v)
i = yield interstitial_result.to_monad
Success(v + i)
end
def interstitial_result
Success(100)
end
end
e = Example.new
e.call(1) # => Success(101)
# when [1]:
e.internal_method # => Success(101)
# when [2]:
e.internal_method # => NoMethodError (private method `internal_method' called for #<Example:0xdeadbeef>
Thanks for investigating this!
I think the intention behind this API was that youād only decorate one public method. As you noticed, fixing it would be a challenge and I honestly donāt think itās worth the effort. OTOH it would probably be relatively simple to rely on method_added hook and decorate methods there (which would allow you to inspect visibility and act accordingly).
Iāll put what Iāve done up as a PR and you can decide whether itās worth a merge. If we decide not to try and fix Do.for, I think itās worth putting an explanation in the docs to highlight this surprising behaviour. WDYT?
All of these visibility problems should be fixed, Iāll take a closer look soon. Personally, I donāt pay attention to visibility since most of my classes have only one āpublicā, namely call.
for fixing this with Do::All
While I usually only have one public call method, I use do notation a lot to clean up private methods too, and I donāt want that to make them public.