Skip to content

Commit

Permalink
Fix issue where Registry fails to resolve first-time lookups on insta…
Browse files Browse the repository at this point in the history
…nce methods

This is due to incorrect cache invalidation in #1260.

The fix is to add a callback on NamespaceMapper when invalidation occurs.

Also adds tests for RegistryResolver and NamespaceMapper
  • Loading branch information
lsegal committed Jan 7, 2020
1 parent 445267b commit aa0ed28
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 28 deletions.
33 changes: 30 additions & 3 deletions lib/yard/code_objects/namespace_mapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ module NamespaceMapper
# Registers a separator with an optional set of valid types that
# must follow the separator lexically.
#
# Calls all callbacks defined by {NamespaceMapper.on_invalidate} after
# the separator is registered.
#
# @param sep [String] the separator string for the namespace
# @param valid_types [Array<Symbol>] a list of object types that
# must follow the separator. If the list is empty, any type can
Expand All @@ -20,6 +23,7 @@ module NamespaceMapper
# register_separator "#", :method
# # Anything after a "." denotes a method object
# register_separator ".", :method
# @see .on_invalidate
def register_separator(sep, *valid_types)
NamespaceMapper.invalidate

Expand All @@ -32,6 +36,19 @@ def register_separator(sep, *valid_types)
NamespaceMapper.map[sep] += valid_types
end

# Unregisters a separator by a type.
#
# @param type [Symbol] the type to unregister
# @see #register_separator
def unregister_separator_by_type(type)
seps = NamespaceMapper.rev_map[type]
return unless seps

seps.each {|s| NamespaceMapper.map.delete(s) }
NamespaceMapper.rev_map.delete(type)
NamespaceMapper.invalidate
end

# Clears the map of separators.
#
# @return [void]
Expand All @@ -50,6 +67,7 @@ def clear_separators
# default_separator "::"
def default_separator(value = nil)
if value
NamespaceMapper.invalidate
NamespaceMapper.default_separator = Regexp.quote value
else
NamespaceMapper.default_separator
Expand All @@ -71,17 +89,25 @@ def separators_match
# @param sep [String] the separator to return types for
# @return [Array<Symbol>] a list of types registered to a separator
def types_for_separator(sep)
NamespaceMapper.map[sep]
NamespaceMapper.map[sep] || []
end

# @param type [String] the type to return separators for
# @return [Array<Symbol>] a list of separators registered to a type
def separators_for_type(type)
NamespaceMapper.rev_map[type]
NamespaceMapper.rev_map[type] || []
end

# Internal methods to act as a singleton registry
class << self
# @!group Invalidation callbacks

# Adds a callback that triggers when a new separator is registered or
# the cache is cleared by {#invalidate}
def on_invalidate(&block)
(@invalidation_callbacks ||= []).push(block)
end

# @!visibility private

# @return [Hash] a mapping of types to separators
Expand All @@ -98,11 +124,12 @@ def rev_map
# @return [void]
def invalidate
@map_match = nil
(@invalidation_callbacks || []).each(&:call)
end

# @return [Regexp] the full list of separators as a regexp match
def map_match
@map_match ||= @map.keys.map {|k| Regexp.quote k }.join('|')
@map_match ||= map.keys.map {|k| Regexp.quote k }.join('|')
end

# @return [String] the default separator when no separator can begin
Expand Down
4 changes: 3 additions & 1 deletion lib/yard/parser/ruby/token_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module Ruby
# constant or identifier token.
class TokenResolver
include Enumerable
include CodeObjects::NamespaceMapper

# Creates a token resolver for given source.
#
Expand Down Expand Up @@ -114,7 +115,8 @@ def lookup(toktype, name)

if toktype == :const
types.any? do |type|
prefix = (type ? type.path : "") + last_sep.to_s
prefix = type ? type.path : ""
prefix += last_sep.to_s if separators.include?(last_sep.to_s)
self.object = Registry.resolve(@default_namespace, "#{prefix}#{name}", true)
end
else # ident
Expand Down
34 changes: 10 additions & 24 deletions lib/yard/registry_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ class RegistryResolver
def initialize(registry = Registry)
@registry = registry
@default_sep = nil

# Preload all code objects for separator declarations
YARD::CodeObjects.constants.map {|t| YARD::CodeObjects.const_get(t) }
end

# Performs a lookup on a given path in the registry. Resolution will occur
Expand Down Expand Up @@ -186,46 +189,29 @@ def collect_namespaces(object)
nss
end

# @see NamespaceMapper#register_separator
def register_separator(*)
super
invalidate_memoized_matchers
end

# @see NamespaceMapper#clear_separators
def clear_separators
super
invalidate_memoized_matchers
end

# @see NamespaceMapper#default_separator
def default_separator(value = nil)
@starts_with_default_separator_match = nil if value
super
end

# @return [Regexp] the regexp match of the default separator
def starts_with_default_separator_match
@starts_with_default_separator_match ||= /\A#{default_separator}/
@@starts_with_default_separator_match ||= /\A#{default_separator}/
end

# @return [Regexp] the regexp that matches strings starting with
# a separator
def starts_with_separator_match
@starts_with_separator_match ||= /\A(#{separators_match})/
@@starts_with_separator_match ||= /\A(#{separators_match})/
end

# @return [Regexp] the regexp that can be used to split a string on all
# occurrences of separator tokens
def split_on_separators_match
@split_on_separators_match ||= /(.+?)(#{separators_match}|$)/
@@split_on_separators_match ||= /(.+?)(#{separators_match}|$)/
end

# Additional invalidations to done when NamespaceMapper API methods are
# called on this class
def invalidate_memoized_matchers
@starts_with_separator_match = nil
@split_on_separators_match = nil
YARD::CodeObjects::NamespaceMapper.on_invalidate do
@@starts_with_default_separator_match = nil
@@starts_with_separator_match = nil
@@split_on_separators_match = nil
end
end
end
32 changes: 32 additions & 0 deletions spec/code_objects/namespace_mapper_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true
require File.dirname(__FILE__) + '/spec_helper'

RSpec.describe YARD::CodeObjects::NamespaceMapper do
include YARD::CodeObjects::NamespaceMapper

describe '#register_separator' do
it 'should allow separators to be registered' do
register_separator '!', :test_type

expect(separators_for_type(:test_type)).to eq ['!']
expect(types_for_separator('!')).to eq [:test_type]

unregister_separator_by_type :test_type

expect(separators_for_type(:test_type)).to be_empty
expect(types_for_separator('!')).to be_empty
end
end

describe '.on_invalidate' do
it 'receives a callback when a new separator is added' do
invalidated = false
NamespaceMapper.on_invalidate { invalidated = true }

register_separator '!', :test_type
expect(invalidated).to be true

unregister_separator_by_type :test_type
end
end
end
15 changes: 15 additions & 0 deletions spec/registry_resolver_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# frozen_string_literal: true

RSpec.describe YARD::RegistryResolver do
include YARD::CodeObjects::NamespaceMapper

describe '#starts_with_separator_match' do
subject { RegistryResolver.new }

it 'should clear cache when a namespace separator is registered' do
expect(subject.send(:starts_with_separator_match).to_s).not_to include('!')
register_separator '!', :test_type
expect(subject.send(:starts_with_separator_match).to_s).to include('!')
end
end
end

0 comments on commit aa0ed28

Please sign in to comment.