pin CLI: resolve paths through method lookup, unsilence messages, fix bare arrows

lib/solargraph/complex_type/unique_type.rb

@@ -312,7 +312,10 @@ def to_rbs
         elsif name.downcase == 'nil'
           'nil'
         elsif name == GENERIC_TAG_NAME
-          all_params.first&.name
+          # A generic with no parameter would otherwise render nil, which
+          # callers interpolate into signatures as an empty string
+          # ("def foo: () -> ")
+          all_params.first&.name || 'untyped'
         elsif %w[Class Module].include?(name)
           rbs_name
         elsif %w[Tuple Array].include?(name) && fixed_parameters?

lib/solargraph/logging.rb

@@ -4,24 +4,30 @@
 
 module Solargraph
   module Logging
+    # @type [Integer]
     DEFAULT_LOG_LEVEL = Logger::WARN
 
+    # @type [Hash{String => Integer}]
     LOG_LEVELS = {
       'warn' => Logger::WARN,
       'info' => Logger::INFO,
       'debug' => Logger::DEBUG
     }.freeze
-    configured_level = ENV.fetch('SOLARGRAPH_LOG', nil)
-    level = if LOG_LEVELS.keys.include?(configured_level)
-              LOG_LEVELS.fetch(configured_level)
-            else
-              if configured_level
-                warn "Invalid value for SOLARGRAPH_LOG: #{configured_level.inspect} - " \
+    # @param configured_level [String, nil]
+    # @return [Integer]
+    # @sg-ignore https://github.com/apiology/solargraph/pull/65
+    # @sg-ignore https://github.com/castwide/solargraph/pull/1223
+    def self.resolve_level configured_level = ENV.fetch('SOLARGRAPH_LOG', nil)
+      return LOG_LEVELS.fetch(configured_level) if LOG_LEVELS.key?(configured_level)
+
+      if configured_level
+        $stderr.puts "Invalid value for SOLARGRAPH_LOG: #{configured_level.inspect} - " \
                      "valid values are #{LOG_LEVELS.keys}"
-              end
-              DEFAULT_LOG_LEVEL
-            end
-    @@logger = Logger.new($stderr, level: level)
+      end
+      DEFAULT_LOG_LEVEL
+    end
+
+    @@logger = Logger.new($stderr, level: resolve_level)
     # @sg-ignore Fix cvar issue
     @@logger.formatter = proc do |severity, _datetime, _progname, msg|
       "[#{severity}] #{msg}\n"

lib/solargraph/pin/base.rb

@@ -245,7 +245,7 @@ def choose other, attr
         return results.first if results.any? { |r| r.is_a? AST::Node }
         results.min
       rescue StandardError
-        warn("Problem handling #{attr} for \n#{inspect}\n and \n#{other.inspect}\n\n#{send(attr).inspect} vs #{other.send(attr).inspect}")
+        logger.warn("Problem handling #{attr} for \n#{inspect}\n and \n#{other.inspect}\n\n#{send(attr).inspect} vs #{other.send(attr).inspect}")
         raise
       end
 

lib/solargraph/shell.rb

@@ -122,7 +122,7 @@ def cache gem, version = nil
         PinCache.serialize_rbs_collection_gem(gemspec, rbs_map.cache_key, rbs_map.pins)
       end
     rescue Gem::MissingSpecError
-      warn "Gem '#{gem}' not found"
+      $stderr.puts "Gem '#{gem}' not found"
     end
 
     desc 'uncache GEM [...GEM]', 'Delete specific cached gem documentation'
@@ -186,7 +186,7 @@ def gems *names
         Gem::Specification.to_a.each { |spec| do_cache spec, rebuild: options[:rebuild] }
         $stderr.puts "Documentation cached for all #{Gem::Specification.count} gems."
       else
-        warn("Caching these gems: #{names}")
+        $stderr.puts("Caching these gems: #{names}")
         names.each do |name|
           if name == 'core'
             # @sg-ignore cache_core and core? are dynamically defined
@@ -196,7 +196,7 @@ def gems *names
 
           gemspec = workspace.find_gem(*name.split('='))
           if gemspec.nil?
