Give warning on unused require

[thiamsantos]

This a draft PR for the for the proposal: https://groups.google.com/g/elixir-lang-core/c/d9ZjViQaOY8/m/tZN0jn6LAAAJ. The implementation is based on the unused import warning.

I plan to compile a project like plug to check the impact that such warning would have in codebases.

[josevalim]

Maybe instead of from_import we could do [{warn, false} | Opts] instead?

[thiamsantos]

Done :)

[josevalim]

We already test these different conditions in the unit tests in lexical tracker. In here, we need only a single test that verifies the format of the warning itself.

[thiamsantos]

Done :)

[thiamsantos]

The results of the compilation of some popular libraries using this branch are promising.
The new warnings appear to be legit.

plug

# plug
> mix compile
Compiling 1 file (.ex)
    warning: unused require Logger
    │
 59 │   require Logger
    │   ~
    │
    └─ lib/plug/request_id.ex:59:3

     warning: unused require Bitwise
     │
 105 │   require Bitwise
     │   ~
     │
     └─ lib/plug/csrf_protection.ex:105:3

     warning: the variable "prefix_size" is accessed inside size(...) of a bitstring but it was defined outside of the match. You must precede it with the pin operator
     │
 153 │         <<prefix::binary-size(prefix_size), char, suffix::binary-size(suffix_size)>> = segment
     │                               ~
     │
     └─ lib/plug/router/utils.ex:153:31: Plug.Router.Utils.build_path_clause/7

     warning: the variable "suffix_size" is accessed inside size(...) of a bitstring but it was defined outside of the match. You must precede it with the pin operator
     │
 153 │         <<prefix::binary-size(prefix_size), char, suffix::binary-size(suffix_size)>> = segment
     │                                                                       ~
     │
     └─ lib/plug/router/utils.ex:153:71: Plug.Router.Utils.build_path_clause/7

Generated plug app

ecto_sql

# ecto_sql
~/dev/thiamsantos/ecto_sql (master)
> mix compile
warning: setting :preferred_cli_env in your mix.exs "def project" is deprecated, set it inside "def cli" instead:

    def cli do
      [preferred_envs: ["test.all": :test, "test.adapters": :test]]
    end

  (mix 1.20.0-dev) lib/mix/cli.ex:189: Mix.CLI.preferred_cli_env/3
  (mix 1.20.0-dev) lib/mix/cli.ex:172: Mix.CLI.maybe_change_env_and_target/2
  (mix 1.20.0-dev) lib/mix/cli.ex:59: Mix.CLI.proceed/1
  /Users/thiago.santos/dev/thiamsantos/elixir/bin/mix:7: (file)
  (elixir 1.20.0-dev) src/elixir_compiler.erl:81: :elixir_compiler.dispatch/4
  (elixir 1.20.0-dev) src/elixir_compiler.erl:56: :elixir_compiler.compile/4

Compiling 25 files (.ex)
      warning: the variable "size" is accessed inside size(...) of a bitstring but it was defined outside of the match. You must precede it with the pin operator
      │
 1977 │       <<val::size(size)>> = value
      │                   ~
      │
      └─ lib/ecto/adapters/postgres/connection.ex:1977:19: Ecto.Adapters.Postgres.Connection.bitstring_literal/1

     warning: unused require Ecto.Query
     │
 107 │   require Ecto.Query
     │   ~
     │
     └─ lib/ecto/migrator.ex:107:3

     warning: unused require Ecto.Query
     │
 140 │   require Ecto.Query
     │   ~
     │
     └─ lib/ecto/adapters/tds.ex:140:3

     warning: unused require Logger
     │
 139 │   require Logger
     │   ~
     │
     └─ lib/ecto/adapters/tds.ex:139:3

    warning: unused require Ecto.Schema
    │
  8 │     require Ecto.Schema
    │     ~
    │
    └─ lib/ecto/adapters/tds/connection.ex:8:5

    warning: unused require Logger
    │
  4 │     require Logger
    │     ~
    │
    └─ lib/ecto/adapters/tds/connection.ex:4:5

Generated ecto_sql app

phoenix

# phoenix
~/dev/thiamsantos/phoenix (master)
> mix compile
    warning: unused require Phoenix.Endpoint
    │
  6 │   require Phoenix.Endpoint
    │   ~
    │
    └─ lib/phoenix/channel/server.ex:6:3

    warning: unused require Phoenix.Endpoint
    │
  6 │   require Phoenix.Endpoint
    │   ~
    │
    └─ lib/phoenix/controller.ex:6:3

     warning: unused require Logger
     │
 101 │   require Logger
     │   ~
     │
     └─ lib/phoenix/token.ex:101:3

    warning: unused require Logger
    │
 69 │   require Logger
    │   ~
    │
    └─ lib/phoenix/socket/pool_supervisor.ex:69:3

    warning: the variable "prefix_size" is accessed inside size(...) of a bitstring but it was defined outside of the match. You must precede it with the pin operator
    │
 46 │       <<prefix::binary-size(prefix_size), ^suffix::binary>> -> prefix
    │                             ~
    │
    └─ lib/phoenix/naming.ex:46:29: Phoenix.Naming.unsuffix/2

    warning: unused require Phoenix.Endpoint
    │
 17 │   require Phoenix.Endpoint
    │   ~
    │
    └─ lib/phoenix/endpoint/render_errors.ex:17:3

     warning: unused require Logger
     │
 283 │   require Logger
     │   ~
     │
     └─ lib/phoenix/endpoint.ex:283:3

     warning: unused require Phoenix.Endpoint
     │
 199 │   require Phoenix.Endpoint
     │   ~
     │
     └─ lib/phoenix/socket.ex:199:3

    warning: unused require Logger
    │
  9 │   require Logger
    │   ~
    │
    └─ lib/phoenix/config.ex:9:3

Generated phoenix app

[josevalim]

💚 💙 💜 💛 ❤️

[lukaszsamson]

@josevalim @thiamsantos I suspect this may not work correctly with require Foo, as: Bar when Foo is not used in macro calls but Bar alias is used. There are not tests covering that case.

[josevalim]

In this case the require is not used, so it should warn (and the user should use an alias instead)! A test would be nice indeed!

[lukaszsamson]

Yes, the warning should say that, e.g. unused require Foo. Consider using alias Foo, as: Bar instead

[thiamsantos]

In this case the require is not used, so it should warn (and the user should use an alias instead)! A test would be nice indeed!

I am not sure if I followed the as: in the require statement is something discouraged?

--
Another point:
I think we are missing some documentation about the warnings.
Probably a note on the documentation similar to alias docs would be nice as well.

[sabiwara]

I am not sure if I followed the as: in the require statement is something discouraged?

Not discouraged if you need both require + alias, but in the case no macro is used, it should be a regular alias 🙂

[thiamsantos]

@sabiwara yeah, makes sense. Now I was able to connect the dots 😂 thank you ♥️