Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Move ENVClass definition under RBS::Unnamed #1041

Merged
merged 17 commits into from
Jun 22, 2022
Merged

Conversation

soutaro
Copy link
Member

@soutaro soutaro commented Jun 20, 2022

Some Ruby objects like ENV doesn't have corresponding class exposed to user scripts. However, defining such a class is not allowed in RBS.

We introduce RBS::Unnamed pseudo module to define classes of those objects. Note that there are no objects defined corresponding to the classes.

buzztaiki and others added 11 commits April 7, 2022 21:45
run the following and rename ARGF.class to ARGFClass

  bundle exec rbs prototype runtime ARGF.class > core/argf.rbs
bundle exec rbs annotate core/argf.rbs
run the following and rename ARGFClass to ARGF or ARGF.class:

  bundle exec rake generate:stdlib_test[ARGFClass]
Starting with ruby v3.0, Random::Base was created to facilitate algorithm changes,
and the inheritance relationship was changed.
ref: ruby/ruby#3024
@soutaro
Copy link
Member Author

soutaro commented Jun 20, 2022

I plan to define classes for ARGF (#975) and Random::Base (#977) under RBS::Unnamed.

Copy link
Collaborator

@ksss ksss left a comment

Choose a reason for hiding this comment

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

This is an idea to cut the namespace under RBS.
It is a very good idea for ARGF, ENV, etc.
But Random::Base exists as a real Ruby class, so we may need to consider about inheritance.

core/random.rbs Outdated
class Random < Object
include Random::Formatter
class Random < RBS::Unnamed::Random_Base
Base: singleton(RBS::Unnamed::Random_Base)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are there plans to make Random::Base inheritable?

class Sample < Random::Base
end

I know it is not a priority since there are few examples of its use, but it would be better if it were considered.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, good point.
Currently defining Random::Base as a class would be the solution?

class Random::Base < RBS::Unnamed::Random_Base
end

But, it's not straightforward...

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm... Cycle still exists with Random_Base and Random::Formatter.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because Random::Formatter is a module, not class, we cannot inherit a module.

Copy link
Member Author

@soutaro soutaro Jun 21, 2022

Choose a reason for hiding this comment

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

So, I would give up for this. We would take one of the two:

  1. Ignore Random::Base for now
  2. Define the module and class as a constant

Neither allows using the classes/module from RBS. (This should be a good motivating example for adding a feature to define alias of classes/modules.)

Copy link
Member Author

@soutaro soutaro Jun 22, 2022

Choose a reason for hiding this comment

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

We can include the Unnamed::Random_Formatter module. 💡

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see this as much of a problem in RBS, since superclasses can be omitted during open class.
Thanks for your consideration!

It cannot be defined directly in RBS. `Random` inherits `Random::Base`, and `Random::Base` is defined inside `Random`, which results in a circular dependency.

Extracting the class and `Random::Formatter` module to `RBS::Unnamed` solves the problem.

* `Random::Base` inherits `Random_Base` so that inheriting `Random::Base` works in RBS
* `Random::Formatter` includes `Random_Formatter` so that including `Random::Formatter` works in RBS
@soutaro soutaro merged commit 7669e92 into master Jun 22, 2022
@soutaro soutaro deleted the move-unnamed-classes branch June 22, 2022 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants