Feature proposal: defmodulep

[Myron Marston]

I find Elixir's warnings to be very, very helpful and on my projects I have our CI builds configured to fail if there are any warnings.  One of my favorite warnings is this one:

> lib/s3.ex:122: warning: function foo/1 is unused

Elixir provides this warning when a private function is never used.  I love getting this warning as it helps me eliminate dead code.  However, there's a similar situation where Elixir does not provide a warning: when private helper functions have been extracted into a helper module, and are no longer called.  In this case, the module is essentially a private module and exists only to support some public interface (and perhaps support multiple public modules).  Elixir's source code has some examples like this:

https://github.com/elixir-lang/elixir/blob/v1.2.4/lib/elixir/lib/kernel/utils.ex

I believe `Kernel.Utils` is meant to be a private module that serves only to support public `Kernel` functionality (but is itself not meant to be ever called by end-users).  I'd like to propose that Elixir gain a new `defmodulep` macro that would define a private module.  Here's what I'm imagining this would do:

• Set `@moduledoc false`.
• Provide a "function is unused" warning when any of its public functions have no callers in the project.  If all of its public functions are unused, it would be nice if it provided a "private module is unused" warning instead of multiple "function is unused" warnings.
• Issue a warning when code in depending projects uses the private module in any way.  Alternately, we may want to totally disallow uses from depending projects, but I think I'd prefer the warning.  I would never commit code that uses a private module from a dependency but it can be useful to use it temporarily during a debugging session (or in iex or whatever) so having a warning would allow this while making it clear to the user that they are doing something that is not recommended.

Thoughts?

Myron

[José Valim]

While I believe the name defmodulep is counter-intuitive, however I do understand the core of the proposal.

I am wondering if this would be better done as a post-processing step. There is a lot of information we can get from post processing:

1. A module that was not defined anywhere is being called.

2. A function with @doc false is not being called

3. A function in a module with @moduledoc false is not being called (as you posted)

Erlang xref has a lot of interesting ideas on the topic: http://erlang.org/doc/apps/tools/xref_chapter.html

There is one issue though. For example, the functions in Kernel.Utils are sometimes called by generated code. This means xref would warn such cases and it would be a false positive.

Further thoughts?

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/eb89b3b5-5c18-40d0-b05e-f83fe5ff9713%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Myron Marston]

I don't think the feature needs to be available via `defmodulep` macro, but that seemed like a natural interface for it, given all the other `xyz`/`xyzp` paired macros (e.g. `def`/`defp`, `defmacro`/`defmacrop`, `@type`/`typep`, etc).  Also, "private modules" seems like a natural name for this feature -- and if that's what we call the feature, `defmodulep` seems like the right macro for it.

But I'd be perfectly happy with the feature being called something else and exposed using a different interface.

I am wondering if this would be better done as a post-processing step.

I think it'd have to be checked in a post-processing check.  My assumption was that `defmodulep` (or however we support this feature) would simply define the module with some extra metadata that would then be checked in a post-processing step (and also stored with the module for use when depending projects are compiled).

There is a lot of information we can get from post processing:
1. A module that was not defined anywhere is being called.
2. A function with @doc false is not being called
3. A function in a module with @moduledoc false is not being called (as you posted)

This information sounds like what you'd need to implement this feature.  I think the missing piece is a way for the author of the module to signal that they consider the module to be private and therefore a warning should be generated if there are not any callers within the project, or if there are any callers in a depending project.

Your mention of `@doc false` and `@moduledoc false` is interesting.  We could perhaps implement this feature using just those, with no need for the introduction of a new macro.  But I think it would perhaps be surprising to end-users if `@doc false` and `@moduledoc false` triggered these warnings as those attributes seem to be only about documentation, and not privateness.  So I think having a new, alternate way to signify that you want a module to be treated as private has value.

There is one issue though. For example, the functions in Kernel.Utils are sometimes called by generated code. This means xref would warn such cases and it would be a false positive.

