Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make Kernel methods to be available as module functions #549

Conversation

pocke
Copy link
Member

@pocke pocke commented Dec 26, 2020

This pull request makes Kernel methods to be available as module functions.

Problem

Most Kernel methods are available as module functions, but currently any Kernel methods aren't.

For example, Kernel.puts "Hello" is a valid Ruby code but Steep complain it.
Because Kernel.puts is typed as a private method accidentally.

Solution

Most methods are module functions, but it is not all methods. So I need to extract the target methods in the beginning.

I extracted the target methods with the following code on IRB.

Kernel.methods.map { Kernel.method(_1) }.select { _1.owner == Kernel.singleton_class }.map(&:name)
# => [:pp, :Pathname, :local_variables, :rand, :srand, :sprintf, :format, :Integer, :String, :Array, :Hash, :warn, :trap, :test, :require, :raise, :fail, :global_variables, :__method__, :__callee__, :__dir__, :require_relative, :autoload, :autoload?, :system, :eval, :iterator?, :block_given?, :catch, :throw, :loop, :exec, :binding, :gets, :abort, :proc, :lambda, :spawn, :Rational, :caller, :caller_locations, :set_trace_func, :exit!, :trace_var, :untrace_var, :at_exit, :Complex, :fork, :URI, :select, :load, :exit, :`, :syscall, :open, :printf, :print, :putc, :puts, :readline, :readlines, :p, :Float, :sleep]

And I changed the methods kinds to self? for the above methods.

Note

I found some missing methods while I updated the RBS.
The following methods are available as Kernel's singleton methods, but RBS doesn't have any definitions.

  • trap
  • spawn
  • set_trace_func
  • trace_var
  • untrace_var

So we need to add them to kernel.rbs as module functions, but this pull request doesn't add them because of out of scope.
I'll work on them with another pull request.

--

I noticed this problem with this bloc post. Thanks! https://narazaka.hatenablog.jp/entry/2020/12/26/Ruby_3.0%E3%81%AE%E9%9D%99%E7%9A%84%E5%9E%8B%E5%AE%9A%E7%BE%A9%E3%82%92TypeScript%E3%81%BF%E3%81%9F%E3%81%84%E3%81%AB%E3%83%A9%E3%82%A4%E3%83%96%E3%83%A9%E3%83%AA%E3%81%AB%E6%9B%B8%E3%81%84%E3%81%A6

@pocke
Copy link
Member Author

pocke commented Dec 26, 2020

Well, this change doesn't affect the accessibility of Kernel.puts singleton method.

The accessibility is still private with this patch.

$ exe/rbs method --singleton Kernel puts
::Kernel.puts
  defined_in: ::Kernel
  implementation: ::Kernel
  accessibility: private
  types:
      (*untyped arg0) -> ::NilClass

I guess the cause is a mismatch of Ruby's module_function and RBS's self?.

In Ruby, module_function works as the following:

  • Make the instance method private
  • Create an alias of the instance method as singleton method, but it is public.

But in RBS, I guess self? syntax doesn't affect accessibility. Kernel module has private member on the top of the module definition, so both of instance and singleton methods are private.


So, I think we have two solutions to fix this problem.

First, we can define private instance methods and public singleton methods by hand.
For example:

# kernel.rbs

module Kernel : BasicObject
  private
  def puts: (*untyped) -> nil
  public
  def self.puts: (*untyped) -> nil

  # ...
end

It works, but I think it is not efficient.

Second, we can change the semantics of self? to make it the same with Ruby's module_function.
I guess we can write Kernel's definition and other module_function efficiently with this change.
But it may introduce an incompatibility, and it means self? syntax ignores public and private syntax. Maybe it introduce confusion about accessibility.

module M
  private
  def self?.foo: () -> untyped
end

# M.foo is public or private???

@pocke pocke force-pushed the Make_Kernel_methods_to_be_available_as_module_functions branch from 07e9da3 to 515475e Compare December 30, 2020 10:06
| [T] (::Array[T] x) -> ::Array[T]
| [T] (::Range[T] x) -> ::Array[T]
| [K, V] (::Hash[K, V] x) -> ::Array[[K, V]]
| [T] (T x) -> ::Array[T]

def Complex: (Numeric | String x, ?Numeric | String y, ?exception: bool exception) -> Complex
def self?.Complex: (Numeric | String x, ?Numeric | String y, ?exception: bool exception) -> Complex
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Goodcheck] Prefer boolish over bool for method arguments and block return values (view)

See the doc below:
https://github.com/ruby/rbs/blob/78d04a2db0f1c4925d2b13c2939868edf9465d6c/docs/syntax.md#bool-or-boolish

Rule
rbs.prefer_boolish

You can close this issue if no need to fix it. Learn more.

👍 true of false is required here.

| (String arg, ?Integer base, ?exception: bool exception) -> Integer

def Rational: (Numeric | String | Object x, ?Numeric | String y, ?exception: bool exception) -> Rational
def self?.Rational: (Numeric | String | Object x, ?Numeric | String y, ?exception: bool exception) -> Rational
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Goodcheck] Prefer boolish over bool for method arguments and block return values (view)

See the doc below:
https://github.com/ruby/rbs/blob/78d04a2db0f1c4925d2b13c2939868edf9465d6c/docs/syntax.md#bool-or-boolish

Rule
rbs.prefer_boolish

You can close this issue if no need to fix it. Learn more.

I closed this issue, because:

  • When you strictly want true | false.

@soutaro soutaro force-pushed the Make_Kernel_methods_to_be_available_as_module_functions branch from 515475e to 6ae39a8 Compare January 16, 2021 07:25
core/kernel.rbs Outdated
@@ -54,7 +54,7 @@ module Kernel : BasicObject
# try { "hello" } #=> "hello"
# try do "hello" end #=> "hello"
# ```
def block_given?: () -> bool
def self?.block_given?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved the conflict caused by deletion of iterator? #569

@soutaro soutaro enabled auto-merge January 16, 2021 07:26
@soutaro soutaro force-pushed the Make_Kernel_methods_to_be_available_as_module_functions branch from 6ae39a8 to 6aec641 Compare January 16, 2021 07:28
@soutaro soutaro disabled auto-merge January 16, 2021 07:32
@soutaro soutaro merged commit f8f6f19 into ruby:master Jan 16, 2021
@pocke pocke deleted the Make_Kernel_methods_to_be_available_as_module_functions branch January 16, 2021 08:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants