avro-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nielsbas...@apache.org
Subject avro git commit: AVRO-1047: Java: Fix remaining javac warnings.
Date Wed, 13 Dec 2017 09:29:06 GMT
Repository: avro
Updated Branches:
  refs/heads/master 7bbbf92cd -> b8c37b322


AVRO-1047: Java: Fix remaining javac warnings.

AVRO-1913 fixed most warnings, but missed a few unchecked usages and
left in some redundant casts.  This patch eliminates remaining
warnings even with the -Xlint:all flag added to javac.  With no more
warnings to surpress, the @SuppressWarnings("all") annotation is
removed from the generated code, as well.  The TestSpecificCompiler
test has been modified to pass -Xlint:all to the compiler and assert
that no warnings are present in compiled code.


Project: http://git-wip-us.apache.org/repos/asf/avro/repo
Commit: http://git-wip-us.apache.org/repos/asf/avro/commit/b8c37b32
Tree: http://git-wip-us.apache.org/repos/asf/avro/tree/b8c37b32
Diff: http://git-wip-us.apache.org/repos/asf/avro/diff/b8c37b32

Branch: refs/heads/master
Commit: b8c37b32256113bd64f35f0b97e673d74ef61db2
Parents: 7bbbf92
Author: Michael Werle <mkw-github@coruscations.com>
Authored: Tue Sep 26 21:12:20 2017 -0500
Committer: Niels Basjes <nbasjes@bol.com>
Committed: Wed Dec 13 10:13:48 2017 +0100

----------------------------------------------------------------------
 CHANGES.txt                                     |  3 ++
 .../specific/templates/java/classic/enum.vm     |  1 -
 .../specific/templates/java/classic/fixed.vm    |  5 +--
 .../specific/templates/java/classic/protocol.vm |  1 -
 .../specific/templates/java/classic/record.vm   | 11 +++--
 .../compiler/specific/TestSpecificCompiler.java | 42 +++++++++++++++++++-
 .../test/resources/union_and_fixed_fields.avsc  | 18 +++++++++
 .../test/compiler/output-string/Position.java   | 12 ++++++
 .../avro/examples/baseball/Player.java          |  1 -
 .../avro/examples/baseball/Position.java        |  1 -
 .../tools/src/test/compiler/output/Player.java  |  1 -
 .../src/test/compiler/output/Position.java      |  1 -
 12 files changed, 80 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/avro/blob/b8c37b32/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index a621c65..9af445f 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -90,6 +90,9 @@ Trunk (not yet released)
     AVRO-2055: Remove Magic Value From org.apache.avro.hadoop.io.AvroSequenceFile
     (Beluga Behr via gabor)
 
+    AVRO-1047: Remaining warnings in generated Java code exposed by -Xlint:all fixed.
+    (Michael Werle via Niels Basjes)
+
   BUG FIXES
 
     AVRO-1741: Python3: Fix error when codec is not in the header.

http://git-wip-us.apache.org/repos/asf/avro/blob/b8c37b32/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/enum.vm
----------------------------------------------------------------------
diff --git a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/enum.vm
b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/enum.vm
index 2117cd4..92d0c05 100644
--- a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/enum.vm
+++ b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/enum.vm
@@ -18,7 +18,6 @@
 #if ($schema.getNamespace())
 package $schema.getNamespace();
 #end
-@SuppressWarnings("all")
 #if ($schema.getDoc())
 /** $schema.getDoc() */
 #end

http://git-wip-us.apache.org/repos/asf/avro/blob/b8c37b32/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/fixed.vm
----------------------------------------------------------------------
diff --git a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/fixed.vm
b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/fixed.vm
index b06854e..b81425c 100644
--- a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/fixed.vm
+++ b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/fixed.vm
@@ -18,7 +18,6 @@
 #if ($schema.getNamespace())
 package $schema.getNamespace();
 #end
-@SuppressWarnings("all")
 #if ($schema.getDoc())
 /** $schema.getDoc() */
 #end
@@ -46,7 +45,7 @@ public class ${this.mangle($schema.getName())} extends org.apache.avro.specific.
     super(bytes);
   }
 
-  private static final org.apache.avro.io.DatumWriter
+  private static final org.apache.avro.io.DatumWriter<${this.mangle($schema.getName())}>
     WRITER$ = new org.apache.avro.specific.SpecificDatumWriter<${this.mangle($schema.getName())}>(SCHEMA$);
 
   @Override public void writeExternal(java.io.ObjectOutput out)
@@ -54,7 +53,7 @@ public class ${this.mangle($schema.getName())} extends org.apache.avro.specific.
     WRITER$.write(this, org.apache.avro.specific.SpecificData.getEncoder(out));
   }
 
-  private static final org.apache.avro.io.DatumReader
+  private static final org.apache.avro.io.DatumReader<${this.mangle($schema.getName())}>
     READER$ = new org.apache.avro.specific.SpecificDatumReader<${this.mangle($schema.getName())}>(SCHEMA$);
 
   @Override public void readExternal(java.io.ObjectInput in)

http://git-wip-us.apache.org/repos/asf/avro/blob/b8c37b32/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/protocol.vm
----------------------------------------------------------------------
diff --git a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/protocol.vm
b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/protocol.vm
index b330796..068bd75 100644
--- a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/protocol.vm
+++ b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/protocol.vm
@@ -19,7 +19,6 @@
 package $protocol.getNamespace();
 #end
 
-@SuppressWarnings("all")
 #if ($protocol.getDoc())
 /** $protocol.getDoc() */
 #end

http://git-wip-us.apache.org/repos/asf/avro/blob/b8c37b32/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
----------------------------------------------------------------------
diff --git a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
index ccec4b6..b38fbd0 100644
--- a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
+++ b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
@@ -26,7 +26,6 @@ import org.apache.avro.message.BinaryMessageDecoder;
 import org.apache.avro.message.SchemaStore;
 #end
 
-@SuppressWarnings("all")
 #if ($schema.getDoc())
 /** $schema.getDoc() */
 #end
@@ -176,7 +175,7 @@ public class ${this.mangle($schema.getName())}#if ($schema.isError())
extends or
     switch (field$) {
 #set ($i = 0)
 #foreach ($field in $schema.getFields())
-    case $i: ${this.mangle($field.name(), $schema.isError())} = (${this.javaType($field.schema())})value$;
break;
+    case $i: ${this.mangle($field.name(), $schema.isError())} = #if(${this.javaType($field.schema())}
!= "java.lang.Object")(${this.javaType($field.schema())})#{end}value$; break;
 #set ($i = $i + 1)
 #end
     default: throw new org.apache.avro.AvroRuntimeException("Bad index");
@@ -435,16 +434,16 @@ public class ${this.mangle($schema.getName())}#if ($schema.isError())
extends or
           record.${this.mangle($field.name(), $schema.isError())} = this.${this.mangle($field.name(),
$schema.isError())}Builder.build();
         } else {
 #if ($this.hasLogicalTypeField($schema))
-          record.${this.mangle($field.name(), $schema.isError())} = fieldSetFlags()[$field.pos()]
? this.${this.mangle($field.name(), $schema.isError())} : (${this.javaType($field.schema())})
defaultValue(fields()[$field.pos()], record.getConversion($field.pos()));
+          record.${this.mangle($field.name(), $schema.isError())} = fieldSetFlags()[$field.pos()]
? this.${this.mangle($field.name(), $schema.isError())} : #if(${this.javaType($field.schema())}
!= "java.lang.Object")(${this.javaType($field.schema())})#{end} defaultValue(fields()[$field.pos()],
record.getConversion($field.pos()));
 #else
-          record.${this.mangle($field.name(), $schema.isError())} = fieldSetFlags()[$field.pos()]
? this.${this.mangle($field.name(), $schema.isError())} : (${this.javaType($field.schema())})
defaultValue(fields()[$field.pos()]);
+          record.${this.mangle($field.name(), $schema.isError())} = fieldSetFlags()[$field.pos()]
? this.${this.mangle($field.name(), $schema.isError())} : #if(${this.javaType($field.schema())}
!= "java.lang.Object")(${this.javaType($field.schema())})#{end} defaultValue(fields()[$field.pos()]);
 #end
         }
 #else
 #if ($this.hasLogicalTypeField($schema))
-        record.${this.mangle($field.name(), $schema.isError())} = fieldSetFlags()[$field.pos()]
? this.${this.mangle($field.name(), $schema.isError())} : (${this.javaType($field.schema())})
defaultValue(fields()[$field.pos()], record.getConversion($field.pos()));
+        record.${this.mangle($field.name(), $schema.isError())} = fieldSetFlags()[$field.pos()]
? this.${this.mangle($field.name(), $schema.isError())} : #if(${this.javaType($field.schema())}
!= "java.lang.Object")(${this.javaType($field.schema())})#{end} defaultValue(fields()[$field.pos()],
record.getConversion($field.pos()));
 #else
