Lazily fetch entry comments

lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb

@@ -17,10 +17,11 @@ class DeclarationListener
         dispatcher: Prism::Dispatcher,
         parse_result: Prism::ParseResult,
         file_path: String,
+        collect_comments: T::Boolean,
         enhancements: T::Array[Enhancement],
       ).void
     end
-    def initialize(index, dispatcher, parse_result, file_path, enhancements: [])
+    def initialize(index, dispatcher, parse_result, file_path, collect_comments: false, enhancements: [])
       @index = index
       @file_path = file_path
       @enhancements = enhancements
@@ -40,6 +41,7 @@ def initialize(index, dispatcher, parse_result, file_path, enhancements: [])
       # A stack of namespace entries that represent where we currently are. Used to properly assign methods to an owner
       @owner_stack = T.let([], T::Array[Entry::Namespace])
       @indexing_errors = T.let([], T::Array[String])
+      @collect_comments = collect_comments
 
       dispatcher.register(
         self,
@@ -540,9 +542,11 @@ def add_constant(node, name, value = nil)
       )
     end
 
-    sig { params(node: Prism::Node).returns(T::Array[String]) }
+    sig { params(node: Prism::Node).returns(T.nilable(String)) }
     def collect_comments(node)
-      comments = []
+      return unless @collect_comments
+
+      comments = +""
 
       start_line = node.location.start_line - 1
       start_line -= 1 unless @comments_by_line.key?(start_line)
@@ -551,7 +555,7 @@ def collect_comments(node)
         comment = @comments_by_line[line]
         break unless comment
 
-        comment_content = comment.location.slice.chomp
+        comment_content = comment.location.slice
 
         # invalid encodings would raise an "invalid byte sequence" exception
         if !comment_content.valid_encoding? || comment_content.match?(@index.configuration.magic_comment_regex)
@@ -560,9 +564,10 @@ def collect_comments(node)
 
         comment_content.delete_prefix!("#")
         comment_content.delete_prefix!(" ")
-        comments.prepend(comment_content)
+        comments.prepend("#{comment_content}\n")
       end
 
+      comments.chomp!
       comments
     end
 

lib/ruby_indexer/lib/ruby_indexer/entry.rb

@@ -24,9 +24,6 @@ class Visibility < T::Enum
 
     alias_method :name_location, :location
 
-    sig { returns(T::Array[String]) }
-    attr_reader :comments
-
     sig { returns(Visibility) }
     attr_accessor :visibility
 
@@ -35,7 +32,7 @@ class Visibility < T::Enum
         name: String,
         file_path: String,
         location: T.any(Prism::Location, RubyIndexer::Location),
-        comments: T::Array[String],
+        comments: T.nilable(String),
       ).void
     end
     def initialize(name, file_path, location, comments)
@@ -79,6 +76,37 @@ def file_name
       File.basename(@file_path)
     end
 
+    sig { returns(String) }
+    def comments
+      @comments ||= begin
+        # Parse only the comments based on the file path, which is much faster than parsing the entire file
+        parsed_comments = Prism.parse_file_comments(@file_path)
+
+        # Group comments based on whether they belong to a single block of comments
+        grouped = parsed_comments.slice_when do |left, right|
+          left.location.start_line + 1 != right.location.start_line
+        end
+
+        # Find the group that is either immediately or two lines above the current entry
+        correct_group = grouped.find do |group|
+          comment_end_line = group.last.location.start_line
+          (comment_end_line - 1..comment_end_line).cover?(@location.start_line - 1)
+        end
+
+        # If we found something, we join the comments together. Otherwise, the entry has no documentation and we don't
+        # want to accidentally re-parse it, so we set it to an empty string. If an entry is updated, the entire entry
+        # object is dropped, so this will not prevent updates
+        if correct_group
+          correct_group.filter_map do |comment|
+            content = comment.slice
+            content if content.valid_encoding?
+          end.join("\n")
+        else
+          ""
+        end
+      end
+    end
+
     class ModuleOperation
       extend T::Sig
       extend T::Helpers
