Improve type of Kernel#system

[pocke]

This pull request improves types of Kernel#system and so on.

TODO

• [ ] Reduce duplication by the environment argument (see below)
• [ ] Reduce duplication by the redirect argument (see below)
• [x] Support system(['echo', 'display-name'], 'this', 'is', 'arg') style
• [ ] Apply the type to other method, e.g. Kernel#spawn.
• [ ] Fix test with type alias (rake test is failing with type alias)

Reduce duplication by the environment argument

I uses overload as a workaround to write the env argument.

def system: (*String args) -> ...
  | (Hash[String, String] env, *String args) -> ...

Because optional argument and rest argument do not work well if both are used. If I write them, the testing will fail.

I have the same problem for system(['echo', 'display-name'], 'this', 'is', 'arg') style.

I think I can reduce the overload by fixing Ruby::Signature::Test::Hook.

Reduce duplication by the redirect

I also uses overload as a workaround for redirect argument.

def system: (*String args) -> ...
  | (*String args, ?redirect) -> ...

Because optional argument is not allowed after rest argument. It is the same as Ruby, but I guess we can allow optional argument after rest argument.

I think I can reduce the overload by fixing Ruby::Signature::Test::Hook also.

---

Maybe @soutaro is trying to fix the problems, so I wait your patch. But I can tackle to fix the problems, so if you have no much time, feel free to assign the work to me.

Thanks!

[soutaro]

I was confusing. I found I cannot simply fix this.

It's not a trivial problem, and should be careful what to do for this. I understand writing overloading would cause a difficulty which requires you to repeat writing argument types many times. But, I'm not sure how to improve yet.

[pocke]

Thank you for your comment, and sorry for my absence.

I've updated the pull request. It has redundant union types but works correctly. I also added type for exec method.

I think writing the types is too hard, so I wrote a script to generate them. I guess it also works well for Kernel.spawn, Open3, and so on in the future.

def indent(str, level)
  str.lines.map{|line| "#{' ' * (level * 2)}#{line}"}.join
end

def flag(name, val)
  "#{name}: #{val ? 'on' : 'off'}"
end

def gen_part(env:, redirect:, method:)
  kwargs = <<~SIG
    ?unsetenv_others: (TrueClass | FalseClass),
    ?pgroup: (TrueClass | Integer | NilClass),
    ?new_pgroup: (TrueClass | FalseClass),
    ?umask: Integer,
    ?close_others: (TrueClass | FalseClass),
    ?chdir: String,

    # See Process.setrlimit
    ?rlimit_as: (Integer | [Integer, Integer]),
    ?rlimit_core: (Integer | [Integer, Integer]),
    ?rlimit_cpu: (Integer | [Integer, Integer]),
    ?rlimit_data: (Integer | [Integer, Integer]),
    ?rlimit_fsize: (Integer | [Integer, Integer]),
    ?rlimit_memlock: (Integer | [Integer, Integer]),
    ?rlimit_msgqueue: (Integer | [Integer, Integer]),
    ?rlimit_nice: (Integer | [Integer, Integer]),
    ?rlimit_nofile: (Integer | [Integer, Integer]),
    ?rlimit_nproc: (Integer | [Integer, Integer]),
    ?rlimit_rss: (Integer | [Integer, Integer]),
    ?rlimit_rtprio: (Integer | [Integer, Integer]),
    ?rlimit_rttime: (Integer | [Integer, Integer]),
    ?rlimit_sbsize: (Integer | [Integer, Integer]),
    ?rlimit_sigpending: (Integer | [Integer, Integer]),
    ?rlimit_stack: (Integer | [Integer, Integer]),
  SIG

  kwargs_for_sytem = <<~SIG

    # exception is available only for system
    ?exception: (TrueClass | FalseClass),
  SIG

  res = +''
  res << indent("# #{flag :env, env}, #{flag :redirect, redirect}\n", 2)
  res << indent("Hash[String, String] env,\n", 2) if env
  res << indent("([String, String] | String) cmd,\n", 2)
  res << indent("*String args,\n", 2)
  res << indent("spawn_redirect_hash,\n", 2) if redirect
  res << indent(kwargs, 2)
  res << indent(kwargs_for_sytem, 2) if method == 'system'
  if method == 'exec'
    res << indent(") -> bot\n", 1)
  else
    res << indent(") -> (NilClass | FalseClass | TrueClass)\n", 1)
  end

  res
end

def gen(method_name)
  res = [true, false].repeated_permutation(2).map do |env, redirect|
    gen_part(env: env, redirect: redirect, method: method_name)
  end.join("| (\n")

  header = indent("def #{method_name}: (\n", 1)

  puts header + indent(res, (header.chomp.size / 2) - 1)
end


gen(ARGV.first || raise)