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

Add URI singleton methods #529

Merged
merged 1 commit into from
Dec 19, 2020
Merged

Add URI singleton methods #529

merged 1 commit into from
Dec 19, 2020

Conversation

ybiquitous
Copy link
Contributor

This change adds signatures of the URI module singleton methods (and constants).
This also adds URI:RFC* modules and classes to pass the test but their content are empty still.

I refer to the following resources:

@ybiquitous
Copy link
Contributor Author

Oh, I forget to add the Kernel::URI method. I'll add it soon.

@soutaro soutaro self-assigned this Dec 18, 2020
@soutaro soutaro added this to the Ruby 3.0 milestone Dec 18, 2020
@ybiquitous
Copy link
Contributor Author

Done. (squashed)

#
# See URI.decode_www_form_component, URI.encode_www_form.
#
def self.decode_www_form: (String str, ?encoding enc, ?isindex: bool, ?use__charset_: bool, ?separator: String) -> Array[[ String, String ]]
Copy link
Member

Choose a reason for hiding this comment

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

It seems like isindex and use__charset_ accepts any value, not only true and false.
boolish would be better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Surely, boolish is more flexible. I'll fix it soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed via 708bbf4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've rebased master and squashed to one commit.

Copy link
Member

@soutaro soutaro left a comment

Choose a reason for hiding this comment

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

boolish looks better to me.

@ybiquitous
Copy link
Contributor Author

By the way, does adding a Goodcheck rule below make sense? 👇

  - id: prefer_boolish_over_bool
    pattern:
      - regexp: '\([^(]*\bbool\b[^\n]*\)'
      - token: "-> bool }"
    message: |
      Prefer `boolish` over `bool` for method arguments and block return values

      See the doc below:
      https://github.com/ruby/rbs/blob/78d04a2db0f1c4925d2b13c2939868edf9465d6c/docs/syntax.md#bool-or-boolish
    glob:
      - "**/*.rbs"
    justification:
      - When the method arguments or block return values require `true` or `false` strictly.
    pass:
      - "(arg: boolish)"
      - "{ () -> boolish }"
    fail:
      - "(arg: bool)"
      - "{ () -> bool }"

According to https://github.com/ruby/rbs/blob/78d04a2db0f1c4925d2b13c2939868edf9465d6c/docs/syntax.md#bool-or-boolish.

E.g.

$ be goodcheck check -R prefer_boolish_over_bool core
core/complex.rbs:168:14: Prefer `boolish` over `bool` for method arguments and block return values
  def clone: (?freeze: bool) -> self
             ^~~~~~~~~~~~~~~
core/exception.rbs:193:21: Prefer `boolish` over `bool` for method arguments and block return values
  def full_message: (?highlight: bool, ?order: :top | :bottom) -> String
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
core/float.rbs:174:14: Prefer `boolish` over `bool` for method arguments and block return values
  def clone: (?freeze: bool) -> self
             ^~~~~~~~~~~~~~~

@ybiquitous
Copy link
Contributor Author

I've open PR #531 to add a Goodcheck rule as a test, but please close the PR if you don't want.

Copy link
Member

@soutaro soutaro left a comment

Choose a reason for hiding this comment

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

Thanks!

@soutaro soutaro merged commit a6f313d into ruby:master Dec 19, 2020
@ybiquitous ybiquitous deleted the add-uri-singleton-methods branch December 20, 2020 02:18
# 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