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

Update String's definitions #1641

Merged
merged 6 commits into from
Dec 11, 2023
Merged

Conversation

sampersand
Copy link
Contributor

@sampersand sampersand commented Nov 23, 2023

IMPORTANT: The gsub! function doesn't have tests (but has been tested manually) because of a bug in the test harness

Test harness updates:

  • TypeAssertions::RUBY_EXE added RUBY_EXE, which is the path to the ruby interpreter—Ruby's the executable guaranteed to be installed on any system we're testing (eg for Kernel#system)
  • TypeAssertions#pass: Added pass, which is used in a few functions to indicate a test passes without using the type-checking assertions directly.

String updates (bolds are significant changes imo):

  • String#{*,+,==,byteindex,center,concat,include?,{r,}index,{l,r}just,match{,?},{r,}partition,scan,sum,to_i,upto}: Updated variable names

  • String#{capitalize{,!},chop!}: Added a TODO to add a variant for when arbitrary constraints come out.

  • String#{bytes,chars,codepoints}: Returns self in the block variant

  • String#{capitalize,downcase}!: All variants now return self?

  • String#{grapheme_clusters,lines}: Added the block variant

  • String#{clear,insert,prepend,replace,succ}: Returns self now

  • String::selector: an alias for string used to indicate methods take a string whose contents have a special meaning

  • String#{count,{delete,squeeze,tr{,_s}}{,!}}: Now uses String::selector, updated variable names

  • String.try_convert: Added (String) -> String | (_ToStr) -> String | (untyped) -> nil

  • String#initialize{,_copy}: Moved to the top of the class; initialize's return type was updated.

  • String#%: The variants can be implicit

  • String#{+,-}@: Return self | instance now

  • String#<<: Separated variants out for the variable names

  • String#<=>: Annotated the previously untyped variant, now untyped returns nil.

  • String#===: Now an alias for String#==

  • String#=~: Added the variant for when the other is not a Regexp but defines =~

  • String#[]{,=}: Collapsed variants, added implicits, the regexp variants now uses MatchData::capture

  • String#byteslice: Collapsed Range variants, and made implicit

  • String#bytesplice: Added updates from Ruby 3.3

  • String#casecmp{,?}: Split out string vs untyped variants; string variant in casecmp variant now returns (-1 | 0 | 1)?

  • String#chomp: Separator can now be nil

  • String#chomp!: Now returns self?, and nil separator always returns nil.

  • String#delete_{prefix,suffix}!: Now returns self? not String?

  • String#each_{byte,char,codepoint,grapheme_cluster,line}: Enumerator variant is now first, to be consistent with other core lib definitions.

  • String#encode{,!}: All keyword args are now exhaustively annotated. The technically correct **untyped is not supplied.

  • String::_EncodeFallbackAref: Added for use with String#encode{,!}

  • String#force_encoding: Now uses encoding

  • String#gsub{,!}: The Hash variant is now implicit, with a _ToS value; Enumerator is now above the block variant; !! gsub! isn't testable because of a bug in the test harness !!

  • String#next{,!}: Now aliases for String#succ{,!}

  • String#scrub: Added ?nil to the block variant

  • String#setbyte: Now returns second variable, which is now _ToInt.

  • String#slice!: Updated variable names, uses MatchData:capture, added implicits.

  • String#split: Added nil value for pattern

  • String#sub{,!}: The Hash variant is now implicit, with a _ToS value; self is now returned in the block form

  • String#to_s: Now returns self | String

  • String#to_str: An alias for String#to_s

  • String#to_sym: An alias for String#intern

  • String#unicode_normalize: Can now return self, named the variable

  • String#unicode_normalize!: Always returns self, named the variable

  • String#unpack: Implcits, and added the block variant

  • String#unpack1: Implcits, added ?offset: int parameter

@sampersand
Copy link
Contributor Author

sampersand commented Nov 23, 2023

<todo, clean this up, collapse commits down>
done

core/match_data.rbs Outdated Show resolved Hide resolved
@sampersand sampersand changed the title swesterman/23 11 18/string Update String's definitions Nov 24, 2023
@sampersand sampersand force-pushed the swesterman/23-11-18/string branch 10 times, most recently from f695dfc to c054b20 Compare November 24, 2023 06:04
@sampersand
Copy link
Contributor Author

This PR is waiting for #1643 to be merged

@sampersand sampersand force-pushed the swesterman/23-11-18/string branch 5 times, most recently from 9546cc1 to 9e40388 Compare November 28, 2023 22:12
@sampersand sampersand marked this pull request as ready for review November 28, 2023 22:13
@sampersand sampersand force-pushed the swesterman/23-11-18/string branch 6 times, most recently from 21f7bf8 to 3779c4c Compare November 29, 2023 00:19
core/string.rbs Outdated
@@ -765,7 +765,7 @@ class String
#
# Otherwise returns `self.dup`, which is not frozen.
#
def +@: () -> (self | instance)
Copy link
Member

Choose a reason for hiding this comment

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

self | instance type doesn't make much sense.
It will be a String instance or it's subclass, so self is good here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean? instance

@@ -798,8 +798,7 @@ class String
#
# Related: String#concat, which takes multiple arguments.
#
def <<: (Integer codepoint) -> self
| (string other_string) -> self
def <<: (string | Integer str_or_codepoint) -> self
Copy link
Member

Choose a reason for hiding this comment

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

Combine the two overloads for the case passing a value of String | Integer.
(This can be improved with future version of Steep, but currently it doesn't work for this.)

@@ -822,19 +821,9 @@ class String
# 'foo' <=> 'FOO' # => 1
# 'foo' <=> 1 # => nil
#
def <=>: (string | _Spaceship[self] other) -> (-1 | 0 | 1)?
def <=>: (string) -> (-1 | 0 | 1)
Copy link
Member

Choose a reason for hiding this comment

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

As we discussed at Discord?

@@ -1361,8 +1348,8 @@ class String
# Like String#chomp, but modifies `self` in place; returns `nil` if no
# modification made, `self` otherwise.
#
def chomp!: (?string separator) -> self?
| (nil) -> nil
def chomp!: (nil) -> nil
Copy link
Member

Choose a reason for hiding this comment

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

For the case we pass String?.

@@ -3436,8 +3415,7 @@ class String
#
# Related: String#unicode_normalize!, String#unicode_normalized?.
#
def unicode_normalize: (?:nfc | :nfd | :nfkc | :nfkd form) -> (self | String)
# NOTE: ^ technically can return `self` if `self.encoding` is ascii.
def unicode_normalize: (?:nfc | :nfd | :nfkc | :nfkd form) -> self
Copy link
Member

Choose a reason for hiding this comment

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

There may be a case that returns exact String, not self?

@soutaro
Copy link
Member

soutaro commented Dec 6, 2023

Also replaced self types in tests with String.

@soutaro soutaro added this to the RBS 3.4 milestone Dec 11, 2023
@soutaro soutaro added this pull request to the merge queue Dec 11, 2023
Merged via the queue into ruby:master with commit a4681af Dec 11, 2023
24 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Dec 13, 2023
A tiny followup for #1641’s oopsies
soutaro added a commit that referenced this pull request Dec 20, 2023
…string"

This reverts commit a4681af, reversing
changes made to 1a50819.
@soutaro soutaro added the Released PRs already included in the released version label Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Released PRs already included in the released version
Development

Successfully merging this pull request may close these issues.

3 participants