Skip to content

Add error position and json_path to the JRuby extension - #1067

Merged
byroot merged 2 commits into
ruby:masterfrom
ojuschugh1:java-parser-positions
Sep 16, 2026
Merged

byroot merged 2 commits into
ruby:masterfrom
ojuschugh1:java-parser-positions

Conversation

@ojuschugh1

Copy link
Copy Markdown
Contributor

Follow-up to #1062, as suggested there: this brings ParserError#line, #column and #json_path to the JRuby extension.

The Java parser already shares the C parser's architecture (frame stack + value stack), so the implementation mirrors it closely: positions are computed by scanning backwards from the error offset like cursor_position does in C, and @json_path is collected as raw segments from the frame stack, then formatted by the shared Ruby code from #1062. Everything happens at raise time, so nothing is added to the happy path. NestingError stays bare, matching the C extension.

Positions and paths come out identical to the C parser:

**jruby 10.0.2.0**
unexpected token at 'xyz}]}}' at line 1 column 34
line=1 column=34 json_path=$.user.roles[1].admin
duplicate key "c" at line 1 column 17
line=1 column=17 json_path=$.a.b[1].c


The duplicate key case reuses the existing cursor rewind to the object's opening brace, so line/column match C exactly there too. Error message wording still differs between the two extensions ("unexpected token at" vs "unexpected character:"); I left that alone to keep the diff focused.

The four json_path test methods now run on JRuby (omit guards removed), plus a new test_parse_error_position asserting line/column values on both extensions. JRuby suite goes from 53 omissions to 49 with 0 failures; CRuby stays at 0 failures, 0 omissions.

@ojuschugh1

Copy link
Copy Markdown
Contributor Author

Hey @byroot could you kindly review this.

@byroot

byroot commented Sep 16, 2026

Copy link
Copy Markdown
Member

Uh, no idea what I haven't seen this until now.

@byroot
byroot force-pushed the java-parser-positions branch from 63352da to 4994fe3 Compare September 16, 2026 12:46
@byroot
byroot force-pushed the java-parser-positions branch from 3220ded to 442961f Compare September 16, 2026 12:55
@byroot
byroot merged commit 729af7a into ruby:master Sep 16, 2026
42 checks passed
@headius

headius commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Why did coverage need to be disabled?

@byroot

byroot commented Sep 17, 2026

Copy link
Copy Markdown
Member

Why did coverage need to be disabled?

You can see the failing builds on the first commit.

LoadError: cannot load such file -- prism
           require at org/jruby/RubyKernel.java:1384
   replace_require at /home/runner/.rubies/jruby-10.1.1.0/lib/ruby/stdlib/bundled_gems.rb:81
            <main> at /home/runner/work/json/json/vendor/bundle/jruby/4.0.0/gems/simplecov-1.3.0/lib/simplecov/static_coverage_extractor.rb:3
           require at org/jruby/RubyKernel.java:1384
  require_relative at org/jruby/RubyKernel.java:1408
            <main> at /home/runner/work/json/json/vendor/bundle/jruby/4.0.0/gems/simplecov-1.3.0/lib/simplecov/source_file.rb:4
           require at org/jruby/RubyKernel.java:1384
  require_relative at org/jruby/RubyKernel.java:1408
            <main> at /home/runner/work/json/json/vendor/bundle/jruby/4.0.0/gems/simplecov-1.3.0/lib/simplecov.rb:175
           require at org/jruby/RubyKernel.java:1384
   replace_require at /home/runner/.rubies/jruby-10.1.1.0/lib/ruby/stdlib/bundled_gems.rb:81
            <main> at /home/runner/work/json/json/test/json/test_helper.rb:14
           require at org/jruby/RubyKernel.java:1384
  require_relative at org/jruby/RubyKernel.java:1408
            <main> at /home/runner/work/json/json/test/json/json_coder_test.rb:3
           require at org/jruby/RubyKernel.java:1384
   replace_require at /home/runner/.rubies/jruby-10.1.1.0/lib/ruby/stdlib/bundled_gems.rb:81
            <main> at /home/runner/work/json/json/vendor/bundle/jruby/4.0.0/gems/rake-13.4.2/lib/rake/rake_test_loader.rb:21
            select at org/jruby/RubyArrayNative.java:2488
            <main> at /home/runner/work/json/json/vendor/bundle/jruby/4.0.0/gems/rake-13.4.2/lib/rake/rake_test_loader.rb:6
rake aborted!

@ojuschugh1

Copy link
Copy Markdown
Contributor Author

Hey @byroot thanks for the quick review and approval.

No it is not your fault, the thing the github automated algo falsely flagged my github account , so my github account, repo etc were not visible to other, even though i raised my PR for this follow fix, but still is was not showing to others and as I am not a prod users , so it took more than 2 weeks to get their human support for get my account back :(

@headius

headius commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

You can see the failing builds on the first commit.

Ah, so it is not including prism as a gem dependency and expecting it to just be available. We don't include prism in the box yet because JRuby is cross-platform and prism requires a native library. If it were in a Gemfile, it would install and work just fine.

I'm working on shipping a cross-platform version using wasm but it's not ready yet.

It would probably be healthy to include it as an explicit dependency.

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.

3 participants