Crystal: More helpful `Nil assertion failed` error message

[Blacksmoke16]

https://play.crystal-lang.org/#/r/4kwn

class Foo
  property! name : String
end


foo = Foo.new

p foo.name

Unhandled exception: Nil assertion failed (Exception)
  from /usr/lib/crystal/class.cr:148:0 in 'not_nil!'
  from /eval:0:3 in 'name'
  from /eval:8:1 in '__crystal_main'
  from /usr/lib/crystal/crystal/main.cr:104:5 in 'main_user_code'
  from /usr/lib/crystal/crystal/main.cr:93:7 in 'main'
  from /usr/lib/crystal/crystal/main.cr:133:3 in 'main'
  from __libc_start_main
  from _start
  from ???

The exception for Nil assertion failed, right now, is quite lacking on details. Including the variable name and/or line number would aid greatly in trying to debug larger programs where it may not be super obvious where the error is coming from.

EDIT: I see the variable name is referenced in the 3rd line, better visibility of that might be better?

EDIT2: Maybe something along the lines of Unhandled exception: 'name' cannot be Nil?

[asterite]

A better exception message sounds good. Improving the line number is a different issue (has to do with the debug info generated by the program, it has nothing to do with the property! macro)

[j8r]

What do you think of having a ~reserved~ method #not Nil, that will be the opposite of #as T?
Following @bew 's solution:

class Object
  def not(type : T.class) forall T
    raise "should not be a #{T}" if self.is_a?(T)
    self
  end
end

var = 1.as(Int32 | String | Nil)

p! typeof(var)
p! typeof(var.not String)

Output:

typeof(var) # => (Int32 | String | Nil)
typeof(var.not(String)) # => (Int32 | Nil)

Then not_nil! could be deprecated, no more cryptic Nil assertion failed.

[bew]

no more cryptic Nil assertion failed.

Well you'd still have the same kind of message.. How would that #not be better than #not_nil! ?

[j8r]

Hum that was my first thought - it may be only possible with a reserved method.

[bew]

@j8r Same thing for a reserved keyword..
I don't understand, what are you expecting exactly? Can you give an example of a "Nice error" ?

[j8r]

I can be done with a macro at least, but that's not really OOP. I think something at least like String | Nil shouldn't be Nil

[ysbaddaden]

The property! macro can be fixed to print something like:

nil assertion error, expected T but got Nil

Where T is the non nilable property type, when its present, otherwise we just print the basic "nil assertion error".

The #not(T) proposal is unrelated to this issue, and wouldn't help to print a nicer error message over #not_nil!.

[bew]

Could we allow something like this:

class Object
  def foo
    p! typeof(self)
  end
end

v = 1.as(Int32 | String)
v.foo

To print Int32 | String instead of Int32 ?

_Maybe not this exact syntax_

[bcardiff]

@bew That is unrelated. The 1 is a value and the type is Int32. That value can be used as a value of Int32 | String but the value, semantically speaking, is the same. Even if the memory representation differs, the value is 1.

[bew]

Hmm maybe v : Int32 | String = 1; v.foo is more accurate. Basically a way to get the compile type of v from a method on Object..

But I guess this is stupid/useless/not wanted.

[straight-shoota]

@bew That's not possible. typeof shows the type of an expression. So you typeof(v) is Int32 | String. But the instance itself doesn't know anything about being assigned to a variable.
This should become obvious when you consider assigning the same value to a second variable having different type restrictions.

[bew]

Yeah you're right, let's forget about that.

So how could we show the wanted type without Nil (if wanted) when doing #not_nil! ? _maybe we can't then.._

[asterite]

Yes we can: typeof(exp.not_nil!)

[bew]

But from inside not_nil! ? (when building the error message)

[asterite]

Ah, right. That's not possible. But I don't understand why would that be useful, you can look up the code that triggered the error to find that out.

[Blacksmoke16]

If anything I think changing how the property!/getter! macro works would be a step in the right direction.

I.e. instead of

def name
  @name.not_nil!
end

make it define like

def name : String
  @name || raise NilAssertionError.new "Klass#name cannot be nil"
end

It still works the same but the error message makes it much much clearer as to what failed.

EDIT: I see @ysbaddaden suggested this already https://github.com/crystal-lang/crystal/issues/6431#issuecomment-472768406.

[Blacksmoke16]

It seems this aspect was already implemented by #8200.