From fbae52b8bdff433fc7e9c1ef703133382bebe60e Mon Sep 17 00:00:00 2001 From: TheGrizzlyDev Date: Tue, 26 Oct 2021 20:37:55 +0100 Subject: [PATCH 1/4] Add spec for String#eql? --- spec/core/string/eql_spec.rb | 21 ++++++++++ spec/core/string/fixtures/classes.rb | 60 ++++++++++++++++++++++++++++ spec/core/string/shared/eql.rb | 34 ++++++++++++++++ 3 files changed, 115 insertions(+) create mode 100644 spec/core/string/eql_spec.rb create mode 100644 spec/core/string/fixtures/classes.rb create mode 100644 spec/core/string/shared/eql.rb diff --git a/spec/core/string/eql_spec.rb b/spec/core/string/eql_spec.rb new file mode 100644 index 000000000..397974d9f --- /dev/null +++ b/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 diff --git a/spec/core/string/fixtures/classes.rb b/spec/core/string/fixtures/classes.rb new file mode 100644 index 000000000..26fcd51b5 --- /dev/null +++ b/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 diff --git a/spec/core/string/shared/eql.rb b/spec/core/string/shared/eql.rb new file mode 100644 index 000000000..85b861f4f --- /dev/null +++ b/spec/core/string/shared/eql.rb @@ -0,0 +1,34 @@ +# -*- 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 + + it "ignores encoding difference of compatible string" do + "hello".force_encoding("utf-8").send(@method, "hello".force_encoding("iso-8859-1")).should be_true + end + + it "considers encoding difference of incompatible string" do + "\xff".force_encoding("utf-8").send(@method, "\xff".force_encoding("iso-8859-1")).should be_false + end + + it "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 From 45e997524cec673765cb488a9307562d00c5e6ae Mon Sep 17 00:00:00 2001 From: TheGrizzlyDev Date: Wed, 27 Oct 2021 11:37:02 +0100 Subject: [PATCH 2/4] Skip tests that require encodings that are not yet supported --- spec/core/string/shared/eql.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/spec/core/string/shared/eql.rb b/spec/core/string/shared/eql.rb index 85b861f4f..3e4ea6395 100644 --- a/spec/core/string/shared/eql.rb +++ b/spec/core/string/shared/eql.rb @@ -12,15 +12,18 @@ "less".send(@method, "greater").should be_false end - it "ignores encoding difference of compatible string" do + #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 - it "considers encoding difference of incompatible string" do + #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 - it "considers encoding compatibility" do + #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 From 174ec53136cf99d1eb35dbe4b3442b9d23428f9a Mon Sep 17 00:00:00 2001 From: TheGrizzlyDev Date: Wed, 27 Oct 2021 11:39:52 +0100 Subject: [PATCH 3/4] Add spec for String#== --- spec/core/string/equal_value_spec.rb | 8 +++++++ spec/core/string/shared/equal_value.rb | 29 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 spec/core/string/equal_value_spec.rb create mode 100644 spec/core/string/shared/equal_value.rb diff --git a/spec/core/string/equal_value_spec.rb b/spec/core/string/equal_value_spec.rb new file mode 100644 index 000000000..b9c9c372f --- /dev/null +++ b/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 diff --git a/spec/core/string/shared/equal_value.rb b/spec/core/string/shared/equal_value.rb new file mode 100644 index 000000000..fccafb582 --- /dev/null +++ b/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 From a1afb3c51f1adbec530064b2da33fd7ab22c1c99 Mon Sep 17 00:00:00 2001 From: TheGrizzlyDev Date: Wed, 27 Oct 2021 11:50:41 +0100 Subject: [PATCH 4/4] Separate String#== and eql? and make the former spec-compliant --- include/natalie/string_value.hpp | 3 ++- lib/natalie/compiler/binding_gen.rb | 6 +++--- src/string_value.cpp | 6 ++++++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/include/natalie/string_value.hpp b/include/natalie/string_value.hpp index 9d32cb0c9..7d05a763c 100644 --- a/include/natalie/string_value.hpp +++ b/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); diff --git a/lib/natalie/compiler/binding_gen.rb b/lib/natalie/compiler/binding_gen.rb index b9a12ba93..74558f9c8 100644 --- a/lib/natalie/compiler/binding_gen.rb +++ b/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) diff --git a/src/string_value.cpp b/src/string_value.cpp index 5996a8524..c5c00ab64 100644 --- a/src/string_value.cpp +++ b/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);