@@ -116,7 +144,7 @@ class Namespace < Entry
           file_path: String,
           location: T.any(Prism::Location, RubyIndexer::Location),
           name_location: T.any(Prism::Location, Location),
-          comments: T::Array[String],
+          comments: T.nilable(String),
         ).void
       end
       def initialize(nesting, file_path, location, name_location, comments)
@@ -177,7 +205,7 @@ class Class < Namespace
           file_path: String,
           location: T.any(Prism::Location, RubyIndexer::Location),
           name_location: T.any(Prism::Location, Location),
-          comments: T::Array[String],
+          comments: T.nilable(String),
           parent_class: T.nilable(String),
         ).void
       end
@@ -195,7 +223,7 @@ def ancestor_hash
     class SingletonClass < Class
       extend T::Sig
 
-      sig { params(location: Prism::Location, name_location: Prism::Location, comments: T::Array[String]).void }
+      sig { params(location: Prism::Location, name_location: Prism::Location, comments: T.nilable(String)).void }
       def update_singleton_information(location, name_location, comments)
         # Create a new RubyIndexer::Location object from the Prism location
         @location = Location.new(
@@ -210,7 +238,7 @@ def update_singleton_information(location, name_location, comments)
           name_location.start_column,
           name_location.end_column,
         )
-        @comments.concat(comments)
+        (@comments ||= +"") << comments if comments
       end
     end
 
@@ -332,7 +360,7 @@ def parameters
           name: String,
           file_path: String,
           location: T.any(Prism::Location, RubyIndexer::Location),
-          comments: T::Array[String],
+          comments: T.nilable(String),
           visibility: Visibility,
           owner: T.nilable(Entry::Namespace),
         ).void
@@ -400,7 +428,7 @@ class Method < Member
           file_path: String,
           location: T.any(Prism::Location, RubyIndexer::Location),
           name_location: T.any(Prism::Location, Location),
-          comments: T::Array[String],
+          comments: T.nilable(String),
           signatures: T::Array[Signature],
           visibility: Visibility,
           owner: T.nilable(Entry::Namespace),
@@ -451,7 +479,7 @@ class UnresolvedConstantAlias < Entry
           name: String,
           file_path: String,
           location: T.any(Prism::Location, RubyIndexer::Location),
-          comments: T::Array[String],
+          comments: T.nilable(String),
         ).void
       end
       def initialize(target, nesting, name, file_path, location, comments) # rubocop:disable Metrics/ParameterLists
@@ -488,7 +516,7 @@ class InstanceVariable < Entry
           name: String,
           file_path: String,
           location: T.any(Prism::Location, RubyIndexer::Location),
-          comments: T::Array[String],
+          comments: T.nilable(String),
           owner: T.nilable(Entry::Namespace),
         ).void
       end
@@ -517,7 +545,7 @@ class UnresolvedMethodAlias < Entry
           owner: T.nilable(Entry::Namespace),
           file_path: String,
           location: T.any(Prism::Location, RubyIndexer::Location),
-          comments: T::Array[String],
+          comments: T.nilable(String),
         ).void
       end
       def initialize(new_name, old_name, owner, file_path, location, comments) # rubocop:disable Metrics/ParameterLists
@@ -541,10 +569,9 @@ class MethodAlias < Entry
 
       sig { params(target: T.any(Member, MethodAlias), unresolved_alias: UnresolvedMethodAlias).void }
       def initialize(target, unresolved_alias)
