Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add -Werror for javac #107

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;
Expand Down Expand Up @@ -73,8 +72,6 @@ public Node addNodeToVertices(Fix fix) {
* Colors the graph based on edges, no two vertices which there is an edge connecting them will be
* in the same group. A greedy algorithm is used to find the solution.
*/
// TODO: Remove SuppressWarnings below later.
@SuppressWarnings("JdkObsolete")
public void findGroups() {
this.groups.clear();
Collection<Node> allNodes = nodes.values();
Expand All @@ -83,17 +80,17 @@ public void findGroups() {
node.id = counter++;
}
int size = allNodes.size();
LinkedList<Integer>[] adj = new LinkedList[size];
List<List<Integer>> adj = new ArrayList<>();
for (int i = 0; i < size; ++i) {
adj[i] = new LinkedList<>();
adj.add(i, new ArrayList<>());
}
for (Node node : allNodes) {
for (Node other : allNodes) {
if (node.equals(other)) {
continue;
}
if (node.hasConflictInRegions(other)) {
adj[node.id].add(other.id);
adj.get(node.id).add(other.id);
}
}
}
Expand All @@ -106,7 +103,7 @@ public void findGroups() {
* @param adj Martic of adjancey.
* @param nodes Nodes in the graph.
*/
private void colorGraph(LinkedList<Integer>[] adj, Collection<Node> nodes) {
private void colorGraph(List<List<Integer>> adj, Collection<Node> nodes) {
int v = nodes.size();
List<Node> allNodes = new ArrayList<>(nodes);
int[] result = new int[v];
Expand All @@ -115,7 +112,7 @@ private void colorGraph(LinkedList<Integer>[] adj, Collection<Node> nodes) {
boolean[] available = new boolean[v];
Arrays.fill(available, true);
for (int u = 1; u < v; u++) {
for (int i : adj[u]) {
for (int i : adj.get(u)) {
if (result[i] != -1) {
available[result[i]] = false;
}
Expand Down
4 changes: 4 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ subprojects { proj ->
proj.dependencies {
errorprone deps.build.errorProneCore
errorproneJavac deps.build.errorProneJavac
// just to prevent javac warnings, we currently pass "-Werror" as compiler arguments,
// all warnings must be resolved.
compileOnly deps.build.errorProneCoreOld
Comment on lines +51 to +53
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this change

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if(proj.name != "ban-mutable-static"){
annotationProcessor project(":checks:ban-mutable-static")
}
Expand All @@ -66,6 +69,7 @@ subprojects { proj ->
}
}
}
options.compilerArgs += '-Werror'
}

repositories {
Expand Down
1 change: 1 addition & 0 deletions gradle/dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ def build = [
javaparser : "com.github.javaparser:javaparser-core:${versions.javaparser}",
commonscli : "commons-cli:commons-cli:${versions.cli}",
commonsio : "commons-io:commons-io:${versions.commonsio}",
errorProneCoreOld : "com.google.errorprone:error_prone_core:${oldestErrorProneApi}",
errorProneCore : "com.google.errorprone:error_prone_core:${versions.errorProne}",
errorProneJavac : "com.google.errorprone:javac:9+181-r4173-1",
errorProneCheckApi : "com.google.errorprone:error_prone_check_api:${versions.errorProneApi}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public <T extends NodeWithAnnotations<?> & NodeWithRange<?>> Modification visit(
}

@Override
@SuppressWarnings("unchecked")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again why suppress

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just created a followup PR that removes the added @SuppressWarnings in this PR and all existing ones by changing dependency to org.json. #123.

public JSONObject getJson() {
JSONObject res = super.getJson();
res.put("INJECT", false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,10 @@ public static Location createLocationFromArrayInfo(String[] values) {
return new OnMethod(path, clazz, values[2]);
case PARAMETER:
return new OnParameter(path, clazz, values[2], Integer.parseInt(values[4]));
default:
throw new RuntimeException(
"Unsupported location type: " + type + ", with values: " + Arrays.toString(values));
}
throw new RuntimeException("Cannot reach this statement, values: " + Arrays.toString(values));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import edu.ucr.cs.riple.injector.location.OnMethod;
import java.io.ByteArrayOutputStream;
import java.io.PrintStream;
import java.nio.charset.Charset;
import java.util.Collections;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -465,7 +466,7 @@ public void properReportInErrStdForClassNotFoundTest() {
new OnMethod("Main.java", "com.test.NotIncluded", "foo(java.lang.Object)"),
"javax.annotation.Nullable"))
.start();
assertTrue(err.toString().contains(expectedErrorMessage));
assertTrue(err.toString(Charset.defaultCharset()).contains(expectedErrorMessage));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

package edu.ucr.cs.riple.injector.tools;

import com.google.common.base.Splitter;
import java.nio.file.Path;

public class Utility {
Expand All @@ -38,7 +39,7 @@ public class Utility {
*/
public static Path pathOf(Path root, String path) {
Path ans = root;
String[] dirs = path.split("/");
Iterable<String> dirs = Splitter.on('/').split(path);
for (String dir : dirs) {
ans = ans.resolve(dir);
}
Expand Down