Contrary to the doc comment, List#map might not return an array.

[isikyus]

Describe the bug

https://github.com/dry-rb/dry-monads/blob/master/lib/dry/monads/list.rb#L139 states:

Note that this method returns an Array instance, not a List

However, this isn't true; when called with a block, List#map will return a List.
It also conflicts with the @return [List,Enumerator] annotation.

I expected the doc comment would be accurate and consistent.
Either the comment needs updating, or the method is working incorrectly.

To Reproduce

require 'dry/monads'
require 'dry/monads/list'

list = Dry::Monads::List['a', 'b', 'c']
mapped = list.map { |char| char.upcase }

pp mapped

Output:

$ ruby list-test.rb 
List["A", "B", "C"]

Expected behavior

I expected the output to be an array, like ["A", "B", "C"]

Your environment

• Affects my production application: NO
• Ruby version: ruby 2.6.3p62 (2019-04-16 revision 67580) [x86_64-linux]
• OS: Linux, Ubuntu 14.04.6 LTS

[flash-gordon]

Nice catch. It's puzzling, I'll take a deeper look later to find out why it states so.

[niksosf]

Here's a bit puzzling in line 145

dry-monads/lib/dry/monads/list.rb

Line 145 in ceb5cae

else

in the else part of the statement, the array value is called Array#map on with the block but the if part is testing whether block exists.

def map(&block)
        if block
          fmap(block)
        else
          value.map(&block)
        end
      end

Is List#map, where the output is just ruby Array, intended to be distinct from fmap where the semigroup type is preserved

[flash-gordon]

haven't looked into code yet but I'm pretty sure, yes, it's intentional. Think about it as two different methods

[niksosf]

Here's the part that I don't quite get. If there's no block given, then it code runs to the else part which in turn calls the block which is non-existent. Maybe it was meant to be something else in the else? The signature in the code for the return value [List, Enumerator] is correct though, so there must have been deliberation over this.

[isikyus]

If there's no block given, then it code runs to the else part which in turn calls the block which is non-existent.

Looking at it again now, I think that's correct. When block is missing, value.map(&:block) is equivalent to value.map(&nil) — i.e. it returns an enumerator:

2.3.0 :003 > [].map(&nil)
 => #<Enumerator: []:map> 

This matches the return type annotation and test in the commit that added this (464b7ad).
In light of this, should the second line of the doc comment say something like this:

If called without a block, this method returns an Enumerator instance, not a List

[solnic]

@niksosf thanks for tracking this down, I believe you're correct, especially that this is what @flash-gordon wrote in the changelog