Skip expensive parser gem YARD generation when RBS collection available

lib/solargraph/doc_map.rb

@@ -107,7 +107,8 @@ def cache_rbs_collection_pins gemspec, out
     # @param out [IO, StringIO, nil] output stream for logging
     # @return [void]
     def cache gemspec, rebuild: false, out: nil
-      build_yard = uncached_yard_gemspecs.include?(gemspec) || rebuild
+      build_yard = (uncached_yard_gemspecs.include?(gemspec) || rebuild) &&
+                   !PinCache.suppress_yard_cache?(gemspec, rbs_cache_key(gemspec))
       build_rbs_collection = uncached_rbs_collection_gemspecs.include?(gemspec) || rebuild
       if build_yard || build_rbs_collection
         type = []
@@ -161,6 +162,11 @@ def combined_pins_in_memory
       self.class.all_combined_pins_in_memory
     end
 
+    # @return [Hash{String => String}] Indexed by gemspec full name
+    def rbs_cache_keys
+      @rbs_cache_keys ||= {}
+    end
+
     # @return [Array<String>]
     def yard_plugins
       @environ.yard_plugins
@@ -211,6 +217,13 @@ def preference_map
       @preference_map ||= preferences.to_h { |gemspec| [gemspec.name, gemspec] }
     end
 
+    # @param gemspec [Gem::Specification]
+    # @return [String]
+    def rbs_cache_key gemspec
+      rbs_cache_keys[gemspec.full_name] ||=
+        RbsMap.from_gemspec(gemspec, rbs_collection_path, rbs_collection_config_path).cache_key
+    end
+
     # @param gemspec [Gem::Specification]
     # @return [Array<Pin::Base>, nil]
     def deserialize_yard_pin_cache gemspec
@@ -237,8 +250,7 @@ def deserialize_combined_pin_cache gemspec
         return combined_pins_in_memory[[gemspec.name, gemspec.version]]
       end
 
-      rbs_map = RbsMap.from_gemspec(gemspec, rbs_collection_path, rbs_collection_config_path)
-      rbs_version_cache_key = rbs_map.cache_key
+      rbs_version_cache_key = rbs_cache_key(gemspec)
 
       cached = PinCache.deserialize_combined_gem(gemspec, rbs_version_cache_key)
       if cached
@@ -249,11 +261,14 @@ def deserialize_combined_pin_cache gemspec
 
       rbs_collection_pins = deserialize_rbs_collection_cache gemspec, rbs_version_cache_key
 
-      yard_pins = deserialize_yard_pin_cache gemspec
+      # A suppressed gem never gets a YARD cache, so combine against an
+      # empty set rather than treating its absence as a cache miss.
+      suppress_yard = PinCache.suppress_yard_cache?(gemspec, rbs_version_cache_key)
+      yard_pins = deserialize_yard_pin_cache(gemspec) unless suppress_yard
 
-      if !rbs_collection_pins.nil? && !yard_pins.nil?
+      if !rbs_collection_pins.nil? && (suppress_yard || !yard_pins.nil?)
         logger.debug { "Combining pins for #{gemspec.name}:#{gemspec.version}" }
-        combined_pins = GemPins.combine(yard_pins, rbs_collection_pins)
+        combined_pins = GemPins.combine(yard_pins || [], rbs_collection_pins)
         PinCache.serialize_combined_gem(gemspec, rbs_version_cache_key, combined_pins)
         combined_pins_in_memory[[gemspec.name, gemspec.version]] = combined_pins
         logger.info { "Generated #{combined_pins_in_memory[[gemspec.name, gemspec.version]].length} combined pins for #{gemspec.name} #{gemspec.version}" }

lib/solargraph/pin_cache.rb

@@ -5,6 +5,11 @@
 
 module Solargraph
   module PinCache
+    # Gems whose YARD build costs far more than the pins are worth once
+    # their RBS collection types resolve; `parser` alone takes over a
+    # minute.
+    YARD_SUPPRESSED_GEMS = ['parser'].freeze
+
     class << self
       include Logging
 
@@ -102,6 +107,18 @@ def has_yard? gemspec
         exist?(yard_gem_path(gemspec))
       end
 
+      # Whether YARD pins for this gem should be skipped in favor of its
+      # RBS collection pins alone.
+      #
+      # @param gemspec [Gem::Specification, Bundler::LazySpecification]
+      # @param rbs_version_cache_key [String, nil]
+      # @return [Boolean]
+      def suppress_yard_cache? gemspec, rbs_version_cache_key
+        return false unless YARD_SUPPRESSED_GEMS.include?(gemspec.name)
+
+        rbs_version_cache_key != RbsMap::CACHE_KEY_UNRESOLVED
+      end
+
       # @param gemspec [Gem::Specification]
       # @param hash [String, nil]
       # @return [String]

lib/solargraph/shell.rb

@@ -111,13 +111,15 @@ def clear
     def cache gem, version = nil
       gemspec = Gem::Specification.find_by_name(gem, version)
 
-      if options[:rebuild] || !PinCache.has_yard?(gemspec)
+      workspace = Solargraph::Workspace.new(Dir.pwd) if File.exist?('rbs_collection.yaml')
+      rbs_map = RbsMap.from_gemspec(gemspec, workspace&.rbs_collection_path, workspace&.rbs_collection_config_path)
+
+      if (options[:rebuild] || !PinCache.has_yard?(gemspec)) &&
+         !PinCache.suppress_yard_cache?(gemspec, rbs_map.cache_key)
         pins = GemPins.build_yard_pins(['yard-activesupport-concern'], gemspec)
         PinCache.serialize_yard_gem(gemspec, pins)
       end
 
-      workspace = Solargraph::Workspace.new(Dir.pwd) if File.exist?('rbs_collection.yaml')
-      rbs_map = RbsMap.from_gemspec(gemspec, workspace&.rbs_collection_path, workspace&.rbs_collection_config_path)
       if options[:rebuild] || !PinCache.has_rbs_collection?(gemspec, rbs_map.cache_key)
         PinCache.serialize_rbs_collection_gem(gemspec, rbs_map.cache_key, rbs_map.pins)
       end
@@ -198,13 +200,15 @@ def gems *names
           if gemspec.nil?
             warn "Gem '#{name}' not found"
           else
-            if options[:rebuild] || !PinCache.has_yard?(gemspec)
+            workspace = Solargraph::Workspace.new(Dir.pwd)
+            rbs_map = RbsMap.from_gemspec(gemspec, workspace.rbs_collection_path, workspace.rbs_collection_config_path)
+
+            if (options[:rebuild] || !PinCache.has_yard?(gemspec)) &&
+               !PinCache.suppress_yard_cache?(gemspec, rbs_map.cache_key)
               pins = GemPins.build_yard_pins(['yard-activesupport-concern'], gemspec)
               PinCache.serialize_yard_gem(gemspec, pins)
             end
 
-            workspace = Solargraph::Workspace.new(Dir.pwd)
-            rbs_map = RbsMap.from_gemspec(gemspec, workspace.rbs_collection_path, workspace.rbs_collection_config_path)
             if options[:rebuild] || !PinCache.has_rbs_collection?(gemspec, rbs_map.cache_key)
               # cache pins even if result is zero, so we don't retry building pins
               pins = rbs_map.pins || []
@@ -604,13 +608,15 @@ def do_cache gemspec, rebuild: false
       if gemspec.nil?
         warn "Gem '#{gemspec&.name}' not found"
       else
-        if rebuild || !PinCache.has_yard?(gemspec)
+        workspace = Solargraph::Workspace.new(Dir.pwd)
+        rbs_map = RbsMap.from_gemspec(gemspec, workspace.rbs_collection_path, workspace.rbs_collection_config_path)
+
+        if (rebuild || !PinCache.has_yard?(gemspec)) &&
+           !PinCache.suppress_yard_cache?(gemspec, rbs_map.cache_key)
           pins = GemPins.build_yard_pins(['yard-activesupport-concern'], gemspec)
           PinCache.serialize_yard_gem(gemspec, pins)
         end
 
-        workspace = Solargraph::Workspace.new(Dir.pwd)
-        rbs_map = RbsMap.from_gemspec(gemspec, workspace.rbs_collection_path, workspace.rbs_collection_config_path)
         if rebuild || !PinCache.has_rbs_collection?(gemspec, rbs_map.cache_key)
           # cache pins even if result is zero, so we don't retry building pins
           pins = rbs_map.pins || []

spec/doc_map_spec.rb

@@ -146,4 +146,24 @@ def global doc_map
       Solargraph::Convention.unregister dummy_convention
     end
   end
+
+  context 'with a gem whose RBS collection types make YARD redundant' do
+    let(:pre_cache) { false }
+
+    it 'skips the YARD build even when asked to rebuild' do
+      allow(Solargraph::GemPins).to receive(:build_yard_pins).and_return([])
+
+      doc_map.cache(Gem::Specification.find_by_name('parser'), rebuild: true, out: nil)
+
+      expect(Solargraph::GemPins).not_to have_received(:build_yard_pins)
+    end
+
+    it 'still builds RBS collection pins' do
+      allow(Solargraph::PinCache).to receive(:serialize_rbs_collection_gem)
+
+      doc_map.cache(Gem::Specification.find_by_name('parser'), rebuild: true, out: nil)
+
+      expect(Solargraph::PinCache).to have_received(:serialize_rbs_collection_gem)
+    end
+  end
 end

