Skip to content

Commit

Permalink
Use string instead of array to store comments
Browse files Browse the repository at this point in the history
  • Loading branch information
vinistock committed Jul 23, 2024
1 parent c36f1c2 commit f3be528
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 36 deletions.
4 changes: 2 additions & 2 deletions lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ def add_constant(node, name, value = nil)
)
end

sig { params(node: Prism::Node).returns(T::Array[String]) }
sig { params(node: Prism::Node).returns(String) }
def collect_comments(node)
comments = []

Expand All @@ -544,7 +544,7 @@ def collect_comments(node)
comments.prepend(comment_content)
end

comments
comments.join("\n")
end

sig { params(name: String).returns(String) }
Expand Down
25 changes: 11 additions & 14 deletions lib/ruby_indexer/lib/ruby_indexer/entry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class Visibility < T::Enum

alias_method :name_location, :location

sig { returns(T::Array[String]) }
sig { returns(String) }
attr_reader :comments

sig { returns(Visibility) }
Expand All @@ -35,7 +35,7 @@ class Visibility < T::Enum
name: String,
file_path: String,
location: T.any(Prism::Location, RubyIndexer::Location),
comments: T::Array[String],
comments: String,
).void
end
def initialize(name, file_path, location, comments)
Expand Down Expand Up @@ -116,7 +116,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: String,
).void
end
def initialize(nesting, file_path, location, name_location, comments)
Expand Down Expand Up @@ -177,7 +177,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: String,
parent_class: T.nilable(String),
).void
end
Expand All @@ -195,7 +195,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: String).void }
def update_singleton_information(location, name_location, comments)
# Create a new RubyIndexer::Location object from the Prism location
@location = Location.new(
Expand Down Expand Up @@ -321,7 +321,7 @@ def parameters
name: String,
file_path: String,
location: T.any(Prism::Location, RubyIndexer::Location),
comments: T::Array[String],
comments: String,
visibility: Visibility,
owner: T.nilable(Entry::Namespace),
).void
Expand Down Expand Up @@ -376,7 +376,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: String,
signatures: T::Array[Signature],
visibility: Visibility,
owner: T.nilable(Entry::Namespace),
Expand Down Expand Up @@ -427,7 +427,7 @@ class UnresolvedAlias < Entry
name: String,
file_path: String,
location: T.any(Prism::Location, RubyIndexer::Location),
comments: T::Array[String],
comments: String,
).void
end
def initialize(target, nesting, name, file_path, location, comments) # rubocop:disable Metrics/ParameterLists
Expand Down Expand Up @@ -464,7 +464,7 @@ class InstanceVariable < Entry
name: String,
file_path: String,
location: T.any(Prism::Location, RubyIndexer::Location),
comments: T::Array[String],
comments: String,
owner: T.nilable(Entry::Namespace),
).void
end
Expand Down Expand Up @@ -493,7 +493,7 @@ class UnresolvedMethodAlias < Entry
owner: T.nilable(Entry::Namespace),
file_path: String,
location: Prism::Location,
comments: T::Array[String],
comments: String,
).void
end
def initialize(new_name, old_name, owner, file_path, location, comments) # rubocop:disable Metrics/ParameterLists
Expand All @@ -517,10 +517,7 @@ 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#{unresolved_alias.comments}\n#{target.comments}"

super(
unresolved_alias.new_name,
Expand Down
2 changes: 1 addition & 1 deletion lib/ruby_indexer/lib/ruby_indexer/index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ def existing_or_new_singleton_class(name)
attached_ancestor.file_path,
attached_ancestor.location,
attached_ancestor.name_location,
[],
+"",
nil,
)
add(singleton, skip_prefix_tree: true)
Expand Down
4 changes: 2 additions & 2 deletions lib/ruby_indexer/lib/ruby_indexer/rbs_indexer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def handle_class_or_module_declaration(declaration, pathname)
nesting = [declaration.name.name.to_s]
file_path = pathname.to_s
location = to_ruby_indexer_location(declaration.location)
comments = Array(declaration.comment&.string)
comments = declaration.comment&.string || ""
entry = if declaration.is_a?(RBS::AST::Declarations::Class)
parent_class = declaration.super_class&.name&.name&.to_s
Entry::Class.new(nesting, file_path, location, location, comments, parent_class)
Expand Down Expand Up @@ -103,7 +103,7 @@ def handle_method(member, owner)
name = member.name.name
file_path = member.location.buffer.name
location = to_ruby_indexer_location(member.location)
comments = Array(member.comment&.string)
comments = member.comment&.string || ""

visibility = case member.visibility
when :private
Expand Down
20 changes: 10 additions & 10 deletions lib/ruby_indexer/test/classes_and_modules_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions lib/ruby_indexer/test/constant_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions lib/ruby_indexer/test/method_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -355,9 +355,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")
Expand Down
2 changes: 1 addition & 1 deletion lib/ruby_lsp/requests/support/common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,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
Expand Down

0 comments on commit f3be528

Please sign in to comment.