Annotation fixes for strong typechecking

.github/workflows/linting.yml

@@ -115,11 +115,13 @@ jobs:
     - name: Run RuboCop against todo file
       continue-on-error: true
       run: |
-        bundle exec rubocop --auto-gen-config --no-exclude-limit --no-offense-counts --no-auto-gen-timestamp
+        cmd="bundle exec rubocop --auto-gen-config --exclude-limit=5 --no-offense-counts --no-auto-gen-timestamp"
+        ${cmd:?}
+        set +e
         if [ -n "$(git status --porcelain)" ]
         then
           git status --porcelain
           git diff -u .
-           >&2 echo "Please fix deltas if bad or run 'bundle exec rubocop --auto-gen-config --no-exclude-limit --no-offense-counts --no-auto-gen-timestamp' and push up changes if good"
+           >&2 echo "Please address any new issues, then run '${cmd:?}' and push up any improvements"
            exit 1
         fi

.rubocop.yml

@@ -37,7 +37,6 @@ Metrics/ParameterLists:
   Max: 7
   CountKeywordArgs: false
 
-
 # we tend to use @@ and the risk doesn't seem high
 Style/ClassVars:
   Enabled: false

lib/solargraph/api_map.rb

@@ -179,6 +179,7 @@ def clip_at filename, position
     # Create an ApiMap with a workspace in the specified directory.
     #
     # @param directory [String]
+    #
     # @return [ApiMap]
     def self.load directory
       api_map = new
@@ -214,6 +215,7 @@ class << self
     #
     # @param directory [String]
     # @param out [IO] The output stream for messages
+    #
     # @return [ApiMap]
     def self.load_with_cache directory, out
       api_map = load(directory)
@@ -259,7 +261,7 @@ def namespace_exists? name, context = ''
     #
     # @param namespace [String] The namespace
     # @param contexts [Array<String>] The contexts
-    # @return [Array<Solargraph::Pin::Base>]
+    # @return [Array<Solargraph::Pin::Constant, Solargraph::Pin::Namespace>]
     def get_constants namespace, *contexts
       namespace ||= ''
       gates = contexts.clone
@@ -499,7 +501,8 @@ def get_complex_type_methods complex_type, context = '', internal = false
     # @param name [String] Method name to look up
     # @param scope [Symbol] :instance or :class
     # @param visibility [Array<Symbol>] :public, :protected, and/or :private
-    # @param preserve_generics [Boolean]
+    # @param preserve_generics [Boolean] True to preserve any
+    #   unresolved generic parameters, false to erase them
     # @return [Array<Solargraph::Pin::Method>]
     def get_method_stack rooted_tag, name, scope: :instance, visibility: [:private, :protected, :public], preserve_generics: false
       rooted_type = ComplexType.parse(rooted_tag)
@@ -728,7 +731,6 @@ def store
     # @param skip [Set<String>]
     # @param no_core [Boolean] Skip core classes if true
     # @return [Array<Pin::Base>]
-    # rubocop:disable Metrics/CyclomaticComplexity
     def inner_get_methods rooted_tag, scope, visibility, deep, skip, no_core = false
       rooted_type = ComplexType.parse(rooted_tag).force_rooted
       fqns = rooted_type.namespace
@@ -802,7 +804,6 @@ def inner_get_methods rooted_tag, scope, visibility, deep, skip, no_core = false
       end
       result
     end
-    # rubocop:enable Metrics/CyclomaticComplexity
 
     # @return [Hash]
     def path_macros

lib/solargraph/api_map/index.rb

@@ -95,6 +95,7 @@ def deep_clone
       end
 
       # @param new_pins [Enumerable<Pin::Base>]
+      #
       # @return [self]
       def catalog new_pins
         # @type [Hash{Class<generic<T>> => Set<generic<T>>}]

lib/solargraph/api_map/store.rb

@@ -57,7 +57,7 @@ def inspect
 
       # @param fqns [String]
       # @param visibility [Array<Symbol>]
-      # @return [Enumerable<Solargraph::Pin::Base>]
+      # @return [Enumerable<Solargraph::Pin::Namespace, Solargraph::Pin::Constant>]
       def get_constants fqns, visibility = [:public]
         namespace_children(fqns).select { |pin|
           !pin.name.empty? && (pin.is_a?(Pin::Namespace) || pin.is_a?(Pin::Constant)) && visibility.include?(pin.visibility)
@@ -134,7 +134,8 @@ def get_instance_variables(fqns, scope = :instance)
       end
 
       # @param fqns [String]
-      # @return [Enumerable<Solargraph::Pin::Base>]
+      #
+      # @return [Enumerable<Solargraph::Pin::ClassVariable>]
       def get_class_variables(fqns)
         namespace_children(fqns).select { |pin| pin.is_a?(Pin::ClassVariable)}
       end
@@ -268,7 +269,8 @@ def index
       end
 
       # @param pinsets [Array<Enumerable<Pin::Base>>]
-      # @return [Boolean]
+      #
+      # @return [void]
       def catalog pinsets
         @pinsets = pinsets
         # @type [Array<Index>]

lib/solargraph/complex_type/unique_type.rb

@@ -280,7 +280,7 @@ def resolve_generics_from_context generics_to_resolve, context_type, resolved_ge
       end
 
       # @param generics_to_resolve [Enumerable<String>]
-      # @param context_type [UniqueType]
+      # @param context_type [UniqueType, nil]
       # @param resolved_generic_values [Hash{String => ComplexType}]
       # @yieldreturn [Array<ComplexType>]
       # @return [Array<ComplexType>]
@@ -446,6 +446,7 @@ def rooted?
         !can_root_name? || @rooted
       end
 
+      # @param name_to_check [String]
       def can_root_name?(name_to_check = name)
         self.class.can_root_name?(name_to_check)
       end

lib/solargraph/convention/data_definition/data_definition_node.rb

@@ -76,7 +76,7 @@ def body_node
         # @return [Parser::AST::Node]
         attr_reader :node
 
-        # @return [Parser::AST::Node]
+        # @return [Parser::AST::Node, nil]
         def data_node
           node.children[1]
         end

lib/solargraph/doc_map.rb

@@ -177,10 +177,10 @@ def load_serialized_gem_pins
       @uncached_yard_gemspecs = []
       @uncached_rbs_collection_gemspecs = []
       with_gemspecs, without_gemspecs = required_gems_map.partition { |_, v| v }
-      # @sg-ignore Wrong argument type for Hash.[]: arg_0 expected _ToHash<Array(String, Array<Gem::Specification>), undefined>, received Array<Array(String, Array<Gem::Specification>)>
+      # @sg-ignore Need support for RBS duck interfaces like _ToHash
       # @type [Array<String>]
       paths = Hash[without_gemspecs].keys
-      # @sg-ignore Wrong argument type for Hash.[]: arg_0 expected _ToHash<Array(String, Array<Gem::Specification>), undefined>, received Array<Array(String, Array<Gem::Specification>)>
+      # @sg-ignore Need support for RBS duck interfaces like _ToHash
       # @type [Array<Gem::Specification>]
       gemspecs = Hash[with_gemspecs].values.flatten.compact + dependencies.to_a
 
@@ -212,7 +212,7 @@ def preference_map
     end
 
     # @param gemspec [Gem::Specification]
-    # @return [Array<Pin::Base>]
+    # @return [Array<Pin::Base>, nil]
     def deserialize_yard_pin_cache gemspec
       if yard_pins_in_memory.key?([gemspec.name, gemspec.version])
         return yard_pins_in_memory[[gemspec.name, gemspec.version]]
@@ -381,7 +381,7 @@ def inspect
       self.class.inspect
     end
 
-    # @return [Array<Gem::Specification>]
+    # @return [Array<Gem::Specification>, nil]
     def gemspecs_required_from_bundler
       # @todo Handle projects with custom Bundler/Gemfile setups
       return unless workspace.gemfile?
@@ -404,7 +404,7 @@ def gemspecs_required_from_bundler
       end
     end
 
-    # @return [Array<Gem::Specification>]
+    # @return [Array<Gem::Specification>, nil]
     def gemspecs_required_from_external_bundle
       logger.info 'Fetching gemspecs required from external bundle'
       return [] unless workspace&.directory

lib/solargraph/environ.rb

@@ -22,7 +22,7 @@ class Environ
     # @param requires [Array<String>]
     # @param domains [Array<String>]
     # @param pins [Array<Pin::Base>]
-    # @param yard_plugins[Array<String>]
+    # @param yard_plugins [Array<String>]
     def initialize requires: [], domains: [], pins: [], yard_plugins: []
       @requires = requires
       @domains = domains

lib/solargraph/language_server/host.rb

@@ -94,7 +94,8 @@ def process request
       # processed, caller is responsible for sending the response.
       #
       # @param request [Hash{String => unspecified}] The contents of the message.
-      # @return [Solargraph::LanguageServer::Message::Base, nil] The message handler.
+      #
+      # @return [Solargraph::LanguageServer::Message::Base, Solargraph::LanguageServer::Request, nil] The message handler.
       def receive request
         if request['method']
           logger.info "Host received ##{request['id']} #{request['method']}"
@@ -534,7 +535,7 @@ def formatter_config uri
       # @param uri [String]
       # @param line [Integer]
       # @param column [Integer]
-      # @return [Solargraph::SourceMap::Completion]
+      # @return [Solargraph::SourceMap::Completion, nil]
       def completions_at uri, line, column
         library = library_for(uri)
         library.completions_at uri_to_file(uri), line, column
@@ -548,7 +549,7 @@ def has_pending_completions?
       # @param uri [String]
       # @param line [Integer]
       # @param column [Integer]
-      # @return [Array<Solargraph::Pin::Base>]
+      # @return [Array<Solargraph::Pin::Base>, nil]
       def definitions_at uri, line, column
         library = library_for(uri)
         library.definitions_at(uri_to_file(uri), line, column)
@@ -557,7 +558,7 @@ def definitions_at uri, line, column
       # @param uri [String]
       # @param line [Integer]
       # @param column [Integer]
-      # @return [Array<Solargraph::Pin::Base>]
+      # @return [Array<Solargraph::Pin::Base>, nil]
       def type_definitions_at uri, line, column
         library = library_for(uri)
         library.type_definitions_at(uri_to_file(uri), line, column)

lib/solargraph/language_server/message/text_document/definition.rb

@@ -10,7 +10,7 @@ def process
 
     private
 
-    # @return [Array<Hash>]
+    # @return [Array<Hash>, nil]
     def code_location
       suggestions = host.definitions_at(params['textDocument']['uri'], @line, @column)
       return nil if suggestions.empty?
@@ -22,7 +22,7 @@ def code_location
       end
     end
 
-    # @return [Array<Hash>]
+    # @return [Array<Hash>, nil]
     def require_location
       # @todo Terrible hack
       lib = host.library_for(params['textDocument']['uri'])

lib/solargraph/language_server/message/text_document/formatting.rb

@@ -96,8 +96,9 @@ def formatter_class(config)
           end
 
           # @param value [Array, String]
-          # @return [String]
+          # @return [String, nil]
           def cop_list(value)
+            # @type [String]
             value = value.join(',') if value.respond_to?(:join)
             return nil if value == '' || !value.is_a?(String)
             value

lib/solargraph/language_server/message/text_document/type_definition.rb

@@ -10,7 +10,7 @@ def process
 
     private
 
-    # @return [Array<Hash>]
+    # @return [Array<Hash>, nil]
     def code_location
       suggestions = host.type_definitions_at(params['textDocument']['uri'], @line, @column)
       return nil if suggestions.empty?

lib/solargraph/language_server/request.rb

@@ -11,7 +11,9 @@ def initialize id, &block
       end
 
       # @param result [Object]
-      # @return [void]
+      # @generic T
+      # @yieldreturn [generic<T>]
+      # @return [generic<T>, nil]
       def process result
         @block.call(result) unless @block.nil?
       end

lib/solargraph/parser/flow_sensitive_typing.rb

@@ -171,7 +171,7 @@ def process_conditional(conditional_node, true_ranges)
       end
 
       # @param isa_node [Parser::AST::Node]
-      # @return [Array(String, String)]
+      # @return [Array(String, String), nil]
       def parse_isa(isa_node)
         return unless isa_node&.type == :send && isa_node.children[1] == :is_a?
         # Check if conditional node follows this pattern:

lib/solargraph/parser/parser_gem/node_processors/sclass_node.rb

@@ -7,9 +7,18 @@ module NodeProcessors
         class SclassNode < Parser::NodeProcessor::Base
           def process
             sclass = node.children[0]
-            if sclass.is_a?(AST::Node) && sclass.type == :self
+            # @todo Changing Parser::AST::Node to AST::Node below will
+            #   cause type errors at strong level because the combined
+            #   pin for AST::Node#children has return type
+            #   "Array<AST::Node>, Array".  YARD annotations in AST
+            #   provided the Array, RBS for Array<AST::Node>.  We
+            #   should probably have a rule to combine "A, A<T>""
+            #   types to "A<T>" if the "A" comes from YARD, with the
+            #   rationale that folks tend to be less formal with types in
+            #   YARD.
+            if sclass.is_a?(::Parser::AST::Node) && sclass.type == :self
               closure = region.closure
-            elsif sclass.is_a?(AST::Node) && sclass.type == :casgn
+            elsif sclass.is_a?(::Parser::AST::Node) && sclass.type == :casgn
               names = [region.closure.namespace, region.closure.name]
               if sclass.children[0].nil? && names.last != sclass.children[1].to_s
                 names << sclass.children[1].to_s
@@ -18,7 +27,7 @@ def process
               end
               name = names.reject(&:empty?).join('::')
               closure = Solargraph::Pin::Namespace.new(name: name, location: region.closure.location, source: :parser)
-            elsif sclass.is_a?(AST::Node) && sclass.type == :const
+            elsif sclass.is_a?(::Parser::AST::Node) && sclass.type == :const
               names = [region.closure.namespace, region.closure.name]
               also = NodeMethods.unpack_name(sclass)
               if also != region.closure.name

lib/solargraph/parser/parser_gem/node_processors/send_node.rb

@@ -66,7 +66,7 @@ def process_visibility
                   return process_children
                 end
                 # :nocov:
-                if child.is_a?(AST::Node) && (child.type == :sym || child.type == :str)
+                if child.is_a?(::Parser::AST::Node) && (child.type == :sym || child.type == :str)
                   name = child.children[0].to_s
                   matches = pins.select{ |pin| pin.is_a?(Pin::Method) && pin.name == name && pin.namespace == region.closure.full_context.namespace && pin.context.scope == (region.scope || :instance)}
                   matches.each do |pin|
@@ -198,7 +198,7 @@ def process_module_function
             elsif node.children[2].type == :sym || node.children[2].type == :str
               node.children[2..-1].each do |x|
                 cn = x.children[0].to_s
-                # @type [Pin::Method]
+                # @type [Pin::Method, nil]
                 ref = pins.find { |p| p.is_a?(Pin::Method) && p.namespace == region.closure.full_context.namespace && p.name == cn }
                 unless ref.nil?
                   pins.delete ref

lib/solargraph/pin/base.rb

@@ -338,7 +338,7 @@ def choose_pin_attr_with_same_name(other, attr)
       # @param other [self]
       # @param attr [::Symbol]
       #
-      # @sg-ignore
+      # @sg-ignore Missing @return tag for Solargraph::Pin::Base#choose_pin_attr
       # @return [undefined]
       def choose_pin_attr(other, attr)
         # @type [Pin::Base, nil]
@@ -456,7 +456,7 @@ def nearly? other
       # Pin equality is determined using the #nearly? method and also
       # requiring both pins to have the same location.
       #
-      # @param other [self]
+      # @param other [Object]
       def == other
         return false unless nearly? other
         comments == other.comments && location == other.location
@@ -579,7 +579,7 @@ def to_rbs
         return_type.to_rbs
       end
 
-      # @return [String]
+      # @return [String, nil]
       def type_desc
         rbs = to_rbs
         # RBS doesn't have a way to represent a Class<x> type

lib/solargraph/pin/callable.rb

@@ -27,13 +27,15 @@ def method_namespace
       end
 
       # @param other [self]
-      # @return [Pin::Block, nil]
+      #
+      # @return [Pin::Signature, nil]
       def combine_blocks(other)
         if block.nil?
           other.block
         elsif other.block.nil?
           block
         else
+          # @type [Pin::Signature, nil]
           choose_pin_attr(other, :block)
         end
       end
@@ -61,6 +63,7 @@ def generics
       end
 
       # @param other [self]
+      #
       # @return [Array<Pin::Parameter>]
       def choose_parameters(other)
         raise "Trying to combine two pins with different arities - \nself =#{inspect}, \nother=#{other.inspect}, \n\n self.arity=#{self.arity}, \nother.arity=#{other.arity}" if other.arity != arity
@@ -210,7 +213,6 @@ def mandatory_positional_param_count
         parameters.count(&:arg?)
       end
 
-      # @return [String]
       def to_rbs
         rbs_generics + '(' + parameters.map { |param| param.to_rbs }.join(', ') + ') ' + (block.nil? ? '' : '{ ' + block.to_rbs + ' } ') + '-> ' + return_type.to_rbs
       end

lib/solargraph/pin/closure.rb

@@ -60,7 +60,6 @@ def generics
         @generics ||= docstring.tags(:generic).map(&:name)
       end
 
-      # @return [String]
       def to_rbs
         rbs_generics + return_type.to_rbs
       end