solargraph-rails
solargraph-rails copied to clipboard
iftheshoefritz
.first and .last on has_many associations are missing return types
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
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"
}
...
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
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.
Interesting 🤔 I didn’t know it was working before. Maybe return type needs to be adjusted
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.
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.
I tested with https://github.com/castwide/solargraph/pull/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?
@iftheshoefritz (and anybody else following) not sure if you saw my comments here https://github.com/castwide/solargraph/pull/585#issuecomment-1263776765 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.
