Crystal: Unused code is not checked by the compiler

[jwaldrip]

I noticed that the crystal compiler does not validate unused code. IMHO, crystal should check the unused code, if I write a library, If I don't write a test that uses my code, the issue may not be found until someone includes the code in their project.

Example:

The following example compiles and runs:

class Foo

  def bar
    this_is_not_a_method_or_var
  end 
end

puts Foo.new

$ crystal run ./sample.cr
#<Foo:0x108335ff0>

It does not fail until you call the code that uses the invalid code.

class Foo

  def bar
    this_is_not_a_method_or_var
  end 
end

puts Foo.new.bar

$ crystal run ./sample.cr
puts Foo.new.bar
             ^~~

in script.cr:4: undefined local variable or method 'this_is_not_a_method_or_var'

    this_is_not_a_method_or_var
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~

[refi64]

This is a duplicate of some other issue, and there was a good reason for it (namely, checking all unused code would seriously slow down builds).

FWIW it's not any dead code, but just methods that are never called. If you can make a library with methods that never get called, your tests are screwed up anyway...

[bew]

Similar to: #3801 (but this was for compiler sources, but it's the same issue)

I think a special flag --dead-code or crystal tool dead-code would be a good idea!

[ozra]

@jwaldrip - meaning: you're actually writing code you never run and you let your users test for the first time!!?? Makes no sense. Write tests. Or use them yourself. Why write code that is never used?

[veelenga]

@ozra I think this is mostly about accidentally written dead code. Human is not ideal and can do mistakes. And we all are looking for a great software that can help to avoid those mistakes. This is what we love Crystal for :)

[luislavena]

It does not fail until you call the code that uses the invalid code.

Please note that the error is coming at compile time, not execution time. Since you're using crystal run it might be confusing, but Crystal doesn't evaluate or validate code that is not invoked.

Now, in relation to testing, that is a complete different subject 😄

[ozra]

@veelenga - sometimes I wish I spent one more second thinking before writing ;-)

Code-coverage check feature!

[refi64]

Is this really for dead code, though? OP wrote:

f I write a library, If I don't write a test that uses my code, the issue may not be found until someone includes the code in their project.

which would imply that the code is very much not dead. Maybe zombie code, but not dead.

[bcardiff]

There are different scenarios.

1. In #3801 the idea was to find defs that where never called. It's kind of a static analysis for code coverage.
2. Dead code is when the code can't be used do to certain reasons (nested conditions, case/when that already cover all possible cases, etc).
3. A third, pointed at the beginning of the issue is to fail to compile when a method just does not make sense even if it is not called. This could detect typos or just pending thing to be done in the source.

The compiler right now is lazy in the sense that won't analyze that the method types unless there is an invocation. Because from the invocation the type of the arguments will be known and the type analysis can begin. This is something that could change. Maybe leading to have 3. as a consequence of the compiler/typing process or it could be added as a phase to check. But adding it as an additional phase will slow thing a bit more today since it will require to identify 1. first and check 3. from them.

[ozra]

I'd definitely vote for on demand static analysis, not bloating the regular compilation process.

[asterite]

This is the same as in Ruby: unused code is not checked, because it's not instantiated. If we want to change this it means we need to add mandatory types for method arguments, traits, interfaces, etc.

[faustinoaq]

@veelenga Interesting issue, Can ameba help with this?