Merge pull request #411 from TheGrizzlyDev/string-reverse

Make String reverse and reverse! spec compliant

include/natalie/string_object.hpp

@@ -104,6 +104,12 @@ class StringObject : public Object {
 
     StringObject *inspect(Env *);
 
+    StringObject &operator=(StringObject other) {
+        this->m_string = other.m_string;
+        this->m_encoding = other.m_encoding;
+        return *this;
+    }
+
     bool operator==(const Object &value) const {
         if (!value.is_string())
             return false;
@@ -163,6 +169,7 @@ class StringObject : public Object {
     Value ord(Env *);
     Value ref(Env *, Value);
     Value reverse(Env *);
+    Value reverse_in_place(Env *);
     Value rstrip(Env *) const;
     Value rstrip_in_place(Env *);
     Value size(Env *);

lib/natalie/compiler/binding_gen.rb

@@ -814,6 +814,7 @@ def generate_name
   gen.binding('String', 'match', 'StringObject', 'match', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
   gen.binding('String', 'ord', 'StringObject', 'ord', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
   gen.binding('String', 'reverse', 'StringObject', 'reverse', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
+  gen.binding('String', 'reverse!', 'StringObject', 'reverse_in_place', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
   gen.binding('String', 'rstrip', 'StringObject', 'rstrip', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
   gen.binding('String', 'rstrip!', 'StringObject', 'rstrip_in_place', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
   gen.binding('String', 'size', 'StringObject', 'size', argc: 0, pass_env: true, pass_block: false, return_type: :Object)

spec/core/string/reverse_spec.rb

@@ -0,0 +1,65 @@
+# encoding: utf-8
+
+require_relative '../../spec_helper'
+require_relative 'fixtures/classes'
+
+describe "String#reverse" do
+  it "returns a new string with the characters of self in reverse order" do
+    "stressed".reverse.should == "desserts"
+    "m".reverse.should == "m"
+    "".reverse.should == ""
+  end
+
+  ruby_version_is '3.0' do
+    it "returns String instances when called on a subclass" do
+      StringSpecs::MyString.new("stressed").reverse.should be_an_instance_of(String)
+      StringSpecs::MyString.new("m").reverse.should be_an_instance_of(String)
+      StringSpecs::MyString.new("").reverse.should be_an_instance_of(String)
+    end
+  end
+
+  ruby_version_is ''...'3.0' do
+    it "returns subclass instances when called on a subclass" do
+      StringSpecs::MyString.new("stressed").reverse.should be_an_instance_of(StringSpecs::MyString)
+      StringSpecs::MyString.new("m").reverse.should be_an_instance_of(StringSpecs::MyString)
+      StringSpecs::MyString.new("").reverse.should be_an_instance_of(StringSpecs::MyString)
+    end
+  end
+
+  ruby_version_is ''...'2.7' do
+    it "taints the result if self is tainted" do
+      "".taint.reverse.should.tainted?
+      "m".taint.reverse.should.tainted?
+    end
+  end
+
+  it "reverses a string with multi byte characters" do
+    "微軟正黑體".reverse.should == "體黑正軟微"
+  end
+end
+
+describe "String#reverse!" do
+  it "reverses self in place and always returns self" do
+    a = "stressed"
+    a.reverse!.should equal(a)
+    a.should == "desserts"
+
+    "".reverse!.should == ""
+  end
+
+  it "raises a FrozenError on a frozen instance that is modified" do
+    -> { "anna".freeze.reverse!  }.should raise_error(FrozenError)
+    -> { "hello".freeze.reverse! }.should raise_error(FrozenError)
+  end
+
+  # see [ruby-core:23666]
+  it "raises a FrozenError on a frozen instance that would not be modified" do
+    -> { "".freeze.reverse! }.should raise_error(FrozenError)
+  end
+
+  it "reverses a string with multi byte characters" do
+    str = "微軟正黑體"
+    str.reverse!
+    str.should == "體黑正軟微"
+  end
+end

src/string_object.cpp

@@ -869,6 +869,12 @@ Value StringObject::reverse(Env *env) {
     return ary->join(env, nullptr);
 }
 
+Value StringObject::reverse_in_place(Env *env) {
+    this->assert_not_frozen(env);
+    *this = *reverse(env)->as_string();
+    return this;
+}
+
 void StringObject::prepend_char(Env *env, char c) {
     m_string.prepend_char(c);
 }