diff --git a/lib/solargraph/complex_type/unique_type.rb b/lib/solargraph/complex_type/unique_type.rb index 4bbdda5b2..85954484d 100644 --- a/lib/solargraph/complex_type/unique_type.rb +++ b/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? diff --git a/lib/solargraph/logging.rb b/lib/solargraph/logging.rb index c4415176a..2371bc9df 100644 --- a/lib/solargraph/logging.rb +++ b/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" diff --git a/lib/solargraph/pin/base.rb b/lib/solargraph/pin/base.rb index f7ae58d38..89d69617d 100644 --- a/lib/solargraph/pin/base.rb +++ b/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 diff --git a/lib/solargraph/shell.rb b/lib/solargraph/shell.rb index 89859da21..b2351ab00 100755 --- a/lib/solargraph/shell.rb +++ b/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 - 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] + 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) diff --git a/spec/complex_type_spec.rb b/spec/complex_type_spec.rb index 309606561..9064069fb 100644 --- a/spec/complex_type_spec.rb +++ b/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 diff --git a/spec/logging_spec.rb b/spec/logging_spec.rb index 7dcd52000..bb39f6d88 100644 --- a/spec/logging_spec.rb +++ b/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 diff --git a/spec/pin/base_spec.rb b/spec/pin/base_spec.rb index e11566d38..fdfb52d52 100644 --- a/spec/pin/base_spec.rb +++ b/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 diff --git a/spec/shell_spec.rb b/spec/shell_spec.rb index 3d8a254bf..d9c6f68d5 100644 --- a/spec/shell_spec.rb +++ b/spec/shell_spec.rb @@ -78,6 +78,55 @@ def bundle_exec(*cmd) expect(output).to include('Scanned ').and include(' seconds.') end + + it 'reports and exits when typifying a pin raises' do + pin = instance_double(Solargraph::Pin::Method, path: 'Foo#bar', location: nil) + allow(api_map).to receive(:pins).and_return([pin]) + allow(pin).to receive(:typify).and_raise(StandardError, 'boom') + + output = capture_both do + shell.options = { directory: 'spec/fixtures/workspace' } + shell.scan + rescue SystemExit + # Ignore the SystemExit raised when a pin fails to typify + end + + expect(output).to include('Error testing Foo#bar') + expect(output).to include('[StandardError]: boom') + end + + context 'with a pin in the workspace' do + let(:pin) { instance_double(Solargraph::Pin::Base, path: 'Foo#bar', location: nil) } + + before do + allow(api_map).to receive(:pins).and_return([pin]) + end + + it 'typifies and probes the pin' do + allow(pin).to receive(:typify) + allow(pin).to receive(:probe) + + capture_stdout do + shell.options = { directory: 'spec/fixtures/workspace', verbose: false } + shell.scan + end + + expect(pin).to have_received(:typify).with(api_map) + expect(pin).to have_received(:probe).with(api_map) + end + + it 'prints the pin description when --verbose is set' do + allow(pin).to receive(:typify) + allow(pin).to receive(:probe) + + output = capture_stdout do + shell.options = { directory: 'spec/fixtures/workspace', verbose: true } + shell.scan + end + + expect(output).to include('Foo#bar') + end + end end end @@ -113,6 +162,22 @@ def bundle_exec(*cmd) expect(output).to include("Gem 'nonexistentgem' not found") end + it 'complains even with Ruby warnings disabled, as under bin/solargraph' do + # bin/solargraph sets $VERBOSE = nil, which turns Kernel#warn into a + # no-op - a warn-based message would never reach the user. + old_verbose = $VERBOSE + $VERBOSE = nil + begin + output = capture_both do + shell.gems('nonexistentgem') + end + ensure + $VERBOSE = old_verbose + end + + expect(output).to include("Gem 'nonexistentgem' not found") + end + it 'caches core without erroring out' do capture_both do shell.uncache('core') @@ -129,6 +194,45 @@ def bundle_exec(*cmd) expect(output).to include("Gem 'solargraph123' not found") end end + + context 'with mocked workspace' do + let(:workspace) { instance_double(Solargraph::Workspace) } + + before do + allow(Solargraph::Workspace).to receive(:new).and_return(workspace) + end + + it 'reports a gem whose lookup resolves to no gemspec' do + allow(workspace).to receive(:find_gem).with('no-gemspec').and_return(nil) + + output = capture_both do + shell.gems('no-gemspec') + end + + expect(output).to include("Gem 'no-gemspec' not found") + end + + it 'reports a gem that raises Gem::MissingSpecError' do + allow(workspace).to receive(:find_gem).with('flaky-gem').and_raise(Gem::MissingSpecError.new('flaky-gem', '1.0')) + + output = capture_both do + shell.gems('flaky-gem') + end + + expect(output).to include("Gem 'flaky-gem' not found") + end + + it 'reports a gem that raises Gem::Requirement::BadRequirementError' do + allow(workspace).to receive(:find_gem).with('bad-gem').and_raise(Gem::Requirement::BadRequirementError, 'bad requirement string') + + output = capture_both do + shell.gems('bad-gem') + end + + expect(output).to include("Gem 'bad-gem' failed while loading") + expect(output).to include('bad requirement string') + end + end end describe 'cache' do @@ -146,6 +250,24 @@ def bundle_exec(*cmd) end end + describe 'do_cache' do + it 'complains when the gemspec is nil, even with Ruby warnings disabled' do + # bin/solargraph sets $VERBOSE = nil, which turns Kernel#warn into a + # no-op - a warn-based message would never reach the user. + old_verbose = $VERBOSE + $VERBOSE = nil + begin + output = capture_both do + shell.send(:do_cache, nil) + end + ensure + $VERBOSE = old_verbose + end + + expect(output).to include("Gem '' not found") + end + end + # @type cmd [Array] # @return [String] def bundle_exec(*cmd) @@ -271,18 +393,90 @@ def bundle_exec(*cmd) end context 'with no pin' do - it 'prints error' do + before do allow(api_map).to receive(:get_path_pins).with('Not#found').and_return([]) + allow(api_map).to receive(:get_method_stack).with('Not', 'found', scope: :instance).and_return([]) allow(Solargraph::Pin::Method).to receive(:===).with(nil).and_return(false) + end + it 'prints error' do out = capture_both do - shell.options = {} + shell.options = { resolve: true } shell.pin('Not#found') rescue SystemExit # Ignore the SystemExit raised by the shell when no pin is found end expect(out).to include("Pin not found for path 'Not#found'") end + + it 'prints the error even with Ruby warnings disabled, as under bin/solargraph' do + # bin/solargraph sets $VERBOSE = nil, which turns Kernel#warn into a + # no-op - a warn-based message would never reach the user. + old_verbose = $VERBOSE + $VERBOSE = nil + begin + out = capture_both do + shell.options = { resolve: true } + shell.pin('Not#found') + rescue SystemExit + # Ignore the SystemExit raised by the shell when no pin is found + end + ensure + $VERBOSE = old_verbose + end + expect(out).to include("Pin not found for path 'Not#found'") + end + end + + context 'when the path names no pin of its own' do + let(:mixin_pin) do + instance_double(Solargraph::Pin::Method, path: 'Mixin#helper') + end + + before do + allow(api_map).to receive(:get_path_pins).with('Child#helper').and_return([]) + allow(api_map).to receive(:get_method_stack) + .with('Child', 'helper', scope: :instance).and_return([mixin_pin]) + allow(mixin_pin).to receive(:inspect).and_return('helper pin inspect') + end + + it 'describes the inherited definition by default, printing nothing else' do + out = capture_both do + shell.options = { resolve: true } + shell.pin('Child#helper') + end + # Just the pin: resolution is the documented default, not an event + # worth annotating. + expect(out).to eq("helper pin inspect\n") + end + + it 'describes only the exact path with --no-resolve' do + out = capture_both do + shell.options = { resolve: false } + shell.pin('Child#helper') + rescue SystemExit + # Ignore the SystemExit raised by the shell when no pin is found + end + expect(out).to include("Pin not found for path 'Child#helper'") + expect(out).not_to include('helper pin inspect') + expect(api_map).not_to have_received(:get_method_stack) + end + end + + context 'when the path names a pin of its own' do + it 'describes it without consulting method lookup' do + allow(to_s_pin).to receive(:inspect).and_return('pin inspect result') + allow(api_map).to receive(:get_method_stack) + + out = capture_both do + shell.options = { resolve: true } + shell.pin('String#to_s') + end + + # Byte-identical to the pre-resolution output for an exact hit. + expect(out).to eq("pin inspect result\n") + expect(api_map).not_to have_received(:get_method_stack) + end end end