Merge pull request #378 from FnControlOption/string/include

Implement String#include?

include/natalie/string.hpp

@@ -47,9 +47,7 @@ class String : public Cell {
 
     String(size_t length, char c) {
         char buf[length];
-        for (size_t i = 0; i < length; ++i) {
-            buf[i] = c;
-        }
+        memset(buf, c, sizeof(char) * length);
         set_str(buf, length);
     }
 
@@ -137,7 +135,7 @@ class String : public Cell {
         assert(str);
         auto old_str = m_str;
         m_str = new char[length + 1];
-        memcpy(m_str, str, length);
+        memcpy(m_str, str, sizeof(char) * length);
         m_str[length] = 0;
         m_length = length;
         m_capacity = length;
@@ -165,7 +163,7 @@ class String : public Cell {
         size_t new_length = strlen(str);
         if (new_length == 0) return;
         char buf[m_length + 1];
-        memcpy(buf, m_str, m_length + 1);
+        memcpy(buf, m_str, sizeof(char) * (m_length + 1));
         set_str(str);
         append(buf);
     }
@@ -245,8 +243,7 @@ class String : public Cell {
         if (str->length() == 0) return;
         size_t total_length = m_length + str->length();
         grow_at_least(total_length);
-        for (size_t i = 0; i < str->length(); ++i)
-            m_str[i + m_length] = (*str)[i];
+        memcpy(m_str + m_length, str->m_str, sizeof(char) * str->length());
         m_length = total_length;
     }
 
@@ -255,29 +252,22 @@ class String : public Cell {
     void append(size_t n, char c) {
         size_t total_length = m_length + n;
         grow_at_least(total_length);
-        for (size_t i = 0; i < n; ++i)
-            m_str[i + m_length] = c;
+        memset(m_str + m_length, c, sizeof(char) * n);
         m_length = total_length;
         m_str[m_length] = 0;
     }
 
     bool operator==(const String &other) const {
         if (length() != other.length())
             return false;
-        for (size_t i = 0; i < m_length; ++i)
-            if (m_str[i] != other[i])
-                return false;
-        return true;
+        return memcmp(m_str, other.c_str(), sizeof(char) * m_length) == 0;
     }
 
     bool operator==(const char *other) const {
         assert(other);
         if (length() != strlen(other))
             return false;
-        for (size_t i = 0; i < m_length; ++i)
-            if (m_str[i] != other[i])
-                return false;
-        return true;
+        return memcmp(m_str, other, sizeof(char) * m_length) == 0;
     }
 
     bool operator!=(const String &other) const {
@@ -293,10 +283,15 @@ class String : public Cell {
     }
 
     ssize_t find(const String &needle) const {
-        const char *index = strstr(m_str, needle.c_str());
-        if (index == nullptr)
+        if (m_length < needle.length())
             return -1;
-        return index - m_str;
+        size_t max_index = m_length - needle.length();
+        size_t byte_count = sizeof(char) * needle.length();
+        for (size_t index = 0; index <= max_index; ++index) {
+            if (memcmp(m_str + index, needle.c_str(), byte_count) == 0)
+                return index;
+        }
+        return -1;
     }
 
     ssize_t find(const char c) const {

include/natalie/string_object.hpp

@@ -153,6 +153,7 @@ class StringObject : public Object {
     bool eq(Env *, Value arg);
     Value eqtilde(Env *, Value);
     Value force_encoding(Env *, Value);
+    bool include(Env *, Value);
     Value ljust(Env *, Value, Value);
     Value lstrip(Env *) const;
     Value lstrip_in_place(Env *);

lib/natalie/compiler/binding_gen.rb

@@ -797,6 +797,7 @@ def generate_name
 gen.binding('String', 'eql?', 'StringObject', 'eql', argc: 1, pass_env: false, pass_block: false, return_type: :bool)
 gen.binding('String', 'force_encoding', 'StringObject', 'force_encoding', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
 gen.binding('String', 'gsub', 'StringObject', 'gsub', argc: 1..2, pass_env: true, pass_block: true, return_type: :Object)
+gen.binding('String', 'include?', 'StringObject', 'include', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
 gen.binding('String', 'index', 'StringObject', 'index', argc: 1, pass_env: true, pass_block: false, return_type: :Object)
 gen.binding('String', 'initialize', 'StringObject', 'initialize', argc: 0..1, pass_env: true, pass_block: false, return_type: :Object)
 gen.binding('String', 'inspect', 'StringObject', 'inspect', argc: 0, pass_env: true, pass_block: false, return_type: :Object)

spec/core/string/include_spec.rb

@@ -0,0 +1,36 @@
+require_relative '../../spec_helper'
+require_relative 'fixtures/classes'
+
+describe "String#include? with String" do
+  it "returns true if self contains other_str" do
+    "hello".include?("lo").should == true
+    "hello".include?("ol").should == false
+  end
+
+  it "ignores subclass differences" do
+    "hello".include?(StringSpecs::MyString.new("lo")).should == true
+    StringSpecs::MyString.new("hello").include?("lo").should == true
+    StringSpecs::MyString.new("hello").include?(StringSpecs::MyString.new("lo")).should == true
+  end
+
+  it "tries to convert other to string using to_str" do
+    other = mock('lo')
+    other.should_receive(:to_str).and_return("lo")
+
+    "hello".include?(other).should == true
+  end
+
+  it "raises a TypeError if other can't be converted to string" do
+    -> { "hello".include?([])       }.should raise_error(TypeError)
+    -> { "hello".include?('h'.ord)  }.should raise_error(TypeError)
+    -> { "hello".include?(mock('x')) }.should raise_error(TypeError)
+  end
+
+  # NATFIXME: Implement EUC-JP encoding
+  xit "raises an Encoding::CompatibilityError if the encodings are incompatible" do
+    pat = "ア".encode Encoding::EUC_JP
+    -> do
+      "あれ".include?(pat)
+    end.should raise_error(Encoding::CompatibilityError)
+  end
+end

src/string_object.cpp

@@ -667,6 +667,14 @@ Value StringObject::split(Env *env, Value splitter, Value max_count_value) {
     }
 }
 
+bool StringObject::include(Env *env, Value arg) {
+    auto to_str = "to_str"_s;
+    if (!arg->is_string() && arg->respond_to(env, to_str))
+        arg = arg->send(env, to_str);
+    arg->assert_type(env, Object::Type::String, "String");
+    return m_string.find(arg->as_string()->m_string) != -1;
+}
+
 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();

test/natalie/string_test.rb

@@ -449,4 +449,11 @@ def initialize(s)
       'tim'.reverse.should == 'mit'
     end
   end
+
+  describe '#include?' do
+    it 'works on strings containing null character' do
+      "foo\x00bar".include?('bar').should == true
+      'foo'.include?("foo\x00bar").should == false
+    end
+  end
 end