Skip to content

Conversation

@Earlopain
Copy link
Contributor

The long-term goal is to deprecate ripper: https://bugs.ruby-lang.org/issues/21827

So, this starts using prism to parse. Prism already knows if a comment is preceeded by code via trailing?, so that makes the RB case a bit simpler.

@eregon
Copy link
Member

eregon commented Jan 26, 2026

Nice. Incidentally today I was trying the rbs test suite on TruffleRuby and noticed quite a few failures, I'm not sure they are Ripper-related though, but I'd be happy to not have to worry about that :)

UPDATE: seems it's not Ripper related, basically same before & after this PR:

before:
Finished in 557.880725985 seconds.
------------------------------------------------------------------------------------------------------------------------------
882 tests, 6871 assertions, 34 failures, 76 errors, 0 pendings, 1 omissions, 0 notifications
87.5142% passed
------------------------------------------------------------------------------------------------------------------------------

after:
Finished in 550.67752058 seconds.
------------------------------------------------------------------------------------------------------------------------------
878 tests, 6958 assertions, 34 failures, 76 errors, 0 pendings, 1 omissions, 0 notifications
87.4572% passed
------------------------------------------------------------------------------------------------------------------------------

else
hash[line] = comment
end
comments = Prism.parse(string).yield_self do |parse_result|
Copy link
Member

Choose a reason for hiding this comment

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

Should use version: "current", right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yeah. I kept ripper around for ruby 3.2 since prism can't parse 3.2 syntax.

Since the two implementations for RB and RBI are pretty much identical except for trailing comments I consolidated their implementations so that there is not so much duplication.

The long-term goal is to deprecate ripper: https://bugs.ruby-lang.org/issues/21827

So, this starts using prism to parse. Prism already knows if a comment is preceeded by code via `trailing?`,
so that makes the `RB` case a bit simpler.
@Earlopain Earlopain force-pushed the no-ripper branch 2 times, most recently from c67fe7e to 67563ca Compare January 26, 2026 19:16
Copy link
Contributor Author

@Earlopain Earlopain left a comment

Choose a reason for hiding this comment

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

The rbi of prism is not correct, missing the version option. I created ruby/prism#3874 to fix this, anything I can do here to work around it for now?

spec.required_ruby_version = ">= 3.2"
spec.add_dependency "logger"
spec.add_dependency "prism", ">= 1.3.0"
spec.add_dependency "prism", ">= 1.6.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the prism version that added support for Prism.parse(version: "current"). Basically it is Prism.parse(version: RUBY_VERSION) with a nicer error message when the prism gem is outdated.

@ksss
Copy link
Collaborator

ksss commented Jan 27, 2026

As a workaround, you can add # steep:ignore at the end of the line where the source code points out.

The long-term goal is to deprecate ripper: https://bugs.ruby-lang.org/issues/21827

So, this starts using prism to parse. Prism already knows if a comment is preceeded by code via `trailing?`, so that makes the `RB` case a bit simpler.

Ripper is still used when running as ruby 3.2 because prism can't parse 3.2 syntax.

When runtime support for 3.2 is dropped, the fallback code can be dropped as well.
@Earlopain
Copy link
Contributor Author

Thanks! I fixed the typecheck by applying your suggestion. The next prism release will contain correct signatures, so it can be removed once that happens.

@eregon eregon mentioned this pull request Jan 27, 2026
Comment on lines +8 to +9
# Prism can't parse Ruby 3.2 code
if RUBY_VERSION >= "3.3"
Copy link
Member

Choose a reason for hiding this comment

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

Could use Prism for all Ruby versions if we're OK to parse 3.2 code with Prism 3.3 syntax, which should be fine for comments: #348 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly, seems fine to keep around for now. The ripper code has already been written and can just be removed at a later time.

Copy link
Member

@soutaro soutaro left a comment

Choose a reason for hiding this comment

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

Thanks!

@soutaro soutaro merged commit 088d66a into ruby:master Jan 28, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants