Regression in JRuby-head

The test-suite for rubocop-ast (a core dependency of RuboCop) fails massively for current JRuby-head.

This gem is very stable; last run was 2 months ago using "jruby-head-ubuntu-20.04.tar.gz"

The failure is sadly deep inside the metaprogramming of the gem. I suspect that this method is called and that somehow node.child is nil?

I have quite an extensive backlog so can't investigate further right now. Maybe a git bisect would help pinpoint what could cause the issue?

@marcandre I will take a look today. I expect jruby-head to have issues but we may as well focus a bit more on people running it in CI.

This may be related to #7247. So #7273 could improve this.

Did it fix the issue?

Apparently not.

umm...

JRuby build version on GitHub Actions is too old

jruby 9.4.0.0-SNAPSHOT (3.1.0) 2022-04-27 cefd578cef OpenJDK 64-Bit Server VM 11.0.16+8 on 11.0.16+8 [x86_64-linux]

https://github.com/rubocop/rubocop-ast/runs/7724638022?check_suite_focus=true#step:4:24

@eregon

Do you have the next release date of jruby-dev-builder ?

It runs daily automatically but fails since April because of https://github.com/jruby/jruby/issues/7106

That makes sense.

https://github.com/jruby/jruby/issues/7308 got fixed, so now there is a recent build of jruby-head for setup-ruby.

Latest runs appear to be green, so I'm optimistically closing this.

@headius runs of which project? I tried it just now: https://github.com/rubocop/rubocop-ast/actions/runs/3210248983/jobs/5373178395

@marcandre Oops, I must have been looking at an old build or a different branch. Will investigate a bit locally.

One failure on 9.3:

  1) RuboCop::AST::RegexpNode#to_regexp with a regexp with an "n" option is expected to eq "/abc/n"
     Failure/Error: it { expect(regexp_node.to_regexp.inspect).to eq('/abc/n') }
     
       expected: "/abc/n"
            got: "/abc/"
     
       (compared using ==)
     # ./spec/rubocop/ast/regexp_node_spec.rb:60:in `block in <main>'

Six on master HEAD:

  1) RuboCop::AST::RegexpNode#to_regexp with a regexp with an "n" option is expected to eq "/abc/n"
     Failure/Error: it { expect(regexp_node.to_regexp.inspect).to eq('/abc/n') }
     
       expected: "/abc/n"
            got: "/abc/"
     
       (compared using ==)
     # ./spec/rubocop/ast/regexp_node_spec.rb:60:in `block in <main>'

  2) RuboCop::AST::NodePattern predicates with a named argument for which the predicate is true is expected to match code s(:send,
  s(:int, 1), :+,
  s(:int, 2)) and {:param=>1}
     Failure/Error: @cache[:lambda].call(*args, block: block, **rest)
     
     ArgumentError:
       wrong number of arguments (given 3, expected 1)
     # ./lib/rubocop/ast/node_pattern.rb:71:in `match'
     # ./spec/rubocop/ast/node_pattern_spec.rb:33:in `block in matches?'
     # ./spec/rubocop/ast/node_pattern_spec.rb:1224:in `block in <main>'

  3) RuboCop::AST::NodePattern params as named parameters when provided as argument to match is expected to match code s(:int, 10) and {:foo=>#<Object:0x7f5a3e41>}
     Failure/Error: @cache[:lambda].call(*args, block: block, **rest)
     
     ArgumentError:
       wrong number of arguments (given 3, expected 1)
     # ./lib/rubocop/ast/node_pattern.rb:71:in `match'
     # ./spec/rubocop/ast/node_pattern_spec.rb:33:in `block in <main>'
     # ./spec/rubocop/ast/node_pattern_spec.rb:1338:in `block in <main>'

  4) Changelog parses correctly
     Failure/Error: Changelog::Entry.new(type: type, body: "Do something cool#{'x' * i}", user: "johndoe#{'x' * i}")
     
     ArgumentError:
       wrong number of arguments (given 1, expected 0)
     # ./tasks/changelog.rb:24:in `initialize'
     # ./spec/tasks/changelog_spec.rb:46:in `block in entries'
     # ./spec/tasks/changelog_spec.rb:45:in `block in entries'
     # ./spec/tasks/changelog_spec.rb:10:in `block in changelog'
     # ./spec/tasks/changelog_spec.rb:60:in `block in <main>'

  5) Changelog merges correctly
     Failure/Error: Changelog::Entry.new(type: type, body: "Do something cool#{'x' * i}", user: "johndoe#{'x' * i}")
     
     ArgumentError:
       wrong number of arguments (given 1, expected 0)
     # ./tasks/changelog.rb:24:in `initialize'
     # ./spec/tasks/changelog_spec.rb:46:in `block in entries'
     # ./spec/tasks/changelog_spec.rb:45:in `block in entries'
     # ./spec/tasks/changelog_spec.rb:10:in `block in changelog'
     # ./spec/tasks/changelog_spec.rb:64:in `block in <main>'

  6) Changelog Changelog::Entry generates correct content
     Failure/Error: Changelog::Entry.new(type: type, body: "Do something cool#{'x' * i}", user: "johndoe#{'x' * i}")
     
     ArgumentError:
       wrong number of arguments (given 1, expected 0)
     # ./tasks/changelog.rb:24:in `initialize'
     # ./spec/tasks/changelog_spec.rb:46:in `block in entries'
     # ./spec/tasks/changelog_spec.rb:45:in `block in entries'
     # ./spec/tasks/changelog_spec.rb:49:in `block in entry'
     # ./spec/tasks/changelog_spec.rb:53:in `block in <main>'