That's definitely an issue I hadn't considered.  Maybe `Kernel.Utils` could not benefit from this feature if it was implemented due to that issue.  Or maybe we can come up with a solution that checks to see if a depending project calls something on a private module before macros are expanded?

Thanks for considering my proposal!

Myron

--
You received this message because you are subscribed to a topic in the Google Groups "elixir-lang-core" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/elixir-lang-core/X18SZnSDW7U/unsubscribe.
To unsubscribe from this group and all its topics, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4L9ZyAr7R7JdOPtP%2B8oKGphUW0nn05DX_242vre4x-GdA%40mail.gmail.com.

[José Valim]

Your mention of `@doc false` and `@moduledoc false` is interesting.  We could perhaps implement this feature using just those, with no need for the introduction of a new macro.  But I think it would perhaps be surprising to end-users if `@doc false` and `@moduledoc false` triggered these warnings as those attributes seem to be only about documentation, and not privateness.

That's a very good point. My concern with the use of private for modules is that private in Elixir is tied to runtime visibility. A private function cannot be used outside of the module that defines it. So calling it a private module may be misleading because we don't restrict its visibility in any way. So while you brought valid points that the @doc should not be used for such, I believe the term private shouldn't as well. It just means we should keep looking. :)

This proposal for Erlang could be useful in this discussion: https://github.com/erlang/eep/blob/master/eeps/eep-0005.md

The proposal above would be closer to private modules where you would explicitly choose the consumer for every function. The best part is that it can likely be implemented in pure Elixir, without the compiler help (therefore it could fit a separate project).

> Thanks for considering my proposal!

And thank you for the discussion!

[Myron Marston]

Interesting.  If we implemented something akin to that `export_to` proposal, I guess the feature would apply at the individual function level instead of at the module level: essentially defining a function's visibility to be limited to the project in which it is defined.  This makes me think a bit of C++'s `friend` scope.  (I don't think I like the name "friend" for this, though.)

--

You received this message because you are subscribed to a topic in the Google Groups "elixir-lang-core" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/elixir-lang-core/X18SZnSDW7U/unsubscribe.
To unsubscribe from this group and all its topics, send an email to elixir-lang-co...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4LiiV6sFyYGde2FXCpzHRoOVLT3gGEgazwpaenTpa5UMA%40mail.gmail.com.

[José Valim]

Interesting.  If we implemented something akin to that `export_to` proposal, I guess the feature would apply at the individual function level instead of at the module level: essentially defining a function's visibility to be limited to the project in which it is defined.  This makes me think a bit of C++'s `friend` scope.  (I don't think I like the name "friend" for this, though.)

That may be quite hard to implement though, even in Elixir itself, since the Elixir compiler does not expect a function you are calling during compilation to exist at compile time. For example, if you have the following code:

def foo do

  Bar.baz

end

At compilation time, we don't check Bar exists.

However, if any function could limit its exports, by definition we need to know the module metadata for every function we call so we can control its access. So modules with such constraints must be required before use.

I can see two approaches:

1. Provide a @export_to that works as an attribute. The compiler won't check it isn't violated but a post-compiler check may.

2. Effectively implement something like "defmodulep" that would slightly change the semantics of its defined functions but ensure a strict compile time and runtime access. 

For example, if you want to provide a "defmodulep", you could implement it like this:

a. defmodulep must be required before used and it will just define a regular module with its own definition for def, defp, defmacro and defmacrop

b. When you define a function like def foo/2 inside defmodulep, you would actually define a macro named foo/2 and an actual function called "(export) foo"/2

c. The macro foo/2 would check if the __CALLER__.module belongs to the allowed exports and emit an AST that calls "(export) foo"/2 with the exactly same arguments

So there is a slight change of semantics, like stacktraces will see "(export) foo/2" instead of "foo/2", but it should provide the required strictness.

[Myron Marston]

1. Provide a @export_to that works as an attribute. The compiler won't check it isn't violated but a post-compiler check may.

This would give you very fine-grain control but at the expense of extra work to maintain the list of modules passed to `export_to`.  For my use case, this would be pretty annoying; I want to declare (and enforce) that a module is usable by other modules in the same mix project but not by depending projects.  I'd rather not have to keep the `export_to` list up-to-date with all the modules in the same mix project that need access.

2. Effectively implement something like "defmodulep" that would slightly change the semantics of its defined functions but ensure a strict compile time and runtime access. 

This approach sounds better to me.

The macro foo/2 would check if the __CALLER__.module belongs to the allowed exports and emit an AST that calls "(export) foo"/2 with the exactly same arguments

How would it now what the allowed exports are?  (Or is this meant to work in conjunction with `@export_to`?)

Also, putting parens in a function name seems kinda weird. Maybe `__exported_foo/2` would be better?  Or `__private_foo/2`.

Would this approach allow depending projects to call the private module's function using `ModName.__exported_foo`?

--
You received this message because you are subscribed to a topic in the Google Groups "elixir-lang-core" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/elixir-lang-core/X18SZnSDW7U/unsubscribe.
To unsubscribe from this group and all its topics, send an email to elixir-lang-co...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4LNxw8UfrE8QaLUqMq%3Da-%2BH3k9JQg7aaNrqZrTyQFpnyA%40mail.gmail.com.

[José Valim]

How would it now what the allowed exports are?  (Or is this meant to work in conjunction with `@export_to`?)

In conjuction with @export_to but you could also define rules like: everything in MyApp.*.

Although this does not solve your original problem of warnings, I think I got sidetracked. Sorry. :)

 

Also, putting parens in a function name seems kinda weird. Maybe `__exported_foo/2` would be better?  Or `__private_foo/2`.

We use parentheses for overridable functions via defoverridable. It reads nicely in stacktraces and so on.

 

Would this approach allow depending projects to call the private module's function using `ModName.__exported_foo`?

Parens would also make this harder. ;)

