Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support double splatting on NamedTuple union. #14811

Open
zw963 opened this issue Jul 14, 2024 · 2 comments
Open

Support double splatting on NamedTuple union. #14811

zw963 opened this issue Jul 14, 2024 · 2 comments

Comments

@zw963
Copy link
Contributor

zw963 commented Jul 14, 2024

Bug Report

Both of following code give same error message

def foo(a, b, c = false)
  p [a, b, c]
end

args = {
  a: 1,
  b: 2,
}

# Both of following code not work

if rand(5) > 3
  args = args.merge(g: true)
end

# ----------------------

if rand(5) > 3
  args = {
    a: 1, b: 2,
  }
else
  args = {
    a: 1, b: 2, c: true,
  }
end

foo(**args)

The very detailed discussion, check here

I didn't see an opened issue for this, considering this issue has been reported by the compiler itself for a long time, and I'm not the only one who meet this issue, I created a new issue.

@zw963 zw963 added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Jul 14, 2024
@straight-shoota straight-shoota added kind:feature topic:lang and removed kind:bug A bug in the code. Does not apply to documentation, specs, etc. labels Jul 14, 2024
@devnote-dev
Copy link
Contributor

I'm curious as to when you would ever need to do this; there are so many alternative ways to handle this that it makes me question if this should be implemented at all.

@ysbaddaden
Copy link
Contributor

Work arounds for those that need a solution:

  1. the obvious one: avoid creating an union; either by always merging, or with a distinct double splat in each branch of a condition;

    foo(**args.merge(checked: rand(0..2) == 0))
    if rand(0..2) == 0
      foo(**args.merge(checked: true))
    else
      foo(**args)
    end
  2. add a method overload that takes a NamedTuple:

    def foo(**args)
      foo(args)
    end
    
    def foo(args : NamedTuple)
      p args
    end
    
    if rand(0..2) == 0
      args = args.merge(checked: true))
    end
    
    foo(args)

I believe libraries should always provide an overload to accept a NamedTuple directly. The point is to spare an expensive Hash when args are known at compile-time. Double splats are mostly sugar coat: they hide a merge (for example foo(**args, checked: true) reads much better than foo(args.merge(checked: true)) and allow intermediary methods to anonymously pass args around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants