Add support for Prism.parse(foo, version: "current")

[Earlopain]

The docs currently say to use Prism.parse(foo, version: RUBY_VERSION) for this. By specifying "current" instead, we can have prism raise a more specifc error. It doesn't add much code so I think being more explicit is worth it. It's similar to Prism::Translation::ParserCurrent but instead of warning it raises.

For a smooth experience, it requires a prism release right around when a new ruby version is released. But that is also already true for using RUBY_VERSION.

Note: Does not use ruby_version from ruby/version.h because writing a test for that is not really possible. I instead pull it from RUBY_VERSION constant at runtime so it can be stubbed.

Ref https://bugs.ruby-lang.org/issues/21618

[Earlopain]

Unsure about the ffi backend. My understanding is that alternate ruby versions will use it, basically the interface to the version of prism shipped with their runtime itself. So it should also be supported there. Is that right?

[eregon]

This looks good to me 👍

[eregon]

Unsure about the ffi backend. My understanding is that alternate ruby versions will use it, basically the interface to the version of prism shipped with their runtime itself.

Not quite, you can think of it as Prism gem having 2 backends: a C extension, only used on CRuby, and the FFI backend used on JRuby and TruffleRuby.
Whether it's the prism default gem or later-installed prism gem doesn't really make a difference.

So it should also be supported there. Is that right?

Yes.

[eregon]

I wonder if Prism.parse(foo, version: :current) could be easily supported?
That seems nice to me because :current is a "symbolic value" to mean current version.
It's a detail though, so if not trivial probably not worth it.

[Earlopain]

Thanks for the the review!

Not quite, you can think of it as Prism gem having 2 backends: a C extension, only used on CRuby, and the FFI backend used on JRuby and TruffleRuby.
Whether it's the prism default gem or later-installed prism gem doesn't really make a difference.

Ah, right. I thought that at least on truffleruby it would use the c extension when part of the gemfile (i.e. explicitly installed)

I wonder if Prism.parse(foo, version: :current) could be easily supported?

Probably, just convert it to a string somewhere. There is already Prism.parse(foo, version: "latest") though, so I would not do it in this PR. I'm not so interested in it, feel free to open an issue for that if you want it.

[kddnewton]

In general I'm totally fine with this approach, but we're doing a lot of string parsing here that I think we should do away with, as this is not going to be a common path. Instead, I would prefer something like:

if 3.3, 3.4, 3.5 -> use version
else -> raise SomeError, given

and then declare SomeError in Ruby and have it shared between the native API and the serialization API.

[Earlopain]

I guess you're saying to move the C extension code for version handling into ruby as well, and then share that between ffi? Something like Prism.resolve_ruby_version_or_raise! or something. Is there a place for this already? I didn't really find something that would fit.

[kddnewton]

Not quite. What I mean is that instead of:

if (RSTRING_LEN(value) == 7 && strncmp(version, "current", 7) == 0) {
    if (strncmp || strncmp || strncmp || strncmp) { rb_raise }
    if (!pm_options_version_set(options, current_version, 3)) { rb_raise(ArgumentError, "...") }
}

and:

if current
  case RUBY_VERSION
  when ...
    raise ArgumentError, "..."
  end
end

and in the tests:

error = assert_raise(ArgumentError) { Prism.parse(...) }
assert_equal error.message, "..."

you somewhat simplify to:

# prism.rb
class CurrentVersionError < ArgumentError
  def initialize(version)
    super("Prism does not support #{version}. Please specify a supported version instead of 'current'.")
  end
end

and:

if (RSTRING_LEN(value) == 7 && strncmp(version, "current", 7) == 0) {
    if (!pm_options_version_set(options, current_version, 3)) { rb_raise(rb_cCurrentVersionError, "...") }
}

and:

if current && RUBY_VERSION !~ /supported-versions-pattern/
  raise CurrentVersionError, "..."
end

and in the tests:

error = assert_raise(CurrentVersionError) { Prism.parse(...) }

[Earlopain]

Ah, gotcha. That is nicer. Thanks for the explanation

[Earlopain]

ruby/ruby CI is failing here because Gem is not loaded: https://github.com/ruby/ruby/actions/runs/18561653353/job/52911907413

[kddnewton]

Ugh of course. I'll fix it. Shouldn't have cowboy'd.

[Earlopain]

I seem to have messed something up as well https://github.com/ruby/ruby/actions/runs/18562695205/job/52915425235
Can't look into that right now though

(This one doesn't crash: https://github.com/ruby/ruby/actions/runs/18562695142/job/52915424818)

[kddnewton]

It's weird. I think RUBY_VERSION is just not consistent inside hte CRuby test suite. I reduced a lot of the tests and just ended up with some bare minimum stuff, and it appears to have resolved.