Emit warnings if undefined function is called on a required module

[josevalim]

Both Elixir and Erlang do not expect the function you are calling to exist during compilation time. However, if a module has been required, we could perform such checks because then, by definition, it is available during compilation time.

The idea of this feature is to check if a function actually exists when compiling code and the function being called belongs to a module that has been required.

[x4lldux]

That would surely make our software safer. But specifying all used modules could be tedious, so we could implement a glob-like feature:

defmodule SafeModule do
  require *, except: [ModuleA, ModuleB, ModuleC]
  ...
end

which would mean, that all "called" modules need to be present at compile time. With a :except keyword to specify optional modules.

But I'd rather lean towards requiring all modules, which functions were called, to be present at compile time and give a way to define a list of optional modules. I think this way would cover a lot more use-cases:

defmodule SafeModule do
  @optional_modules [ModuleA, ModuleB]
  @optional_modules ModuleC

  ...
end

(this is a potentially breaking change)

[josevalim]

@x4lldux that feels to be another separate discussion: how to make it easy to require multiple modules?

I am quite sure we can never make something like * with :except work because the only way to know all modules is by compiling all of them. And then if you define require * in two different modules, how to solve that? I believe we should move this discussion elsewhere, possibly the elixir-lang-core mailing list, to keep this one focused on warnings on top of the existing require feature. What do you think?

[michalmuskala]

I think such a feature of checking modules in the entire project could be done much easier as a post processing step to the compilation. I'm thinking about something similar to what xref does, but make a convenient mix task of it and promote adding it to the test alias so it runs after (or even before) the tests.

Using require for this has also a significant drawback - adding compile-time dependency between modules will most probably significantly decrease compilation speed by limiting the concurrency of the compiler (because you need to compile your dependencies strictly before the particular module is compiled).

We can do a lot more with a separate analysis step - for instance analyzing undefined or unused modules and functions (similar to recently proposed defmodulep but based on @moduledoc false and @doc false). I also think this will be a more extensible solution that will enable adding additional checks later on (I could see a linter integrating into the post-compilation check).

[josevalim]

Using require for this has also a significant drawback

@michalmuskala that's only if you are using this feature for the purpose of getting warnings.

Maybe another way to put it is: would be worth to emit those warnings given the module was already required? It is considerably cheaper than a precompile check.

[michalmuskala]

I'm just afraid that people will start adding additional requires just to get those checks (because they are useful), and they will get slow compilation even not realizing why that is.

[josevalim]

I am not sure if it would increase compilation time by any noticiable measure. If one file halts, we will always put another one in the compilation pipeline.

[michalmuskala]

The problem is that with something like require * you put every file in the compilation pipeline. With single modules I agree this won't be an issue.

I think using a struct from a module also adds compile time dependency - should we do the detection for those modules as well?

Another issue I see is that it might be confusing why you sometimes get those warnings and sometimes you don't.

And to be clear, I'm not against the idea - just trying to be a devil's advocate.

[josevalim]

The problem is that with something like require * you put every file in the compilation pipeline.

Yes, that's why I asked to move the discussion about require * elsewhere. I don't think it could actually work.

Another issue I see is that it might be confusing why you sometimes get those warnings and sometimes you don't.

The struct is a good point but I think this comment may be enough reason for keeping it only for required modules.

And to be clear, I'm not against the idea - just trying to be a devil's advocate.

Thank you :)

[x4lldux]

After spending some time playing with xref, exref and xref_runner, I would side with @michalmuskala i. It will be confusing why sometimes compiler warns us and sometimes not. And it would be a lacking duplication of a tool that already exists - xref (it'd be better to integrate it to mix, like eprof was, or maybe even add it to the list of default compilers to be run always after mix compile).

[josevalim]

@x4lldux well, the issue with integrating with xref is that if it is an optional step, it won't run, and if it is always run, it will slow compilation down. So I like xref for its feature set but it is most likely that we would have our own tiny implementation, integrated into Mix, if we ever add something of sorts.

The advantage of relying on require is that the feedback would be immediate and not depend on Mix.