[regression] Fix crash while typechecking files with Struct use (#1031)

* [regression] Fix crash while typechecking files with Struct use

It looks like a mis-set closure field on a Parameter in is causing
exceptions while typechecking code with Struct use inside:

https://github.com/iftheshoefritz/solargraph-rails/actions/runs/17040847020/job/48303963447?pr=158

* Set types in parameters and instance variables

* Fix specs

* Fix linting issues

* Rebaseline .rubocop_todo.yml

* Rebaseline .rubocop_todo.yml

Author: apiology

.rubocop_todo.yml

@@ -57,7 +57,6 @@ Gemspec/RequiredRubyVersion:
 # SupportedStyles: with_first_argument, with_fixed_indentation
 Layout/ArgumentAlignment:
   Exclude:
-    - 'lib/solargraph/convention/struct_definition.rb'
     - 'lib/solargraph/pin/callable.rb'
     - 'spec/source/source_chainer_spec.rb'
 
@@ -117,14 +116,12 @@ Layout/EmptyLines:
   Exclude:
     - 'lib/solargraph/bench.rb'
     - 'lib/solargraph/complex_type/unique_type.rb'
-    - 'lib/solargraph/convention.rb'
     - 'lib/solargraph/doc_map.rb'
     - 'lib/solargraph/language_server/message/extended/check_gem_version.rb'
     - 'lib/solargraph/language_server/message/initialize.rb'
     - 'lib/solargraph/pin/delegated_method.rb'
     - 'lib/solargraph/rbs_map/conversions.rb'
     - 'spec/complex_type_spec.rb'
-    - 'spec/convention/struct_definition_spec.rb'
     - 'spec/pin/local_variable_spec.rb'
     - 'spec/pin/symbol_spec.rb'
     - 'spec/type_checker/levels/strict_spec.rb'
@@ -233,7 +230,6 @@ Layout/FirstHashElementIndentation:
 # SupportedLastArgumentHashStyles: always_inspect, always_ignore, ignore_implicit, ignore_explicit
 Layout/HashAlignment:
   Exclude:
-    - 'lib/solargraph/convention/struct_definition.rb'
     - 'lib/solargraph/workspace/config.rb'
 
 # This cop supports safe autocorrection (--autocorrect).
@@ -265,6 +261,7 @@ Layout/IndentationWidth:
     - 'lib/solargraph/type_checker/rules.rb'
     - 'lib/solargraph/yard_map/mapper.rb'
     - 'spec/api_map/config_spec.rb'
+    - 'spec/shell_spec.rb'
     - 'spec/source_map/mapper_spec.rb'
 
 # This cop supports safe autocorrection (--autocorrect).
@@ -444,20 +441,12 @@ Layout/SpaceInsideParens:
     - 'lib/solargraph/pin/namespace.rb'
     - 'lib/solargraph/source_map.rb'
 
-# This cop supports safe autocorrection (--autocorrect).
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: final_newline, final_blank_line
-Layout/TrailingEmptyLines:
-  Exclude:
-    - 'spec/convention/struct_definition_spec.rb'
-
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: AllowInHeredoc.
 Layout/TrailingWhitespace:
   Exclude:
     - 'lib/solargraph/language_server/message/client/register_capability.rb'
     - 'spec/api_map/config_spec.rb'
-    - 'spec/convention/struct_definition_spec.rb'
     - 'spec/convention_spec.rb'
 
 # This cop supports safe autocorrection (--autocorrect).
@@ -532,7 +521,6 @@ Lint/DuplicateMethods:
     - 'lib/solargraph/complex_type.rb'
     - 'lib/solargraph/location.rb'
     - 'lib/solargraph/pin/base.rb'
-    - 'lib/solargraph/pin/common.rb'
     - 'lib/solargraph/pin/signature.rb'
     - 'lib/solargraph/rbs_map.rb'
     - 'lib/solargraph/rbs_map/core_map.rb'
@@ -674,6 +662,7 @@ Lint/UnusedMethodArgument:
     - 'lib/solargraph/source/chain/literal.rb'
     - 'lib/solargraph/source/chain/variable.rb'
     - 'lib/solargraph/source/chain/z_super.rb'
+    - 'spec/doc_map_spec.rb'
 
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: AutoCorrect, ContextCreatingMethods, MethodCreatingMethods.
@@ -747,6 +736,7 @@ Metrics/BlockLength:
     - 'lib/solargraph/api_map.rb'
     - 'lib/solargraph/api_map/source_to_yard.rb'
     - 'lib/solargraph/complex_type.rb'
+    - 'lib/solargraph/convention/struct_definition.rb'
     - 'lib/solargraph/parser/parser_gem/node_processors/send_node.rb'
     - 'lib/solargraph/source/chain/call.rb'
     - 'lib/solargraph/type_checker.rb'
@@ -809,7 +799,6 @@ Metrics/ParameterLists:
 # Configuration parameters: AllowedMethods, AllowedPatterns, Max.
 Metrics/PerceivedComplexity:
   Exclude:
-    - 'lib/solargraph/api_map/source_to_yard.rb'
     - 'lib/solargraph/complex_type.rb'
     - 'lib/solargraph/parser/parser_gem/node_chainer.rb'
     - 'lib/solargraph/source/chain/call.rb'
@@ -866,7 +855,6 @@ Naming/PredicateMethod:
     - 'lib/solargraph/api_map/store.rb'
     - 'lib/solargraph/convention/data_definition.rb'
     - 'lib/solargraph/convention/struct_definition.rb'
-    - 'lib/solargraph/doc_map.rb'
     - 'lib/solargraph/language_server/progress.rb'
     - 'lib/solargraph/library.rb'
     - 'lib/solargraph/parser/node_processor/base.rb'
@@ -907,11 +895,6 @@ RSpec/BeEq:
     - 'spec/complex_type_spec.rb'
     - 'spec/pin/method_spec.rb'
 
-# This cop supports unsafe autocorrection (--autocorrect-all).
-RSpec/BeEql:
-  Exclude:
-    - 'spec/convention/struct_definition_spec.rb'
-
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: be, be_nil
@@ -1190,6 +1173,7 @@ RSpec/MultipleExpectations:
     - 'spec/rbs_map/core_map_spec.rb'
     - 'spec/rbs_map/stdlib_map_spec.rb'
     - 'spec/rbs_map_spec.rb'
+    - 'spec/shell_spec.rb'
     - 'spec/source/chain/call_spec.rb'
     - 'spec/source/chain/class_variable_spec.rb'
     - 'spec/source/chain/global_variable_spec.rb'
@@ -1306,6 +1290,7 @@ RSpec/SpecFilePathFormat:
     - 'spec/api_map/source_to_yard_spec.rb'
     - 'spec/api_map/store_spec.rb'
     - 'spec/api_map_spec.rb'
+    - 'spec/convention/activesupport_concern_spec.rb'
     - 'spec/convention/struct_definition_spec.rb'
     - 'spec/convention_spec.rb'
     - 'spec/diagnostics/base_spec.rb'
@@ -1360,6 +1345,7 @@ RSpec/SpecFilePathFormat:
     - 'spec/rbs_map/core_map_spec.rb'
     - 'spec/rbs_map/stdlib_map_spec.rb'
     - 'spec/rbs_map_spec.rb'
+    - 'spec/shell_spec.rb'
     - 'spec/source/chain/array_spec.rb'
     - 'spec/source/chain/call_spec.rb'
     - 'spec/source/chain/class_variable_spec.rb'
@@ -1440,7 +1426,6 @@ Style/AccessModifierDeclarations:
 # SupportedStyles: separated, grouped
 Style/AccessorGrouping:
   Exclude:
-    - 'lib/solargraph/doc_map.rb'
     - 'lib/solargraph/parser/flow_sensitive_typing.rb'
     - 'lib/solargraph/pin/base.rb'
     - 'lib/solargraph/pin/method.rb'
@@ -1866,6 +1851,7 @@ Style/FrozenStringLiteralComment:
     - 'spec/rbs_map/core_map_spec.rb'
     - 'spec/rbs_map/stdlib_map_spec.rb'
     - 'spec/rbs_map_spec.rb'
+    - 'spec/shell_spec.rb'
     - 'spec/source/chain/array_spec.rb'
     - 'spec/source/chain/call_spec.rb'
     - 'spec/source/chain/class_variable_spec.rb'
@@ -2083,6 +2069,7 @@ Style/MethodDefParentheses:
     - 'lib/solargraph/type_checker/checks.rb'
     - 'lib/solargraph/yard_map/helpers.rb'
     - 'lib/solargraph/yardoc.rb'
+    - 'spec/doc_map_spec.rb'
     - 'spec/fixtures/rdoc-lib/lib/example.rb'
     - 'spec/source_map_spec.rb'
     - 'spec/spec_helper.rb'
@@ -2412,7 +2399,6 @@ Style/StringConcatenation:
   Exclude:
     - 'lib/solargraph/api_map.rb'
     - 'lib/solargraph/api_map/index.rb'
-    - 'lib/solargraph/convention/struct_definition.rb'
     - 'lib/solargraph/pin/base.rb'
     - 'lib/solargraph/pin/callable.rb'
     - 'lib/solargraph/pin/closure.rb'
@@ -2494,6 +2480,7 @@ Style/StringLiterals:
     - 'spec/rbs_map/conversions_spec.rb'
     - 'spec/rbs_map/core_map_spec.rb'
     - 'spec/rbs_map/stdlib_map_spec.rb'
+    - 'spec/shell_spec.rb'
     - 'spec/source/chain/array_spec.rb'
     - 'spec/source/chain/call_spec.rb'
     - 'spec/source/chain/class_variable_spec.rb'
@@ -2598,7 +2585,6 @@ Style/TrailingCommaInArrayLiteral:
 # SupportedStylesForMultiline: comma, consistent_comma, diff_comma, no_comma
 Style/TrailingCommaInHashLiteral:
   Exclude:
-    - 'lib/solargraph/pin/base.rb'
     - 'lib/solargraph/pin/base_variable.rb'
     - 'lib/solargraph/pin/callable.rb'
     - 'lib/solargraph/pin/closure.rb'
@@ -2628,6 +2614,7 @@ Style/WordArray:
     - 'lib/solargraph/complex_type/unique_type.rb'
     - 'lib/solargraph/diagnostics/type_check.rb'
     - 'lib/solargraph/language_server/message/text_document/formatting.rb'
+    - 'spec/doc_map_spec.rb'
     - 'spec/parser/node_chainer_spec.rb'
     - 'spec/pin/method_spec.rb'
     - 'spec/source/cursor_spec.rb'
@@ -2664,18 +2651,12 @@ YARD/CollectionType:
 # SupportedStylesPrototypeName: before, after
 YARD/MismatchName:
   Exclude:
-    - 'lib/solargraph/api_map.rb'
     - 'lib/solargraph/complex_type.rb'
     - 'lib/solargraph/complex_type/unique_type.rb'
-    - 'lib/solargraph/convention.rb'
-    - 'lib/solargraph/convention/data_definition.rb'
-    - 'lib/solargraph/doc_map.rb'
     - 'lib/solargraph/gem_pins.rb'
     - 'lib/solargraph/language_server/host.rb'
     - 'lib/solargraph/language_server/host/dispatch.rb'
-    - 'lib/solargraph/language_server/message/text_document/formatting.rb'
     - 'lib/solargraph/language_server/request.rb'
-    - 'lib/solargraph/parser/flow_sensitive_typing.rb'
     - 'lib/solargraph/parser/parser_gem/node_methods.rb'
     - 'lib/solargraph/parser/region.rb'
     - 'lib/solargraph/pin/base.rb'
@@ -2693,8 +2674,6 @@ YARD/MismatchName:
     - 'lib/solargraph/pin/until.rb'
     - 'lib/solargraph/pin/while.rb'
     - 'lib/solargraph/pin_cache.rb'
-    - 'lib/solargraph/rbs_map.rb'
-    - 'lib/solargraph/shell.rb'
     - 'lib/solargraph/source/chain.rb'
     - 'lib/solargraph/source/chain/call.rb'
     - 'lib/solargraph/source/chain/z_super.rb'

lib/solargraph/convention/struct_definition.rb

@@ -20,7 +20,8 @@ def process
               name: struct_definition_node.class_name,
               docstring: docstring,
               visibility: :public,
-              gates: region.closure.gates.freeze
+              gates: region.closure.gates.freeze,
+              source: :struct_definition
             )
             pins.push nspin
 
