Simplify CrossReference resolve (ruby#1571)

Simply return nil if crossref resolve failed.
Crossref suppression (already done in another part) is not what
`CrossReference#resolve` have to do.
So `resolve(name, text)` don't need `text` arg.
Cache mechanism of `resolve` will be simple: no string label cache, only
cache `ref` or `nil`.

Author: tompng

lib/rdoc/cross_reference.rb

@@ -190,19 +190,13 @@ def resolve_local_symbol(name)
   ##
   # Returns a reference to +name+.
   #
-  # If the reference is found and +name+ is not documented +text+ will be
-  # returned.  If +name+ is escaped +name+ is returned.  If +name+ is not
-  # found +text+ is returned.
+  # If the reference is found and +name+ is not documented +nil+ will be
+  # returned.  If +name+ is not found +nil+ is returned.
 
-  def resolve(name, text)
+  def resolve(name)
     return @seen[name] if @seen.include? name
 
-    ref = case name
-          when /^\\(#{CLASS_REGEXP_STR})$/o then
-            @context.find_symbol $1
-          else
-            @context.find_symbol name
-          end
+    ref = @context.find_symbol name
 
     ref = resolve_local_symbol name unless ref
 
@@ -211,25 +205,11 @@ def resolve(name, text)
 
     ref = nil if RDoc::Alias === ref # external alias, can't link to it
 
-    out = if name == '\\' then
-            name
-          elsif name =~ /^\\/ then
-            # we remove the \ only in front of what we know:
-            # other backslashes are treated later, only outside of <tt>
-            ref ? $' : name
-          elsif ref then
-            if ref.display? then
-              ref
-            else
-              text
-            end
-          else
-            text
-          end
+    ref = nil unless ref&.display?
 
-    @seen[name] = out
+    @seen[name] = ref
 
-    out
+    ref
   end
 
 end

lib/rdoc/markup/to_html_crossref.rb

@@ -57,26 +57,23 @@ def init_link_notation_regexp_handlings
   ##
   # Creates a link to the reference +name+ if the name exists.  If +text+ is
   # given it is used as the link text, otherwise +name+ is used.
+  # Returns +nil+ if the link target could not be resolved.
 
   def cross_reference(name, text = nil, code = true, rdoc_ref: false)
-    # What to show when the reference doesn't resolve to a link:
-    # caller-provided text if any, otherwise the original name (preserving '#').
-    fallback = text || name
-
     # Strip '#' for link display text (e.g. #method shows as "method" in links)
     display = !@show_hash && name.start_with?('#') ? name[1..] : name
 
     if !display.end_with?('+@', '-@') && match = display.match(/(.*[^#:])?@(.*)/)
       context_name = match[1]
-      label = RDoc::Text.decode_legacy_label(match[2])
-      text ||= "#{label} at <code>#{context_name}</code>" if context_name
+      label = convert_string(RDoc::Text.decode_legacy_label(match[2]))
+      text ||= "#{label} at <code>#{convert_string(context_name)}</code>" if context_name
       text ||= label
       code = false
     else
-      text ||= display
+      text ||= convert_string(display)
     end
 
-    link(name, text, code, rdoc_ref: rdoc_ref) || fallback
+    link(name, text, code, rdoc_ref: rdoc_ref)
   end
 
   ##
@@ -98,8 +95,7 @@ def handle_regexp_CROSSREF(name)
       # cross-references to "new" in text, for instance)
       return name if name =~ /\A[a-z]*\z/
     end
-
-    cross_reference name, rdoc_ref: false
+    cross_reference(name, rdoc_ref: false) || convert_string(name)
   end
 
   ##
@@ -111,7 +107,8 @@ def handle_regexp_HYPERLINK(url)
 
     case url
     when /\Ardoc-ref:/
-      cross_reference $', rdoc_ref: true
+      ref = $'
+      cross_reference(ref, rdoc_ref: true) || convert_string(ref)
     else
       super
     end
@@ -131,7 +128,8 @@ def handle_regexp_RDOCLINK(url)
       if in_tidylink_label?
         convert_string(url)
       else
-        cross_reference $', rdoc_ref: true
+        ref = $'
+        cross_reference(ref, rdoc_ref: true) || convert_string(ref)
       end
     else
       super
@@ -145,46 +143,46 @@ def handle_regexp_RDOCLINK(url)
   def gen_url(url, text)
     if url =~ /\Ardoc-ref:/
       name = $'
-      cross_reference name, text, name == text, rdoc_ref: true
+      cross_reference(name, text, name == text, rdoc_ref: true) || text
     else
       super
     end
   end
 
   ##
-  # Creates an HTML link to +name+ with the given +text+.
+  # Creates an HTML link to +name+ with the given +html_string+.
+  # +html_string+ should be already escaped and may contain HTML tags.
   # Returns the link HTML string, or +nil+ if the reference could not be resolved.
 
-  def link(name, text, code = true, rdoc_ref: false)
+  def link(name, html_string, code = true, rdoc_ref: false)
     if !(name.end_with?('+@', '-@')) and name =~ /(.*[^#:])?@/
       name = $1
       label = $'
     end
 
-    ref = @cross_reference.resolve name, text if name
+    ref = @cross_reference.resolve name if name
 
     # Non-text source files (C, Ruby, etc.) don't get HTML pages generated,
     # so don't auto-link to them. Explicit rdoc-ref: links are still allowed.
     if !rdoc_ref && RDoc::TopLevel === ref && !ref.text?
       return
     end
 
-    case ref
-    when String
+    if ref
+      path = ref.as_href(@from_path)
+
+      if code and RDoc::CodeObject === ref and !(RDoc::TopLevel === ref)
+        html_string = "<code>#{html_string}</code>"
+      end
+    elsif name
       if rdoc_ref && @warn_missing_rdoc_ref
-        puts "#{@from_path}: `rdoc-ref:#{name}` can't be resolved for `#{text}`"
+        puts "#{@from_path}: `rdoc-ref:#{name}` can't be resolved for `#{html_string}`"
       end
       return
-    when nil
+    else
       # A bare label reference like @foo still produces a valid anchor link
       return unless label
       path = +""
-    else
-      path = ref.as_href(@from_path)
-
-      if code and RDoc::CodeObject === ref and !(RDoc::TopLevel === ref)
-        text = "<code>#{CGI.escapeHTML text}</code>"
-      end
     end
 
     if label
@@ -219,33 +217,45 @@ def link(name, text, code = true, rdoc_ref: false)
       end
     end
 
-    "<a href=\"#{path}\">#{text}</a>"
+    "<a href=\"#{path}\">#{html_string}</a>"
   end
 
   def handle_TT(code)
-    emit_inline(tt_cross_reference(code) || "<code>#{CGI.escapeHTML code}</code>")
+    emit_inline(tt_cross_reference(code) || "<code>#{convert_string(code)}</code>")
   end
 
   # Applies additional special handling on top of the one defined in ToHtml.
   # When a tidy link is <tt>{Foo}[rdoc-ref:Foo]</tt>, the label part is surrounded by <tt><code></code></tt>.
   # TODO: reconsider this workaround.
   def apply_tidylink_label_special_handling(label, url)
-    if url == "rdoc-ref:#{label}" && cross_reference(label).include?('<code>')
+    if url == "rdoc-ref:#{label}" && cross_reference(label)&.include?('<code>')
       "<code>#{convert_string(label)}</code>"
     else
       super
     end
   end
 
+  # Handles cross-reference and suppressed-crossref inside tt tag.
+  # Returns nil if code is not an existing cross-reference nor a suppressed-crossref.
   def tt_cross_reference(code)
     return if in_tidylink_label?
 
     crossref_regexp = @hyperlink_all ? ALL_CROSSREF_REGEXP : CROSSREF_REGEXP
-    match = crossref_regexp.match(code)
+    # REGEXP sometimes matches a string that starts with a backslash but is not a
+    # suppressed cross-reference (for example, `\+`), so the backslash-removed
+    # part needs to be checked against crossref_regexp.
+    match = crossref_regexp.match(code.delete_prefix('\\'))
     return unless match && match.begin(1).zero?
     return unless match.post_match.match?(/\A[[:punct:]\s]*\z/)
 
-    ref = cross_reference(code)
-    ref if ref != code
+    # cross_reference(file_page) may return a link without code tag.
+    # We need to check it because this method shouldn't return an html text without code tag.
+    if code.start_with?('\\')
+      # Remove leading backslash if crossref exists
+      "<code>#{convert_string(code[1..])}</code>" if cross_reference(code[1..])&.include?('<code>')
+    else
+      html = cross_reference(code)
+      html if html&.include?('<code>')
+    end
   end
 end

test/rdoc/markup/to_html_crossref_test.rb

@@ -48,6 +48,35 @@ def test_convert_CROSSREF_backslash_in_tt
     assert_equal para('<code>.bar.hello(\\)</code>'), result
   end
 
+  def test_convert_suppressed_CROSSREF_in_tt
+    result = @to.convert '<tt>C1</tt> <tt>\\C1</tt>'
+    assert_equal para('<a href="C1.html"><code>C1</code></a> <code>C1</code>'), result
+
+    result = @to.convert '<tt>C1#m()</tt> <tt>\\C1#m()</tt>'
+    assert_equal para('<a href="C1.html#method-i-m"><code>C1#m()</code></a> <code>C1#m()</code>'), result
+
+    # Keep backshash if crossref doesn't exist
+    result = @to.convert '<tt>C1#&</tt> <tt>\\n</tt> <tt>\\C1#&</tt>'
+    assert_equal para('<code>C1#&amp;</code> <code>\\n</code> <code>\\C1#&amp;</code>'), result
+  end
+
+  def test_convert_file_CROSSREF_in_tt
+    readme = @store.add_file 'README.txt'
+    readme.parser = RDoc::Parser::Simple
+
+    result = @to.convert 'Link to README.txt'
+    assert_equal para('Link to <a href="README_txt.html">README.txt</a>'), result
+
+    # Don't link to file in code. Only CodeObjects are linked in <tt>.
+    result = @to.convert '<tt>README.txt</tt>'
+    assert_equal para('<code>README.txt</code>'), result
+
+    # Since <tt>README.txt</tt> is not treated as a cross-reference,
+    # there's nothing to suppress. Backslash shouldn't be removed.
+    result = @to.convert '<tt>\README.txt</tt>'
+    assert_equal para('<code>\README.txt</code>'), result
+  end
+
   def test_convert_CROSSREF_ignored_excluded_words
     @to = RDoc::Markup::ToHtmlCrossref.new 'index.html', @c1,
       hyperlink_all: true, warn_missing_rdoc_ref: true,
@@ -118,7 +147,7 @@ def test_convert_CROSSREF_section_with_spaces
 
   def test_convert_CROSSREF_legacy_label
     result = @to.convert 'C1@What-27s+Here'
-    assert_equal para("<a href=\"C1.html#class-c1-whats-here\">What's Here at <code>C1</code></a>"), result
+    assert_equal para("<a href=\"C1.html#class-c1-whats-here\">What&#39;s Here at <code>C1</code></a>"), result
   end
 
   def test_convert_CROSSREF_legacy_label_colon
@@ -130,7 +159,7 @@ def test_convert_CROSSREF_legacy_section
     @c1.add_section "What's Here"
 
     result = @to.convert "C1@What-27s+Here"
-    assert_equal para("<a href=\"C1.html#whats-here\">What's Here at <code>C1</code></a>"), result
+    assert_equal para("<a href=\"C1.html#whats-here\">What&#39;s Here at <code>C1</code></a>"), result
   end
 
   def test_convert_CROSSREF_constant
@@ -430,9 +459,9 @@ def test_handle_regexp_CROSSREF_hash_preserved_for_unresolved
     assert_equal "#no", REGEXP_HANDLING('#no')
   end
 
-  def test_cross_reference_preserves_explicit_text_for_unresolved
-    # When explicit text is provided, it should be preserved on unresolved refs
-    assert_equal "Foo", @to.cross_reference("Missing", "Foo")
+  def test_cross_reference_returns_nil_for_unresolved
+    # Fallback of unresolved refs depends on the context, so cross_reference should return nil for unresolved refs
+    assert_nil @to.cross_reference("Missing", "Foo")
   end
 
   private

test/rdoc/rdoc_cross_reference_test.rb

@@ -13,11 +13,11 @@ def setup
   end
 
   def assert_ref(expected, name)
-    assert_equal expected, @xref.resolve(name, 'fail')
+    assert_equal expected, @xref.resolve(name)
   end
 
   def refute_ref(name)
-    assert_equal name, @xref.resolve(name, name)
+    assert_nil @xref.resolve(name)
   end
 
   def test_METHOD_REGEXP_STR
@@ -202,22 +202,21 @@ def assert_resolve_method(x)
   end
 
   def test_resolve_no_ref
-    assert_equal '', @xref.resolve('', '')
+    refute_ref('')
 
-    assert_equal "bogus",     @xref.resolve("bogus",     "bogus")
-    assert_equal "\\bogus",   @xref.resolve("\\bogus",   "\\bogus")
-    assert_equal "\\\\bogus", @xref.resolve("\\\\bogus", "\\\\bogus")
+    refute_ref("bogus")
+    refute_ref("\\bogus")
 
-    assert_equal "\\#n",    @xref.resolve("\\#n",    "fail")
-    assert_equal "\\#n()",  @xref.resolve("\\#n()",  "fail")
-    assert_equal "\\#n(*)", @xref.resolve("\\#n(*)", "fail")
+    refute_ref("\\#n")
+    refute_ref("\\#n()")
+    refute_ref("\\#n(*)")
 
-    assert_equal "C1",   @xref.resolve("\\C1",   "fail")
-    assert_equal "::C3", @xref.resolve("\\::C3", "fail")
+    refute_ref("\\C1")
+    refute_ref("\\::C3")
 
-    assert_equal "succeed",      @xref.resolve("::C3::H1#n",    "succeed")
-    assert_equal "succeed",      @xref.resolve("::C3::H1#n(*)", "succeed")
-    assert_equal "\\::C3::H1#n", @xref.resolve("\\::C3::H1#n",  "fail")
+    refute_ref("::C3::H1#n")
+    refute_ref("::C3::H1#n(*)")
+    refute_ref("\\::C3::H1#n")
   end
 
 end