Fix escaping of argument lists in doc generator, expose JSON (#10109)

crystal-lang · Jun 13, 2021 · 16fd7df · 16fd7df
1 parent 6103d5a
commit 16fd7df
Show file tree

Hide file tree

Showing 8 changed files with 155 additions and 102 deletions.
diff --git a/spec/compiler/crystal/tools/doc/macro_spec.cr b/spec/compiler/crystal/tools/doc/macro_spec.cr
@@ -1,5 +1,10 @@
 require "../../../spec_helper"
 
+private def assert_args_to_s(item, to_s_output, to_html_output = to_s_output, *, file = __FILE__, line = __LINE__)
+  item.args_to_s.should eq(to_s_output), file: file, line: line
+  item.args_to_html.should eq(to_html_output), file: file, line: line
+end
+
 describe Doc::Macro do
   describe "args_to_s" do
     it "shows simple args" do
@@ -9,7 +14,7 @@ describe Doc::Macro do
 
       a_macro = Macro.new "foo", ["foo".arg, "bar".arg]
       doc_macro = Doc::Macro.new generator, doc_type, a_macro
-      doc_macro.args_to_s.should eq("(foo, bar)")
+      assert_args_to_s(doc_macro, "(foo, bar)")
     end
 
     it "shows splat arg" do
@@ -19,7 +24,7 @@ describe Doc::Macro do
 
       a_macro = Macro.new "foo", ["foo".arg], splat_index: 0
       doc_macro = Doc::Macro.new generator, doc_type, a_macro
-      doc_macro.args_to_s.should eq("(*foo)")
+      assert_args_to_s(doc_macro, "(*foo)")
     end
 
     it "shows simple arg and splat arg" do
@@ -29,7 +34,7 @@ describe Doc::Macro do
 
       a_macro = Macro.new "foo", ["foo".arg, "bar".arg], splat_index: 1
       doc_macro = Doc::Macro.new generator, doc_type, a_macro
-      doc_macro.args_to_s.should eq("(foo, *bar)")
+      assert_args_to_s(doc_macro, "(foo, *bar)")
     end
 
     it "shows double splat arg" do
@@ -39,7 +44,7 @@ describe Doc::Macro do
 
       a_macro = Macro.new "foo", double_splat: "foo".arg
       doc_macro = Doc::Macro.new generator, doc_type, a_macro
-      doc_macro.args_to_s.should eq("(**foo)")
+      assert_args_to_s(doc_macro, "(**foo)")
     end
 
     it "shows double splat arg" do
@@ -49,7 +54,7 @@ describe Doc::Macro do
 
       a_macro = Macro.new "foo", double_splat: "foo".arg
       doc_macro = Doc::Macro.new generator, doc_type, a_macro
-      doc_macro.args_to_s.should eq("(**foo)")
+      assert_args_to_s(doc_macro, "(**foo)")
     end
 
     it "shows simple arg and double splat arg" do
@@ -59,7 +64,7 @@ describe Doc::Macro do
 
       a_macro = Macro.new "foo", ["foo".arg], double_splat: "bar".arg
       doc_macro = Doc::Macro.new generator, doc_type, a_macro
-      doc_macro.args_to_s.should eq("(foo, **bar)")
+      assert_args_to_s(doc_macro, "(foo, **bar)")
     end
 
     it "shows block arg" do
@@ -69,7 +74,7 @@ describe Doc::Macro do
 
       a_macro = Macro.new "foo", block_arg: "foo".arg
       doc_macro = Doc::Macro.new generator, doc_type, a_macro
-      doc_macro.args_to_s.should eq("(&foo)")
+      assert_args_to_s(doc_macro, "(&foo)")
     end
 
     it "shows simple arg and block arg" do
@@ -79,7 +84,7 @@ describe Doc::Macro do
 
       a_macro = Macro.new "foo", ["foo".arg], block_arg: "bar".arg
       doc_macro = Doc::Macro.new generator, doc_type, a_macro
-      doc_macro.args_to_s.should eq("(foo, &bar)")
+      assert_args_to_s(doc_macro, "(foo, &bar)")
     end
 
     it "shows external name of arg" do
@@ -89,7 +94,7 @@ describe Doc::Macro do
 
       a_macro = Macro.new "foo", ["foo".arg(external_name: "bar")]
       doc_macro = Doc::Macro.new generator, doc_type, a_macro
-      doc_macro.args_to_s.should eq("(bar foo)")
+      assert_args_to_s(doc_macro, "(bar foo)")
     end
 
     it "shows external name of arg with quotes and escaping" do
@@ -99,7 +104,9 @@ describe Doc::Macro do
 
       a_macro = Macro.new "foo", ["foo".arg(external_name: "<<-< uouo fish life")]
       doc_macro = Doc::Macro.new generator, doc_type, a_macro
-      doc_macro.args_to_s.should eq("(&quot;&lt;&lt;-&lt; uouo fish life&quot; foo)")
+      assert_args_to_s(doc_macro,
+        %(("<<-< uouo fish life" foo)),
+        "(&quot;&lt;&lt;-&lt; uouo fish life&quot; foo)")
     end
 
     it "shows default value with highlighting" do
@@ -109,7 +116,7 @@ describe Doc::Macro do
 
       a_macro = Macro.new "foo", ["foo".arg(default_value: 1.int32)]
       doc_macro = Doc::Macro.new generator, doc_type, a_macro
-      doc_macro.args_to_s.should eq(%((foo = <span class="n">1</span>)))
+      assert_args_to_s(doc_macro, %((foo = 1)), %((foo = <span class="n">1</span>)))
     end
   end
 end
diff --git a/spec/compiler/crystal/tools/doc/method_spec.cr b/spec/compiler/crystal/tools/doc/method_spec.cr
@@ -1,5 +1,10 @@
 require "../../../spec_helper"
 
+private def assert_args_to_s(item, to_s_output, to_html_output = to_s_output, *, file = __FILE__, line = __LINE__)
+  item.args_to_s.should eq(to_s_output), file: file, line: line
+  item.args_to_html.should eq(to_html_output), file: file, line: line
+end
+
 describe Doc::Method do
   describe "args_to_s" do
     it "shows simple args" do
@@ -9,7 +14,7 @@ describe Doc::Method do
 
       a_def = Def.new "foo", ["foo".arg, "bar".arg]
       doc_method = Doc::Method.new generator, doc_type, a_def, false
-      doc_method.args_to_s.should eq("(foo, bar)")
+      assert_args_to_s(doc_method, "(foo, bar)")
     end
 
     it "shows splat args" do
@@ -19,7 +24,7 @@ describe Doc::Method do
 
       a_def = Def.new "foo", ["foo".arg], splat_index: 0
       doc_method = Doc::Method.new generator, doc_type, a_def, false
-      doc_method.args_to_s.should eq("(*foo)")
+      assert_args_to_s(doc_method, "(*foo)")
     end
 
     it "shows underscore restriction" do
@@ -29,7 +34,7 @@ describe Doc::Method do
 
       a_def = Def.new "foo", ["foo".arg(restriction: Crystal::Underscore.new)], splat_index: 0
       doc_method = Doc::Method.new generator, doc_type, a_def, false
-      doc_method.args_to_s.should eq("(*foo : _)")
+      assert_args_to_s(doc_method, "(*foo : _)")
     end
 
     it "shows double splat args" do
@@ -39,7 +44,7 @@ describe Doc::Method do
 
       a_def = Def.new "foo", double_splat: "foo".arg
       doc_method = Doc::Method.new generator, doc_type, a_def, false
-      doc_method.args_to_s.should eq("(**foo)")
+      assert_args_to_s(doc_method, "(**foo)")
     end
 
     it "shows block args" do
@@ -49,7 +54,7 @@ describe Doc::Method do
 
       a_def = Def.new "foo", block_arg: "foo".arg
       doc_method = Doc::Method.new generator, doc_type, a_def, false
-      doc_method.args_to_s.should eq("(&foo)")
+      assert_args_to_s(doc_method, "(&foo)")
     end
 
     it "shows block args with underscore" do
@@ -59,7 +64,7 @@ describe Doc::Method do
 
       a_def = Def.new "foo", block_arg: "foo".arg(restriction: Crystal::ProcNotation.new(([Crystal::Underscore.new] of Crystal::ASTNode), Crystal::Underscore.new))
       doc_method = Doc::Method.new generator, doc_type, a_def, false
-      doc_method.args_to_s.should eq("(&foo : _ -> _)")
+      assert_args_to_s(doc_method, "(&foo : _ -> _)")
     end
 
     it "shows block args if a def has `yield`" do
@@ -69,7 +74,7 @@ describe Doc::Method do
 
       a_def = Def.new "foo", yields: 1
       doc_method = Doc::Method.new generator, doc_type, a_def, false
-      doc_method.args_to_s.should eq("(&)")
+      assert_args_to_s(doc_method, "(&)")
     end
 
     it "shows return type restriction" do
@@ -79,7 +84,7 @@ describe Doc::Method do
 
       a_def = Def.new "foo", return_type: "Foo".path
       doc_method = Doc::Method.new generator, doc_type, a_def, false
-      doc_method.args_to_s.should eq(" : Foo")
+      assert_args_to_s(doc_method, " : Foo")
     end
 
     it "shows args and return type restriction" do
@@ -89,7 +94,7 @@ describe Doc::Method do
 
       a_def = Def.new "foo", ["foo".arg], return_type: "Foo".path
       doc_method = Doc::Method.new generator, doc_type, a_def, false
-      doc_method.args_to_s.should eq("(foo) : Foo")
+      assert_args_to_s(doc_method, "(foo) : Foo")
     end
 
     it "shows external name of arg" do
@@ -99,7 +104,7 @@ describe Doc::Method do
 
       a_def = Def.new "foo", ["foo".arg(external_name: "bar")]
       doc_method = Doc::Method.new generator, doc_type, a_def, false
-      doc_method.args_to_s.should eq("(bar foo)")
+      assert_args_to_s(doc_method, "(bar foo)")
     end
 
     it "shows external name of arg with quotes and escaping" do
@@ -109,7 +114,9 @@ describe Doc::Method do
 
       a_def = Def.new "foo", ["foo".arg(external_name: "<<-< uouo fish life")]
       doc_method = Doc::Method.new generator, doc_type, a_def, false
-      doc_method.args_to_s.should eq("(&quot;&lt;&lt;-&lt; uouo fish life&quot; foo)")
+      assert_args_to_s(doc_method,
+        %(("<<-< uouo fish life" foo)),
+        "(&quot;&lt;&lt;-&lt; uouo fish life&quot; foo)")
     end
 
     it "shows typeof restriction of arg with highlighting" do
@@ -119,7 +126,9 @@ describe Doc::Method do
 
       a_def = Def.new "foo", ["foo".arg(restriction: TypeOf.new([1.int32] of ASTNode))]
       doc_method = Doc::Method.new generator, doc_type, a_def, false
-      doc_method.args_to_s.should eq(%((foo : <span class="k">typeof</span>(<span class="n">1</span>))))
+      assert_args_to_s(doc_method,
+        %((foo : typeof(1))),
+        %((foo : <span class="k">typeof</span>(<span class="n">1</span>))))
     end
 
     it "shows default value of arg with highlighting" do
@@ -129,7 +138,7 @@ describe Doc::Method do
 
       a_def = Def.new "foo", ["foo".arg(default_value: 1.int32)]
       doc_method = Doc::Method.new generator, doc_type, a_def, false
-      doc_method.args_to_s.should eq(%((foo = <span class="n">1</span>)))
+      assert_args_to_s(doc_method, %((foo = 1)), %((foo = <span class="n">1</span>)))
     end
   end
 

diff --git a/src/compiler/crystal/tools/doc/html/_method_summary.html b/src/compiler/crystal/tools/doc/html/_method_summary.html
@@ -6,7 +6,7 @@ <h2>
   <ul class="list-summary">
     <% methods.each do |method| %>
       <li class="entry-summary">
-        <a href="<%= method.anchor %>" class="signature"><strong><%= method.prefix %><%= method.name %></strong><%= method.args_to_s %></a>
+        <a href="<%= method.anchor %>" class="signature"><strong><%= method.prefix %><%= method.name %></strong><%= method.args_to_html(:highlight) %></a>
         <% if summary = method.formatted_summary %>
           <div class="summary"><%= summary %></div>
         <% end %>

diff --git a/src/compiler/crystal/tools/doc/html/_methods_inherited.html b/src/compiler/crystal/tools/doc/html/_methods_inherited.html
@@ -4,7 +4,7 @@ <h3><%= label %> methods inherited from <%= ancestor.kind %> <code><%= ancestor.
   <% i = 0 %>
   <% method_groups.each do |method_name, methods| %>
     <a href="<%= type.path_to(ancestor) %><%= methods[0].anchor %>" class="tooltip">
-      <span><%= methods.map { |method| method.name + method.args_to_s } .join("<br/>") %></span>
+      <span><%= methods.map { |method| method.name + method.args_to_html(:highlight) } .join("<br/>") %></span>
     <%= method_name %></a><%= ", " if i != method_groups.size - 1 %>
     <% i += 1 %>
   <% end %>

diff --git a/src/compiler/crystal/tools/doc/item.cr b/src/compiler/crystal/tools/doc/item.cr
@@ -7,3 +7,17 @@ module Crystal::Doc::Item
     @generator.summary(self)
   end
 end
+
+enum Crystal::Doc::HTMLOption
+  None
+  Highlight
+  All
+
+  def highlight? : Bool
+    self >= Highlight
+  end
+
+  def links? : Bool
+    self >= All
+  end
+end
diff --git a/src/compiler/crystal/tools/doc/macro.cr b/src/compiler/crystal/tools/doc/macro.cr
@@ -33,7 +33,7 @@ class Crystal::Doc::Macro
 
   def id
     String.build do |io|
-      io << to_s.gsub(/<.+?>/, "").delete(' ')
+      io << to_s.delete(' ')
       io << "-macro"
     end
   end
@@ -68,6 +68,14 @@ class Crystal::Doc::Macro
   end
 
   def args_to_s(io : IO) : Nil
+    args_to_html(io, html: :none)
+  end
+
+  def args_to_html(html : HTMLOption = :all)
+    String.build { |io| args_to_html io, html }
+  end
+
+  def args_to_html(io : IO, html : HTMLOption = :all) : Nil
     return unless has_args?
 
     printed = false
@@ -76,31 +84,35 @@ class Crystal::Doc::Macro
     @macro.args.each_with_index do |arg, i|
       io << ", " if printed
       io << '*' if @macro.splat_index == i
-      arg_to_s arg, io
+      arg_to_html arg, io, html: html
       printed = true
     end
 
     if double_splat = @macro.double_splat
       io << ", " if printed
       io << "**"
-      arg_to_s double_splat, io
+      arg_to_html double_splat, io, html: html
       printed = true
     end
 
     if block_arg = @macro.block_arg
       io << ", " if printed
       io << '&'
-      arg_to_s block_arg, io
+      arg_to_html block_arg, io, html: html
     end
 
     io << ')'
   end
 
-  def arg_to_s(arg : Arg, io : IO) : Nil
+  def arg_to_html(arg : Arg, io, html : HTMLOption = :all)
     if arg.external_name != arg.name
       if name = arg.external_name.presence
         if Symbol.needs_quotes_for_named_argument? name
-          HTML.escape name.inspect, io
+          if html.none?
+            name.inspect io
+          else
+            HTML.escape name.inspect, io
+          end
         else
           io << name
         end
@@ -116,32 +128,32 @@ class Crystal::Doc::Macro
 
     if default_value = arg.default_value
       io << " = "
-      io << Highlighter.highlight(default_value.to_s)
+      if html.highlight?
+        io << Highlighter.highlight(default_value.to_s)
+      else
+        io << default_value
+      end
     end
   end
 
   def has_args?
     !@macro.args.empty? || @macro.double_splat || @macro.block_arg
   end
 
-  def args_to_html
-    args_to_s
-  end
-
   def must_be_included?
     @generator.must_include? @macro
   end
 
   def to_json(builder : JSON::Builder)
     builder.object do
-      builder.field "id", id
-      builder.field "html_id", html_id
+      builder.field "html_id", id
       builder.field "name", name
       builder.field "doc", doc
       builder.field "summary", formatted_summary
       builder.field "abstract", abstract?
       builder.field "args", args
       builder.field "args_string", args_to_s
+      builder.field "args_html", args_to_html
       builder.field "location", location
       builder.field "def", self.macro
     end