fail the build on compilation warnings [LUCENE-5130]

[asfimport]

Many modules compile w/o warnings ... we should lock this in and fail the build if warnings are ever added, and try to fix the warnings in existing modules.

---

Migrated from LUCENE-5130 by Michael McCandless (@mikemccand), updated May 09 2016
Attachments: LUCENE-5130.patch (versions: 2)
Linked issues:

• Fix warnings in the ant build [LUCENE-5551] #6613

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

analyzers-common fixed

[asfimport]

Shawn Heisey (@elyograg) (migrated from JIRA)

I'm trying to coordinate an effort to clean up warnings in the IDEs that we support. Before putting serious effort into that, I am tackling Uwe's recommendation to clean up warnings in the actual build first. I think this may require its own issue rather than doing it on this issue, but I thought I would ask here first.

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

Hi,

one important thing: We should not use something like "+all warning some warnings". In that case we would prevent later java versions with more possible warnings (covered by +all) from suceeding the build. Failing on warnings should be done with an explicit set of warnings given. This is just a warning about the warnings :)

[asfimport]

Shawn Heisey (@elyograg) (migrated from JIRA)

@uschindler, I need to catch you on IRC sometime.

What I am doing today is looking at what actually gets listed as a warning in the output from 'ant clean compile' on an unmodified trunk. Then I ask Eclipse to turn one specific kind of warning that gets found there into an actual error, and I go and change things so that the "problem" is either fixed or the warning is suppressed.

I'm starting with things that are easy to fix. So far I have separate patches for case fallthrough and for redundant casts. I did find a couple of things I need to ask about on the case fallthrough, and right now I am running tests after cleaning up redundant casts.

I don't think this work really fits in this issue, but I wanted to ask before I open a new one.

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

Shawn: this was referring to the actual config of the warnings:

  <property name="javac.args" value="-Xlint -Xlint:-deprecation -Xlint:-serial -Xlint:-options"/>

This setting has the effect that all warnings are enabled (-Xlint) minus the ones listed (-Xlint:-deprecation -Xlint:-serial -Xlint:-options). For pure warnings, this is no problem at all. But if we fail on warnings, this combination of flags is a bad idea: E.g.Java 8 or Java 9 or another compiler like J9's has another set of "default" warnings (the ones enabled by -Xlint). If somebody uses this newer version, the build will fail for no reason.

Because of this: If we fail builds on warnings, we should give an explicit list of warnings to report, like:

  <property name="javac.args" value="-Xlint:+unchecked -Xlint:+rawtypes -Xlint:+fallthrough ..."/>

In that case, we dont compile against a unknown set of warnings (which may fail our build on newer Oracle JDKs or different JDK vendors), but against a set of warnings we agreed on.

In short: I am against to fail the build on warnings, if the set of warnings is not defined explicit.

[asfimport]

Shawn Heisey (@elyograg) (migrated from JIRA)

After a couple of days, it's all starting to filter through and make sense. Thanks for continuing to pound me with information, Uwe.

Is there any way with javac to pick and choose which warnings we want to simply report and which ones we want to actually turn into errors? I suspect that the answer to that question is "no."

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

Move issue to Lucene 4.9.

[asfimport]

Brijesh Nekkare (migrated from JIRA)

<<Non-IBMers please ignore this message>>

Dont see this as a bug against IBM JDK.

Thanks and Regards
Brijesh Nekkare
IBM Java team