-            warn "Gem '#{name}' not found"
+            $stderr.puts "Gem '#{name}' not found"
           else
             if options[:rebuild] || !PinCache.has_yard?(gemspec)
               pins = GemPins.build_yard_pins(['yard-activesupport-concern'], gemspec)
@@ -212,14 +212,14 @@ def gems *names
             end
           end
         rescue Gem::MissingSpecError
-          warn "Gem '#{name}' not found"
+          $stderr.puts "Gem '#{name}' not found"
         rescue Gem::Requirement::BadRequirementError => e
-          warn "Gem '#{name}' failed while loading"
-          warn e.message
+          $stderr.puts "Gem '#{name}' failed while loading"
+          $stderr.puts e.message
           # @sg-ignore Need to add nil check here
-          warn e.backtrace.join("\n")
+          $stderr.puts e.backtrace.join("\n")
         end
-        warn "Documentation cached for #{names.count} gems."
+        $stderr.puts "Documentation cached for #{names.count} gems."
       end
     end
 
@@ -303,13 +303,13 @@ def scan
         rescue StandardError => e
           # @todo to add nil check here
           # @todo should warn on nil dereference below
-          warn "Error testing #{pin_description(pin)} #{if pin.location
+          $stderr.puts "Error testing #{pin_description(pin)} #{if pin.location
                                                           "at #{pin.location.filename}:#{pin.location.range.start.line + 1}"
                                                         end}"
-          warn "[#{e.class}]: #{e.message}"
+          $stderr.puts "[#{e.class}]: #{e.message}"
           # @todo Need to add nil check here
           # @todo flow sensitive typing should be able to handle redefinition
-          warn e.backtrace.join("\n")
+          $stderr.puts e.backtrace.join("\n")
           exit 1
         end
       end
@@ -336,32 +336,37 @@ def list
                    default: false
     option :stack, type: :boolean, desc: 'Show entire stack of a method pin by including definitions in superclasses',
                    default: false
+    option :resolve, type: :boolean, default: true,
+                     desc: 'Follow Ruby method lookup when the path names no pin of its own, describing the ' \
+                           'definition a call would reach; --no-resolve describes only the exact path'
     # @param path [String] The path to the method pin, e.g. 'Class#method' or 'Class.method'
     # @return [void]
     def pin path
       api_map = Solargraph::ApiMap.load_with_cache('.', $stderr)
