Enable typing in Cask::Artifact

[dduugg]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

Enables typing in Cask::Artifact. In this PR, i've introduced a new util module, Utils::Splat, which allows a file to be typed by wrapping splat calls to Ruby core/std library methods. Initially, it has a single method, a Process.kill wrapper. It's admittedly a potentially clumsy abstraction – bringing together a bunch of methods whose only unifying factor is accepting splat arguments, but we could refactor it as necessary (though I think it will mostly be lower-level CLI-wrapped methods like system).

[BrewTestBot]

Review period will end on 2023-02-23 at 17:36:21 UTC.

[dduugg]

This previously converted args to a splat, in order to subsequently call a method that accepts splat arguments. I've cut out the middleman, in order to enable typing, although I think it also makes for slightly more understandable code.

The methods below are all private, which ofc doesn't guarantee that changing the method signature is safe, so this proposed change deserves a critical eye.

[reitermarkus]

Isn't Hash also an Enumerable, i.e. would now be just args instead of [args]?

[dduugg]

@reitermarkus Great question! When it was a Hash, it would be wrapped as an array, then splatted back into a hash arg. Since we've removed the *, we no longer want to wrap hashes as arrays.

This might help explain things:

def foo(args, c: nil)
  args.class
end

argv = 'str'
foo(*argv)
# => String
argv = ['str']
foo(*argv)
# => String
argv = {a: 1, b: 2}
foo(*argv)
# => ArgumentError: wrong number of arguments (given 2, expected 1)
foo(*[argv])
# => Hash

Ruby will convert a hash into kwargs when preceded by *, leading to errors in methods that take a mix of positional and kwargs (iirc without the kwargs it will pass the hash as the first poisitional arg). You can try it for yourself by changing Enumerable to Array above, and running brew tests --only=cask/artifact/uninstall.

(I believe this also relates to why sorbet has a hard time supporting splats, but my memory is hazy here)

I generally find this all pretty hard to wrap my head around, so I try to avoid splats regardless of Sorbet's stance 🤕

[reitermarkus]

Previously, we had e.g.

def uninstall_something(*args)
  args
end

and passing a Hash would look like

uninstall_something(*[{a: 1}]) == [{a: 1}]

while now we have

def uninstall_something(args)
  args
end

and passing a Hash, since it is Enumerable, we effectively have

uninstall_something({a: 1}) == {a: 1}

which isn't the same thing.

[reitermarkus]

Okay, so the only method we actually pass a Hash to is uninstall_script, which was not using a splat to begin with, so passing both *[{a: 1}] and {a: 1} still behave the same for that case.

[MikeMcQuaid]

Looks great, thanks again @dduugg!

[reitermarkus]

Okay, so the only method we actually pass a Hash to is uninstall_script, which was not using a splat to begin with, so passing both *[{a: 1}] and {a: 1} still behave the same for that case.

[MikeMcQuaid]

Thanks again @dduugg!

[BrewTestBot]

Review period ended.