Merge pull request #253 from TheGrizzlyDev/string-equality-methods

include/natalie/string_value.hpp

@@ -117,7 +117,7 @@ class StringValue : public Value {
     ValuePtr initialize(Env *, ValuePtr);
     ValuePtr ltlt(Env *, ValuePtr);
 
-    bool eq(ValuePtr arg) {
+    bool eql(ValuePtr arg) {
         return *this == *arg;
     }
 
@@ -138,6 +138,7 @@ class StringValue : public Value {
     ValuePtr downcase(Env *);
     ValuePtr encode(Env *, ValuePtr);
     ValuePtr encoding(Env *);
+    bool eq(Env *, ValuePtr arg);
     ValuePtr eqtilde(Env *, ValuePtr);
     ValuePtr force_encoding(Env *, ValuePtr);
     ValuePtr ljust(Env *, ValuePtr, ValuePtr);

lib/natalie/compiler/binding_gen.rb

@@ -715,8 +715,8 @@ def generate_name
 gen.binding('String', '+', 'StringValue', 'add', argc: 1, pass_env: true, pass_block: false, return_type: :Value)
 gen.binding('String', '<<', 'StringValue', 'ltlt', argc: 1, pass_env: true, pass_block: false, return_type: :Value)
 gen.binding('String', '<=>', 'StringValue', 'cmp', argc: 1, pass_env: true, pass_block: false, return_type: :Value)
-gen.binding('String', '==', 'StringValue', 'eq', argc: 1, pass_env: false, pass_block: false, return_type: :bool)
-gen.binding('String', '===', 'StringValue', 'eq', argc: 1, pass_env: false, pass_block: false, return_type: :bool)
+gen.binding('String', '==', 'StringValue', 'eq', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
+gen.binding('String', '===', 'StringValue', 'eq', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
 gen.binding('String', '=~', 'StringValue', 'eqtilde', argc: 1, pass_env: true, pass_block: false, return_type: :Value)
 gen.binding('String', '[]', 'StringValue', 'ref', argc: 1, pass_env: true, pass_block: false, return_type: :Value)
 gen.binding('String', 'bytes', 'StringValue', 'bytes', argc: 0, pass_env: true, pass_block: false, return_type: :Value)
@@ -727,7 +727,7 @@ def generate_name
 gen.binding('String', 'encode', 'StringValue', 'encode', argc: 1, pass_env: true, pass_block: false, return_type: :Value)
 gen.binding('String', 'encoding', 'StringValue', 'encoding', argc: 0, pass_env: true, pass_block: false, return_type: :Value)
 gen.binding('String', 'end_with?', 'StringValue', 'end_with', argc: 1, pass_env: true, pass_block: false, return_type: :bool)
-gen.binding('String', 'eql?', 'StringValue', 'eq', argc: 1, pass_env: false, pass_block: false, return_type: :bool)
+gen.binding('String', 'eql?', 'StringValue', 'eql', argc: 1, pass_env: false, pass_block: false, return_type: :bool)
 gen.binding('String', 'force_encoding', 'StringValue', 'force_encoding', argc: 1, pass_env: true, pass_block: false, return_type: :Value)
 gen.binding('String', 'gsub', 'StringValue', 'gsub', argc: 1..2, pass_env: true, pass_block: true, return_type: :Value)
 gen.binding('String', 'index', 'StringValue', 'index', argc: 1, pass_env: true, pass_block: false, return_type: :Value)

spec/core/string/eql_spec.rb

@@ -0,0 +1,21 @@
+require_relative '../../spec_helper'
+require_relative 'shared/eql'
+
+describe "String#eql?" do
+  it_behaves_like :string_eql_value, :eql?
+
+  describe "when given a non-String" do
+    it "returns false" do
+      'hello'.should_not eql(5)
+      not_supported_on :opal do
+        'hello'.should_not eql(:hello)
+      end
+      'hello'.should_not eql(mock('x'))
+    end
+
+    it "does not try to call #to_str on the given argument" do
+      (obj = mock('x')).should_not_receive(:to_str)
+      'hello'.should_not eql(obj)
+    end
+  end
+end

spec/core/string/equal_value_spec.rb

@@ -0,0 +1,8 @@
+require_relative '../../spec_helper'
+require_relative 'shared/eql'
+require_relative 'shared/equal_value'
+
+describe "String#==" do
+  it_behaves_like :string_eql_value, :==
+  it_behaves_like :string_equal_value, :==
+end

spec/core/string/fixtures/classes.rb

@@ -0,0 +1,60 @@
+class Object
+  # This helper is defined here rather than in MSpec because
+  # it is only used in #unpack specs.
+  def unpack_format(count=nil, repeat=nil)
+    format = "#{instance_variable_get(:@method)}#{count}"
+    format *= repeat if repeat
+    format.dup # because it may then become tainted
+  end
+end
+
+module StringSpecs
+  class MyString < String; end
+  class MyArray < Array; end
+  class MyRange < Range; end
+
+  class SubString < String
+    attr_reader :special
+
+    def initialize(str=nil)
+      @special = str
+    end
+  end
+
+  class InitializeString < String
+    attr_reader :ivar
+
+    def initialize(other)
+      super
+      @ivar = 1
+    end
+
+    def initialize_copy(other)
+      ScratchPad.record object_id
+    end
+  end
+
+  module StringModule
+    def repr
+      1
+    end
+  end
+
+  class StringWithRaisingConstructor < String
+    def initialize(str)
+      raise ArgumentError.new('constructor was called') unless str == 'silly:string'
+      self.replace(str)
+    end
+  end
+
+  class SpecialVarProcessor
+    def process(match)
+      if $~ != nil
+        str = $~[0]
+      else
+        str = "unset"
+      end
+      "<#{str}>"
+    end
+  end
+end

spec/core/string/shared/eql.rb

@@ -0,0 +1,37 @@
+# -*- encoding: binary -*-
+require_relative '../../../spec_helper'
+require_relative '../fixtures/classes'
+
+describe :string_eql_value, shared: true do
+  it "returns true if self <=> string returns 0" do
+    'hello'.send(@method, 'hello').should be_true
+  end
+
+  it "returns false if self <=> string does not return 0" do
+    "more".send(@method, "MORE").should be_false
+    "less".send(@method, "greater").should be_false
+  end
+
+  #FIXME: add back once we support iso-8859-1 encoding
+  xit "ignores encoding difference of compatible string" do
+    "hello".force_encoding("utf-8").send(@method, "hello".force_encoding("iso-8859-1")).should be_true
+  end
+
+  #FIXME: add back once we support iso-8859-1 encoding
+  xit "considers encoding difference of incompatible string" do
+    "\xff".force_encoding("utf-8").send(@method, "\xff".force_encoding("iso-8859-1")).should be_false
+  end
+
+  #FIXME: add back once we support utf-32le encoding
+  xit "considers encoding compatibility" do
+    "hello".force_encoding("utf-8").send(@method, "hello".force_encoding("utf-32le")).should be_false
+  end
+
+  it "ignores subclass differences" do
+    a = "hello"
+    b = StringSpecs::MyString.new("hello")
+
+    a.send(@method, b).should be_true
+    b.send(@method, a).should be_true
+  end
+end

spec/core/string/shared/equal_value.rb

@@ -0,0 +1,29 @@
+require_relative '../../../spec_helper'
+require_relative '../fixtures/classes'
+
+describe :string_equal_value, shared: true do
+  it "returns false if obj does not respond to to_str" do
+    'hello'.send(@method, 5).should be_false
+    not_supported_on :opal do
+      'hello'.send(@method, :hello).should be_false
+    end
+    'hello'.send(@method, mock('x')).should be_false
+  end
+
+  it "returns obj == self if obj responds to to_str" do
+    obj = Object.new
+
+    # String#== merely checks if #to_str is defined. It does
+    # not call it.
+    obj.stub!(:to_str)
+
+    # Don't use @method for :== in `obj.should_receive(:==)`
+    obj.should_receive(:==).and_return(true)
+
+    'hello'.send(@method, obj).should be_true
+  end
+
+  it "is not fooled by NUL characters" do
+    "abc\0def".send(@method, "abc\0xyz").should be_false
+  end
+end

src/string_value.cpp

@@ -228,6 +228,12 @@ ValuePtr StringValue::cmp(Env *env, ValuePtr other) {
     return ValuePtr::integer(0);
 }
 
+bool StringValue::eq(Env *env, ValuePtr arg) {
+    if (!arg->is_string() && arg->respond_to(env, SymbolValue::intern("to_str")))
+        return arg->send(env, SymbolValue::intern("=="), { this });
+    return eql(arg);
+}
+
 ValuePtr StringValue::eqtilde(Env *env, ValuePtr other) {
     other->assert_type(env, Value::Type::Regexp, "Regexp");
     return other->as_regexp()->eqtilde(env, this);