Improve pin caching

lib/solargraph/doc_map.rb

@@ -16,9 +16,6 @@ class DocMap
     # @return [Workspace]
     attr_reader :workspace
 
-    # @return [Environ]
-    attr_reader :environ
-
     # @param requires [Array<String>]
     # @param workspace [Workspace, nil]
     # @param out [IO, nil] output stream for logging

lib/solargraph/gem_pins.rb

@@ -43,10 +43,10 @@ def self.combine_method_pins(*pins)
       out
     end
 
-    # @param yard_pins [Array<Pin::Base>]
-    # @param rbs_pins [Array<Pin::Base>]
+    # @param yard_pins [Array<Pin::Method>]
+    # @param rbs_pins [Array<Pin::Method>]
     #
-    # @return [Array<Pin::Base>]
+    # @return [Array<Pin::Method>]
     def self.combine yard_pins, rbs_pins
       in_yard = Set.new
       rbs_store = Solargraph::ApiMap::Store.new(rbs_pins)

lib/solargraph/pin/parameter.rb

@@ -156,7 +156,7 @@ def full_name
       end
 
       def reset_generated!
-        @return_type = nil if param_tag
+        @return_type = nil if param_tag && @return_type&.undefined?
         super
       end
 

lib/solargraph/pin_cache.rb

@@ -99,8 +99,8 @@ def lookup_rbs_version_cache_key gemspec
 
     # @param gemspec [Gem::Specification, Bundler::LazySpecification]
     # @param rbs_version_cache_key [String, nil]
-    # @param yard_pins [Array<Pin::Base>]
-    # @param rbs_collection_pins [Array<Pin::Base>]
+    # @param yard_pins [Array<Pin::Method>]
+    # @param rbs_collection_pins [Array<Pin::Method>]
     # @return [void]
     def cache_combined_pins gemspec, rbs_version_cache_key, yard_pins, rbs_collection_pins
       combined_pins = GemPins.combine(yard_pins, rbs_collection_pins)
@@ -231,6 +231,7 @@ def log_cache_info gemspec,
 
     # @param gemspec [Gem::Specification]
     # @param out [StringIO, IO, nil]
+    #
     # @return [Array<Pin::Base>]
     def cache_yard_pins gemspec, out
       gem_yardoc_path = yardoc_path(gemspec)

lib/solargraph/rbs_map.rb

@@ -117,9 +117,13 @@ def self.from_gemspec gemspec, rbs_collection_path, rbs_collection_config_path
       return rbs_map if rbs_map.resolved?
 
       # try any version of the gem in the collection
-      RbsMap.new(gemspec.name, nil,
-                 rbs_collection_paths: [rbs_collection_path].compact,
-                 rbs_collection_config_path: rbs_collection_config_path)
+      rbs_map = RbsMap.new(gemspec.name, nil,
+                           rbs_collection_paths: [rbs_collection_path].compact,
+                           rbs_collection_config_path: rbs_collection_config_path)
+
+      return rbs_map if rbs_map.resolved?
+
+      StdlibMap.new(gemspec.name)
     end
 
     # @param out [IO, nil] where to log messages

lib/solargraph/workspace.rb

@@ -19,10 +19,6 @@ class Workspace
     # @return [String]
     attr_reader :directory
 
-    # @return [Array<String>]
-    attr_reader :gemnames
-    alias source_gems gemnames
-
     # @todo Remove '' and '*' special cases
     # @param directory [String]
     # @param config [Config, nil]
@@ -38,7 +34,6 @@ def initialize directory = '', config = nil, server = {}
       @config = config
       @server = server
       load_sources
-      @gemnames = []
       require_plugins
     end
 
@@ -70,6 +65,7 @@ def fetch_dependencies gemspec, out: $stderr
     end
 
     # @param require [String] The string sent to 'require' in the code to resolve, e.g. 'rails', 'bundler/require'
+    #
     # @return [Array<Gem::Specification>, nil]
     def resolve_require require
       gemspecs.resolve_require(require)

lib/solargraph/workspace/gemspecs.rb

@@ -70,7 +70,6 @@ def resolve_require require
               "Require path #{require} could not be resolved to a gem via find_by_path or guess of #{gem_name}"
             end
           end
-
           # look ourselves just in case this is hanging out somewhere
           # that find_by_path doesn't index
           gemspec = all_gemspecs.find do |spec|