-        full_comments = ["Alias for #{target.name}\n"]
-        full_comments.concat(unresolved_alias.comments)
-        full_comments << "\n"
-        full_comments.concat(target.comments)
+        full_comments = +"Alias for #{target.name}\n"
+        full_comments << "#{unresolved_alias.comments}\n"
+        full_comments << target.comments
 
         super(
           unresolved_alias.new_name,

lib/ruby_indexer/lib/ruby_indexer/index.rb

@@ -312,12 +312,12 @@ def index_all(indexable_paths: @configuration.indexables, &block)
           break unless block.call(progress)
         end
 
-        index_single(path)
+        index_single(path, collect_comments: false)
       end
     end
 
-    sig { params(indexable_path: IndexablePath, source: T.nilable(String)).void }
-    def index_single(indexable_path, source = nil)
+    sig { params(indexable_path: IndexablePath, source: T.nilable(String), collect_comments: T::Boolean).void }
+    def index_single(indexable_path, source = nil, collect_comments: true)
       content = source || File.read(indexable_path.full_path)
       dispatcher = Prism::Dispatcher.new
 
@@ -327,6 +327,7 @@ def index_single(indexable_path, source = nil)
         dispatcher,
         result,
         indexable_path.full_path,
+        collect_comments: collect_comments,
         enhancements: @enhancements,
       )
       dispatcher.dispatch(result.value)
@@ -607,7 +608,7 @@ def existing_or_new_singleton_class(name)
           attached_ancestor.file_path,
           attached_ancestor.location,
           attached_ancestor.name_location,
-          [],
+          nil,
           nil,
         )
         add(singleton, skip_prefix_tree: true)

lib/ruby_indexer/lib/ruby_indexer/rbs_indexer.rb

@@ -272,10 +272,10 @@ def handle_signature_alias(member, owner_entry)
         RBS::AST::Declarations::Constant,
         RBS::AST::Members::MethodDefinition,
         RBS::AST::Members::Alias,
-      )).returns(T::Array[String])
+      )).returns(T.nilable(String))
     end
     def comments_to_string(declaration)
-      Array(declaration.comment&.string)
+      declaration.comment&.string
     end
   end
 end

lib/ruby_indexer/test/classes_and_modules_test.rb

@@ -213,10 +213,10 @@ class Bar; end
       RUBY
 
       foo_entry = @index["Foo"].first
-      assert_equal("This is a Foo comment\nThis is another Foo comment", foo_entry.comments.join("\n"))
+      assert_equal("This is a Foo comment\nThis is another Foo comment", foo_entry.comments)
 
       bar_entry = @index["Bar"].first
-      assert_equal("This Bar comment has 1 line padding", bar_entry.comments.join("\n"))
+      assert_equal("This Bar comment has 1 line padding", bar_entry.comments)
     end
 
     def test_skips_comments_containing_invalid_encodings
@@ -239,10 +239,10 @@ class Bar; end
       RUBY
 
       foo_entry = @index["Foo"].first
-      assert_equal("This is a Foo comment\nThis is another Foo comment", foo_entry.comments.join("\n"))
+      assert_equal("This is a Foo comment\nThis is another Foo comment", foo_entry.comments)
 
       bar_entry = @index["Foo::Bar"].first
-      assert_equal("This is a Bar comment", bar_entry.comments.join("\n"))
+      assert_equal("This is a Bar comment", bar_entry.comments)
     end
 
     def test_comments_can_be_attached_to_a_reopened_class
@@ -255,10 +255,10 @@ class Foo; end
       RUBY
 
       first_foo_entry = @index["Foo"][0]
-      assert_equal("This is a Foo comment", first_foo_entry.comments.join("\n"))
+      assert_equal("This is a Foo comment", first_foo_entry.comments)
 
       second_foo_entry = @index["Foo"][1]
-      assert_equal("This is another Foo comment", second_foo_entry.comments.join("\n"))
+      assert_equal("This is another Foo comment", second_foo_entry.comments)
     end
 
     def test_comments_removes_the_leading_pound_and_space
@@ -271,10 +271,10 @@ class Bar; end
       RUBY
 
       first_foo_entry = @index["Foo"][0]
-      assert_equal("This is a Foo comment", first_foo_entry.comments.join("\n"))
+      assert_equal("This is a Foo comment", first_foo_entry.comments)
 
       second_foo_entry = @index["Bar"][0]
-      assert_equal("This is a Bar comment", second_foo_entry.comments.join("\n"))
+      assert_equal("This is a Bar comment", second_foo_entry.comments)
     end
 
     def test_private_class_and_module_indexing
@@ -483,7 +483,7 @@ class << self
 
       foo = T.must(@index["Foo::<Class:Foo>"].first)
       assert_equal(4, foo.location.start_line)
