Skip to content

Commit db08df2

Browse files
authored
[regression] Add back bundler/require support for solargraph-rails (#941)
* Add back bundler/require support for solargraph-rails * Fix type issue in error message * Use Gem.loaded_specs for dependencies as well * Use Gem.loaded_specs for dependencies as well * Switch to using Bundler API * Fix error handling, add logging * Update spec
1 parent c3147fa commit db08df2

File tree

3 files changed

+59
-18
lines changed

3 files changed

+59
-18
lines changed

lib/solargraph/doc_map.rb

Lines changed: 42 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ module Solargraph
44
# A collection of pins generated from required gems.
55
#
66
class DocMap
7+
include Logging
8+
79
# @return [Array<String>]
810
attr_reader :requires
911

@@ -28,12 +30,12 @@ def initialize(requires, preferences, rbs_path = nil)
2830

2931
# @return [Array<Gem::Specification>]
3032
def gemspecs
31-
@gemspecs ||= required_gem_map.values.compact
33+
@gemspecs ||= required_gems_map.values.compact.flatten
3234
end
3335

3436
# @return [Array<String>]
3537
def unresolved_requires
36-
@unresolved_requires ||= required_gem_map.select { |_, gemspec| gemspec.nil? }.keys
38+
@unresolved_requires ||= required_gems_map.select { |_, gemspecs| gemspecs.nil? }.keys
3739
end
3840

3941
# @return [Hash{Gem::Specification => Array[Pin::Base]}]
@@ -52,20 +54,22 @@ def dependencies
5254
def generate
5355
@pins = []
5456
@uncached_gemspecs = []
55-
required_gem_map.each do |path, gemspec|
56-
if gemspec
57-
try_cache gemspec
58-
else
57+
required_gems_map.each do |path, gemspecs|
58+
if gemspecs.nil?
5959
try_stdlib_map path
60+
else
61+
gemspecs.each do |gemspec|
62+
try_cache gemspec
63+
end
6064
end
6165
end
6266
dependencies.each { |dep| try_cache dep }
6367
@uncached_gemspecs.uniq!
6468
end
6569

66-
# @return [Hash{String => Gem::Specification, nil}]
67-
def required_gem_map
68-
@required_gem_map ||= requires.to_h { |path| [path, resolve_path_to_gemspec(path)] }
70+
# @return [Hash{String => Array<Gem::Specification>}]
71+
def required_gems_map
72+
@required_gems_map ||= requires.to_h { |path| [path, resolve_path_to_gemspecs(path)] }
6973
end
7074

7175
# @return [Hash{String => Gem::Specification}]
@@ -125,10 +129,26 @@ def update_from_collection gemspec, gempins
125129
end
126130

127131
# @param path [String]
128-
# @return [Gem::Specification, nil]
129-
def resolve_path_to_gemspec path
132+
# @return [::Array<Gem::Specification>, nil]
133+
def resolve_path_to_gemspecs path
130134
return nil if path.empty?
131135

136+
if path == 'bundler/require'
137+
# find only the gems bundler is now using
138+
gemspecs = Bundler.definition.locked_gems.specs.flat_map do |lazy_spec|
139+
logger.info "Handling #{lazy_spec.name}:#{lazy_spec.version} from #{path}"
140+
[Gem::Specification.find_by_name(lazy_spec.name, lazy_spec.version)]
141+
rescue Gem::MissingSpecError => e
142+
logger.info("Could not find #{lazy_spec.name}:#{lazy_spec.version} with find_by_name, falling back to guess")
143+
# can happen in local filesystem references
144+
specs = resolve_path_to_gemspecs lazy_spec.name
145+
logger.info "Gem #{lazy_spec.name} #{lazy_spec.version} from bundle not found: #{e}" if specs.nil?
146+
next specs
147+
end.compact
148+
149+
return gemspecs
150+
end
151+
132152
gemspec = Gem::Specification.find_by_path(path)
133153
if gemspec.nil?
134154
gem_name_guess = path.split('/').first
@@ -142,16 +162,17 @@ def resolve_path_to_gemspec path
142162
gemspec = potential_gemspec if potential_gemspec.files.any? { |gemspec_file| file == gemspec_file }
143163
rescue Gem::MissingSpecError
144164
Solargraph.logger.debug "Require path #{path} could not be resolved to a gem via find_by_path or guess of #{gem_name_guess}"
145-
nil
165+
[]
146166
end
147167
end
148-
gemspec_or_preference gemspec
168+
return nil if gemspec.nil?
169+
[gemspec_or_preference(gemspec)]
149170
end
150171

151-
# @param gemspec [Gem::Specification, nil]
152-
# @return [Gem::Specification, nil]
172+
# @param gemspec [Gem::Specification]
173+
# @return [Gem::Specification]
153174
def gemspec_or_preference gemspec
154-
return gemspec unless gemspec && preference_map.key?(gemspec.name)
175+
return gemspec unless preference_map.key?(gemspec.name)
155176
return gemspec if gemspec.version == preference_map[gemspec.name].version
156177

157178
change_gemspec_version gemspec, preference_map[by_path.name].version
@@ -170,12 +191,15 @@ def change_gemspec_version gemspec, version
170191
# @param gemspec [Gem::Specification]
171192
# @return [Array<Gem::Specification>]
172193
def fetch_dependencies gemspec
194+
# @param spec [Gem::Dependency]
173195
only_runtime_dependencies(gemspec).each_with_object(Set.new) do |spec, deps|
174196
Solargraph.logger.info "Adding #{spec.name} dependency for #{gemspec.name}"
175-
dep = Gem::Specification.find_by_name(spec.name, spec.requirement)
197+
dep = Gem.loaded_specs[spec.name]
198+
# @todo is next line necessary?
199+
dep ||= Gem::Specification.find_by_name(spec.name, spec.requirement)
176200
deps.merge fetch_dependencies(dep) if deps.add?(dep)
177201
rescue Gem::MissingSpecError
178-
Solargraph.logger.warn "Gem dependency #{spec.name} #{spec.requirements} for #{gemspec.name} not found."
202+
Solargraph.logger.warn "Gem dependency #{spec.name} #{spec.requirement} for #{gemspec.name} not found."
179203
end.to_a
180204
end
181205

spec/doc_map_spec.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,14 @@
2929
expect(doc_map.uncached_gemspecs).to eq([gemspec])
3030
end
3131

32+
it 'imports all gems when bundler/require used' do
33+
plain_doc_map = Solargraph::DocMap.new([], [])
34+
35+
doc_map_with_bundler_require = Solargraph::DocMap.new(['bundler/require'], [])
36+
37+
expect(doc_map_with_bundler_require.pins.length - plain_doc_map.pins.length).to be_positive
38+
end
39+
3240
it 'does not warn for redundant requires' do
3341
# Requiring 'set' is unnecessary because it's already included in core. It
3442
# might make sense to log redundant requires, but a warning is overkill.

spec/source_map/mapper_spec.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1394,6 +1394,15 @@ def meth; end
13941394
expect(pin.name).to eq('bundler/require')
13951395
end
13961396

1397+
it 'maps Bundler.require in Rails context to require "bundler/require"' do
1398+
map = Solargraph::SourceMap.load_string(%(
1399+
Bundler.require(*Rails.groups)
1400+
))
1401+
pin = map.pins.select { |pin| pin.is_a?(Solargraph::Pin::Reference::Require) }.first
1402+
# not perfect - would be best if we did something with the argument to limit pins gathered
1403+
expect(pin.name).to eq('bundler/require')
1404+
end
1405+
13971406
it 'correctly orders optargs and blockargs' do
13981407
map = Solargraph::SourceMap.load_string(%(
13991408
def foo bar = nil, &block

0 commit comments

Comments
 (0)