-        record.${this.mangle($field.name(), $schema.isError())} = fieldSetFlags()[$field.pos()]
? this.${this.mangle($field.name(), $schema.isError())} : (${this.javaType($field.schema())})
defaultValue(fields()[$field.pos()]);
+        record.${this.mangle($field.name(), $schema.isError())} = fieldSetFlags()[$field.pos()]
? this.${this.mangle($field.name(), $schema.isError())} : #if(${this.javaType($field.schema())}
!= "java.lang.Object")(${this.javaType($field.schema())})#{end} defaultValue(fields()[$field.pos()]);
 #end
 #end
 #end

http://git-wip-us.apache.org/repos/asf/avro/blob/b8c37b32/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java
----------------------------------------------------------------------
diff --git a/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java
b/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java
index 35d53b5..50adb19 100644
--- a/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java
+++ b/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java
@@ -33,6 +33,7 @@ import java.net.URISyntaxException;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 
 import org.apache.avro.AvroTestUtil;
@@ -46,13 +47,20 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import javax.tools.Diagnostic;
+import javax.tools.DiagnosticListener;
 import javax.tools.JavaCompiler;
+import javax.tools.JavaFileObject;
 import javax.tools.StandardJavaFileManager;
 import javax.tools.ToolProvider;
 
 @RunWith(JUnit4.class)
 public class TestSpecificCompiler {
+  private static final Logger LOG = LoggerFactory.getLogger(TestSpecificCompiler.class);
+
   private final String schemaSrcPath = "src/test/resources/simple_record.avsc";
   private final String velocityTemplateDir =
       "src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/";
@@ -93,11 +101,33 @@ public class TestSpecificCompiler {
       javaFiles.add(o.writeToDestination(null, dstDir));
     }
 
+    final List<Diagnostic<?>> warnings = new ArrayList<Diagnostic<?>>();
+    DiagnosticListener<JavaFileObject> diagnosticListener = new DiagnosticListener<JavaFileObject>()
{
+      @Override
+      public void report(Diagnostic<? extends JavaFileObject> diagnostic) {
+        switch (diagnostic.getKind()) {
+        case ERROR:
+          // Do not add these to warnings becuase they will fail the compile, anyway.
+          LOG.error("{}", diagnostic);
+          break;
+        case WARNING:
+        case MANDATORY_WARNING:
+          LOG.warn("{}", diagnostic);
+          warnings.add(diagnostic);
+          break;
+        case NOTE:
+        case OTHER:
+          LOG.debug("{}", diagnostic);
+          break;
+        }
+      }
+    };
     JavaCompiler.CompilationTask cTask = compiler.getTask(null, fileManager,
-            null, null, null, fileManager.getJavaFileObjects(
-                    javaFiles.toArray(new File[javaFiles.size()])));
+            diagnosticListener, Collections.singletonList("-Xlint:all"), null,
+            fileManager.getJavaFileObjects(javaFiles.toArray(new File[javaFiles.size()])));
     boolean compilesWithoutError = cTask.call();
     assertTrue(compilesWithoutError);
+    assertEquals("Warnings produced when compiling generated code with -Xlint:all", 0, warnings.size());
   }
 
   private static Schema createSampleRecordSchema(int numStringFields, int numDoubleFields)
{
@@ -476,6 +506,14 @@ public class TestSpecificCompiler {
   }
 
   @Test
+  public void testUnionAndFixedFields() throws Exception {
+    Schema unionTypesWithMultipleFields = new Schema.Parser().parse(
+        new File("src/test/resources/union_and_fixed_fields.avsc"));
+    assertCompilesWithJavaCompiler(
+        new SpecificCompiler(unionTypesWithMultipleFields).compile());
+  }
+
+  @Test
   public void testConversionInstanceWithDecimalLogicalTypeDisabled() throws Exception {
     SpecificCompiler compiler = createCompiler();
     compiler.setEnableDecimalLogicalType(false);

http://git-wip-us.apache.org/repos/asf/avro/blob/b8c37b32/lang/java/compiler/src/test/resources/union_and_fixed_fields.avsc
----------------------------------------------------------------------
diff --git a/lang/java/compiler/src/test/resources/union_and_fixed_fields.avsc b/lang/java/compiler/src/test/resources/union_and_fixed_fields.avsc
new file mode 100644
index 0000000..c584d6a
--- /dev/null
+++ b/lang/java/compiler/src/test/resources/union_and_fixed_fields.avsc
@@ -0,0 +1,18 @@
+{
+  "type" : "record",
+  "name" : "UnionAndFixedFields",
+  "doc" : "Schema for UnionAndFixedFields designed to trigger fixed compiler warnings in
genrated code",
+  "namespace" : "org.apache.avro.specific",
+  "fields" : [ {
+    "name" : "u",
+    "type" : [ "boolean", "int", "long", "float", "string" ]
+  },
+  {
+    "name" : "l",
+    "type" : [ "string", { "type": "long", "logicalType": "timestamp-millis" } ]
+  },
+  {
+    "name" : "f",
+    "type" : {"type": "fixed", "size": 16, "name": "md5"}
+  } ]
+}

http://git-wip-us.apache.org/repos/asf/avro/blob/b8c37b32/lang/java/tools/src/test/compiler/output-string/Position.java
----------------------------------------------------------------------
diff --git a/lang/java/tools/src/test/compiler/output-string/Position.java b/lang/java/tools/src/test/compiler/output-string/Position.java
new file mode 100644
index 0000000..5aa206b
--- /dev/null
+++ b/lang/java/tools/src/test/compiler/output-string/Position.java
@@ -0,0 +1,12 @@
+/**
+ * Autogenerated by Avro
+ *
+ * DO NOT EDIT DIRECTLY
+ */
+package avro.examples.baseball;
+@org.apache.avro.specific.AvroGenerated
+public enum Position {
+  P, C, B1, B2, B3, SS, LF, CF, RF, DH  ;
+  public static final org.apache.avro.Schema SCHEMA$ = new org.apache.avro.Schema.Parser().parse("{\"type\":\"enum\",\"name\":\"Position\",\"namespace\":\"avro.examples.baseball\",\"symbols\":[\"P\",\"C\",\"B1\",\"B2\",\"B3\",\"SS\",\"LF\",\"CF\",\"RF\",\"DH\"]}");
+  public static org.apache.avro.Schema getClassSchema() { return SCHEMA$; }
+}

http://git-wip-us.apache.org/repos/asf/avro/blob/b8c37b32/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java
----------------------------------------------------------------------
diff --git a/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java
b/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java
index 4dff5ef..a12a214 100644
--- a/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java
+++ b/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java
@@ -10,7 +10,6 @@ import org.apache.avro.message.BinaryMessageEncoder;
 import org.apache.avro.message.BinaryMessageDecoder;
 import org.apache.avro.message.SchemaStore;
 
-@SuppressWarnings("all")
 /** 選手 is Japanese for player. */
 @org.apache.avro.specific.AvroGenerated
 public class Player extends org.apache.avro.specific.SpecificRecordBase implements org.apache.avro.specific.SpecificRecord
{

http://git-wip-us.apache.org/repos/asf/avro/blob/b8c37b32/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Position.java
----------------------------------------------------------------------
diff --git a/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Position.java
b/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Position.java
index 4effedd..5aa206b 100644
--- a/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Position.java
+++ b/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Position.java
@@ -4,7 +4,6 @@
  * DO NOT EDIT DIRECTLY
  */
 package avro.examples.baseball;
-@SuppressWarnings("all")
 @org.apache.avro.specific.AvroGenerated
 public enum Position {
   P, C, B1, B2, B3, SS, LF, CF, RF, DH  ;

http://git-wip-us.apache.org/repos/asf/avro/blob/b8c37b32/lang/java/tools/src/test/compiler/output/Player.java
----------------------------------------------------------------------
diff --git a/lang/java/tools/src/test/compiler/output/Player.java b/lang/java/tools/src/test/compiler/output/Player.java
index 26fcbc0..aa78034 100644
--- a/lang/java/tools/src/test/compiler/output/Player.java
+++ b/lang/java/tools/src/test/compiler/output/Player.java
@@ -10,7 +10,6 @@ import org.apache.avro.message.BinaryMessageEncoder;
 import org.apache.avro.message.BinaryMessageDecoder;
 import org.apache.avro.message.SchemaStore;
 
-@SuppressWarnings("all")
 /** 選手 is Japanese for player. */
 @org.apache.avro.specific.AvroGenerated
 public class Player extends org.apache.avro.specific.SpecificRecordBase implements org.apache.avro.specific.SpecificRecord
{

http://git-wip-us.apache.org/repos/asf/avro/blob/b8c37b32/lang/java/tools/src/test/compiler/output/Position.java
----------------------------------------------------------------------
diff --git a/lang/java/tools/src/test/compiler/output/Position.java b/lang/java/tools/src/test/compiler/output/Position.java
index 4effedd..5aa206b 100644
--- a/lang/java/tools/src/test/compiler/output/Position.java
+++ b/lang/java/tools/src/test/compiler/output/Position.java
@@ -4,7 +4,6 @@
  * DO NOT EDIT DIRECTLY
  */
 package avro.examples.baseball;
-@SuppressWarnings("all")
 @org.apache.avro.specific.AvroGenerated
 public enum Position {
   P, C, B1, B2, B3, SS, LF, CF, RF, DH  ;


Mime
View raw message