fix wrong error message for nil.func() call

[tonini]

The expected behavior for nil.func() should be:

iex(1)> nil.test()
** (UndefinedFunctionError) undefined function: nil.test/0
   nil.test()

Also via cli elixir -e:

$ elixir -e 'nil.test()'
** (UndefinedFunctionError) undefined function: nil.test/0
   nil.test()
   ....

@josevalim as you can see I explicit give an :no_module symbol as argument for the module inside of normalize_exception/2 function to get around the problem that nil get in front of every function name inside of the UndefinedFunctionError exception.

Maybe it's not the nicest solution, I'm open for feedback and ideas. :)

reference: #1823

[ericmj]

Note about no addition tests, there was no need to add more test to handle the case between nil.test() and foo.test() because there already tests which handle this case: https://github.com/elixir-lang/elixir/blob/master/lib/iex/test/iex/interaction_test.exs?source=cc

I find no tests for this in that file. https://github.com/elixir-lang/elixir/blob/master/lib/elixir/test/elixir/exception_test.exs would be a good place to add tests.

@josevalim as you can see I explicit give an empty string as argument for the module inside of normalize_exception/2 function to get around the problem that nil get in front of every function name inside of the UndefinedFunctionError exception.

I would go with something as :no_module instead of empty string. Maybe there is a nicer solution...

[tonini]

@ericmj Good point.

I would go with something as :no_module instead of empty string. Maybe there is a nicer solution...

I will add tests and also use the :no_moduleinstead of an empty string.

[tonini]

@ericmj @josevalim I made the changes

• :no_module instead of an non-informativ empty string (thx @ericmj :)
• add explicit tests for it.

@josevalim what's about this raise_preserves_the_stacktrace test inside of exception_tests.exs could't the explicit line number be replaced with an _?. Is the explicit line number needed to be a proper test in this case?

good night. 🌔

[josevalim]

Thanks @ericmj and @tonini! I don't think the :no_module is a good approach though because :no_module is still a valid atom name. I think the best to do here is to stop using UndefinedFunctionError and simply return RuntimeError[message: "undefined function #{function}/#{arity}"] (note arity can be a list, so we need to convert it to an integer if so).

[tonini]

:no_module is still a valid atom name

Ok I see what you mean.

I will update the source with using a RuntimeError instead for a review.

[josevalim]

@josevalim what's about this raise_preserves_the_stacktrace test inside of exception_tests.exs could't the explicit line number be replaced with an _?. Is the explicit line number needed to be a proper test in this case?

Yeah, we are testing exactly the number doesn't change. We have a couple tests like that in the suite. :)

[josevalim]

We could probably fix those tests to use some combination of __ENV__.line though.

[tonini]

@josevalim You showed the error message above like : "undefined function #{function}/#{arity}"] but at the moment it's not showing the amount of the arity, instead the list of arguments itself like: undefined function: foo(123, "bar", [123, 4])

Would you like to change the current message style or leave it as it is?

[josevalim]

Let's just show the arity. This error can only happen when the user types no_helper(1, 2, 3) so the actual arguments are just one line above, there is no need to repeat them. :)

[tonini]

ok. :)

—
Sent from Mailbox for iPhone

On Fri, Nov 1, 2013 at 11:38 AM, José Valim notifications@github.com
wrote:

Let's just show the arity. This error can only happen when the user types no_helper(1, 2, 3) so the actual arguments are just one line above, there is no need to repeat them. :)

Reply to this email directly or view it on GitHub:
#1834 (comment)

[tonini]

@josevalim @ericmj Ok I'm done with a first realization, please review and give feedback if needed. ;)

[josevalim]

I don't think it can ever not be one of those, so I would get rid of this clause. Everything else is perfect. :)

[tonini]

do you talk about the true -> clause` ?

[tonini]

@josevalim The clause is updated. :)

[josevalim]

❤️ 💚 💙 💛 💜

[ericmj]

Awesome @tonini!

[tonini]

My pleasure!