spec/pin_cache_spec.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+describe Solargraph::PinCache do
+  describe '.suppress_yard_cache?' do
+    let(:parser_gemspec) { Gem::Specification.new('parser', '3.3.7.1') }
+    let(:other_gemspec) { Gem::Specification.new('backport', '1.2.0') }
+
+    it 'suppresses YARD when the gem has resolved RBS collection types' do
+      expect(described_class.suppress_yard_cache?(parser_gemspec,
+                                                  Solargraph::RbsMap::CACHE_KEY_GEM_EXPORT)).to be true
+    end
+
+    it 'suppresses YARD for any resolved RBS source, including a collection digest' do
+      expect(described_class.suppress_yard_cache?(parser_gemspec, 'abc123')).to be true
+    end
+
+    it 'builds YARD when the gem has no RBS types to fall back on' do
+      expect(described_class.suppress_yard_cache?(parser_gemspec,
+                                                  Solargraph::RbsMap::CACHE_KEY_UNRESOLVED)).to be false
+    end
+
+    it 'builds YARD for a gem outside the suppression list' do
+      expect(described_class.suppress_yard_cache?(other_gemspec,
+                                                  Solargraph::RbsMap::CACHE_KEY_GEM_EXPORT)).to be false
+    end
+  end
+end

spec/shell_spec.rb

@@ -286,6 +286,58 @@ def bundle_exec(*cmd)
     end
   end
 
+  describe '#do_cache' do
+    let(:pin) { Solargraph::Pin::Namespace.new(name: 'Foo') }
+
+    before do
+      workspace = instance_double(Solargraph::Workspace, rbs_collection_path: nil, rbs_collection_config_path: nil)
+      allow(Solargraph::Workspace).to receive(:new).and_return(workspace)
+      allow(Solargraph::PinCache).to receive_messages(has_yard?: false, has_rbs_collection?: false)
+      allow(Solargraph::PinCache).to receive(:serialize_yard_gem)
+      allow(Solargraph::PinCache).to receive(:serialize_rbs_collection_gem)
+      allow(Solargraph::GemPins).to receive(:build_yard_pins).and_return([pin])
+    end
+
+    # @param name [String]
+    # @param cache_key [String]
+    # @return [Gem::Specification] the gemspec handed to do_cache
+    def cache_gem name, cache_key
+      gemspec = instance_double(Gem::Specification, name: name)
+      rbs_map = instance_double(Solargraph::RbsMap, cache_key: cache_key, pins: [pin])
+      allow(Solargraph::RbsMap).to receive(:from_gemspec).and_return(rbs_map)
+      shell.send(:do_cache, gemspec)
+      gemspec
+    end
+
+    it 'skips the YARD build when the RBS collection resolves types' do
+      cache_gem('parser', 'resolved-key')
+
+      expect(Solargraph::GemPins).not_to have_received(:build_yard_pins)
+    end
+
+    it 'caches RBS collection pins for a gem whose YARD build was skipped' do
+      gemspec = cache_gem('parser', 'resolved-key')
+
+      expect(Solargraph::PinCache).to have_received(:serialize_rbs_collection_gem).with(gemspec, 'resolved-key', [pin])
+    end
+
+    it 'builds YARD pins when the RBS collection cannot resolve types' do
+      gemspec = cache_gem('parser', Solargraph::RbsMap::CACHE_KEY_UNRESOLVED)
+
+      expect(Solargraph::PinCache).to have_received(:serialize_yard_gem).with(gemspec, [pin])
+    end
+
+    it 'builds YARD pins for a gem outside the suppression list' do
+      gemspec = cache_gem('rspec', 'resolved-key')
+
+      expect(Solargraph::PinCache).to have_received(:serialize_yard_gem).with(gemspec, [pin])
+    end
+
+    it 'warns when the gemspec is missing' do
+      expect { shell.send(:do_cache, nil) }.to output(/not found/).to_stderr
+    end
+  end
+
   context 'with unbundled environments' do
     let!(:command_path) { File.realpath(File.join('spec', 'fixtures', 'shim.rb')) }
     let!(:unbundled_env) { Bundler.unbundled_env.merge({ 'BUNDLE_GEMFILE' => nil }) }