From 3da23b9973df9bf6a25a961a42dea4bbb5c75057 Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 25 Oct 2026 01:16:22 -0400 Subject: [PATCH 1/3] Remove nil from compact element types Core RBS declares Array#compact and Enumerable#compact as () -> ::Array[Elem] and Hash#compact as () -> ::Hash[K, V], so the result keeps nil from its receiver and every element call reports "Unresolved call to upcase on String, NilClass". There is no block or method type variable for generic binding to use, and RBS cannot write Elem minus nil. Drop nil and NilClass from the element types (Hash: value types) of those three resolved pins in Chain::Call#inferred_pins. compact!, compact methods on other classes and all-nil element types are left unchanged. Removes the now-unneeded ignore marker in Pin::Search#do_query. --- lib/solargraph/pin/search.rb | 1 - lib/solargraph/source/chain/call.rb | 24 ++++++++ spec/source/chain/call_spec.rb | 88 +++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 1 deletion(-) diff --git a/lib/solargraph/pin/search.rb b/lib/solargraph/pin/search.rb index d797d949fd..0aa6a59c4c 100644 --- a/lib/solargraph/pin/search.rb +++ b/lib/solargraph/pin/search.rb @@ -44,7 +44,6 @@ def do_query .compact # @param a [self] # @param b [self] - # @sg-ignore https://github.com/castwide/solargraph/pull/1050 .sort { |a, b| b.match <=> a.match } .map(&:pin) end diff --git a/lib/solargraph/source/chain/call.rb b/lib/solargraph/source/chain/call.rb index 3754f70a7a..5b9931c650 100644 --- a/lib/solargraph/source/chain/call.rb +++ b/lib/solargraph/source/chain/call.rb @@ -11,6 +11,11 @@ class Chain class Call < Chain::Link include Solargraph::Parser::NodeMethods + # Methods whose results drop nil elements (Hash#compact: nil + # values), which RBS signatures like `() -> ::Array[Elem]` cannot + # express. + NIL_DROPPING_METHODS = %w[Array#compact Enumerable#compact Hash#compact].freeze + # @return [String] attr_reader :word @@ -231,6 +236,8 @@ def inferred_pins pins, api_map, name_pin, locals if pin.path == 'Class#new' && name_pin.binder.tag != 'Class' reduced_context = name_pin.binder.reduce_class_type pin.proxy(reduced_context) + elsif NIL_DROPPING_METHODS.include?(pin.path) + without_nil_elements(pin) else # @sg-ignore Need to add nil check here next pin if pin.return_type.undefined? @@ -242,6 +249,23 @@ def inferred_pins pins, api_map, name_pin, locals end end + # @param pin [Pin::Base] a resolved NIL_DROPPING_METHODS pin + # @return [Pin::Base] + def without_nil_elements pin + compacted = ComplexType.new(pin.return_type.items.map { |type| without_nil_subtypes(type) }) + compacted == pin.return_type ? pin : pin.proxy(compacted) + end + + # @param type [ComplexType::UniqueType] + # @return [ComplexType::UniqueType] + def without_nil_subtypes type + non_nil_items = type.subtypes.map do |subtype| + subtype.items.reject { |item| item.nil_type? || item.name == 'NilClass' } + end + subtypes = non_nil_items.reject(&:empty?).map { |items| ComplexType.new(items) } + subtypes.empty? ? type : type.recreate(new_subtypes: subtypes) + end + # @param api_map [ApiMap] # @param method_pin [Pin::Method] # @return [ComplexType, nil] diff --git a/spec/source/chain/call_spec.rb b/spec/source/chain/call_spec.rb index f56dcfa758..9ea95a460d 100644 --- a/spec/source/chain/call_spec.rb +++ b/spec/source/chain/call_spec.rb @@ -124,6 +124,94 @@ def self.bar; end expect(type.tag).to eq('Array') end + it 'removes nil from the element type of Array#compact' do + source = Solargraph::Source.load_string(%( + # @type [Array] + list = things + list.compact + ), 'test.rb') + api_map = Solargraph::ApiMap.new + api_map.map source + clip = api_map.clip_at('test.rb', [3, 12]) + expect(clip.infer.to_s).to eq('Array') + end + + it 'leaves non-nil element types of Array#compact unchanged' do + source = Solargraph::Source.load_string(%( + # @type [Array] + list = things + list.compact + ), 'test.rb') + api_map = Solargraph::ApiMap.new + api_map.map source + clip = api_map.clip_at('test.rb', [3, 12]) + expect(clip.infer.to_s).to eq('Array') + end + + it 'leaves an all-nil element type of Array#compact unchanged' do + source = Solargraph::Source.load_string(%( + # @type [Array] + list = things + list.compact + ), 'test.rb') + api_map = Solargraph::ApiMap.new + api_map.map source + clip = api_map.clip_at('test.rb', [3, 12]) + expect(clip.infer.to_s).to eq('Array') + end + + it 'keeps nil in the element type of Array#compact!' do + source = Solargraph::Source.load_string(%( + # @type [Array] + list = things + list.compact! + ), 'test.rb') + api_map = Solargraph::ApiMap.new + api_map.map source + clip = api_map.clip_at('test.rb', [3, 12]) + expect(clip.infer.to_s).to eq('Array, nil') + end + + it 'removes nil from the value type of Hash#compact' do + source = Solargraph::Source.load_string(%( + # @type [Hash{Symbol, nil => String, nil}] + list = things + list.compact + ), 'test.rb') + api_map = Solargraph::ApiMap.new + api_map.map source + clip = api_map.clip_at('test.rb', [3, 12]) + expect(clip.infer.to_s).to eq('Hash{Symbol, NilClass => String}') + end + + it 'removes nil from the element type of Enumerable#compact' do + source = Solargraph::Source.load_string(%( + # @type [Set] + list = things + list.compact + ), 'test.rb') + api_map = Solargraph::ApiMap.new + api_map.map source + clip = api_map.clip_at('test.rb', [3, 12]) + expect(clip.infer.to_s).to eq('Array') + end + + it 'keeps nil in the return type of a compact method not from core' do + source = Solargraph::Source.load_string(%( + class Foo + # @return [Array] + def compact + [] + end + end + Foo.new.compact + ), 'test.rb') + api_map = Solargraph::ApiMap.new + api_map.map source + clip = api_map.clip_at('test.rb', [7, 15]) + expect(clip.infer.to_s).to eq('Array') + end + it 'infers constant return types via returns, ignoring blocks' do source = Solargraph::Source.load_string(%( def yielder(&blk) From c966c1d21735e218c58977d6d07ae2abf06143bb Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 25 Oct 2026 11:51:12 -0400 Subject: [PATCH 2/3] Drive compact nil removal from an RBS annotation Replace the hard-coded Array#compact, Enumerable#compact and Hash#compact paths in Chain::Call with %a{solargraph:removes-nil}. Conversions records the annotation (on the method, or on every overload) as Pin::Method#removes_nil?, so a user's own RBS methods can use it too. rbs/annotations reopens the core classes with 'def compact: ...'. RbsMap::Core loads it with the fills, keeps its pins out of the core pins, and copies their annotations onto the core methods, which keep their core signatures. --- lib/solargraph/api_map.rb | 1 + lib/solargraph/pin/method.rb | 16 +++++- lib/solargraph/rbs_map/conversions.rb | 19 +++++++ lib/solargraph/rbs_map/core.rb | 23 +++++++- lib/solargraph/source/chain/call.rb | 18 ++---- rbs/annotations/core.rbs | 24 ++++++++ spec/rbs_map/core_spec.rb | 13 +++++ spec/source/chain/call_spec.rb | 80 +++++++++++++++++++++++++++ 8 files changed, 180 insertions(+), 14 deletions(-) create mode 100644 rbs/annotations/core.rbs diff --git a/lib/solargraph/api_map.rb b/lib/solargraph/api_map.rb index 5ec7c591fa..aa682100fd 100755 --- a/lib/solargraph/api_map.rb +++ b/lib/solargraph/api_map.rb @@ -991,6 +991,7 @@ def create_resolved_alias_pin alias_pin, original visibility: original.visibility, signatures: original.signatures.map(&:clone).freeze, attribute: original.attribute?, + removes_nil: original.removes_nil?, generics: original.generics.clone, return_type: original.return_type, source: :resolve_method_alias diff --git a/lib/solargraph/pin/method.rb b/lib/solargraph/pin/method.rb index e1d65326a3..47ebc2e6de 100644 --- a/lib/solargraph/pin/method.rb +++ b/lib/solargraph/pin/method.rb @@ -12,6 +12,9 @@ class Method < Callable attr_writer :signatures + # @return [Boolean] + attr_writer :removes_nil + # @return [Parser::AST::Node] attr_reader :node @@ -22,10 +25,11 @@ class Method < Callable # @param attribute [Boolean] # @param signatures [::Array, nil] # @param anon_splat [Boolean] + # @param removes_nil [Boolean] declared by RBS %a{solargraph:removes-nil} # @param context [ComplexType, ComplexType::UniqueType, nil] # @param [Hash{Symbol => Object}] splat def initialize visibility: :public, explicit: true, block: :undefined, node: nil, attribute: false, signatures: nil, anon_splat: false, - context: nil, **splat + removes_nil: false, context: nil, **splat super(**splat) @visibility = visibility @explicit = explicit @@ -34,6 +38,7 @@ def initialize visibility: :public, explicit: true, block: :undefined, node: nil @attribute = attribute @signatures = signatures @anon_splat = anon_splat + @removes_nil = removes_nil @context = context if context end @@ -68,6 +73,7 @@ def combine_with other, attrs = {} parameters: parameters, signatures: sigs, anon_splat: assert_same(other, :anon_splat?), + removes_nil: removes_nil? || other.removes_nil?, return_type: nil # pulled from signatures on first call }.merge(attrs) super(other, new_attrs) @@ -381,6 +387,14 @@ def attribute? @attribute end + # True when the method returns a collection with its receiver's nil + # elements (Hash: nil values) removed (RBS %a{solargraph:removes-nil}) + # + # @return [Boolean] + def removes_nil? + @removes_nil + end + # @parm other [self] def nearly? other super && diff --git a/lib/solargraph/rbs_map/conversions.rb b/lib/solargraph/rbs_map/conversions.rb index 29cd03331d..793bc94d35 100644 --- a/lib/solargraph/rbs_map/conversions.rb +++ b/lib/solargraph/rbs_map/conversions.rb @@ -551,6 +551,7 @@ def method_def_to_pin decl, closure, context generics = decl.overloads.map(&:method_type).map do |method_type| type_parameter_names method_type end + removes_nil = removes_nil?(decl) if decl.instance? name = decl.name.to_s @@ -565,6 +566,7 @@ def method_def_to_pin decl, closure, context signatures: [], generics: generics, visibility: visibility, + removes_nil: removes_nil, source: :rbs ) pin.signatures.concat method_def_to_sigs(decl, pin) @@ -587,12 +589,29 @@ def method_def_to_pin decl, closure, context scope: final_scope, signatures: [], generics: generics, + removes_nil: removes_nil, source: :rbs ) pin.signatures.concat method_def_to_sigs(decl, pin) pins.push pin end + # %a{solargraph:removes-nil} on the method, or on every overload + # + # @param decl [RBS::AST::Members::MethodDefinition] + # @return [Boolean] + def removes_nil? decl + return true if removes_nil_annotation?(decl.annotations) + + !decl.overloads.empty? && decl.overloads.all? { |overload| removes_nil_annotation?(overload.annotations) } + end + + # @param annotations [Array] + # @return [Boolean] + def removes_nil_annotation? annotations + annotations.any? { |annotation| annotation.string == 'solargraph:removes-nil' } + end + # @param decl [RBS::AST::Members::MethodDefinition] # @param pin [Pin::Method] # @return [void] diff --git a/lib/solargraph/rbs_map/core.rb b/lib/solargraph/rbs_map/core.rb index 94a8b51c27..e4a657b149 100644 --- a/lib/solargraph/rbs_map/core.rb +++ b/lib/solargraph/rbs_map/core.rb @@ -4,6 +4,7 @@ module Solargraph module RbsMap class Core < Base FILLS_DIRECTORY = File.expand_path(File.join(File.dirname(__FILE__), '..', '..', '..', 'rbs', 'fills')) + ANNOTATIONS_DIRECTORY = File.expand_path(File.join(File.dirname(__FILE__), '..', '..', '..', 'rbs', 'annotations')) def pins @pins ||= generate_pins @@ -27,8 +28,14 @@ def generate_pins # Avoid RBS::DuplicatedDeclarationError by loading in a different EnvironmentLoader fill_loader = RBS::EnvironmentLoader.new(core_root: nil, repository: RBS::Repository.new(no_stdlib: false)) fill_loader.add(path: Pathname(FILLS_DIRECTORY)) + fill_loader.add(path: Pathname(ANNOTATIONS_DIRECTORY)) fill_conversions = Conversions.new(loader: fill_loader) - new_pins.concat fill_conversions.pins + annotation_pins, fill_pins = fill_conversions.pins.partition do |pin| + pin.type_location&.filename&.start_with?(ANNOTATIONS_DIRECTORY) + end + new_pins.concat fill_pins + + annotate new_pins, annotation_pins # add some overrides new_pins.concat RbsMap::CoreFills::ALL @@ -39,6 +46,20 @@ def generate_pins # File.write cache_file, serial, mode: 'wb' processed end + + # Copies the annotations that ANNOTATIONS_DIRECTORY declares, e.g. on + # 'def compact: ...' in a reopened Array, onto the core methods, + # keeping their core signatures + # + # @param pins [Array] + # @param annotation_pins [Array] pins from ANNOTATIONS_DIRECTORY + # @return [void] + def annotate pins, annotation_pins + removes_nil_paths = annotation_pins.select { |pin| pin.is_a?(Pin::Method) && pin.removes_nil? }.to_set(&:path) + pins.each do |pin| + pin.removes_nil = true if pin.is_a?(Pin::Method) && removes_nil_paths.include?(pin.path) + end + end end end end diff --git a/lib/solargraph/source/chain/call.rb b/lib/solargraph/source/chain/call.rb index 5b9931c650..a7e9507702 100644 --- a/lib/solargraph/source/chain/call.rb +++ b/lib/solargraph/source/chain/call.rb @@ -11,11 +11,6 @@ class Chain class Call < Chain::Link include Solargraph::Parser::NodeMethods - # Methods whose results drop nil elements (Hash#compact: nil - # values), which RBS signatures like `() -> ::Array[Elem]` cannot - # express. - NIL_DROPPING_METHODS = %w[Array#compact Enumerable#compact Hash#compact].freeze - # @return [String] attr_reader :word @@ -217,6 +212,7 @@ def inferred_pins pins, api_map, name_pin, locals break if type.defined? end p = p.with_single_signature(new_signature_pin) unless new_signature_pin.nil? + type = without_nil_elements(type) if p.removes_nil? && type.defined? next p.proxy(type) if type.defined? if !p.macros.empty? result = process_macro(p, api_map, name_pin.context, locals) @@ -236,8 +232,6 @@ def inferred_pins pins, api_map, name_pin, locals if pin.path == 'Class#new' && name_pin.binder.tag != 'Class' reduced_context = name_pin.binder.reduce_class_type pin.proxy(reduced_context) - elsif NIL_DROPPING_METHODS.include?(pin.path) - without_nil_elements(pin) else # @sg-ignore Need to add nil check here next pin if pin.return_type.undefined? @@ -249,11 +243,11 @@ def inferred_pins pins, api_map, name_pin, locals end end - # @param pin [Pin::Base] a resolved NIL_DROPPING_METHODS pin - # @return [Pin::Base] - def without_nil_elements pin - compacted = ComplexType.new(pin.return_type.items.map { |type| without_nil_subtypes(type) }) - compacted == pin.return_type ? pin : pin.proxy(compacted) + # @param type [ComplexType] the return type of a method annotated + # %a{solargraph:removes-nil} + # @return [ComplexType] + def without_nil_elements type + ComplexType.new(type.items.map { |unique_type| without_nil_subtypes(unique_type) }) end # @param type [ComplexType::UniqueType] diff --git a/rbs/annotations/core.rbs b/rbs/annotations/core.rbs new file mode 100644 index 0000000000..6cd47ac7a6 --- /dev/null +++ b/rbs/annotations/core.rbs @@ -0,0 +1,24 @@ +# Adds Solargraph annotations to core methods. +# +# Each 'def name: ...' adds only its annotations; RbsMap::Core copies them +# onto the core method and keeps the core signatures, and uses nothing +# else from this file. +# +# %a{solargraph:removes-nil} drops nil from the element types +# (Hash: value types) of the returned collection, which a signature like +# '() -> ::Array[Elem]' cannot express. + +class Array[unchecked out Elem] + %a{solargraph:removes-nil} + def compact: ... +end + +module Enumerable[unchecked out Elem] + %a{solargraph:removes-nil} + def compact: ... +end + +class Hash[unchecked out K, unchecked out V] + %a{solargraph:removes-nil} + def compact: ... +end diff --git a/spec/rbs_map/core_spec.rb b/spec/rbs_map/core_spec.rb index f4374cc312..9bdc2009fa 100644 --- a/spec/rbs_map/core_spec.rb +++ b/spec/rbs_map/core_spec.rb @@ -120,4 +120,17 @@ class Foo clip = api_map.clip_at('test.rb', [2, 6]) expect(clip.infer.to_s).to eq('String') end + + it 'copies %a{solargraph:removes-nil} onto core compact methods' do + store = Solargraph::ApiMap::Store.new(core.pins) + flags = %w[Array#compact Enumerable#compact Hash#compact Array#compact!].to_h do |path| + [path, store.get_path_pins(path).first.removes_nil?] + end + expect(flags).to eq('Array#compact' => true, 'Enumerable#compact' => true, 'Hash#compact' => true, 'Array#compact!' => false) + end + + it 'keeps the core signatures of annotated methods' do + store = Solargraph::ApiMap::Store.new(core.pins) + expect(store.get_path_pins('Array#compact').first.signatures).not_to be_empty + end end diff --git a/spec/source/chain/call_spec.rb b/spec/source/chain/call_spec.rb index 9ea95a460d..d9cbd9543c 100644 --- a/spec/source/chain/call_spec.rb +++ b/spec/source/chain/call_spec.rb @@ -212,6 +212,86 @@ def compact expect(clip.infer.to_s).to eq('Array') end + it 'keeps nil in the element type of an unannotated core method' do + source = Solargraph::Source.load_string(%( + # @type [Array] + list = things + list.uniq + ), 'test.rb') + api_map = Solargraph::ApiMap.new + api_map.map source + clip = api_map.clip_at('test.rb', [3, 12]) + expect(clip.infer.to_s).to eq('Array') + end + + context 'with a user RBS method' do + around do |example| + require 'tmpdir' + Dir.mktmpdir('rspec-solargraph-') do |dir| + File.write(File.join(dir, 'box.rbs'), rbs) + loader = RBS::EnvironmentLoader.new(core_root: nil, repository: RBS::Repository.new(no_stdlib: false)) + loader.add(path: Pathname(dir)) + @rbs_pins = Solargraph::RbsMap::Conversions.new(loader: loader).pins + example.run + end + end + + let(:type) do + source = Solargraph::Source.load_string(%( + # @type [Box] + box = make_box + box.present + ), 'test.rb') + source_map = Solargraph::SourceMap.map(source) + api_map = Solargraph::ApiMap.new.index(@rbs_pins + source_map.pins + source_map.locals) + chain = Solargraph::Source::SourceChainer.chain(source, Solargraph::Position.new(3, 13)) + chain.infer(api_map, Solargraph::Pin::ROOT_PIN, source_map.locals) + end + + context 'when annotated %a{solargraph:removes-nil}' do + let(:rbs) do + <<~RBS + class Box[Elem] + %a{solargraph:removes-nil} + def present: () -> Array[Elem] + end + RBS + end + + it 'removes nil from the element type' do + expect(type.to_s).to eq('Array') + end + end + + context 'when annotated on every overload' do + let(:rbs) do + <<~RBS + class Box[Elem] + def present: %a{solargraph:removes-nil} () -> Array[Elem] + end + RBS + end + + it 'removes nil from the element type' do + expect(type.to_s).to eq('Array') + end + end + + context 'when not annotated' do + let(:rbs) do + <<~RBS + class Box[Elem] + def present: () -> Array[Elem] + end + RBS + end + + it 'keeps nil in the element type' do + expect(type.to_s).to eq('Array') + end + end + end + it 'infers constant return types via returns, ignoring blocks' do source = Solargraph::Source.load_string(%( def yielder(&blk) From d05ab148e0492cd726a013393658e0b84b65fc26 Mon Sep 17 00:00:00 2001 From: Vince Broz Date: Sun, 25 Oct 2026 13:03:23 -0400 Subject: [PATCH 3/3] Name the removes-nil annotation file for its annotation rbs/annotations holds every annotation file RbsMap::Core reads; one file per annotation keeps them apart as more are added. --- lib/solargraph/rbs_map/core.rb | 2 ++ rbs/annotations/{core.rbs => removes_nil.rbs} | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) rename rbs/annotations/{core.rbs => removes_nil.rbs} (92%) diff --git a/lib/solargraph/rbs_map/core.rb b/lib/solargraph/rbs_map/core.rb index e4a657b149..66dd6a530e 100644 --- a/lib/solargraph/rbs_map/core.rb +++ b/lib/solargraph/rbs_map/core.rb @@ -4,6 +4,8 @@ module Solargraph module RbsMap class Core < Base FILLS_DIRECTORY = File.expand_path(File.join(File.dirname(__FILE__), '..', '..', '..', 'rbs', 'fills')) + # Every .rbs file in it is loaded with the fills, its annotations + # copied onto core methods (see #annotate) ANNOTATIONS_DIRECTORY = File.expand_path(File.join(File.dirname(__FILE__), '..', '..', '..', 'rbs', 'annotations')) def pins diff --git a/rbs/annotations/core.rbs b/rbs/annotations/removes_nil.rbs similarity index 92% rename from rbs/annotations/core.rbs rename to rbs/annotations/removes_nil.rbs index 6cd47ac7a6..ff7537e4fc 100644 --- a/rbs/annotations/core.rbs +++ b/rbs/annotations/removes_nil.rbs @@ -1,4 +1,4 @@ -# Adds Solargraph annotations to core methods. +# Adds %a{solargraph:removes-nil} to core methods. # # Each 'def name: ...' adds only its annotations; RbsMap::Core copies them # onto the core method and keeps the core signatures, and uses nothing