Add -Werror for javac #107
Conversation
| @@ -74,7 +74,7 @@ public Node addNodeToVertices(Fix fix) { | |||
| * in the same group. A greedy algorithm is used to find the solution. | |||
| */ | |||
| // TODO: Remove SuppressWarnings below later. | |||
| @SuppressWarnings("JdkObsolete") | |||
| @SuppressWarnings({"JdkObsolete", "unchecked"}) | |||
There was a problem hiding this comment.
Why not just fix the error instead of suppressing?
There was a problem hiding this comment.
I didn't know how I can resolve the warning for creating array of LinkedLists. Since I was confident that there will be no problem with the code I just silenced it.
However, I just changed it to use List of Lists 6517fba, I don't think this will impact the performance. But if you think this will actually impact the performance, please let me know and I will undo this. Thank you.
| // just to prevent javac warnings, we currently pass "-Werror" as compiler arguments, | ||
| // all warnings must be resolved. | ||
| compileOnly deps.build.errorProneCoreOld |
There was a problem hiding this comment.
it seems once error prone is in the path we get the error below for compiling annotator-core:
warning: unknown enum constant SeverityLevel.SUGGESTION
reason: class file for com.google.errorprone.BugPattern$SeverityLevel not found
Added a dependency to resolve this issue.
| @@ -64,6 +64,7 @@ public Modification visit(NodeWithAnnotations<?> node, Range range) { | |||
| } | |||
|
|
|||
| @Override | |||
| @SuppressWarnings("unchecked") | |||
There was a problem hiding this comment.
@msridhar The only way to resolve this warning is to change the json dependency to org.json from com.googlecode.json-simple:json-simple since it is compiled with a very old javac (the latest release is for 2012). I am definitely interested in updating this dependency to use org.json but that will include a lot of changes which makes this PR pretty large. Should I do it in a follow up PR with an issue for now to track it ?
There was a problem hiding this comment.
Just created a followup PR that removes the added @SuppressWarnings in this PR and all existing ones by changing dependency to org.json. #123.
|
@msridhar Thank you for the review, this is ready for another round. |
|
@nimakarimipour do you still need a review on this one? |
Low Priority
This PR adds
-Werrorjavac flag to builds and gets rid of all existing warnings.@msridhar @lazaroclapp this is low priority but just wanted to get rid of warnings 🙂.