.first and .last on has_many associations are missing return types

[iftheshoefritz]

Describe the bug
Given:

MyModel has_many :my_other_models
MyOtherModel belongs_to :my_model

and MyOtherModel has an attribute .my_attribute

I expect to be able to autocomplete the entire chain: MyModel.new.my_other_models.first.my_attribute

I can autocomplete to MyModel.my_other_models.first, but the returned object does not have sufficient information to allow me to reach my_attribute.

To Reproduce
Create a rails project with solargraph-rails 1.0.0.pre.1 installed and set up the models above.

Debug log

Run the following command in the project you are having problems:

> ruby -r'solargraph-rails' -e 'Solargraph::Rails::Debug.run()'
[INFO] Indexing workspace files in ./
[DEBUG] [Rails][Schema] added ["created_at", "updated_at", "author_id", "price"] to Book
[DEBUG] [Rails][Model] added ["author"] to Book
[DEBUG] [Rails][Schema] added ["name", "created_at", "updated_at"] to Author
[DEBUG] [Rails][Model] added ["books"] to Author
[DEBUG] [Rails][RailsApi] added ["ActiveRecord::ConnectionAdapters::SchemaStatements", "ActiveRecord::ConnectionAdapters::SchemaStatements"] to AddPriceToBook
[DEBUG] [Rails][RailsApi] added ["ActiveRecord::ConnectionAdapters::SchemaStatements", "ActiveRecord::ConnectionAdapters::SchemaStatements"] to AddAssociations
[DEBUG] [Rails][RailsApi] added ["ActiveRecord::ConnectionAdapters::SchemaStatements", "ActiveRecord::ConnectionAdapters::SchemaStatements"] to CreateAuthors
[DEBUG] [Rails][RailsApi] added ["ActiveRecord::ConnectionAdapters::SchemaStatements", "ActiveRecord::ConnectionAdapters::SchemaStatements"] to CreateBooks
[INFO] Loading gems for bundler/require
[DEBUG] [Rails][Rails] found 33 pins in annotations
[INFO] Loading railties 6.0.4.4 from cache
[INFO] Loading activesupport 6.1.4.1 from cache
[INFO] Loading i18n 1.8.11 from cache
[INFO] Loading concurrent-ruby 1.1.9 from cache
[INFO] Loading tzinfo 2.0.4 from cache
[INFO] Loading zeitwerk 2.5.3 from cache
[INFO] Loading minitest 5.15.0 from cache
[INFO] Loading actionpack 6.1.3.2 from cache
[INFO] Loading rack 2.2.3 from cache
[INFO] Loading rack-test 1.1.0 from cache
[INFO] Loading rails-html-sanitizer 1.4.2 from cache
[INFO] Loading loofah 2.13.0 from cache
[INFO] Loading crass 1.0.6 from cache
[INFO] Loading nokogiri 1.13.1 from cache
[INFO] Loading racc 1.6.0 from cache
[INFO] Loading rails-dom-testing 2.0.3 from cache
[INFO] Loading actionview 6.1.3.2 from cache
[INFO] Loading builder 3.2.4 from cache
[INFO] Loading erubi 1.10.0 from cache
[INFO] Loading rake 13.0.6 from cache
[INFO] Loading thor 1.2.1 from cache
[INFO] Loading method_source 1.0.0 from cache
[INFO] Loading bundler 2.2.20 from cache
[INFO] Loading bootsnap 1.10.2 from cache
[INFO] Loading msgpack 1.4.3 from cache
[INFO] Loading solargraph 0.44.3 from cache
[INFO] Loading backport 1.2.0 from cache
[INFO] Loading benchmark 0.2.0 from cache
[INFO] Loading diff-lcs 1.5.0 from cache
[INFO] Loading e2mmap 0.1.0 from cache
[INFO] Loading jaro_winkler 1.5.4 from cache
[INFO] Loading kramdown 2.3.1 from cache
[INFO] Loading rexml 3.2.5 from cache
[INFO] Loading kramdown-parser-gfm 1.1.0 from cache
[INFO] Loading parser 3.1.0.0 from cache
[INFO] Loading ast 2.4.2 from cache
[INFO] Loading reverse_markdown 2.1.1 from cache
[INFO] Loading rubocop 1.25.1 from cache
[INFO] Loading parallel 1.21.0 from cache
[INFO] Loading rainbow 3.1.1 from cache
[INFO] Loading regexp_parser 2.2.0 from cache
[INFO] Loading rubocop-ast 1.15.1 from cache
[INFO] Loading ruby-progressbar 1.11.0 from cache
[INFO] Loading unicode-display_width 2.1.0 from cache
[INFO] Loading tilt 2.0.10 from cache
[INFO] Loading yard 0.9.27 from cache
[INFO] Loading webrick 1.7.0 from cache
[INFO] Loading thread_safe 0.3.6 from cache
[INFO] Loading nio4r 2.5.8 from cache
[INFO] Loading websocket-extensions 0.1.5 from cache
[INFO] Loading websocket-driver 0.7.5 from cache
[INFO] Loading actioncable 6.0.4.4 from cache
[INFO] Loading globalid 1.0.0 from cache
[INFO] Loading activejob 6.0.4.4 from cache
[INFO] Loading activemodel 6.0.4.4 from cache
[INFO] Loading activerecord 6.0.4.4 from cache
[INFO] Loading marcel 1.0.2 from cache
[INFO] Loading activestorage 6.0.4.4 from cache
[INFO] Loading mini_mime 1.1.2 from cache
[INFO] Loading mail 2.7.1 from cache
[INFO] Loading actionmailbox 6.0.4.4 from cache
[INFO] Loading actionmailer 6.0.4.4 from cache
[INFO] Loading actiontext 6.0.4.4 from cache
[INFO] Loading ffi 1.15.5 from cache
[INFO] Loading jbuilder 2.11.5 from cache
[INFO] Loading puma 3.12.6 from cache
[INFO] Loading rack-proxy 0.7.2 from cache
[INFO] Loading sprockets 3.7.2 from cache
[INFO] Loading sprockets-rails 3.4.2 from cache
[INFO] Loading rb-fsevent 0.11.0 from cache
[INFO] Loading rb-inotify 0.10.1 from cache
[INFO] Loading sass-listen 4.0.0 from cache
[INFO] Loading sass 3.7.4 from cache
[INFO] Loading sass-rails 5.1.0 from cache
[INFO] Loading sqlite3 1.4.2 from cache
[INFO] Loading turbolinks-source 5.2.0 from cache
[INFO] Loading turbolinks 5.2.1 from cache
[INFO] Loading webpacker 4.3.0 from cache
Ruby version: 2.7.2
Solargraph version: 0.44.3
Solargraph Rails version: 1.0.0.pre.1

