Nicer error message for "Error: undefined method 'size' for Nil" when do multiple assignment.

[zw963]

When i run some code on my local, i get following error message:

 ╰─ $ 1  cr run src/procodile.cr --error-trace
In src/procodile.cr:76:34

 76 | ap = Procodile::AppDetermination.new(FileUtils.pwd, options[:root], options[:procfile], global_config)
                                       ^--
Error: instantiating 'Procodile::AppDetermination.class#new(String, String, String, (Procodile::GlobalOption | Nil))'

In src/procodile/app_determination.cr:26:45

 26 | @root, @procfile, @in_app_directory = calculate(@pwd, @given_root, @given_procfile, @global_options)
                                            ^---
Error: undefined method 'size' for Nil

Nil trace:

  src/procodile/app_determination.cr:26

          @root, @procfile, @in_app_directory = calculate(@pwd, @given_root, @given_procfile, @global_options)
                                                ^~~~~~~~~

  src/procodile/app_determination.cr:70

        private def calculate(pwd, given_root, given_procfile, global_options)
                    ^~~~~~~~~

  src/procodile/app_determination.cr:72

          root = find_root_and_procfile(pwd, given_root, given_procfile)

  src/procodile/app_determination.cr:73

          if !root

  src/procodile/app_determination.cr:75

            find_root_and_procfile_from_options(global_options)
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  src/procodile/app_determination.cr:106

        private def find_root_and_procfile_from_options(options)
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  src/procodile/app_determination.cr:107

          case options

  src/procodile/app_determination.cr:107

          case options

  src/procodile/app_determination.cr:111

          when Array

  src/procodile/app_determination.cr:114

            if @app_id

I don't know why, it should ouput some message like following too, right? (the compile time type report is missing in this case)

Error: undefined method 'size' for Nil (compile-time type is (Array(Int32) | Nil))

But, the key point is, why we output undefined method 'size' for Nil for a potentially array type, even, never a #size method be invoked by user, is it appropriate to use the #size method to stand for arrays?

above case can be reproduce like this:

def calculate
  if true
    [1, 2, 3]
  end
end

x, y, z = calculate

 ╰─ $ cr 1.cr 
Showing last frame. Use --error-trace for full trace.

In 1.cr:7:11

 7 | x, y, z = calculate
               ^---
Error: undefined method 'size' for Nil (compile-time type is (Array(Int32) | Nil))

Although, i really don't know why (compile-time type is (Array(Int32) | Nil)) is not output correctly occasionaly.

[Blacksmoke16]

But, the key point is, why we output undefined method 'size' for Nil for a potentially array type, even, never a #size method be invoked by user

It's because your x, y, z = calculate is equivalent to something like:

__temp_1 = calculate
if __temp_1.size < 3
  ::raise("BUG: multiple assignment count mismatch")
end
x = __temp_1[0]
y = __temp_1[1]
z = __temp_1[2]

which is internally generated by the compiler.

Sounds like that code could also use a nil check.

EDIT: It actually seems your code produces Error: undefined method '[]' for Nil (compile-time type is (Array(Int32) | Nil)). Which seems to be different than your example. Which also seems to suggest there is no .size call in this context, so it's prob failing on the __temp[0] assignment.

[zw963]

It actually seems your code produces Error: undefined method '[]' for Nil (compile-time type is (Array(Int32) | Nil))

No, when run on my local linux, it produces

In 1.cr:7:11

 7 | x, y, z = calculate
               ^---
Error: undefined method 'size' for Nil (compile-time type is (Array(Int32) | Nil))

which is internally generated by the compiler.

Okay, i consider those code generated by compiler should check right op should be a Array or a Tuple, and, raise error use more clearly message instead of undefined method '[]' or '.size', what do you think?

[Blacksmoke16]

No, when run on my local linux, it produces

Same thing on 1.6.1

Are you sure the code you're running is:

def calculate
  if true
    [1, 2, 3]
  end
end

x, y, z = calculate

But yes, I would agree the error could be a bit more clear on what the problem is.

[zw963]

Are you sure the code you're running is:

def calculate
  if true
    [1, 2, 3]
  end
end

x, y, z = calculate

I test both on static(musl) and dynamic 1.6.1, all same result.

[straight-shoota]

Whether size is checked or not depends on the flag strict_multi_assign:

$ crystal build .test/multiassign.cr
Showing last frame. Use --error-trace for full trace.

error in line 7
Error: undefined method '[]' for Nil (compile-time type is (Array(Int32) | Nil))
$ crystal build .test/multiassign.cr -Dstrict_multi_assign
Showing last frame. Use --error-trace for full trace.

error in line 7
Error: undefined method 'size' for Nil (compile-time type is (Array(Int32) | Nil))

[zw963]

Whether size is checked or not depends on the flag strict_multi_assign:

$ crystal build .test/multiassign.cr
Showing last frame. Use --error-trace for full trace.

error in line 7
Error: undefined method '[]' for Nil (compile-time type is (Array(Int32) | Nil))
$ crystal build .test/multiassign.cr -Dstrict_multi_assign
Showing last frame. Use --error-trace for full trace.

error in line 7
Error: undefined method 'size' for Nil (compile-time type is (Array(Int32) | Nil))

Oops, you are right, i enable some option globally, i forget it.

 ╰─ $ echo $CRYSTAL_OPTS 
-Dstrict_multi_assign -Dno_number_autocast

[straight-shoota]

Back to improving the error message: Just a nil check (ref https://github.com/crystal-lang/crystal/issues/12682#issuecomment-1295834138) would improve this specific use case. But: any other type that doesn't implement [] and size (depending on the flag) would still create an unhelpful error message. So I think the improvement should really involve checking that the type implements these methods. However, I don't see a simple implementation for that at compile time. We might need to enhance the macro API or some other kind of compiler support. Or consider requiring the Indexable type for multi assignment 🤷

[asterite]

It should be another module like MultiAssignTarget