String Enums concatenate values when repeatable

[mslinn]

This demo program lives in a project with Thor v1.2.2 as a dependency:

require 'thor'

class Cli < Thor
  desc 'test NAME', 'I do not exist therefore I am confused.'

  method_option :type, type: :string, default: 'tag', enum: %w[tag block generator], 
    repeatable: true, desc: 'Specifies the types of plugin.'

  def test
    puts options[:type]
  end
end

Cli.start

When called this way:

$ ruby lib/test.rb test --type block

Then options has value {"type"=>"tagblock"}, which is an error.

However, if repeatable: false (or is not specified), then options has value {"type"=>"block"}, as it should.

[rafaelfranca]

This is indeed a bug. Mind to send a PR?

[mslinn]

I will leave that to others

[jmpage]

The given case does print the following deprecation warning on thor v1.2.2:

Deprecation warning: Expected array default value for '--type'; got "tag" (string).
This will be rejected in the future unless you explicitly pass the options `check_default_type: false` or call `allow_incompatible_default_type!` in your code
You can silence deprecations warning by setting the environment variable THOR_SILENCE_DEPRECATION.

With check_default_type: true set on the options, the validity of the default option is properly evaluated, resulting in the following error and a non-zero exit code:

Expected array default value for '--type'; got "tag" (string) (ArgumentError)

Given the presence of the deprecation warning, it appears that the usage of repeatable is not intended to be used with a default that is anything other than an Array or Hash. Is this worth a patch given that this will presumably be validated in the future?

[mslinn]

This sounds like you are asking if Strings should be able to be validated against an array of allowable values. I think it is obvious that this ability is valuable. Apparently, YMMV.

[jmpage]

No, I don't think that that's a fair assessment of what I wrote, especially given my last paragraph:

it appears that the usage of repeatable is not intended to be used with a default that is anything other than an Array or Hash

To elaborate:

1. The input option is being validated as a member of the enum. This can be verified by providing a value outside the enum and observing an error such as: Expected '--type' to be one of tag, block, generator; got not-in-enum
2. The bug is not with enum validation itself (it's clearly doing this, see the previous point) but that it's erroneously concatenating the input to the option's default because it expects that default to be an array (see the deprecation warning from my previous comment)

[mslinn]

This conclusion may be correct, but it is not helpful. Strings need to be constrained to a list of acceptable values also.

On 2023-06-12 9:45 p.m., Jennifer Page wrote:

it appears that the usage of repeatable is not intended to be used with a default that is anything other than an Array or Hash

[rafaelfranca]

Thank you for the investigation @jmpage.

@jmpage is correct. The option definition should be:

 method_option :type, type: :string, default: ['tag'], enum: %w[tag block generator], 
    repeatable: true, desc: 'Specifies the types of plugin.'

Not

 method_option :type, type: :string, default: 'tag', enum: %w[tag block generator], 
    repeatable: true, desc: 'Specifies the types of plugin.'