Skip to content

Commit

Permalink
Fix regression introduced by frozen symbol fix
Browse files Browse the repository at this point in the history
There was a recent change [1] made to how punctuation is escaped to address
an issue introduced by a proposal in Ruby 2.7 (which is to change the
behaviour of `Symbol#to_s` so that it returns a frozen string) [2].

This ended up breaking support for calling the `memoize` method with a
punctuated string [3], e.g.

```ruby
class Dog
  extend Memoist

  def woof?
    true
  end

  memoize 'woof?'
end
```

I have added an additional test which should prevent further regressions
from being introduced.

[1] a893ce6
[2] https://bugs.ruby-lang.org/issues/16150
[3] #85
  • Loading branch information
sebjacobs committed Dec 4, 2019
1 parent 34f90dd commit 72d40af
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
4 changes: 1 addition & 3 deletions lib/memoist.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,10 @@ def self.unmemoized_prefix(identifier = nil)
end

def self.escape_punctuation(string)
string = string.to_s
string = string.is_a?(String) ? string.dup : string.to_s.dup

return string unless string.end_with?('?'.freeze, '!'.freeze)

string = string.dup if string.frozen?

# A String can't end in both ? and !
if string.sub!(/\?\Z/, '_query'.freeze)
else
Expand Down
17 changes: 15 additions & 2 deletions test/memoist_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ def age

memoize :name, :age

def age?
@counter.call(:age?)
true
end
memoize 'age?'

def sleep(hours = 8)
@counter.call(:sleep)
hours
Expand Down Expand Up @@ -272,6 +278,13 @@ def test_memoization_with_punctuation
@person.unmemoize_all
end

def test_memoization_when_memoize_is_called_with_punctuated_string
assert_equal true, @person.age?

@person.memoize_all
@person.unmemoize_all
end

def test_memoization_flush_with_punctuation
assert_equal true, @person.name?
@person.flush_cache(:name?)
Expand Down Expand Up @@ -344,11 +357,11 @@ def test_unmemoize_all
end

def test_all_memoized_structs
# Person memoize :age, :is_developer?, :memoize_protected_test, :name, :name?, :sleep, :update, :update_attributes
# Person memoize :age, :age?, :is_developer?, :memoize_protected_test, :name, :name?, :sleep, :update, :update_attributes
# Student < Person memoize :name, :identifier => :student
# Teacher < Person memoize :seniority

expected = %w[age is_developer? memoize_protected_test name name? sleep update update_attributes]
expected = %w[age age? is_developer? memoize_protected_test name name? sleep update update_attributes]
structs = Person.all_memoized_structs
assert_equal expected, structs.collect(&:memoized_method).collect(&:to_s).sort
assert_equal '@_memoized_name', structs.detect { |s| s.memoized_method == :name }.ivar
Expand Down

0 comments on commit 72d40af

Please sign in to comment.