Skip to content

Support RBS comment translation on multi-name attrs - #973

Draft
KaanOzkan wants to merge 3 commits into
mainfrom
ko-fix-rbs-comments-multi-attr
Draft

Support RBS comment translation on multi-name attrs#973
KaanOzkan wants to merge 3 commits into
mainfrom
ko-fix-rbs-comments-multi-attr

Conversation

@KaanOzkan

@KaanOzkan KaanOzkan commented Jul 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Rewrite attr_reader, attr_writer, and attr_accessor calls with multiple arguments so that each generated method has its own pending Sorbet sig.

As a result of this PR

#: Integer
attr_reader :a, :b

is translated into

sig { returns(Integer) }
attr_reader :a

sig { returns(Integer) }
attr_reader :b

@KaanOzkan KaanOzkan mentioned this pull request Jul 13, 2026
Open
Morriar
Morriar reviewed Jul 16, 2026
View reviewed changes

@Morriar Morriar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this is related to Shopify/tapioca#2662. Shouldn't this be fixed at the sorbet-runtime layer instead?

@KaanOzkan

KaanOzkan commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@Morriar I gave it a go in https://github.com/Shopify/sorbet/pull/870/changes but couldn't reduce the surface area. We need to keep track of multiple method_added calls and associate it with the same signature. I feel like fixing it in Spoom is a cleaner implementation, wdyt?

@KaanOzkan
KaanOzkan force-pushed the ko-fix-rbs-comments-multi-attr branch from 1352c0f to 5528b84 Compare August 19, 2026 14:05
@KaanOzkan
KaanOzkan requested a review from Morriar August 19, 2026 15:13
@KaanOzkan
KaanOzkan force-pushed the ko-fix-rbs-comments-multi-attr branch from 5528b84 to 1716ad5 Compare August 24, 2026 15:10
@KaanOzkan KaanOzkan changed the title Split typed multi-name attrs into per-name sigs Support signatures on multi-name attrs Aug 27, 2026
@KaanOzkan KaanOzkan changed the title Support signatures on multi-name attrs Support RBS comment translation on multi-name attrs Aug 27, 2026
@KaanOzkan
KaanOzkan force-pushed the ko-fix-rbs-comments-multi-attr branch 2 times, most recently from e2c4436 to 216cafc Compare August 28, 2026 15:15
@KaanOzkan
KaanOzkan force-pushed the ko-fix-rbs-comments-multi-attr branch 3 times, most recently from 636d21d to 4e396b7 Compare August 31, 2026 15:06
@KaanOzkan
Fix signatures for multi-name attrs
184f5a3 
Given this RBS:

```ruby
#: Integer
attr_reader :foo, :bar
```

Before this fix, Spoom translated it to:

```ruby
sig { returns(Integer) }
attr_reader :foo, :bar
```

After this fix, Spoom translates it to:

```ruby
sig { returns(Integer) }
attr_reader :foo
sig { returns(Integer) }
attr_reader :bar
```
@KaanOzkan
Preserve comments when splitting attrs
d722f87 
Given this RBS:

```ruby
#: Integer
attr_reader(
  :foo,
  :bar, # internal name
  # keep this note
)
```

Spoom now translates it to:

```ruby
sig { returns(Integer) }
attr_reader(:foo)
sig { returns(Integer) }
attr_reader(:bar) # internal name
# keep this note
```
@KaanOzkan
KaanOzkan force-pushed the ko-fix-rbs-comments-multi-attr branch from 4e396b7 to d722f87 Compare August 31, 2026 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Morriar Morriar Awaiting requested review from Morriar

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@KaanOzkan @Morriar