-
Notifications
You must be signed in to change notification settings - Fork 174
Open
Description
irb -Ilib -rprism
irb(main):001> src = Prism::Source.for("1 + 2")
=> #<Prism::ASCIISource:0x00007f59cab9f840 @offsets=[], @source="1 + 2", @start_line=1>
irb(main):002> src.line_start(3)
=> nilThe problem is @offsets is [] which is never valid.
(@start_line defaults to 1 which is probably fine)
Another way to see it:
irb(main):004> Prism.parse("1\n2").source
=> #<Prism::ASCIISource:0x00007f59c9affc60 @offsets=[0, 2], @source="1\n2", @start_line=1>
irb(main):005> Prism::Source.for("1\n2")
=> #<Prism::ASCIISource:0x00007f59c998cf68 @offsets=[], @source="1\n2", @start_line=1>Prism::Source.for is notably used by Prism::DSL#source and might be used by code outside of Prism (I don't know).
It's also used in the Ripper translation layer, I'm fixing that usage in #3859.
The method takes arguments for the offsets and start_line:
prism/lib/prism/parse_result.rb
Line 13 in d3d3de9
| def self.for(source, start_line = 1, offsets = []) |
But because they are optional and Prism doesn't provide a way to compute the offsets (AFAIK) I don't think that's good enough.
Some solutions I can imagine:
- Remove
Prism::Source.for, only allow to create complete Source objects through Prism.{lex,parse}*. This is what the Java code does, the Source constructor is private, and the only way to get a Source is through lexing/parsing and get it from the ParseResult. - Have the default compute the offsets itself, probably most convenient but a bit of a performance trap (at least we should document it on the method). In the past we have seen that computing line offsets from Ruby is pretty slow.
- Have the default be
niland automatically compute the offsets when needed.
Metadata
Metadata
Assignees
Labels
No labels