Repository: joelittlejohn/jsonschema2pojo (java)

Issue:
Unnecessary @SuppressWarnings java warnings after upgrade
After upgrading to version 0.4.18 we've started receiving a load of Java warnings like so:

Unnecessary @SuppressWarnings("unchecked")	




Pull request description:
Remove unused suppress warning annotations
This PR adds an integration test to detect compilation warnings in the source files generated by the project.  Warnings being produced by the includeDynamicAccessors feature have also been resolved.

__Note__: there is currently a bug with the Eclipse compiler that causes it to report suppressed warnings.  In those cases, the warnings being suppressed have been allowed.

Fixes #498 

Patch:
diff --git a/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/DynamicPropertiesRule.java b/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/DynamicPropertiesRule.java
index 21f5f168..e202d341 100644
--- a/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/DynamicPropertiesRule.java
+++ b/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/DynamicPropertiesRule.java
@@ -166,7 +166,6 @@ public class DynamicPropertiesRule implements Rule<JDefinedClass, JDefinedClass>
         JMethod method = jclass.method(PROTECTED, jclass.owner()._ref(Object.class), DEFINED_GETTER_NAME);
         JVar nameParam = method.param(String.class, "name");
         JVar notFoundParam = method.param(jclass.owner()._ref(Object.class), "notFoundValue");
-        Models.suppressWarnings(method, "unchecked");
         JBlock body = method.body();
         JSwitch propertySwitch = body._switch(nameParam);
         if (propertiesNode != null) {
@@ -197,7 +196,6 @@ public class DynamicPropertiesRule implements Rule<JDefinedClass, JDefinedClass>
         JMethod method = jclass.method(PROTECTED, jclass.owner()._ref(Object.class), DEFINED_GETTER_NAME);
         JVar nameParam = method.param(String.class, "name");
         JVar notFoundParam = method.param(jclass.owner()._ref(Object.class), "notFoundValue");
-        Models.suppressWarnings(method, "unchecked");
         JBlock body = method.body();
         JConditional propertyConditional = null;
         if (propertiesNode != null) {
@@ -245,7 +243,6 @@ public class DynamicPropertiesRule implements Rule<JDefinedClass, JDefinedClass>
         JMethod method = jclass.method(PUBLIC, jclass.owner().VOID, SETTER_NAME);
         JVar nameParam = method.param(String.class, "name");
         JVar valueParam = method.param(Object.class, "value");
-        Models.suppressWarnings(method, "unchecked");
         JBlock body = method.body();
         JBlock notFound = body._if(JOp.not(invoke(internalSetMethod).arg(nameParam).arg(valueParam)))._then();
 
@@ -273,7 +270,6 @@ public class DynamicPropertiesRule implements Rule<JDefinedClass, JDefinedClass>
         JMethod method = jclass.method(PUBLIC, jclass, BUILDER_NAME);
         JVar nameParam = method.param(String.class, "name");
         JVar valueParam = method.param(Object.class, "value");
-        Models.suppressWarnings(method, "unchecked");
         JBlock body = method.body();
         JBlock notFound = body._if(JOp.not(invoke(internalSetMethod).arg(nameParam).arg(valueParam)))._then();
 
@@ -297,7 +293,6 @@ public class DynamicPropertiesRule implements Rule<JDefinedClass, JDefinedClass>
         JMethod method = jclass.method(PROTECTED, jclass.owner().BOOLEAN, DEFINED_SETTER_NAME);
         JVar nameParam = method.param(String.class, "name");
         JVar valueParam = method.param(Object.class, "value");
-        Models.suppressWarnings(method, "unchecked");
         JBlock body = method.body();
         JSwitch propertySwitch = body._switch(nameParam);
         if (propertiesNode != null) {
@@ -326,7 +321,6 @@ public class DynamicPropertiesRule implements Rule<JDefinedClass, JDefinedClass>
         JMethod method = jclass.method(PROTECTED, jclass.owner().BOOLEAN, DEFINED_SETTER_NAME);
         JVar nameParam = method.param(String.class, "name");
         JVar valueParam = method.param(Object.class, "value");
-        Models.suppressWarnings(method, "unchecked");
         JBlock body = method.body();
         JConditional propertyConditional = null;
         if (propertiesNode != null) {
diff --git a/pom.xml b/pom.xml
index 643d8a39..c67c8519 100644
--- a/pom.xml
+++ b/pom.xml
@@ -362,6 +362,12 @@
                 <artifactId>jackson-mapper-asl</artifactId>
                 <version>1.9.11</version>
             </dependency>
+            <dependency>
+                <groupId>org.eclipse.jdt.core.compiler</groupId>
+                <artifactId>ecj</artifactId>
+                <version>4.4.2</version>
+                <scope>test</scope>
+            </dependency>
             <dependency>
                 <groupId>org.hamcrest</groupId>
                 <artifactId>hamcrest-all</artifactId>


Test patch:
diff --git a/jsonschema2pojo-integration-tests/pom.xml b/jsonschema2pojo-integration-tests/pom.xml
index 3a8b00f6..3c5e7d4e 100644
--- a/jsonschema2pojo-integration-tests/pom.xml
+++ b/jsonschema2pojo-integration-tests/pom.xml
@@ -123,6 +123,10 @@
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.eclipse.jdt.core.compiler</groupId>
+            <artifactId>ecj</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.hamcrest</groupId>
             <artifactId>hamcrest-all</artifactId>
diff --git a/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/CompilerWarningIT.java b/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/CompilerWarningIT.java
new file mode 100644
index 00000000..7ddca924
--- /dev/null
+++ b/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/CompilerWarningIT.java
@@ -0,0 +1,129 @@
+/**
+ * Copyright ¬© 2010-2014 Nokia
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jsonschema2pojo.integration;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+
+import org.apache.commons.io.output.NullWriter;
+import org.hamcrest.FeatureMatcher;
+import org.hamcrest.Matcher;
+import org.hamcrest.Matchers;
+import org.jsonschema2pojo.integration.util.Compiler;
+import org.jsonschema2pojo.integration.util.Jsonschema2PojoRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+import javax.tools.Diagnostic;
+import javax.tools.Diagnostic.Kind;
+import javax.tools.JavaCompiler;
+import javax.tools.JavaFileObject;
+import static org.jsonschema2pojo.integration.util.CodeGenerationHelper.config;
+
+import static org.hamcrest.Matchers.*;
+import static org.hamcrest.MatcherAssert.*;
+
+/**
+ * <p>Tests looking for warning coming from generated output.</p>
+ *
+ * <p>Notes: The eclipse compiler used in these tests has an open issue with the SuppressWarnings annotation.  As a result, some warnings must be
+ * accepted here that would not be present in practice.
+ * <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=469725">Bug 469725 - ECJ compiler: @SuppressWarnings annotation is ignored when ecj is invoked via java compiler tool API</a>
+ * </p>
+ *
+ * @author Christian Trimble
+ *
+ */
+@RunWith(Parameterized.class)
+public class CompilerWarningIT {
+  @Rule public Jsonschema2PojoRule schemaRule = new Jsonschema2PojoRule();
+  
+  @Parameters(name="{0}")
+  public static Collection<Object[]> parameters() {
+    JavaCompiler systemJavaCompiler = Compiler.systemJavaCompiler();
+    JavaCompiler eclipseCompiler = Compiler.eclipseCompiler();
+    return Arrays.asList(new Object[][] {
+      {
+        "includeAccessorsWithSystemJavaCompiler",
+        systemJavaCompiler,
+        config("includeDynamicAccessors", true),
+        "/schema/dynamic/parentType.json",
+        Matchers.empty()
+      },
+      {
+        "includeAccessorsWithEclipseCompiler",
+        eclipseCompiler,
+        config("includeDynamicAccessors", true),
+        "/schema/dynamic/parentType.json",
+        onlyCastExceptions()
+      }
+    });
+  }
+
+  private JavaCompiler compiler;
+  private Map<String, Object> config;
+  private String schema;
+  private Matcher<List<Diagnostic<? extends JavaFileObject>>> matcher;
+
+  public CompilerWarningIT(String label, JavaCompiler compiler, Map<String, Object> config, String schema, Matcher<List<Diagnostic<? extends JavaFileObject>>> matcher) {
+    this.compiler = compiler;
+    this.config = config;
+    this.schema = schema;
+    this.matcher = matcher;
+  }
+
+  @Test
+  public void checkWarnings() {
+    schemaRule.generate(schema, "com.example", config);
+    schemaRule.compile(compiler, new NullWriter(), new ArrayList<File>(), config);
+    
+    List<Diagnostic<? extends JavaFileObject>> warnings = warnings(schemaRule.getDiagnostics());
+    
+    assertThat(warnings, matcher);
+  }
+
+  public static List<Diagnostic<? extends JavaFileObject>> warnings(List<Diagnostic<? extends JavaFileObject>> all) {
+    List<Diagnostic<? extends JavaFileObject>> warnings = new ArrayList<>();
+    for( Diagnostic<? extends JavaFileObject> entry : all ) {
+      if( entry.getKind() == Kind.WARNING ) {
+        warnings.add(entry);
+      }
+    }
+    return warnings;
+  }
+
+  public static Matcher<Iterable<Diagnostic<? extends JavaFileObject>>> onlyCastExceptions() {
+    return Matchers.everyItem(hasMessage(containsString("Type safety: Unchecked cast from")));
+  }
+
+  public static Matcher<Diagnostic<? extends JavaFileObject>> hasMessage( Matcher<String> messageMatcher ) {
+    return new FeatureMatcher<Diagnostic<? extends JavaFileObject>, String>(messageMatcher, "message", "message") {
+      @Override
+      protected String featureValueOf(Diagnostic<? extends JavaFileObject> actual) {
+        return actual.getMessage(Locale.ENGLISH);
+      }
+    };
+  }
+}
diff --git a/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/util/CodeGenerationHelper.java b/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/util/CodeGenerationHelper.java
index 203bdb8b..e222f0bd 100644
--- a/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/util/CodeGenerationHelper.java
+++ b/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/util/CodeGenerationHelper.java
@@ -20,9 +20,11 @@ import static org.apache.commons.io.FileUtils.*;
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
+import static org.jsonschema2pojo.integration.util.Compiler.systemJavaCompiler;
 
 import java.io.File;
 import java.io.IOException;
+import java.io.Writer;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLClassLoader;
@@ -34,6 +36,10 @@ import java.util.Map;
 import java.util.UUID;
 import java.util.regex.Pattern;
 
+import javax.tools.DiagnosticListener;
+import javax.tools.JavaCompiler;
+import javax.tools.JavaFileObject;
+
 import org.apache.maven.artifact.DependencyResolutionRequiredException;
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.project.MavenProject;
@@ -135,12 +141,16 @@ public class CodeGenerationHelper {
     }
     
     public static ClassLoader compile(File sourceDirectory, File outputDirectory, List<File> classpath, Map<String, Object> config) {
+      return compile(systemJavaCompiler(), null, sourceDirectory, outputDirectory, classpath, config, null);
+    }
+
+    public static ClassLoader compile(JavaCompiler compiler, Writer out, File sourceDirectory, File outputDirectory, List<File> classpath, Map<String, Object> config, DiagnosticListener<? super JavaFileObject> listener) {
 
         List<File> fullClasspath = new ArrayList<File>();
         fullClasspath.addAll(classpath);
         fullClasspath.addAll(CodeGenerationHelper.classpathToFileArray(System.getProperty("java.class.path")));
 
-        new Compiler().compile(sourceDirectory, outputDirectory, fullClasspath, (String)config.get("targetVersion"));
+        new Compiler().compile(compiler, out, sourceDirectory, outputDirectory, fullClasspath, listener, (String)config.get("targetVersion"));
 
         try {
             return URLClassLoader.newInstance(new URL[] { outputDirectory.toURI().toURL() }, Thread.currentThread().getContextClassLoader());
diff --git a/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/util/Compiler.java b/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/util/Compiler.java
index 37c3db6a..2db2984f 100644
--- a/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/util/Compiler.java
+++ b/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/util/Compiler.java
@@ -18,18 +18,22 @@ package org.jsonschema2pojo.integration.util;
 
 import java.io.File;
 import java.io.IOException;
+import java.io.Writer;
 import java.util.ArrayList;
 import java.util.Arrays;
 
 import java.util.Collection;
 import java.util.List;
 
+import javax.tools.DiagnosticListener;
 import javax.tools.JavaCompiler;
 import javax.tools.JavaFileObject;
 import javax.tools.StandardJavaFileManager;
 import javax.tools.StandardLocation;
 import javax.tools.ToolProvider;
 
+import org.eclipse.jdt.internal.compiler.tool.EclipseCompiler;
+
 import static org.apache.commons.io.FileUtils.*;
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
@@ -42,9 +46,12 @@ import static org.junit.Assert.*;
 public class Compiler {
 
     public void compile(File sourceDirectory, File outputDirectory, List<File> classpath, String targetVersion ) {
+      compile(null, null, sourceDirectory, outputDirectory, classpath, null, targetVersion);
+    }
+
+    public void compile(JavaCompiler javaCompiler, Writer out, File sourceDirectory, File outputDirectory, List<File> classpath, DiagnosticListener<? super JavaFileObject> diagnosticListener, String targetVersion ) {
         targetVersion = targetVersion == null ? "1.6" : targetVersion;
 
-        JavaCompiler javaCompiler = ToolProvider.getSystemJavaCompiler();
         StandardJavaFileManager fileManager = javaCompiler.getStandardFileManager(null, null, null);
 
         if (outputDirectory != null) {
@@ -69,7 +76,7 @@ public class Compiler {
         options.add("-Xlint:-options");
         options.add("-Xlint:unchecked");
         if (compilationUnits.iterator().hasNext()) {
-            Boolean success = javaCompiler.getTask(null, fileManager, null, options, null, compilationUnits).call();
+            Boolean success = javaCompiler.getTask(out, fileManager, diagnosticListener, options, null, compilationUnits).call();
             assertThat("Compilation was not successful, check stdout for errors", success, is(true));
         }
 
@@ -98,4 +105,11 @@ public class Compiler {
         }
     }
 
+    public static JavaCompiler systemJavaCompiler() {
+      return ToolProvider.getSystemJavaCompiler();
+    }
+
+    public static JavaCompiler eclipseCompiler() {
+      return new EclipseCompiler();
+    }
 }
diff --git a/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/util/Jsonschema2PojoRule.java b/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/util/Jsonschema2PojoRule.java
index 881abea0..4bf5bb15 100644
--- a/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/util/Jsonschema2PojoRule.java
+++ b/jsonschema2pojo-integration-tests/src/test/java/org/jsonschema2pojo/integration/util/Jsonschema2PojoRule.java
@@ -18,6 +18,7 @@ package org.jsonschema2pojo.integration.util;
 
 import java.io.File;
 import java.io.IOException;
+import java.io.Writer;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -26,6 +27,11 @@ import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import javax.tools.Diagnostic;
+import javax.tools.DiagnosticListener;
+import javax.tools.JavaCompiler;
+import javax.tools.JavaFileObject;
+
 import org.junit.rules.TestRule;
 import org.junit.runner.Description;
 import org.junit.runners.model.Statement;
@@ -33,6 +39,7 @@ import static org.apache.commons.io.FileUtils.*;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.junit.Assert.assertThat;
+import static org.jsonschema2pojo.integration.util.Compiler.systemJavaCompiler;
 
 /**
  * A JUnit rule that executes JsonSchema2Pojo.
@@ -48,6 +55,7 @@ public class Jsonschema2PojoRule implements TestRule {
     private boolean sourceDirInitialized = false;
     private boolean classesDirInitialized = false;
     private ClassLoader classLoader;
+    private List<Diagnostic<? extends JavaFileObject>> diagnostics;
 
     /**
      * Gets the target directory for generate calls.
@@ -82,12 +90,18 @@ public class Jsonschema2PojoRule implements TestRule {
         return classLoader;
     }
 
+    public List<Diagnostic<? extends JavaFileObject>> getDiagnostics() {
+      checkActive();
+      return diagnostics;
+    }
+
     @Override
     public Statement apply(final Statement base, final Description description) {
         return new Statement() {
             @Override
             public void evaluate() throws Throwable {
                 active = true;
+                diagnostics = new ArrayList<Diagnostic<? extends JavaFileObject>>();
                 try {
                     File testRoot = methodNameDir(classNameDir(rootDirectory(), description.getClassName()),
                             description.getMethodName());
@@ -101,6 +115,7 @@ public class Jsonschema2PojoRule implements TestRule {
                     classLoader = null;
                     sourceDirInitialized = false;
                     classesDirInitialized = false;
+                    diagnostics = null;
                     active = false;
                 }
             }
@@ -133,10 +148,14 @@ public class Jsonschema2PojoRule implements TestRule {
     }
 
     public ClassLoader compile(List<File> classpath, Map<String, Object> config) {
+      return compile(systemJavaCompiler(), null, classpath, config);
+    }
+
+    public ClassLoader compile(JavaCompiler compiler, Writer out, List<File> classpath, Map<String, Object> config) {
         if (classLoader != null) {
             throw new IllegalStateException("cannot recompile sources");
         }
-        classLoader = CodeGenerationHelper.compile(getGenerateDir(), getCompileDir(), classpath, config);
+        classLoader = CodeGenerationHelper.compile(compiler, out, getGenerateDir(), getCompileDir(), classpath, config, new CapturingDiagnosticListener());
         return classLoader;
     }
 
@@ -170,6 +189,13 @@ public class Jsonschema2PojoRule implements TestRule {
         }
     }
 
+    class CapturingDiagnosticListener implements DiagnosticListener<JavaFileObject> {
+      @Override
+      public void report(Diagnostic<? extends JavaFileObject> diagnostic) {
+        diagnostics.add(diagnostic);
+      }
+    }
+
     private static List<File> emptyClasspath() {
         return new ArrayList<File>();
     }