You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by ie...@apache.org on 2021/04/28 13:48:04 UTC

[avro] branch master updated: AVRO-3116: Mangle Java16 keywords appropriately. (#1202)

This is an automated email from the ASF dual-hosted git repository.

iemejia pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/avro.git


The following commit(s) were added to refs/heads/master by this push:
     new 3f34bf8  AVRO-3116: Mangle Java16 keywords appropriately. (#1202)
3f34bf8 is described below

commit 3f34bf8b65df1190c54d80f437d47f76a029de76
Author: RyanSkraba <ry...@skraba.com>
AuthorDate: Wed Apr 28 15:47:54 2021 +0200

    AVRO-3116: Mangle Java16 keywords appropriately. (#1202)
---
 .../java/org/apache/avro/reflect/ReflectData.java  |  2 +-
 .../org/apache/avro/specific/SpecificData.java     | 37 ++++++++----
 .../compiler/specific/TestSpecificCompiler.java    | 69 ++++++++++++++++++++--
 .../test/resources/simple_record_for_scape.avsc    |  9 ---
 4 files changed, 90 insertions(+), 27 deletions(-)

diff --git a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java
index 7f15a65..4ead6b8 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java
@@ -924,7 +924,7 @@ public class ReflectData extends SpecificData {
         else if (annotation instanceof Nullable) // nullable
           paramSchema = makeNullable(paramSchema);
       }
-      fields.add(new Schema.Field(parameter.getName(), paramSchema, null /* doc */, null));
+      fields.add(new Schema.Field(unmangle(parameter.getName()), paramSchema, null /* doc */, null));
     }
 
     Schema request = Schema.createRecord(fields);
diff --git a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java
index b595b28..7c4f7d6 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java
@@ -91,19 +91,24 @@ public class SpecificData extends GenericData {
   public static final String ELEMENT_PROP = "java-element-class";
 
   /**
-   * List of Java reserved words from
-   * https://docs.oracle.com/javase/specs/jls/se8/html/jls-3.html#jls-3.9 combined
-   * with the boolean and null literals. combined with the classnames used
-   * internally in the generated avro code.
+   * Reserved words from
+   * https://docs.oracle.com/javase/specs/jls/se16/html/jls-3.html require
+   * mangling in order to be used in generated Java code.
    */
-  public static final Set<String> RESERVED_WORDS = new HashSet<>(
-      Arrays.asList("abstract", "assert", "boolean", "break", "byte", "case", "catch", "char", "class", "const",
-          "continue", "default", "do", "double", "else", "enum", "extends", "false", "final", "finally", "float", "for",
-          "goto", "if", "implements", "import", "instanceof", "int", "interface", "long", "native", "new", "null",
-          "package", "private", "protected", "public", "return", "short", "static", "strictfp", "super", "switch",
-          "synchronized", "this", "throw", "throws", "transient", "true", "try", "void", "volatile", "while",
-          /* classnames use internally by the avro code generator */
-          "Builder"));
+  public static final Set<String> RESERVED_WORDS = new HashSet<>(Arrays.asList(
+      // Keywords from Section 3.9 can't be used as identifiers.
+      "_", "abstract", "assert", "boolean", "break", "byte", "case", "catch", "char", "class", "const", "continue",
+      "default", "do", "double", "else", "enum", "extends", "final", "finally", "float", "for", "goto", "if",
+      "implements", "import", "instanceof", "int", "interface", "long", "native", "new", "package", "private",
+      "protected", "public", "return", "short", "static", "strictfp", "super", "switch", "synchronized", "this",
+      "throw", "throws", "transient", "try", "void", "volatile", "while",
+      // Literals from Section 3.10 can't be used as identifiers.
+      "true", "false", "null",
+      // Some keywords from Section 3.8 can't be used as type identifiers.
+      "var", "yield", "record",
+      // Note that module-related restricted keywords can still be used.
+      // Class names used internally by the avro code generator
+      "Builder"));
 
   /**
    * Read/write some common builtin classes as strings. Representing these as
@@ -238,6 +243,14 @@ public class SpecificData extends GenericData {
   }.getClass();
   private static final Schema NULL_SCHEMA = Schema.create(Schema.Type.NULL);
 
+  /** Undoes mangling for reserved words. */
+  protected static String unmangle(String word) {
+    while (word.endsWith("$")) {
+      word = word.substring(0, word.length() - 1);
+    }
+    return word;
+  }
+
   /** Return the class that implements a schema, or null if none exists. */
   public Class getClass(Schema schema) {
     switch (schema.getType()) {
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 1ab3b5a..77efaeb 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
@@ -36,10 +36,12 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 
+import org.apache.avro.AvroTypeException;
 import org.apache.avro.LogicalTypes;
 import org.apache.avro.Schema;
 import org.apache.avro.SchemaBuilder;
 import org.apache.avro.generic.GenericData.StringType;
+import org.apache.avro.specific.SpecificData;
 import org.junit.*;
 import org.junit.rules.TemporaryFolder;
 import org.junit.rules.TestName;
@@ -607,12 +609,69 @@ public class TestSpecificCompiler {
     Assert.assertEquals("org.apache.avro.data.TimeConversions.DateConversion", usedConversionClasses.iterator().next());
   }
 
+  /**
+   * Checks that identifiers that may cause problems in Java code will compile
+   * correctly when used in a generated specific record.
+   *
+   * @param schema                         A schema with an identifier __test__
+   *                                       that will be replaced.
+   * @param throwsTypeExceptionOnPrimitive If true, using a reserved word that is
+   *                                       also an Avro primitive type name must
+   *                                       throw an exception instead of
+   *                                       generating code.
+   * @param dstDirPrefix                   Where to generate the java code before
+   *                                       compiling.
+   */
+  public void testManglingReservedIdentifiers(String schema, boolean throwsTypeExceptionOnPrimitive,
+      String dstDirPrefix) throws IOException {
+    for (String reserved : SpecificData.RESERVED_WORDS) {
+      try {
+        Schema s = new Schema.Parser().parse(schema.replace("__test__", reserved));
+        assertCompilesWithJavaCompiler(new File(OUTPUT_DIR.getRoot(), dstDirPrefix + "_" + reserved),
+            new SpecificCompiler(s).compile());
+      } catch (AvroTypeException e) {
+        if (!(throwsTypeExceptionOnPrimitive && e.getMessage().contains("Schemas may not be named after primitives")))
+          throw e;
+      }
+    }
+  }
+
   @Test
-  public void testPackageNameScape() throws Exception {
-    Schema unionTypesWithMultipleFields = new Schema.Parser()
-        .parse(new File("src/test/resources/simple_record_for_scape.avsc"));
-    assertCompilesWithJavaCompiler(new File(this.outputFile, name.getMethodName()),
-        new SpecificCompiler(unionTypesWithMultipleFields).compile());
+  public void testMangleRecordName() throws Exception {
+    testManglingReservedIdentifiers(
+        SchemaBuilder.record("__test__").fields().requiredInt("field").endRecord().toString(), true,
+        name.getMethodName());
+  }
+
+  @Test
+  public void testMangleRecordNamespace() throws Exception {
+    testManglingReservedIdentifiers(
+        SchemaBuilder.record("__test__.Record").fields().requiredInt("field").endRecord().toString(), false,
+        name.getMethodName());
+  }
+
+  @Test
+  public void testMangleField() throws Exception {
+    testManglingReservedIdentifiers(
+        SchemaBuilder.record("Record").fields().requiredInt("__test__").endRecord().toString(), false,
+        name.getMethodName());
+  }
+
+  @Test
+  public void testMangleEnumName() throws Exception {
+    testManglingReservedIdentifiers(SchemaBuilder.enumeration("__test__").symbols("reserved").toString(), true,
+        name.getMethodName());
+  }
+
+  @Test
+  public void testMangleEnumSymbol() throws Exception {
+    testManglingReservedIdentifiers(SchemaBuilder.enumeration("Enum").symbols("__test__").toString(), false,
+        name.getMethodName());
+  }
+
+  @Test
+  public void testMangleFixedName() throws Exception {
+    testManglingReservedIdentifiers(SchemaBuilder.fixed("__test__").size(2).toString(), true, name.getMethodName());
   }
 
   @Test
diff --git a/lang/java/compiler/src/test/resources/simple_record_for_scape.avsc b/lang/java/compiler/src/test/resources/simple_record_for_scape.avsc
deleted file mode 100644
index 637f3d4..0000000
--- a/lang/java/compiler/src/test/resources/simple_record_for_scape.avsc
+++ /dev/null
@@ -1,9 +0,0 @@
-{
-  "type": "record", 
-  "name": "SimpleRecord",
-  "namespace" : "org.public.int.void",
-  "fields" : [
-    {"name": "value", "type": "int"},
-    {"name": "nullableValue", "type": ["null","int"], "doc" : "doc"}
-  ]
-}