-      assert_equal("Some extra comments", foo.comments.join("\n"))
+      assert_equal("Some extra comments", foo.comments)
     end
 
     def test_dynamic_singleton_class_blocks
@@ -501,7 +501,7 @@ class << bar
       # That pattern cannot be properly analyzed statically and assuming that it's always a regular singleton simplifies
       # the implementation considerably.
       assert_equal(3, singleton.location.start_line)
-      assert_equal("Some extra comments", singleton.comments.join("\n"))
+      assert_equal("Some extra comments", singleton.comments)
     end
 
     def test_namespaces_inside_singleton_blocks

lib/ruby_indexer/test/constant_test.rb

@@ -86,16 +86,16 @@ class A
         A::BAZ = 1
       RUBY
 
-      foo_comment = @index["FOO"].first.comments.join("\n")
+      foo_comment = @index["FOO"].first.comments
       assert_equal("FOO comment", foo_comment)
 
-      a_foo_comment = @index["A::FOO"].first.comments.join("\n")
+      a_foo_comment = @index["A::FOO"].first.comments
       assert_equal("A::FOO comment", a_foo_comment)
 
-      bar_comment = @index["BAR"].first.comments.join("\n")
+      bar_comment = @index["BAR"].first.comments
       assert_equal("::BAR comment", bar_comment)
 
-      a_baz_comment = @index["A::BAZ"].first.comments.join("\n")
+      a_baz_comment = @index["A::BAZ"].first.comments
       assert_equal("A::BAZ comment", a_baz_comment)
     end
 

lib/ruby_indexer/test/enhancements_test.rb

@@ -38,7 +38,7 @@ def on_call_node(index, owner, node, file_path)
               file_path,
               location,
               location,
-              [],
+              nil,
               [Entry::Signature.new([Entry::RequiredParameter.new(name: :a)])],
               Entry::Visibility::PUBLIC,
               owner,
@@ -120,7 +120,7 @@ def on_call_node(index, owner, node, file_path)
             file_path,
             location,
             location,
-            [],
+            nil,
             [],
             Entry::Visibility::PUBLIC,
             owner,

lib/ruby_indexer/test/method_test.rb

@@ -382,9 +382,9 @@ class Foo
       RUBY
 
       assert_entry("bar", Entry::Accessor, "/fake/path/foo.rb:2-15:2-18")
-      assert_equal("Hello there", @index["bar"].first.comments.join("\n"))
+      assert_equal("Hello there", @index["bar"].first.comments)
       assert_entry("other", Entry::Accessor, "/fake/path/foo.rb:2-21:2-26")
-      assert_equal("Hello there", @index["other"].first.comments.join("\n"))
+      assert_equal("Hello there", @index["other"].first.comments)
       assert_entry("baz=", Entry::Accessor, "/fake/path/foo.rb:3-15:3-18")
       assert_entry("qux", Entry::Accessor, "/fake/path/foo.rb:4-17:4-20")
       assert_entry("qux=", Entry::Accessor, "/fake/path/foo.rb:4-17:4-20")

lib/ruby_indexer/test/rbs_indexer_test.rb

@@ -345,7 +345,7 @@ def test_signature_alias
       assert_equal("all?", entry.old_name)
       assert_equal("Array", entry.owner.name)
       assert(entry.file_path.end_with?("core/array.rbs"))
-      assert_includes(entry.comments[0], "Returns `true` if any element of `self` meets a given criterion.")
+      assert_includes(entry.comments, "Returns `true` if any element of `self` meets a given criterion.")
     end
 
     private

lib/ruby_lsp/requests/support/common.rb

@@ -99,7 +99,7 @@ def categorized_markdown_from_index_entries(title, entries, max_entries = nil)
             )
 
             definitions << "[#{entry.file_name}](#{uri})"
-            content << "\n\n#{entry.comments.join("\n")}" unless entry.comments.empty?
+            content << "\n\n#{entry.comments}" unless entry.comments.empty?
           end
 
           additional_entries_text = if max_entries && entries.length > max_entries