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

Implement String prepend #410

Merged
merged 3 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/natalie/string_object.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ class StringObject : public Object {
Value match(Env *, Value);
Value mul(Env *, Value) const;
Value ord(Env *);
Value prepend(Env *, size_t, Value *);
Value ref(Env *, Value);
Value reverse(Env *);
Value rstrip(Env *) const;
Expand Down
1 change: 1 addition & 0 deletions lib/natalie/compiler/binding_gen.rb
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,7 @@ def generate_name
gen.binding('String', 'lstrip', 'StringObject', 'lstrip', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('String', 'lstrip!', 'StringObject', 'lstrip_in_place', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('String', 'match', 'StringObject', 'match', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
gen.binding('String', 'prepend', 'StringObject', 'prepend', argc: :any, 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', 'rstrip', 'StringObject', 'rstrip', argc: 0, pass_env: true, pass_block: false, return_type: :Object)
Expand Down
64 changes: 64 additions & 0 deletions spec/core/string/prepend_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
require_relative '../../spec_helper'
require_relative 'fixtures/classes'

describe "String#prepend" do
it "prepends the given argument to self and returns self" do
str = "world"
str.prepend("hello ").should equal(str)
str.should == "hello world"
end

it "converts the given argument to a String using to_str" do
obj = mock("hello")
obj.should_receive(:to_str).and_return("hello")
a = " world!".prepend(obj)
a.should == "hello world!"
end

it "raises a TypeError if the given argument can't be converted to a String" do
-> { "hello ".prepend [] }.should raise_error(TypeError)
-> { 'hello '.prepend mock('x') }.should raise_error(TypeError)
end

it "raises a FrozenError when self is frozen" do
a = "hello"
a.freeze

-> { a.prepend "" }.should raise_error(FrozenError)
-> { a.prepend "test" }.should raise_error(FrozenError)
end

it "works when given a subclass instance" do
a = " world"
a.prepend StringSpecs::MyString.new("hello")
a.should == "hello world"
end

ruby_version_is ''...'2.7' do
it "taints self if other is tainted" do
x = "x"
x.prepend("".taint).tainted?.should be_true

x = "x"
x.prepend("y".taint).tainted?.should be_true
end
end

it "takes multiple arguments" do
str = " world"
str.prepend "he", "", "llo"
str.should == "hello world"
end

it "prepends the initial value when given arguments contain 2 self" do
str = "hello"
str.prepend str, str
str.should == "hellohellohello"
end

it "returns self when given no arguments" do
str = "hello"
str.prepend.should equal(str)
str.should == "hello"
end
end
34 changes: 34 additions & 0 deletions src/string_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,40 @@ Value StringObject::ord(Env *env) {
return Value::integer(code);
}

Value StringObject::prepend(Env *env, size_t argc, Value *args) {
assert_not_frozen(env);

StringObject *original = new StringObject(*this);

auto to_str = "to_str"_s;
String appendable;
for (size_t i = 0; i < argc; i++) {
auto arg = args[i];

if (arg == this)
arg = original;

StringObject *str_obj;
if (arg->is_string()) {
str_obj = arg->as_string();
} else if (arg->is_integer() && arg->as_integer()->to_nat_int_t() < 0) {
env->raise("RangeError", "less than 0");
} else if (arg->is_integer()) {
str_obj = arg.send(env, "chr"_s)->as_string();
} else if (arg->respond_to(env, to_str)) {
str_obj = arg.send(env, to_str)->as_string();
} else {
env->raise("TypeError", "cannot call to_str", arg->inspect_str(env));
}

str_obj->assert_type(env, Object::Type::String, "String");
appendable.append(str_obj->c_str());
}
m_string.prepend(appendable.c_str());
Copy link
Member

Choose a reason for hiding this comment

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

One thing that @FnControlOption worked on was making our String class properly handle internal null characters \0. I think we need to avoid dropping down to cstrs as much as possible for that reason.

s = "foo"
s.prepend("bar\0baz")
p s # => "bar\u0000bazfoo"

Copy link
Member

Choose a reason for hiding this comment

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

You should be able to add a String::prepend() that accepts another String....


return this;
}

Value StringObject::bytes(Env *env) const {
ArrayObject *ary = new ArrayObject { length() };
for (size_t i = 0; i < length(); i++) {
Expand Down