Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include convention (eg. solargraph-rspec) pins in document_symbols #724

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ class Solargraph::LanguageServer::Message::TextDocument::DocumentSymbol < Solarg
def process
pins = host.document_symbols params['textDocument']['uri']
info = pins.map do |pin|
next nil unless pin.location&.filename
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

solargraph-rspec happens to be adding some RSpec DSL methods without a filename, which should be excluded.


result = {
name: pin.name,
containerName: pin.namespace,
Expand All @@ -17,7 +19,8 @@ def process
deprecated: pin.deprecated?
}
result
end
end.compact

set_result info
end
end
17 changes: 15 additions & 2 deletions lib/solargraph/source_map.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def initialize source, pins, locals
@pins = pins
@locals = locals
environ.merge Convention.for_local(self) unless filename.nil?
self.convention_pins = environ.pins
@pin_class_hash = pins.to_set.classify(&:class).transform_values(&:to_a)
@pin_select_cache = {}
end
Expand Down Expand Up @@ -65,11 +66,12 @@ def environ
@environ ||= Environ.new
end

# all pins except Solargraph::Pin::Reference::Reference
# @return [Array<Pin::Base>]
def document_symbols
@document_symbols ||= pins.select { |pin|
@document_symbols ||= (pins + convention_pins).select do |pin|
pin.path && !pin.path.empty?
}
end
end

# @param query [String]
Expand Down Expand Up @@ -159,6 +161,17 @@ def map source

private

# @return [Array<Pin::Base>]
def convention_pins
@convention_pins || []
end

def convention_pins=(pins)
# unmemoizing the document_symbols in case it was called from any of convnetions
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, solargraph-rails:

https://github.com/iftheshoefritz/solargraph-rails/blob/144cec0d91028b8073029df0bbaf58f6a2adbb3b/lib/solargraph-rails.rb#L39

This leads to subsequent convention pins not being included, hence we need to clear the memorization after all convention pins are collected.

@document_symbols = nil
@convention_pins = pins
end

# @param line [Integer]
# @param character [Integer]
# @param klasses [Array<Class>]
Expand Down
43 changes: 43 additions & 0 deletions spec/source_map_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,49 @@ def baz_qux; end
expect(map.query_symbols("bazqux")).to eq(map.document_symbols.select{ |pin_namespace| pin_namespace.name == "baz_qux" })
end

it 'returns all pins, except for references as document symbols' do
map = Solargraph::SourceMap.load_string(%(
class FooBar
require 'foo'
include SomeModule
extend SomeOtherModule

def baz_qux; end
end
), 'test.rb')

expect(map.document_symbols.map(&:path)).to eq(['FooBar', 'FooBar#baz_qux'])
expect(map.document_symbols.map(&:class)).not_to include(an_instance_of(Solargraph::Pin::Reference))
end

it 'includes convention pins in document symbols' do
dummy_convention = Class.new(Solargraph::Convention::Base) do
def local(source_map)
source_map.document_symbols # call memoized method

Solargraph::Environ.new(
pins: [
Solargraph::Pin::Method.new(
closure: Solargraph::Pin::Namespace.new(name: 'FooBar', type: :class),
name: 'baz_convention',
scope: :instance
)
]
)
end
end

Solargraph::Convention.register dummy_convention

map = Solargraph::SourceMap.load_string(%(
class FooBar
def baz_qux; end
end
), 'test.rb')

expect(map.document_symbols.map(&:path)).to include('FooBar#baz_convention')
end

it "locates block pins" do
map = Solargraph::SourceMap.load_string(%(
class Foo
Expand Down