From 42612f82edd583b87a12e6071aa1014bbae90edd Mon Sep 17 00:00:00 2001 From: Ryan Chandler Date: Thu, 3 Mar 2022 21:33:38 +0000 Subject: [PATCH 1/3] Add String#ljust spec --- spec/core/string/ljust_spec.rb | 131 +++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 spec/core/string/ljust_spec.rb diff --git a/spec/core/string/ljust_spec.rb b/spec/core/string/ljust_spec.rb new file mode 100644 index 000000000..0c3b2a2f4 --- /dev/null +++ b/spec/core/string/ljust_spec.rb @@ -0,0 +1,131 @@ +# -*- 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 + + describe "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 + + describe "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 From 0c5c3187f7df4477945f82160a1e3abcd4ec6ad5 Mon Sep 17 00:00:00 2001 From: Ryan Chandler Date: Thu, 3 Mar 2022 21:40:06 +0000 Subject: [PATCH 2/3] Make String#ljust spec compliant --- spec/core/string/ljust_spec.rb | 6 ++++-- src/string_object.cpp | 27 +++++++++++++++++++++------ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/spec/core/string/ljust_spec.rb b/spec/core/string/ljust_spec.rb index 0c3b2a2f4..b148456fb 100644 --- a/spec/core/string/ljust_spec.rb +++ b/spec/core/string/ljust_spec.rb @@ -104,7 +104,8 @@ end end - describe "with width" do + # 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 @@ -113,7 +114,8 @@ end end - describe "with width, pattern" do + # 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, "あ" diff --git a/src/string_object.cpp b/src/string_object.cpp index 9aa39351e..a8f75bfdd 100644 --- a/src/string_object.cpp +++ b/src/string_object.cpp @@ -801,15 +801,30 @@ 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(); - StringObject *padstr; - if (pad_obj) { - pad_obj->assert_type(env, Object::Type::String, "String"); + nat_int_t length_i = length_obj->to_int(env)->to_nat_int_t(); + size_t length = length_i < 0 ? 0 : length_i; + + auto to_str = "to_str"_s; + StringObject *padstr = new StringObject { " " }; + + if (!pad_obj) { + padstr = new StringObject { " " }; + } else if (pad_obj->is_string()) { padstr = pad_obj->as_string(); + } else if (pad_obj->respond_to(env, to_str)) { + auto result = pad_obj->send(env, to_str); + + if (!result->is_string()) + env->raise("TypeError", "padstr can't be converted to a string"); + + padstr = result->as_string(); } else { - padstr = new StringObject { " " }; + env->raise("TypeError", "padstr can't be converted to a string"); } + + 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; From 209ba60742942ee0023374f8d1f56d1225b60f25 Mon Sep 17 00:00:00 2001 From: Ryan Chandler Date: Fri, 4 Mar 2022 16:30:05 +0000 Subject: [PATCH 3/3] Refactor String#ljust to use Object::to_str --- src/string_object.cpp | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/string_object.cpp b/src/string_object.cpp index a8f75bfdd..d14fc2903 100644 --- a/src/string_object.cpp +++ b/src/string_object.cpp @@ -804,22 +804,11 @@ Value StringObject::ljust(Env *env, Value length_obj, Value pad_obj) { nat_int_t length_i = length_obj->to_int(env)->to_nat_int_t(); size_t length = length_i < 0 ? 0 : length_i; - auto to_str = "to_str"_s; - StringObject *padstr = new StringObject { " " }; - + StringObject *padstr; if (!pad_obj) { padstr = new StringObject { " " }; - } else if (pad_obj->is_string()) { - padstr = pad_obj->as_string(); - } else if (pad_obj->respond_to(env, to_str)) { - auto result = pad_obj->send(env, to_str); - - if (!result->is_string()) - env->raise("TypeError", "padstr can't be converted to a string"); - - padstr = result->as_string(); } else { - env->raise("TypeError", "padstr can't be converted to a string"); + padstr = pad_obj->to_str(env); } if (padstr->string().is_empty())