Make String#ljust spec-compliant

spec/core/string/ljust_spec.rb

@@ -0,0 +1,133 @@
+# -*- encoding: utf-8 -*-
+require_relative '../../spec_helper'
+require_relative 'fixtures/classes'
+
+describe "String#ljust with length, padding" do
+  it "returns a new string of specified length with self left justified and padded with padstr" do
+    "hello".ljust(20, '1234').should == "hello123412341234123"
+
+    "".ljust(1, "abcd").should == "a"
+    "".ljust(2, "abcd").should == "ab"
+    "".ljust(3, "abcd").should == "abc"
+    "".ljust(4, "abcd").should == "abcd"
+    "".ljust(6, "abcd").should == "abcdab"
+
+    "OK".ljust(3, "abcd").should == "OKa"
+    "OK".ljust(4, "abcd").should == "OKab"
+    "OK".ljust(6, "abcd").should == "OKabcd"
+    "OK".ljust(8, "abcd").should == "OKabcdab"
+  end
+
+  it "pads with whitespace if no padstr is given" do
+    "hello".ljust(20).should == "hello               "
+  end
+
+  it "returns self if it's longer than or as long as the specified length" do
+    "".ljust(0).should == ""
+    "".ljust(-1).should == ""
+    "hello".ljust(4).should == "hello"
+    "hello".ljust(-1).should == "hello"
+    "this".ljust(3).should == "this"
+    "radiology".ljust(8, '-').should == "radiology"
+  end
+
+  ruby_version_is ''...'2.7' do
+    it "taints result when self or padstr is tainted" do
+      "x".taint.ljust(4).should.tainted?
+      "x".taint.ljust(0).should.tainted?
+      "".taint.ljust(0).should.tainted?
+      "x".taint.ljust(4, "*").should.tainted?
+      "x".ljust(4, "*".taint).should.tainted?
+    end
+  end
+
+  it "tries to convert length to an integer using to_int" do
+    "^".ljust(3.8, "_^").should == "^_^"
+
+    obj = mock('3')
+    obj.should_receive(:to_int).and_return(3)
+
+    "o".ljust(obj, "_o").should == "o_o"
+  end
+
+  it "raises a TypeError when length can't be converted to an integer" do
+    -> { "hello".ljust("x")       }.should raise_error(TypeError)
+    -> { "hello".ljust("x", "y")  }.should raise_error(TypeError)
+    -> { "hello".ljust([])        }.should raise_error(TypeError)
+    -> { "hello".ljust(mock('x')) }.should raise_error(TypeError)
+  end
+
+  it "tries to convert padstr to a string using to_str" do
+    padstr = mock('123')
+    padstr.should_receive(:to_str).and_return("123")
+
+    "hello".ljust(10, padstr).should == "hello12312"
+  end
+
+  it "raises a TypeError when padstr can't be converted" do
+    -> { "hello".ljust(20, [])        }.should raise_error(TypeError)
+    -> { "hello".ljust(20, Object.new)}.should raise_error(TypeError)
+    -> { "hello".ljust(20, mock('x')) }.should raise_error(TypeError)
+  end
+
+  it "raises an ArgumentError when padstr is empty" do
+    -> { "hello".ljust(10, '') }.should raise_error(ArgumentError)
+  end
+
+  ruby_version_is ''...'3.0' do
+    it "returns subclass instances when called on subclasses" do
+      StringSpecs::MyString.new("").ljust(10).should be_an_instance_of(StringSpecs::MyString)
+      StringSpecs::MyString.new("foo").ljust(10).should be_an_instance_of(StringSpecs::MyString)
+      StringSpecs::MyString.new("foo").ljust(10, StringSpecs::MyString.new("x")).should be_an_instance_of(StringSpecs::MyString)
+
+      "".ljust(10, StringSpecs::MyString.new("x")).should be_an_instance_of(String)
+      "foo".ljust(10, StringSpecs::MyString.new("x")).should be_an_instance_of(String)
+    end
+  end
+
+  ruby_version_is '3.0' do
+    it "returns String instances when called on subclasses" do
+      StringSpecs::MyString.new("").ljust(10).should be_an_instance_of(String)
+      StringSpecs::MyString.new("foo").ljust(10).should be_an_instance_of(String)
+      StringSpecs::MyString.new("foo").ljust(10, StringSpecs::MyString.new("x")).should be_an_instance_of(String)
+
+      "".ljust(10, StringSpecs::MyString.new("x")).should be_an_instance_of(String)
+      "foo".ljust(10, StringSpecs::MyString.new("x")).should be_an_instance_of(String)
+    end
+  end
+
+  ruby_version_is ''...'2.7' do
+    it "when padding is tainted and self is untainted returns a tainted string if and only if length is longer than self" do
+      "hello".ljust(4, 'X'.taint).tainted?.should be_false
+      "hello".ljust(5, 'X'.taint).tainted?.should be_false
+      "hello".ljust(6, 'X'.taint).tainted?.should be_true
+    end
+  end
+
+  # NATFIXME: Add back once we have encodings.
+  xdescribe "with width" do
+    it "returns a String in the same encoding as the original" do
+      str = "abc".force_encoding Encoding::IBM437
+      result = str.ljust 5
+      result.should == "abc  "
+      result.encoding.should equal(Encoding::IBM437)
+    end
+  end
+
+  # NATFIXME: Add back once we have encodings.
+  xdescribe "with width, pattern" do
+    it "returns a String in the compatible encoding" do
+      str = "abc".force_encoding Encoding::IBM437
+      result = str.ljust 5, "あ"
+      result.should == "abcああ"
+      result.encoding.should equal(Encoding::UTF_8)
+    end
+
+    it "raises an Encoding::CompatibilityError if the encodings are incompatible" do
+      pat = "ア".encode Encoding::EUC_JP
+      -> do
+        "あれ".ljust 5, pat
+      end.should raise_error(Encoding::CompatibilityError)
+    end
+  end
+end

src/string_object.cpp

@@ -801,15 +801,19 @@ bool StringObject::include(const char *arg) const {
 }
 
 Value StringObject::ljust(Env *env, Value length_obj, Value pad_obj) {
-    length_obj->assert_type(env, Object::Type::Integer, "Integer");
-    size_t length = length_obj->as_integer()->to_nat_int_t() < 0 ? 0 : length_obj->as_integer()->to_nat_int_t();
+    nat_int_t length_i = length_obj->to_int(env)->to_nat_int_t();
+    size_t length = length_i < 0 ? 0 : length_i;
+
     StringObject *padstr;
-    if (pad_obj) {
-        pad_obj->assert_type(env, Object::Type::String, "String");
-        padstr = pad_obj->as_string();
-    } else {
+    if (!pad_obj) {
         padstr = new StringObject { " " };
+    } else {
+        padstr = pad_obj->to_str(env);
     }
+
+    if (padstr->string().is_empty())
+        env->raise("ArgumentError", "can't pad with an empty string");
+
     StringObject *copy = dup(env)->as_string();
     while (copy->length() < length) {
         bool truncate = copy->length() + padstr->length() > length;