[Myron Marston]

It seems odd to have both @export_to and defmodulep — the fact that they are individually available suggests someone could use one but not the other, which would be quite confusing. Would this work?

defmodule MyLib.PrivateHelpers do
  @private_module export_to: MyLib.*

  # ...
end

Basically, combine them into a single, one-line declaration. It also perhaps solves the issue of defmodulep implying something about the module that is not true. The export_to option passed when declaring @private_module makes the behavior of the privateness explicit.

​

--
You received this message because you are subscribed to a topic in the Google Groups "elixir-lang-core" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/elixir-lang-core/X18SZnSDW7U/unsubscribe.
To unsubscribe from this group and all its topics, send an email to elixir-lang-co...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4KjL5we7qTfyJxk8VMDLJ-h7KVKtjkBy3MDjvDmrUMsow%40mail.gmail.com.

[José Valim]

The only reason I proposed both is because it is quite hard to implement only one of them without the other with today's compiler semantics.

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

--

You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.

To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CADUxQmuaXUUudTD4UxXLCFM9xkv2iJV8qNwRpkOybdAqW2Nppg%40mail.gmail.com.

[Myron Marston]

Then maybe this?

``` elixir

defmodulep MyLib.PrivateHelpers, export_to: MyLib.* do

  # ...

end

```

That keeps them all in the same line to make it clear they belong together and must both be used.

That said, if something like my `@private_module export_to: MyLib.*` proposal is preferred, but currently infeasible due to the compiler, we might want to wait to implement this until it is more feasible.  I have no urgent need for this, and if the more desirable interface would be easier to achieve in the future, holding off for now makes sense.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4JiaZQuSbQzuNjX2iBy0CdW%3DWowWr2FG3KKYTWDD5Qz8g%40mail.gmail.com.

[w.m.w...@student.rug.nl]

About the @export_to option:

I wonder how often we define a module like `MyMainModule.Utils` where the idea is to export functionality to places other than `MyMainModule`.

To state this in a possibly clearer way: I see modules either:

a) exporting all public functions to any callers.

b) exporting all public functions to the module(s) in its parent naming stack.

If other ways of exporting are not necessary, the logic of @export_to could be a lot simpler. Maybe another name, such as just saying `@protected` might be clearer in this case, but that is just semantics.

Just my two cents.

~W-M