-      is_method = path.include?('#') || path.include?('.')
-      if is_method && options[:stack]
-        scope, ns, meth = if path.include? '#'
-                            [:instance, *path.split('#', 2)]
-                          else
-                            [:class, *path.split('.', 2)]
-                          end
-
-        # @sg-ignore Wrong argument type for
-        #   Solargraph::ApiMap#get_method_stack: rooted_tag
-        #   expected String, received Array<String>
-        pins = api_map.get_method_stack(ns, meth, scope: scope)
-      else
-        pins = api_map.get_path_pins path
-      end
+      pins = if options[:stack] && path.match?(/[#.]/)
+               method_stack_for_path(api_map, path)
+             else
+               api_map.get_path_pins path
+             end
       # @type [Hash{Symbol => Pin::Base}]
       references = {}
       pin = pins.first
+      if pin.nil?
+        # --stack already walks the ancestry, so resolution applies only to
+        # the single-pin default.
+        resolved = options[:stack] || !options[:resolve] ? nil : method_stack_for_path(api_map, path).first
+        if resolved.nil?
+          # $stderr.puts instead of Kernel#warn: bin/solargraph disables Ruby
+          # warnings ($VERBOSE = nil), which also silences Kernel#warn, so
+          # warn-based CLI messages never reach the user.
+          $stderr.puts "Pin not found for path '#{path}'"
+          exit 1
+        else
+          pins = [resolved]
+          pin = resolved
+        end
+      end
       case pin
-      when nil
-        warn "Pin not found for path '#{path}'"
-        exit 1
       when Pin::Namespace
         if options[:references]
           # @sg-ignore Need to add nil check here
@@ -587,6 +592,19 @@ def print_type type
       end
     end
 
+    # Resolve a method path through the receiver's ancestry, for suggesting
+    # (e.g.) 'Comparable#between?' when 'Integer#between?' has no pin of its
+    # own.
+    #
+    # @param api_map [Solargraph::ApiMap]
+    # @param path [String]
+    # @return [Array<Solargraph::Pin::Method>]
+    def method_stack_for_path api_map, path
+      ns, meth = path.split(/[#.]/, 2)
+      return [] if meth.nil?
+      api_map.get_method_stack(ns, meth, scope: path.include?('#') ? :instance : :class)
+    end
+
     # @param pin [Solargraph::Pin::Base]
     # @return [void]
     def print_pin pin
@@ -602,7 +620,7 @@ def print_pin pin
     # @return [void]
     def do_cache gemspec, rebuild: false
       if gemspec.nil?
-        warn "Gem '#{gemspec&.name}' not found"
+        $stderr.puts "Gem '#{gemspec&.name}' not found"
       else
         if rebuild || !PinCache.has_yard?(gemspec)
           pins = GemPins.build_yard_pins(['yard-activesupport-concern'], gemspec)

spec/complex_type_spec.rb

@@ -80,6 +80,21 @@
       expect(types.to_rbs).to eq('untyped')
     end
 
+    it 'renders an unparameterized generic as untyped in RBS' do
+      types = Solargraph::ComplexType.parse('generic')
+      expect(types.length).to eq(1)
+      expect(types.to_rbs).to eq('untyped')
+    end
+
+    it 'does not render a bare arrow for a signature returning an unparameterized generic' do
+      sig = Solargraph::Pin::Signature.new(generics: [], parameters: [],
+                                           return_type: Solargraph::ComplexType.parse('generic'))
+      method_pin = Solargraph::Pin::Method.new(name: 'to_a',
+                                               closure: Solargraph::Pin::Namespace.new(name: 'NodeSet'),
+                                               signatures: [sig])
+      expect(method_pin.to_rbs).to eq('def to_a: () -> untyped')
+    end
+
     #
     # Note that the type specifier list is always an optional field and
     # can be omitted when present in a tag signature. This is the reason

spec/logging_spec.rb

@@ -3,6 +3,27 @@
 require 'tempfile'
 
 describe Solargraph::Logging do
+  it 'gives a class overriding log_level its own logger at that level, leaving the shared one alone' do
+    logging = described_class
+    verbose_class = Class.new do
+      include logging
+
+      def log_level
+        :debug
+      end
+
+      # module_function makes #logger private on includers.
+      def build_logger
+        logger
+      end
+    end
+
+    built = verbose_class.new.build_logger
+
+    expect(built.level).to eq(Logger::DEBUG)
+    expect(built).not_to be(described_class.logger)
+  end
+
   it 'logs messages with levels' do
     file = Tempfile.new('log')
     described_class.logger.reopen file
@@ -14,4 +35,26 @@
     described_class.logger.reopen File::NULL
     expect(msg).to include('WARN')
   end
+
+  describe '.resolve_level' do
+    it 'resolves a recognized level with no warning' do
+      expect { @level = described_class.resolve_level('debug') }.not_to output.to_stderr
+
+      expect(@level).to eq(Logger::DEBUG)
+    end
+
+    it 'warns and falls back to the default level for an unrecognized value' do
+      level = nil
+      output = capture_both { level = described_class.resolve_level('bogus') }
+
+      expect(output).to include('Invalid value for SOLARGRAPH_LOG: "bogus"')
+      expect(level).to eq(Logger::WARN)
+    end
+
+    it 'falls back to the default level with no warning when unset' do
+      expect { @level = described_class.resolve_level(nil) }.not_to output.to_stderr
+
+      expect(@level).to eq(Logger::WARN)
+    end
+  end
 end

spec/pin/base_spec.rb

@@ -88,4 +88,16 @@
       expect { pin1.nearly?(pin2) }.not_to raise_error
     end
   end
+
+  describe '#choose' do
+    it 'logs and re-raises when the two values cannot be compared' do
+      pin1 = described_class.new(location: zero_location, name: 'Foo')
+      pin2 = described_class.new(location: zero_location, name: 'Foo')
+      allow(pin2).to receive(:location).and_return(Object.new)
+      allow(Solargraph.logger).to receive(:warn)
+
+      expect { pin1.choose(pin2, :location) }.to raise_error(StandardError)
+      expect(Solargraph.logger).to have_received(:warn).with(/Problem handling location/)
+    end
+  end
 end