1. RuboCop::AST::RegexpNode#to_regexp with a regexp with an "n" option => I can disable for JRuby, which I imagine does not support this option, right?

2-6) See same error, related to keyword parameters.

Looks like other regression(s) have been fixed though, good.

Some of these seem to be a failure to properly peel off keyword args passed into a block.

diff --git a/spec/rubocop/ast/node_pattern_spec.rb b/spec/rubocop/ast/node_pattern_spec.rb
index e1bd404..957621e 100644
--- a/spec/rubocop/ast/node_pattern_spec.rb
+++ b/spec/rubocop/ast/node_pattern_spec.rb
@@ -24,16 +24,17 @@ RSpec.describe RuboCop::AST::NodePattern do
       instance.match(node, *params, **keyword_params)
     end
   end
 
   RSpec::Matchers.define :match_code do |code, *params, **keyword_params|
     match do |pattern|
       instance = pattern.is_a?(String) ? described_class.new(pattern) : pattern
       code = parse(code) if code.is_a?(String)
+      p params
       if keyword_params.empty? # Avoid bug in Ruby < 2.6
         instance.public_send(method_name, code, *params)
       else
         instance.public_send(method_name, code, *params, **keyword_params)
       end
     end
   end

This params comes out as [{:param=>1}] before the first failure in node_pattern_spec and something similar before the second.

cc @enebo it seems like the related code should be putting kwargs in the **keyword_params parameter here.

@headius yeah I am just trying to repro this now. It looks like something is unmarking a kwarg as a regular arg.

@enebo I will look into the regexp thing then.

JRuby does support //n but it seems like something is stripping that from the given code path.

$ jruby -e 'p /foo/n'
/foo/n

Oops:

$ jruby -e 'p Regexp.new("abc", Regexp::NOENCODING)'
/abc/

$ rvm ruby-3.1 do ruby -e 'p Regexp.new("abc", Regexp::NOENCODING)'
/abc/n

This explanation for the other problems is simple but it will be some work. Our Struct implementation is not Ruby-3 keyword safe yet. It still depends on old logic. Nothing alarming.

This will fix the regexp error on 9.3 and master, once merged: https://github.com/jruby/jruby/pull/7416

51e79577fee fixes part of this by allowing structs calling new to propagate kwargs to initialized. There are still two other errors which also appear to be the same issue.

bd06d0237bb405accf4c4372f31c1122e0c4d26f fixes the other two failures. Once we merge jruby-9.3 this should be resolved.

Merged and spec runs green now.