Reconstruct Comment#string lazily from its location

config.yml

@@ -18,6 +18,7 @@ nodes:
     fields:
       - name: string
         c_type: rbs_string
+        expose_to_ruby: false  # `Comment#string` is reconstructed lazily from the location in Ruby
   - name: RBS::AST::Declarations::Class
     rust_name: ClassNode
     fields:

docs/config.md

@@ -108,6 +108,17 @@ The `rbs_ast_declarations_class` struct is a pure C AST, and `ast_translation.c`
     }
 ```
 
+A field can be excluded from the Ruby translation with `expose_to_ruby: false`. The field is still part of the C struct (and the Rust/serialization side), but `ast_translation.c` does not pass it to the Ruby constructor, so the Ruby class has to compute it by itself. `RBS::AST::Comment#string` uses this to reconstruct the comment text lazily from its `location` instead of holding a copy of it.
+
+```yaml
+  - name: RBS::AST::Comment
+    rust_name: CommentNode
+    fields:
+      - name: string
+        c_type: rbs_string
+        expose_to_ruby: false
+```
+
 ## enums
 
 `enums` defines *enum* data types in C or Rust.

ext/rbs_extension/ast_translation.c

@@ -232,14 +232,12 @@ VALUE rbs_struct_to_ruby_value(rbs_translation_context_t ctx, rbs_node_t *instan
 
         // Compute child VALUEs into locals variables first, before any recursion into `rbs_struct_to_ruby_value()`.
         VALUE arg_location = rbs_location_range_to_ruby_location(ctx, node->base.location);
-        VALUE arg_string = rbs_string_to_ruby_string(&node->string, ctx.encoding);
 
         // Claim the shared kwargs hash, clear it, fill it, and hand it to `.new`.
         // Must not recurse between `rb_hash_clear()` and `CLASS_NEW_INSTANCE()`.
         VALUE h = ctx.reusable_kwargs_hash;
         rb_hash_clear(h);
         rb_hash_aset(h, ID2SYM(rb_intern("location")), arg_location);
-        rb_hash_aset(h, ID2SYM(rb_intern("string")), arg_string);
         return CLASS_NEW_INSTANCE(RBS_AST_Comment, 1, &h);
     }
     case RBS_AST_DECLARATIONS_CLASS: {

lib/rbs/ast/comment.rb

@@ -3,14 +3,17 @@
 module RBS
   module AST
     class Comment
-      attr_reader :string
       attr_reader :location
 
-      def initialize(string:, location:)
+      def initialize(string: nil, location:)
         @string = string
         @location = location
       end
 
+      def string
+        @string ||= string_from_location
+      end
+
       def ==(other)
         other.is_a?(Comment) && other.string == string
       end
@@ -24,6 +27,18 @@ def hash
       def to_json(state = nil)
         { string: string, location: location }.to_json(state)
       end
+
+      private
+
+      def string_from_location
+        loc = location or raise "Comment#string requires a location when no string is given"
+
+        source = loc.buffer.top_buffer.content[loc.start_pos...loc.end_pos] or raise
+
+        source.each_line.map do |line|
+          line.lstrip.delete_prefix("#").delete_prefix(" ").sub(/\r?\n\z/, "") + "\n"
+        end.join
+      end
     end
   end
 end

lib/rbs/wasm/deserializer.rb

@@ -88,7 +88,8 @@ def read_struct(entry)
 
         kwargs = {} #: Hash[Symbol, untyped]
         (fields || []).each do |name, reader|
-          kwargs[name] = read_field(reader)
+          value = read_field(reader)
+          kwargs[name] = value if name
         end
 
         RBS::AST::TypeParam.resolve_variables(kwargs[:type_params]) if resolve_type_params

lib/rbs/wasm/serialization_schema.rb

@@ -21,15 +21,16 @@ module WASM
     # where loc_children is [[name, required?], ...] and fields is
     # [[name, reader], ...] with reader one of :node, :node_list, :hash, :string,
     # :bool, :location_range, :location_range_list, :attr_ivar_name, or
-    # [:enum, [value_or_nil, ...]].
+    # [:enum, [value_or_nil, ...]]. A `nil` name means the field is read but not
+    # passed to the constructor (`expose_to_ruby: false` in config.yml).
     module SerializationSchema
       SYMBOL_TAG = 79
 
       SCHEMA = [
         nil, # tag 0 is reserved for NULL
         [:node, "RBS::AST::Annotation", true, nil, [[:string, :string]], false],
         [:bool],
-        [:node, "RBS::AST::Comment", true, nil, [[:string, :string]], false],
+        [:node, "RBS::AST::Comment", true, nil, [[nil, :string]], false],
         [:node, "RBS::AST::Declarations::Class", true, [[:keyword, true], [:name, true], [:end, true], [:type_params, false], [:lt, false]], [[:name, :node], [:type_params, :node_list], [:super_class, :node], [:members, :node_list], [:annotations, :node_list], [:comment, :node]], true],
         [:node, "RBS::AST::Declarations::Class::Super", true, [[:name, true], [:args, false]], [[:name, :node], [:args, :node_list]], false],
         [:node, "RBS::AST::Declarations::ClassAlias", true, [[:keyword, true], [:new_name, true], [:eq, true], [:old_name, true]], [[:new_name, :node], [:old_name, :node], [:comment, :node], [:annotations, :node_list]], false],

sig/comment.rbs

@@ -9,10 +9,15 @@ module RBS
     # ```
     #
     class Comment
-      attr_reader string: String
+      # Each line with its leading `#` and one following space removed.
+      # Computed from `location` on first access unless given to `initialize`.
+      def string: () -> String
+
+      @string: String?
+
       attr_reader location: Location[bot, bot]?
 
-      def initialize: (string: String, location: Location[bot, bot]?) -> void
+      def initialize: (?string: String?, location: Location[bot, bot]?) -> void
 
       def ==: (untyped other) -> bool
 
@@ -21,6 +26,12 @@ module RBS
       def hash: () -> Integer
 
       include _ToJson
+
+      private
+
+      # Must stay in sync with `parse_comment_lines` in src/parser.c.
+      # Slices the buffer instead of using `Location#source` so no copy is memoized on the Location.
+      def string_from_location: () -> String
     end
   end
 end

templates/ext/rbs_extension/ast_translation.c.erb

@@ -212,6 +212,7 @@ VALUE rbs_struct_to_ruby_value(rbs_translation_context_t ctx, rbs_node_t *instan
             <%- end -%>
         <%- end -%>
         <%- node.fields.each do |field| -%>
+        <%- next unless field.expose_to_ruby? -%>
         <%- case field.type.name -%>
         <%- when "rbs_node_list" -%>
         VALUE arg_<%= field.name %> = rbs_node_list_to_ruby_array(ctx, node-><%= field.c_name %>);
@@ -262,6 +263,7 @@ VALUE rbs_struct_to_ruby_value(rbs_translation_context_t ctx, rbs_node_t *instan
         rb_hash_aset(h, ID2SYM(rb_intern("location")), arg_location);
         <%- end -%>
         <%- node.fields.each do |field| -%>
+        <%- next unless field.expose_to_ruby? -%>
         rb_hash_aset(h, ID2SYM(rb_intern("<%= field.name %>")), arg_<%= field.name %>);
         <%- end -%>
         return CLASS_NEW_INSTANCE(<%= node.c_constant_name %>, 1, &h);

templates/lib/rbs/wasm/serialization_schema.rb.erb

@@ -41,7 +41,9 @@
     when "RBS::TypeName" then "[:type_name]"
     else
       children = (node.locations || []).map { |loc| "[:#{loc.name}, #{loc.required?}]" }
-      fields = node.fields.map { |field| "[:#{field.name}, #{reader_for.call(field)}]" }
+      # A field with `expose_to_ruby: false` is still in the buffer, so the deserializer
+      # must read it, but it is not passed to the Ruby constructor: its name is `nil`.
+      fields = node.fields.map { |field| "[#{field.expose_to_ruby? ? ":#{field.name}" : "nil"}, #{reader_for.call(field)}]" }
       # Emit `nil` rather than `[]` for empty lists, so the generated file has no
       # unannotated empty collections for Steep to flag.
       children_literal = children.empty? ? "nil" : "[#{children.join(", ")}]"
@@ -67,7 +69,8 @@ module RBS
     # where loc_children is [[name, required?], ...] and fields is
     # [[name, reader], ...] with reader one of :node, :node_list, :hash, :string,
     # :bool, :location_range, :location_range_list, :attr_ivar_name, or
-    # [:enum, [value_or_nil, ...]].
+    # [:enum, [value_or_nil, ...]]. A `nil` name means the field is read but not
+    # passed to the constructor (`expose_to_ruby: false` in config.yml).
     module SerializationSchema
       SYMBOL_TAG = <%= nodes.size + 1 %>
 

templates/template.rb

@@ -75,22 +75,28 @@ def c_type_name
     class Field
       attr_reader :name, :type, :c_name #: String
 
-      def initialize(name:, type:, optional:, c_name: nil)
+      def initialize(name:, type:, optional:, c_name: nil, expose_to_ruby: true)
         @name = name
         @type = type
         @c_name = c_name || name
         @optional = optional
+        @expose_to_ruby = expose_to_ruby
       end
 
       def self.from_hash(hash)
         new(
           name: hash["name"],
           type: hash.fetch("c_type", "VALUE"),
           c_name: hash["c_name"],
-          optional: hash.fetch("optional", false)
+          optional: hash.fetch("optional", false),
+          expose_to_ruby: hash.fetch("expose_to_ruby", true)
         )
       end
 
+      def expose_to_ruby? #: bool
+        @expose_to_ruby
+      end
+
       def optional? #: bool
         @optional
       end
@@ -376,6 +382,7 @@ def locals
               type: type,
               optional: field.fetch("optional", false),
               c_name: field["c_name"],
+              expose_to_ruby: field.fetch("expose_to_ruby", true),
             )
           end
 

test/rbs/comment_test.rb

@@ -0,0 +1,28 @@
+require "test_helper"
+
+class RBS::CommentTest < Test::Unit::TestCase
+  include RBS
+
+  def test_new_with_string
+    comment = AST::Comment.new(string: "Hello\n", location: nil)
+
+    assert_equal "Hello\n", comment.string
+    assert_equal AST::Comment.new(string: "Hello\n", location: nil), comment
+  end
+
+  def test_new_without_string_requires_location
+    assert_raises(RuntimeError) do
+      AST::Comment.new(location: nil).string
+    end
+  end
+
+  def test_string_from_location
+    buffer = Buffer.new(name: Pathname("a.rbs"), content: "  # foo\n  #bar\n  #\nclass Foo end\n")
+    location = Location.new(buffer, 2, 17)
+
+    comment = AST::Comment.new(location: location)
+
+    assert_equal "foo\nbar\n\n", comment.string
+    assert_equal AST::Comment.new(string: "foo\nbar\n\n", location: nil), comment
+  end
+end

test/rbs/signature_parsing_test.rb

@@ -1172,6 +1172,37 @@ def bar: () -> void
     end
   end
 
+  def test_comment_with_crlf
+    # The CR of a CRLF line ending is not part of the comment, but a bare CR inside a line is.
+    Parser.parse_signature("# foo\r\n  # bar\r\n#baz\r\nclass Foo\r\nend\r\n").yield_self do |_, _, (foo_decl)|
+      assert_equal "foo\nbar\nbaz\n", foo_decl.comment.string
+    end
+
+    Parser.parse_signature("# a\rb\nclass Foo end").yield_self do |_, _, (foo_decl)|
+      assert_equal "a\rb\n", foo_decl.comment.string
+    end
+  end
+
+  def test_comment_string_is_reconstructed_from_location
+    Parser.parse_signature(<<-EOF).yield_self do |_, _, (foo_decl)|
+# Doc for Foo.
+class Foo
+  #
+  #  indented, blank, and tab lines
+  #\tkeep the tab
+  def bar: () -> void
+end
+EOF
+
+      comment = foo_decl.comment
+      assert_nil comment.instance_variable_get(:@string)
+      assert_equal "Doc for Foo.\n", comment.string
+      assert_same comment.string, comment.string
+
+      assert_equal "\n indented, blank, and tab lines\n\tkeep the tab\n", foo_decl.members[0].comment.string
+    end
+  end
+
   def test_parsing_quoted_method_name
     Parser.parse_signature(<<-EOF).yield_self do |foo_decl,|
 module Kernel