@karlseguin do you have any scenario for this?

I am not convinced the Symbol => Nil will be comfortable when you potentially will expect a hash of other type if the tuple is not empty. NamedTuple#to_a for example return Array(Tuple(NoReturn, NoReturn)).

Tuple does not have a #to_a but if it had, the same problem will be present there. You will sometimes have a Array(Nil) or and Array(T1 | T2 | ...). So, you will still be needing some case over time.

Another alternative could be to do something as follows. Depending on your use case it might be useful or not.

def some_key?(**opt)
  if opt.empty?
    :empty
  else
    opt.each do
      return opt.keys[0]
    end
  end
end

some_key? foo: 1 # => :foo
some_key?        # => :empty

I'm sure I could come up with a different (better) way to solve my specific problem. But it seems weird to me that calling a method on an instance would give a compiler error based on the state of that instance. On the other hand, I guess the fact the they're known at compile time means that we can do the check at compile time.

I definitely think the current behavior is wrong (if for no other reason as being an unhelpful error message), so, we could:

1 - Change the default type to something else, but what?
2 - Raise a exception
3 - Improve the compile-time error message

My case was factories for creating tests objects that looked like:

def user(args = NamedTuple.new())
   # somewhere args.to_h was being called

the simplest solution in this case would be to make it nullable.

For reference, this is how it's currently being handled

in /opt/crystal/src/named_tuple.cr:402: macro didn't expand to a valid program, it expanded to:

================================================================================
--------------------------------------------------------------------------------
   1. 
   2.       {
   3.         
   4.       }
   5.     
--------------------------------------------------------------------------------
Syntax error in expanded macro: macro_4610676704:2: for empty hashes use '{} of KeyType => ValueType'

      {
      ^

================================================================================

    {% begin %}

Why not make it return nil. Because this is done via macros, I don't think that you will actually have to deal with to_h returning nil until there's an empty tuple in the union you're calling to_h on.

I changed it to give a more meaningful compile-time error. This seems like the least risky improvement. I couldn't figure out how to get it under test though.

👍 for returning nil.

Otherwise scenario below becomes impossible:

def foo(**options)
  options.to_h
end

to_h returns hash, not nil. Original PR was OK

I still think that raising in compile time is the right thing to do.

Dealing with a hash version of a named tuple is far from a type safe thing.
If the developer want to to something with the hash coming from options.to_h:

def foo(**options)
  # use options.to_h
end

there should be some basic assumptions of the types of the values. for example: all integers

def foo(**options)
  options.to_h.values.sum
end

So foo(a: 1, b: 2) # => 3 works. And foo(a: 1, b: "2") fails to compile.
foo also fails to, before this PR with a more cryptic error message.
So failing with a clear "Can't convert an empty NamedTuple to a Hash" is an improvement.

What happens if we want to return a hash (of some type) for the empty tuple?
For hash type we choose Symbol => Nil, NoReturn => NoReturn, etc there will be a case that fails to compile when the original named tuple is empty due to a type mismatch between the hash for the empty tuple ad the hash for the non empty tuple.

So, in the sample case of returning the sum of the values of the named tuple the programer is left with two options.

a) overload:

def foo
  0
end

def foo(**options)
  options.to_h.values.sum
end

b) split the case with and weird is_a? (I think there is no other way to express the empty tuple type).

def foo(**options)
  if options.is_a?(typeof(NamedTuple.new))
    0
  else
    options.to_h.values.sum
  end
end

@karlseguin I authored the changeset, there was an unneeded begin/end block since now there is a macro if. After CI pass this should be ready to merge.

I also added a spec for this (yet upon #2391 it could be moved to the std)