@@ -32,7 +33,8 @@ def process
               location: get_node_location(node),
               closure: nspin,
               visibility: :private,
-              docstring: docstring
+              docstring: docstring,
+              source: :struct_definition
             )
 
             pins.push initialize_method_pin
@@ -43,7 +45,8 @@ def process
                   name: attribute_name,
                   decl: struct_definition_node.keyword_init? ? :kwarg : :arg,
                   location: get_node_location(attribute_node),
-                  closure: initialize_method_pin
+                  closure: initialize_method_pin,
+                  source: :struct_definition
                 )
               )
             end
@@ -53,29 +56,40 @@ def process
               [attribute_name, "#{attribute_name}="].each do |name|
                 docs = docstring.tags.find { |t| t.tag_name == 'param' && t.name == attribute_name }
 
+                attribute_type = ComplexType.parse(tag_string(docs))
+                return_type_comment = attribute_comment(docs, false)
+                param_comment = attribute_comment(docs, true)
+
                 method_pin = Pin::Method.new(
                   name: name,
                   parameters: [],
                   scope: :instance,
                   location: get_node_location(attribute_node),
                   closure: nspin,
+                  docstring: YARD::Docstring.new(return_type_comment),
                   # even assignments return the value
-                  comments: attribute_comment(docs, false),
-                  visibility: :public
+                  comments: return_type_comment,
+                  return_type: attribute_type,
+                  visibility: :public,
+                  source: :struct_definition
                 )
 
                 if name.end_with?('=')
                   method_pin.parameters << Pin::Parameter.new(
                     name: attribute_name,
                     location: get_node_location(attribute_node),
-                    closure: nspin,
-                    comments: attribute_comment(docs, true)
+                    closure: method_pin,
+                    return_type: attribute_type,
+                    comments: param_comment,
+                    source: :struct_definition
                   )
 
                   pins.push Pin::InstanceVariable.new(name: "@#{attribute_name}",
                                                       closure: method_pin,
                                                       location: get_node_location(attribute_node),
-                                                      comments: attribute_comment(docs, false))
+                                                      return_type: attribute_type,
+                                                      comments: "@type [#{attribute_type.rooted_tags}]",
+                                                      source: :struct_definition)
                 end
 
                 pins.push method_pin
