warnings as errors not working

[cjimison]

Hi all,

In our code base we have many warnings of the type:

"variable xyz is unused"

I would like to turn on "warnings_as_errors" but that option does not seam to be working. I tried the following with no success (meaning I get the warnings but not the errors):

mix compile --warnings-as-errors

and I tried adding the following to my mix.exs file

def project do
[app: :sandbox,
version: "0.0.1",
deps_path: "../../deps",
lockfile: "../../mix.lock",
elixir: "~> 1.0",
erlc_options: [{:parse_transform, :lager_transform}, :warn_missing_spec, :warnings_as_errors],
elixirc_options: [warnings_as_errors: true], # <<------ added in hopes of getting warnings as errors
deps: deps]
end

[itsgreggreg]

Just tested this with a fresh pull from github and it seems like this flag doesn't work.

# warnings_as_errors_test.ex 
defmodule Warnings do
  def func(a) do
    IO.puts "a not used"
  end
end

> ./bin/elixirc --warnings-as-errors --verbose warnings_as_errors_test.ex 
warning_test.ex:2: warning: variable a is unused
Compiled warning_test.ex

The module is compiled and beam file generated.

[oneeman]

It seems that it does work but the behavior isn't what one might expect intuitively. The documentation says:

--warnings-as-errors - treat warnings as errors and return a non-zero exit code

Trying it out and checking the exit codes:

~ $ elixirc warnings_sample.ex 
warnings_sample.ex:1: warning: redefining module WarningsSample
warnings_sample.ex:3: warning: this clause cannot match because a previous clause at line 2 always matches
~ $ echo $?
0
~ $ elixirc warnings_sample.ex --warnings-as-errors
warnings_sample.ex:1: warning: redefining module WarningsSample
warnings_sample.ex:3: warning: this clause cannot match because a previous clause at line 2 always matches
~ $ echo $?
1

So the exit code is what it should be. And the exit code is also what is tested in cli_test.exs in the test "warnings as errors".

Perhaps what we want is to be more explicit to the user. For example, when running mix compile --warnings-as-errors, it seems to not build (e.g. the line Generated foo.app is not output like it with just mix compile), but other than there's no indication (though I have next to no experience with mix, so I could very well be missing stuff here). So maybe it's just a matter of adding some user-facing messages to the compiler and/or mix?

@josevalim ?

[josevalim]

Oh yes, I remember now. The rationale was that you want to get as many warnings as you want instead of failing on the first one. So this is right, your build tools and what not should fail too, and this is what we want. I don't think we need to do anything code related here, unless we can improve the docs somehow.

Thank you @oneeman!

[ericmj]

It's important to make sure that we do not produce compiled binaries for the files with warnings, otherwise you will get the warning once and the next compilation will be complete successfully.

A quick test shows that we do not produce binaries but we still print "Compiled lib/foo.ex" which is confusing since compilation never completed.

[TheFirstAvenger]

@josevalim Would adding a line at the end of compilation that states "compile failed due to warnings" be a code change to take from this issue? We have added this flag to our semaphore build, but it is not obvious right now to someone looking at the output (which simply contains 'compiled' lines and 'warning' lines) that the reason it failed is because we have enabled the warnings-as-errors flag.

[josevalim]

That would be a great addition, yes. I could swear we did that but
apparently we do not. :)

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Lead Developer

[TheFirstAvenger]

Additionally, --warnings-as-errors on mix test would allow automated build systems to catch warnings in test files. Currently, since test files don't get compiled with mix compile, and --warnings-as-errors isnt recognized on mix test, there is no way to root out these warnings in test files via automated builds or linting tools.

[TheFirstAvenger]

Created this for discussion of the mix test implementation of warnings as errors:

#3223