Why is not compilation aborted when an undefined module attribute is found?

Questions / Help
, , ,
1

A bug going into production without any need:

warning: undefined module attribute @seconds, please remove access to @seconds or explicitly set it before access
  lib/api_baas/accounts_admins/admin_user_token.ex:120: ApiBaas.AccountsAdmins.AdminUserToken (module)

Yes, tests should caught it, but why doesn’t the compiler abort compilation when it finds this?

Yes, I know that warnings can be enabled to behave as errors, but some warnings need to behave as warnings, and not halt the compiler.

I am curious about the reason for this to be considered as an warning and not an error?

1 Like
2

Elixir’s code states that it is a known issue and they just don’t want to add breaking change

        # TODO: Consider raising instead of warning on v2.0 as it usually cascades
1 Like
3

Why only in v2.0, that may never come!?

This a breaking change to solve a serious issue that lets bugs crop into production and it’s a compiler breaking change not a runtime one.

4

Just use --warnings-as-errors which is a sane option for CI

2 Likes
5

I’m curious about what these are, because I can’t remember the last project I worked on that didn’t build with --warnings-as-errors in CI.

1 Like
6

For example:

==> plug
Compiling 1 file (.erl)
Compiling 41 files (.ex)
warning: System.stacktrace/0 is deprecated. Use __STACKTRACE__ instead
  lib/plug/conn/wrapper_error.ex:26: Plug.Conn.WrapperError.reraise/3


==> ecto_sqlite3
Compiling 4 files (.ex)
warning: function query_many/4 required by behaviour Ecto.Adapters.SQL.Connection is not implemented (in module Ecto.Adapters.SQLite3.Connection)
  lib/ecto/adapters/sqlite3/connection.ex:1: Ecto.Adapters.SQLite3.Connection (module)
2 Likes
7

I only have 1.13 + OTP 25 right at hand to test, but on that version I still get the same warning from plug when running mix compile --warnings-as-errors with an empty _build directory.

However, --warnings-as-errors doesn’t turn warnings in dependencies into errors.

Some historical digging reveals that it used to; around the time of Elixir 1.4 that was changed, by commits like:

5 Likes
8

Well IMO this is a reasonable request. An opt-in configuration to allow compilation failure if any warning is in the project’s or any of the dependencies’ code should do the trick. There are orgs that inspect and slightly modify their project deps.

1 Like
9

I really don’t understand the rational of allowing this breaking change and not allowing the breaking change that would really fix the issue and make Elixir compilation to deliver a more robust code, instead of buggy code as it does at the moment.

The rationale of postponing for v2.0 really puzzles me, because that version may never exist as Jose keeps saying, but even if it was scheduled it doesn’t make any sense in my mind to wait, because this is a compiler bug that should be fixed, because a compiler that produces bad code output isn’t helping anyone.

1 Like
10

I think you should ask someone from the core team about this.

1 Like
11

I don’t think it necessarily produces bad code. It’s kind of increasing the risk of having bad code though, that’s true.

12

What valid code can refer to non-existing module attribute?

1 Like
13

Eh, I forgot what I even had in mind back then. Main point was to allow --warnings-as-errors to propagate to dependencies as well as our own code. No clue how was that even related to non-compilable code (like a module attribute missing but being referenced to). :person_shrugging:

1 Like
14

I’m confused. You had an issue and a solution was posed (use warnings-as-errors flag). You thought it wouldn’t work, but then were informed that it would work as warnings in dependencies don’t halt compilation.

So, problem solved, yes?

Re: why behavior was changed in the past: it was Elixir 1.4, we’re now officially on 1.14. My guess is that the core team’s opinions on maintaining a stable environment have changed in the last 5+ years as adoption had increased. Also, the behavior that was changed was the behavior of an opt-in option, not a default, so the impacted group was more clear-cut and well defined.

15

Not a solution… at best a work-around and the problem persists for non informed developers.