@@ -121,13 +135,21 @@ def parse_comments
             Solargraph::Source.parse_docstring(struct_comments).to_docstring
           end
 
+          # @param tag [YARD::Tags::Tag, nil] The param tag for this attribute.xtract_
+          #
+          # @return [String]
+          def tag_string(tag)
+            tag&.types&.join(',') || 'undefined'
+          end
+
           # @param tag [YARD::Tags::Tag, nil] The param tag for this attribute. If nil, this method is a no-op
           # @param for_setter [Boolean] If true, will return a @param tag instead of a @return tag
+          #
           # @return [String] The formatted comment for the attribute
           def attribute_comment(tag, for_setter)
             return "" if tag.nil?
 
-            suffix = "[#{tag.types&.join(',') || 'undefined'}] #{tag.text}"
+            suffix = "[#{tag_string(tag)}] #{tag.text}"
 
             if for_setter
               "@param #{tag.name} #{suffix}"

spec/convention/struct_definition_spec.rb

@@ -21,6 +21,21 @@
       expect(param_baz.return_type.tag).to eql('Integer')
     end
 
+    it 'should set closure to method on assignment operator parameters' do
+      source = Solargraph::SourceMap.load_string(%(
+        # @param bar [String]
+        # @param baz [Integer]
+        Foo = Struct.new(:bar, :baz, keyword_init: true)
+      ), 'test.rb')
+
+      # @type [Array<Solargraph::Pin::Parameter>]
+      parameters = source.pins.find { |p| p.path == 'Foo#bar=' }.parameters
+
+      parameters.each do |param|
+        expect(param.closure).to be_instance_of(Solargraph::Pin::Method)
+      end
+    end
+
     it 'supports positional args' do
       source = Solargraph::SourceMap.load_string(%(
         # @param bar [String]
@@ -110,7 +125,26 @@
         expect(params_baz.length).to be(1)
         expect(params_baz.first.return_type.tag).to eql("Integer")
         expect(params_baz.first.arg?).to be(true)
+
+        iv_bar = source.pins.find { |p| p.name == "@bar" }
+        expect(iv_bar.return_type.tag).to eql("String")
+
+        iv_baz = source.pins.find { |p| p.name == "@baz" }
+        expect(iv_baz.return_type.tag).to eql("Integer")
       end
     end
   end
+
+  context 'with typechecking' do
+    def type_checker code
+      Solargraph::TypeChecker.load_string(code, 'test.rb', :strong)
+    end
+
+    it 'should not crash' do
+      checker = type_checker(%(
+        Foo = Struct.new(:bar, :baz)
+      ))
+      expect { checker.problems }.not_to raise_error
+    end
+  end
 end