@@ -79,6 +78,7 @@ def resolve_require require
             # @sg-ignore Translate to something flow sensitive typing understands
             spec&.files&.any? { |gemspec_file| file == gemspec_file }
           end
+
           # @sg-ignore flow sensitive typing should be able to handle redefinition
           return [gemspec_or_preference(gemspec)] if gemspec
         end
@@ -125,7 +125,15 @@ def fetch_dependencies gemspec, out: $stderr
         # @param runtime_dep [Gem::Dependency]
         # @param deps [Hash{String => Gem::Specification}]
         gem_dep_gemspecs = only_runtime_dependencies(gemspec).each_with_object(deps_so_far) do |runtime_dep, deps|
+          next if deps[runtime_dep.name]
+
+          Solargraph.logger.info "Adding #{runtime_dep.name} dependency for #{gemspec.name}"
+
           dep = find_gem(runtime_dep.name, runtime_dep.requirement)
+          dep ||= Gem::Specification.find_by_name(runtime_dep.name, runtime_dep.requirement)
+        rescue Gem::MissingSpecError
+          dep = resolve_gem_ignoring_local_bundle runtime_dep.name, out: out
+        ensure
           next unless dep
 
           fetch_dependencies(dep, out: out).each { |sub_dep| deps[sub_dep.name] ||= sub_dep }
@@ -170,6 +178,7 @@ def to_gem_specification specish
         # print time including milliseconds
         self.class.gem_specification_cache[specish] ||= case specish
                                                         when Gem::Specification
+                                                          # yay!
                                                           specish
                                                         when Bundler::LazySpecification
                                                           # materializing didn't work.  Let's look in the local
@@ -186,6 +195,11 @@ def to_gem_specification specish
                                                             # A Bundler::StubSpecification is a Bundler::
                                                             # RemoteSpecification which ought to proxy a Gem::
                                                             # Specification
+                                                            @@warned_on_gem_type ||= false
+                                                            unless @@warned_on_gem_type
+                                                              logger.warn "Unexpected type while resolving gem: #{specish.class}"
+                                                              @@warned_on_gem_type = true
+                                                            end
                                                             specish
                                                           end
                                                         # @sg-ignore Unresolved constant Gem::StubSpecification

solargraph.gemspec

@@ -57,6 +57,7 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'public_suffix', '~> 3.1'
   s.add_development_dependency 'rake', '~> 13.2'
   s.add_development_dependency 'rspec', '~> 3.5'
+  s.add_development_dependency 'rspec-time-guard', '~>0.2.0'
   #
   # very specific development-time RuboCop version patterns for CI
   # stability - feel free to update in an isolated PR

spec/api_map_method_spec.rb

@@ -117,15 +117,15 @@ class B
     end
   end
 
-  describe '#get_method_stack' do
+  describe '#get_method_stack', time_limit_seconds: 240 do
     let(:out) { StringIO.new }
     let(:api_map) { described_class.load_with_cache(Dir.pwd, out) }
 
-    context 'with stdlib that has vital dependencies' do
+    context 'with stdlib that has vital dependencies', time_limit_seconds: 240 do
       let(:external_requires) { ['yaml'] }
       let(:method_stack) { api_map.get_method_stack('YAML', 'safe_load', scope: :class) }
 
-      it 'handles the YAML gem aliased to Psych' do
+      it 'handles the YAML gem aliased to Psych', time_limit_seconds: 240 do
         expect(method_stack).not_to be_empty
       end
     end

spec/api_map_spec.rb

@@ -7,7 +7,7 @@
     @api_map = described_class.new
   end
 
-  it 'returns core methods' do
+  it 'returns core methods', time_limit_seconds: 120 do
     pins = @api_map.get_methods('String')
     expect(pins.map(&:path)).to include('String#upcase')
   end

spec/doc_map_spec.rb

@@ -67,14 +67,6 @@
     end
   end
 
-  it 'does not warn for redundant requires' do
-    # Requiring 'set' is unnecessary because it's already included in core. It
-    # might make sense to log redundant requires, but a warning is overkill.
-    allow(Solargraph.logger).to receive(:warn).and_call_original
-    described_class.new(['set'], workspace)
-    expect(Solargraph.logger).not_to have_received(:warn).with(/path set/)
-  end
-
   context 'when deserialization takes a while' do
     let(:pre_cache) { false }
     let(:requires) { ['backport'] }

spec/language_server/host_spec.rb

