Shopify / Shopify/ruby-lsp

Make owner non nilable in all members and instance variables

Open
#2,506 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

chore pinned
Dominant language
Ruby
Stars
2k
Forks
281
Avg merge
2h 14m
Merged PRs (30d)
6

Description

Before we could index RBS, we started out by saying that the owner of a method or instance variable could be nil (since we didn't index Object).

In reality, that's not correct. Every instance variable and method always has an owner in Ruby (since everything is an object!). It would be nice if we could get rid of the nilability on owner for all entries, so that we more accurately represent reality.

Notes

The implementation is something like this:

  • Remove the T.nilable from all owner type annotations in entry.rb
  • Run Sorbet to find all type check errors. Fix them one by one

The challenging part is actually tests. Since any method defined directly on the top level is added to Object, the DeclarationListener will now have a dependency that RBS indexing must run before Ruby indexing for correct results.

However, that would mean that we need to index RBS on every single test, even small ones where we only index one tiny Ruby class. I'm not sure how to speed that up, but we need to come up with a strategy that won't slow down the test suite too much.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in entry.rb by reviewing every owner type annotation, then run Sorbet to identify affected call sites. Pay particular attention to DeclarationListener and the indexing order used by tests. Done means all owners are non-nilable and the test suite has a strategy that preserves correct RBS-before-Ruby indexing without excessive overhead.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
devtools
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.