piotrmurach/tty-option

`NoMethodError` with `convert :pathname_list`

Closed this issue · 6 comments

Quintasan commented

Describe the problem

Using convert :pathname_list results in undefined method 'strip' for ["vendor/spdx.json", "spec/frontend/editor/schema/ci/json_tests/positive_tests/retry.json"]:Array (NoMethodError)

Steps to reproduce the problem

module Jsonlinter
  class Cli
    include TTY::Option

    usage do
      program "jsonlinter"

      command "run"
    end

    argument :path do
      name "path"
      arity one_or_more
      convert :pathname_list
      desc "Paths (or files) to scan"
    end

    flag :help do
      short "-h"
      long "--help"
      desc "Print usage"
    end

    flag :silent do
      long "--silent"
      desc "Do not display any output"
    end

    def run
      if params[:help]
        print help
      elsif params.errors.any?
        puts params.errors.summary
      else
        pp params.to_h
      end
    end
  end
end

Actual behaviour

$ ruby -I/home/quintasan/source/jsonlinter/lib /home/quintasan/source/jsonlinter/exe/jsonlinter vendor/spdx.json spec/frontend/editor/schema/ci/json_tests/positive_tests/retry.json

/home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/conversions.rb:95:in `block (2 levels) in <module:Conversions>': undefined method `strip' for ["vendor/spdx.json", "spec/frontend/editor/schema/ci/json_tests/positive_tests/retry.json"]:Array (NoMethodError)

           .map { |v| v.strip.gsub(/\\\\,/, ",") }
                       ^^^^^^
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/conversions.rb:95:in `map'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/conversions.rb:95:in `block in <module:Conversions>'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/conversions.rb:118:in `block (3 levels) in <module:Conversions>'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/param_conversion.rb:22:in `call'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/pipeline.rb:29:in `block in call'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/pipeline.rb:28:in `each'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/pipeline.rb:28:in `call'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/parser/arguments.rb:161:in `assign_argument'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/parser/arguments.rb:62:in `block in parse'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/parser/arguments.rb:58:in `each'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/parser/arguments.rb:58:in `parse'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/parser.rb:50:in `block in parse'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/parser.rb:45:in `each'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option/parser.rb:45:in `parse'
        from /home/quintasan/.local/share/rtx/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/tty-option-0.3.0/lib/tty/option.rb:54:in `parse'
        from /home/quintasan/source/jsonlinter/exe/jsonlinter:38:in `<main>'

Expected behaviour

I expected it to work with multiple paths

Describe your environment

  • OS version: Ubuntu 22.04
  • Ruby version: 3.1.4
  • TTY::Option version: 0.3.0
Quintasan commented

It does work properly if I pass one path to it:

ruby -I/home/quintasan/source/jsonlinter/lib /home/quintasan/source/jsonlinter/exe/jsonlinter vendor/spdx.json                                                                    
{:help=>false, :silent=>false, :path=>[#<Pathname:vendor/spdx.json>]}
piotrmurach commented

Hi Michał 👋

Thanks for using the tty-option gem.

I don't believe this to be a bug but a misunderstanding on how to use the library.

For your example to work you need to call the 2.6 parse method first.

For instance, passing the following paths to parse converts the paths as expected:

linter = Jsonlinter::Cli.new
linter.parse(%w[vendor/spdx.json spec/frontend/editor/schema/ci/json_tests/positive_tests/retry.json])
linter.run
# =>
# {:help=>false, :silent=>false, :path=>[#<Pathname:vendor/spdx.json>, #<Pathname:spec/frontend/editor/schema/ci/json_tests/positive_tests/retry.json>]}

Alternatively, add the parse method call to your run method:

def run
  parse
  if params[:help]
  ...
end

Then running:

ruby jsonlinter vendor/spdx.json spec/frontend/editor/schema/ci/json_tests/positive_tests/retry.json

Gives the same result.

Quintasan commented

@piotrmurach Fascinating but this is almost exactly the same way I run it:

cmd = Jsonlinter::Cli.new
cmd.parse
cmd.run

See https://gitlab.com/Quintasan/jsonlinter/-/blob/master/exe/jsonlinter?ref_type=heads

piotrmurach commented

I had a quick play with your code.

It seems that when you comment out the require_realtive "runner" and the Jsonlinter::Runner.call(params) from the lib/jsonlinter/cli.rb and simply output puts params.to_h it works fine:

bundle exec exe/jsonlinter vendor/spdx.json spec/frontend/editor/schema/ci/json_tests/positive_tests/retry.json
# =>
# {:autocorrect=>false,
# :help=>false,
# :silent=>false,
# :paths=>[#<Pathname:vendor/spdx.json>, #<Pathname:spec/frontend/editor/schema/ci/json_tests/positive_tests/retry.json>]}

I wonder what is happening inside the runner file to 'override' this behaviour?

Quintasan commented

@piotrmurach fair enough, that seems to squarely put the problem on my side. I don't have access to a computer ATM so I'll play around with it to tomorrow.

piotrmurach commented

As I was already playing with your gem I decided to dig into why this is happening.

The culprit is the activesupport dependency.

In particular, the inclusion of require "active_support/core_ext/array/grouping" in the lib/jsonlinter/discriminator.rb is the real issue. It overrides the Array#split core method. The split is used internally by tty-option to convert any list arguments whether they are pathname or other objects.

I consider activesupport use in gems an antipattern and red flag. Once you include it and override core ruby classes then all bets are off. I'd strongly encourage you to stop using this dependency as you may have a hard time finding bugs in the future. I don't believe there is a safe way to use it and any benefits gained may not outweigh the obvious downsides.

I'd recommend also before submitting issues to any project, isolate the problem and run a minimum case like I did in this issue thread without any other dependencies. This way you will quickly know whether the problem is with your code or external library. Open-source developer's time is limited and could be spent on resolving actual issues and implementing new features. Please don't take it as a reprimand, as I don't believe you meant to waste my time, but as a perspective to carry with you into the future.