[iftheshoefritz]

LSP logs show that the server responds to the request for autocompletion of methods on the association with many items from Enumerable, ActiveRecord::Associations::CollectionProxy etc, but they all have a "return_type": "undefined":

[Trace - 02:59:51 pm] Received response 'textDocument/completion - (129)' in 34ms.
Result: {
  "isIncomplete": null,
  "items": [
    ...
    {
      "label": "first",
      "kind": 2,
      "detail": "(*args)",
      "data": {
        "path": "Enumerable#first",
        "return_type": "undefined",
        "location": null,
        "deprecated": null,
        "uri": "file:///Users/frederickmeissner/devprojects/solartest60/app/models/author.rb"
      },
      "textEdit": {
        "range": {
          "start": {
            "line": 20,
            "character": 21
          },
          "end": {
            "line": 20,
            "character": 21
          }
        },
        "newText": "first"
      },
      "sortText": "0003first"
    }
    ...
    {
      "label": "first",
      "kind": 2,
      "detail": "(limit = nil)",
      "data": {
        "path": "ActiveRecord::FinderMethods#first",
        "return_type": "undefined",
        "location": {
          "filename": "/Users/frederickmeissner/.asdf/installs/ruby/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.0.4.4/lib/active_record/relation/finder_methods.rb",
          "range": {
            "start": {
              "line": 115,
              "character": 0
            },
            "end": {
              "line": 115,
              "character": 0
            }
          }
        },
        "deprecated": null,
        "uri": "file:///Users/frederickmeissner/devprojects/solartest60/app/models/author.rb"
      },
      "textEdit": {
        "range": {
          "start": {
            "line": 19,
            "character": 11
          },
          "end": {
            "line": 19,
            "character": 11
          }
        },
        "newText": "first"
      },
      "sortText": "0002first"
    }
   ...
   

[alisnic]

Most methods are missing return types. Type information is something that needs to be solved somehow. The simplest fix right now is to add types to https://github.com/iftheshoefritz/solargraph-rails/blob/main/lib/solargraph/rails/types.yml.

But I'm not sure how sustainable that is, because there are a lot of methods in Rails, and filling them up by hand is a lot of manual labor. We might somehow automate this by writing a test suite and tracing method types while the suite runs. One example of this is Ruby 3.1 built-in type checker that has a mode for generating RBS type signatures from runtime execution https://evilmartians.com/chronicles/climbing-steep-hills-or-adopting-ruby-types.

But RBS is Ruby 3 specific, and we'd have o port it to solaragraph type definitions so that this works on older versions of Ruby, at least until solargraph catches up with the Ruby vision of type checking

[iftheshoefritz]

Hmmm... I had this working in previous versions of solargraph-rails. With has_many :things, I specified the return type on .things as CollectionProxy<Thing> and based on that, Solargraph was smart enough to work out that .first returned Thing.

I see we still have the code to set the type on the collection method .things, so something else we are doing is interfering with solargraph's inference.

[alisnic]

Interesting 🤔 I didn’t know it was working before. Maybe return type needs to be adjusted

[iftheshoefritz]

Your code at lib/solargraph/rails/model.rb:69 looks right:

types: ["ActiveRecord::Associations::CollectionProxy<#{class_name}>"],

which is converted to a YARD comment:

comments << "@return [#{types.join(',')}]" if types

My best guess is that something else is overriding the return values on the pins created in Model.rb.

[iftheshoefritz]

The mystery deepens. I rolled back to solargraph 0.3.1 and I still don't see this working. Here is what I definitely had in the past:

In the GIF here: you can see the behaviour that I want (watch the whole thing, I only demonstrate this late in the gif).

However that GIF was made when I returned the association type Array<MyType>, because of problems with solargraph understanding Activerecord::Associations::CollectionProxy.

In that issue I document how running solargraph bundle removes pins like #first, #any etc. from CollectionProxy and that running solargraph clear restores them.

I would swear that (at some point after I realised that solargraph clear made the difference) I had a version of the gem where my_books of type Activerecord::Associations::CollectionProxy<MyBook> would autocomplete .first.created_at etc. Current evidence seems to disprove that.

All of that said, going back to the GIF, Array<MyBook> was definitely enough for solargraph to understand that .first is a MyBook... I'd like it to understand that Activerecord::Associations::CollectionProxy<MyBook>#first in the same way.

[iftheshoefritz]

I tested with castwide/solargraph#585, but still first and last have an undefined return_type. I'm wondering if there's something we could add to types.yml that would do the trick?

[grncdr]

@iftheshoefritz (and anybody else following) not sure if you saw my comments here castwide/solargraph#585 (comment) but I really would love to get some feedback on whether my "hacks" branches work well for other codebases. I want to open PRs and get everything merged upstream in solargraph and here, but I'm a bit pressed for time right now.

[iftheshoefritz]

@grncdr I'll check the hacks out shortly