@@ -275,7 +275,7 @@ def initialize(foo); end
       expect(symbols).not_to be_empty
     end
 
-    it 'opens a file outside of prepared libraries' do
+    it 'opens a file outside of prepared libraries', time_limit_seconds: 120 do
       @host.prepare(File.absolute_path(File.join('spec', 'fixtures', 'workspace')))
       @host.open('file:///file.rb', 'class Foo; end', 1)
       symbols = @host.document_symbols('file:///file.rb')

spec/language_server/protocol_spec.rb

@@ -424,7 +424,7 @@ def bar baz
     expect(response['result']['available']).to be_a(String)
   end
 
-  it 'handles $/solargraph/documentGems' do
+  it 'handles $/solargraph/documentGems', time_limit_seconds: 120 do
     @protocol.request '$/solargraph/documentGems', {}
     response = @protocol.response
     expect(response['error']).to be_nil

spec/spec_helper.rb

@@ -2,12 +2,12 @@
 
 require 'bundler/setup'
 require 'webmock/rspec'
+require 'rspec_time_guard'
 WebMock.disable_net_connect!(allow_localhost: true)
 unless ENV['SIMPLECOV_DISABLED']
   # set up lcov reporting for undercover
   require 'simplecov'
   require 'undercover/simplecov_formatter'
-
   SimpleCov.start do
     cname = ENV.fetch('TEST_COVERAGE_COMMAND_NAME', 'ad-hoc')
     command_name cname
@@ -27,6 +27,12 @@
   # Allow use of --only-failures with rspec, handy for local development
   c.example_status_persistence_file_path = 'rspec-examples.txt'
 end
+RspecTimeGuard.setup
+RspecTimeGuard.configure do |config|
+  config.global_time_limit_seconds = 60
+  config.continue_on_timeout = false
+end
+
 require 'solargraph'
 # execute any logging blocks to make sure they don't blow up
 Solargraph::Logging.logger.sev_threshold = Logger::DEBUG

spec/workspace/gemspecs_fetch_dependencies_spec.rb

@@ -77,15 +77,15 @@
     context 'with gem that exists in our bundle' do
       let(:gem_name) { 'undercover' }
 
-      it 'finds dependencies' do
+      it 'finds dependencies', time_limit_seconds: 120 do
         expect(deps.map(&:name)).to include('ast')
       end
     end
 
     context 'with gem does not exist in our bundle' do
       let(:gem_name) { 'activerecord' }
 
-      it 'gives a useful message' do
+      it 'gives a useful message', time_limit_seconds: 120 do
         dep_names = nil
         output = capture_both { dep_names = deps.map(&:name) }
         expect(output).to include('Please install the gem activerecord')

spec/workspace_spec.rb

@@ -148,13 +148,17 @@
 
   describe '#cache_all_for_workspace!' do
     let(:pin_cache) { instance_double(Solargraph::PinCache) }
+    let(:gemspecs)  { instance_double(Solargraph::Workspace::Gemspecs) }
 
     before do
       allow(Solargraph::PinCache).to receive(:cache_core)
       allow(Solargraph::PinCache).to receive(:possible_stdlibs)
       allow(Solargraph::PinCache).to receive(:new).and_return(pin_cache)
       allow(pin_cache).to receive_messages(cache_gem: nil, possible_stdlibs: [])
       allow(Solargraph::PinCache).to receive(:cache_all_stdlibs)
+      allow(Solargraph::Workspace::Gemspecs).to receive(:new).and_return(gemspecs)
+      gemspec = instance_double(Gem::Specification, name: 'test_gem', version: '1.0.0')
+      allow(gemspecs).to receive(:all_gemspecs_from_bundle).and_return([gemspec])
     end
 
     it 'caches core pins' do
@@ -166,5 +170,18 @@
 
       expect(Solargraph::PinCache).to have_received(:cache_core).with(out: nil)
     end
+
+    it 'caches gems' do
+      allow(pin_cache).to receive(:cached?).and_return(false)
+
+      allow(pin_cache).to receive(:cache_all_stdlibs).with(out: nil, rebuild: false)
+
+      allow(Solargraph::PinCache).to receive_messages(core?: true,
+                                                      possible_stdlibs: [])
+
+      workspace.cache_all_for_workspace!(nil, rebuild: false)
+
+      expect(pin_cache).to have_received(:cache_gem)
+    end
   end
 end