Fixes #34141 - Make validators modern Ruby compatible

[ekohl]

In old Ruby versions a hash was pushed as the last argument but in modern Ruby there is first class support for this via kwargs. This worked if a single argument was provided but if they were combined it failed:

validate_presence :setting, if: ->(settings) { true }

This then tried to create a validator of a hash.

It looks like this code currently has no coverage which is why this never showed up. I'm also not sure if this is used in any plugins. I'm taking a look at writing tests.

[ekohl]

Looks like GH had a problem.
[test smart-proxy]

[ekohl]

At first I thought this may have been https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/ but now I'm wondering if this ever worked. However, I'm a bit uncertain about this on Ruby 2. Tests pass, but is this really the right thing? That post doesn't describe Ruby 2.5 and 2.6 has some more warnings.

[lzap]

Make sense.

[lzap]

This was written in a way that only array could be passed in. This is a new feature. Do you still want this to be merged then?

[ekohl]

So I had some difficulty deciding that. Was it previously possible to pass it as the last argument and should it always have been possible? How compatible is it? I'm mostly unsure about changing the validate() method.

[adamruzicka]

To be honest I've never dug too deep into ruby's keyword arguments, but if I were doing this, I'd do it this way. I briefly tried checking this out with ruby 2.5 and it seems to work just fine with the set of plugins I usually run with.

Was it previously possible to pass it as the last argument and should it always have been possible?

As far as I understand (and per[1]) then ruby <2.0 didn't have keyword arguments at all, but keyword-argument-like thing could be implemented by passing a hash as the last argument. In ruby 2.0 they introduced proper keyword arguments and the double splat operator, but under the hood it was (probably) just syntactical sugar around passing a hash as the last argument, so I'd go with yes here.

How compatible is it?

From what I've seen passing an argument as a hash to method with double splat in its signature generates a warning on <3.0 and raises an ArgumentError on >=3.0. The "new way" as proposed here should work on >=2.0.

2.6 has some more warnings.

Do you have any examples?

[1] - https://thoughtbot.com/blog/ruby-2-keyword-arguments

[ekohl]

2.6 has some more warnings.

Do you have any examples?

I meant that https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/ has various notes about how to write it for Ruby 2.6. The section "A compatible delegation that works on Ruby 2.6, 2.7 and Ruby 3". It feels to me like the current patch shouldn't work on Ruby 2.5, but perhaps because we're calling it DSL-style in our plugins it actually works since we're using kwargs.

[adamruzicka]

we're calling it DSL-style in our plugins it actually works since we're using kwargs.

That's most likely what's happening

[ekohl]

In that case I feel safe merging this as a feature for 3.2 but I'm unsure about cherry picking it to 3.1, which we would probably want for REX. Should I modify theforeman/smart_proxy_remote_execution_ssh#57 to use a workaround so it can be cherry picked?

[adamruzicka]

Should I modify theforeman/smart_proxy_remote_execution_ssh#57 to use a workaround so it can be cherry picked?

Please do

[ekohl]

While I don't have time to finish that PR now, is there anything holding back this PR?

[adamruzicka]

Not really, no. Let's get this in