Fix package name support for embedded composites (#915)

* enhanced test case.

* Bugfix: embedded composites were missing import for required types.

* polish

* use Collections.emptySet()

* chain append statements

* renamed method argument additionalPackages -> importedTypesPackages

Author: ratcashdev

sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java

@@ -29,6 +29,7 @@
 import java.io.IOException;
 import java.io.Writer;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Formatter;
 import java.util.HashSet;
 import java.util.List;
@@ -60,6 +61,7 @@ enum CodecType
     private static final String PACKAGE_INFO = "package-info";
     private static final String BASE_INDENT = "";
     private static final String INDENT = "    ";
+    private static final Set<String> NO_PACKAGES = Collections.emptySet();
 
     private final Ir ir;
     private final DynamicPackageOutputManager outputManager;
@@ -184,11 +186,15 @@ public void generateTypeStubs() throws IOException
      */
     private String registerTypesPackageName(final Token token, final Ir ir)
     {
-        if (shouldSupportTypesPackageNames && token.packageName() != null)
+
+        if (!shouldSupportTypesPackageNames)
+        {
+            return ir.applicableNamespace();
+        }
+        if (token.packageName() != null)
         {
             packageNameByTypes.add(token.packageName());
             outputManager.setPackageName(token.packageName());
-
             return token.packageName();
         }
         return ir.applicableNamespace();
@@ -1243,7 +1249,8 @@ private void generateBitSet(final List<Token> tokens) throws IOException
         try (Writer out = outputManager.createOutput(decoderName))
         {
             final Encoding encoding = token.encoding();
-            generateFixedFlyweightHeader(out, token, decoderName, implementsString, readOnlyBuffer, fqReadOnlyBuffer);
+            generateFixedFlyweightHeader(out, token, decoderName, implementsString, readOnlyBuffer, fqReadOnlyBuffer,
+                NO_PACKAGES);
             out.append(generateChoiceIsEmpty(encoding.primitiveType()));
 
             new Formatter(out).format(
@@ -1263,7 +1270,8 @@ private void generateBitSet(final List<Token> tokens) throws IOException
         registerTypesPackageName(token, ir);
         try (Writer out = outputManager.createOutput(encoderName))
         {
-            generateFixedFlyweightHeader(out, token, encoderName, implementsString, mutableBuffer, fqMutableBuffer);
+            generateFixedFlyweightHeader(out, token, encoderName, implementsString, mutableBuffer, fqMutableBuffer,
+                NO_PACKAGES);
             generateChoiceClear(out, encoderName, token);
             generateChoiceEncoders(out, encoderName, choiceList);
             out.append("}\n");
@@ -1276,10 +1284,11 @@ private void generateFixedFlyweightHeader(
         final String typeName,
         final String implementsString,
         final String buffer,
-        final String fqBuffer) throws IOException
+        final String fqBuffer,
+        final Set<String> importedTypesPackages) throws IOException
     {
         final String packageName = registerTypesPackageName(token, ir);
-        out.append(generateFileHeader(packageName, fqBuffer));
+        out.append(generateFileHeader(packageName, importedTypesPackages, fqBuffer));
         out.append(generateDeclaration(typeName, implementsString, token));
         out.append(generateFixedFlyweightCode(typeName, token.encodedLength(), buffer));
     }
@@ -1290,10 +1299,11 @@ private void generateCompositeFlyweightHeader(
         final Writer out,
         final String buffer,
         final String fqBuffer,
-        final String implementsString) throws IOException
+        final String implementsString,
+        final Set<String> importedTypesPackages) throws IOException
     {
         final String packageName = registerTypesPackageName(token, ir);
-        out.append(generateFileHeader(packageName, fqBuffer));
+        out.append(generateFileHeader(packageName, importedTypesPackages, fqBuffer));
         out.append(generateDeclaration(typeName, implementsString, token));
         out.append(generateFixedFlyweightCode(typeName, token.encodedLength(), buffer));
     }
@@ -1329,11 +1339,12 @@ private void generateComposite(final List<Token> tokens) throws IOException
         final String encoderName = encoderName(compositeName);
 
         registerTypesPackageName(token, ir);
+        final Set<String> importedTypesPackages = scanPackagesToImport(tokens);
         try (Writer out = outputManager.createOutput(decoderName))
         {
             final String implementsString = implementsInterface(CompositeDecoderFlyweight.class.getSimpleName());
             generateCompositeFlyweightHeader(
-                token, decoderName, out, readOnlyBuffer, fqReadOnlyBuffer, implementsString);
+                token, decoderName, out, readOnlyBuffer, fqReadOnlyBuffer, implementsString, importedTypesPackages);
 
             for (int i = 1, end = tokens.size() - 1; i < end;)
             {
@@ -1381,7 +1392,8 @@ private void generateComposite(final List<Token> tokens) throws IOException
         try (Writer out = outputManager.createOutput(encoderName))
         {
             final String implementsString = implementsInterface(CompositeEncoderFlyweight.class.getSimpleName());
-            generateCompositeFlyweightHeader(token, encoderName, out, mutableBuffer, fqMutableBuffer, implementsString);
+            generateCompositeFlyweightHeader(token, encoderName, out, mutableBuffer, fqMutableBuffer, implementsString,
+                importedTypesPackages);
 
             for (int i = 1, end = tokens.size() - 1; i < end;)
             {
@@ -1423,6 +1435,30 @@ private void generateComposite(final List<Token> tokens) throws IOException
         }
     }
 
+    private Set<String> scanPackagesToImport(final List<Token> tokens)
+    {
+        if (!shouldSupportTypesPackageNames)
+        {
+            return NO_PACKAGES;
+        }
+
+        final Set<String> packagesToImport = new HashSet<>();
+        for (int i = 1; i < tokens.size() - 1; i++)
+        {
+            final Token typeToken = tokens.get(i);
+            if (typeToken.signal() == Signal.BEGIN_ENUM ||
+                typeToken.signal() == Signal.BEGIN_SET ||
+                typeToken.signal() == Signal.BEGIN_COMPOSITE)
+            {
+                if (typeToken.packageName() != null)
+                {
+                    packagesToImport.add(typeToken.packageName());
+                }
+            }
+        }
+        return packagesToImport;
+    }
+
     private void generateChoiceClear(final Appendable out, final String bitSetClassName, final Token token)
         throws IOException
     {
@@ -1608,6 +1644,23 @@ private CharSequence generateEnumLookupMethod(final List<Token> tokens, final St
         return sb;
     }
 
+    private StringBuilder generateImportStatements(final Set<String> packages, final String currentPackage)
+    {
+        final StringBuilder importsStatements = new StringBuilder();
+        for (final String candidatePackage : packages)
+        {
+            if (!candidatePackage.equalsIgnoreCase(currentPackage))
+            {
+                importsStatements.append("import ").append(candidatePackage).append(".*;\n");
+            }
+        }
+        if (importsStatements.length() > 0)
+        {
+            importsStatements.append("\n\n");
+        }
+        return importsStatements;
+    }
+
     private String interfaceImportLine()
     {
         if (!shouldGenerateInterfaces)
@@ -1618,25 +1671,23 @@ private String interfaceImportLine()
         return "import " + JAVA_INTERFACE_PACKAGE + ".*;\n\n";
     }
 
-    private CharSequence generateFileHeader(final String packageName, final String fqBuffer)
+
+    private CharSequence generateFileHeader(final String packageName, final Set<String> importedTypesPackages,
+        final String fqBuffer)
     {
-        return
-            "/* Generated SBE (Simple Binary Encoding) message codec. */\n" +
+        final StringBuilder importStatements = generateImportStatements(importedTypesPackages, packageName);
+
+        return "/* Generated SBE (Simple Binary Encoding) message codec. */\n" +
             "package " + packageName + ";\n\n" +
             "import " + fqBuffer + ";\n" +
-            interfaceImportLine();
+            interfaceImportLine() +
+            importStatements;
     }
 
     private CharSequence generateMainHeader(
         final String packageName, final CodecType codecType, final boolean hasVarData)
     {
-        final StringBuilder packageImports = new StringBuilder();
-        for (final String typePackage : packageNameByTypes)
-        {
-            packageImports.append("import ");
-            packageImports.append(typePackage);
-            packageImports.append(".*;\n");
-        }
+        final StringBuilder importStatements = generateImportStatements(packageNameByTypes, packageName);
 
         if (fqMutableBuffer.equals(fqReadOnlyBuffer))
         {
@@ -1645,7 +1696,7 @@ private CharSequence generateMainHeader(
                 "package " + packageName + ";\n\n" +
                 "import " + fqMutableBuffer + ";\n" +
                 interfaceImportLine() +
-                packageImports;
+                importStatements;
         }
         else
         {
@@ -1658,7 +1709,7 @@ private CharSequence generateMainHeader(
                 (hasMutableBuffer ? "import " + fqMutableBuffer + ";\n" : "") +
                 (hasReadOnlyBuffer ? "import " + fqReadOnlyBuffer + ";\n" : "") +
                 interfaceImportLine() +
-                packageImports;
+                importStatements;
         }
     }
 

sbe-tool/src/test/resources/explicit-package-test-schema.xml

@@ -18,13 +18,24 @@
         <composite name="Car" description="Message identifiers and length of message root">
             <type name="make" primitiveType="uint16"/>
             <type name="model" primitiveType="uint16"/>
+            <ref name="vintage" type="BooleanType"/>
+            <composite name="Engine">
+                <type name="power" primitiveType="int32"/>
+                <type name="torque" primitiveType="int32"/>
+                <ref name="ice" type="BooleanType"/>
+                <ref name="fuel" type="FuelSpec"/>
+            </composite>
         </composite>
     </types>
     <types package="outside.schema">
         <enum name="BooleanType" encodingType="int8">
             <validValue name="FALSE">0</validValue>
             <validValue name="TRUE">1</validValue>
         </enum>
+        <composite name="FuelSpec">
+           <type name="limit" primitiveType="int32"/>
+           <ref name="fuel" type="FuelType"/>
+        </composite>
         <set name="Days" encodingType="uint8">
             <choice name="Monday">0</choice>
             <choice name="Tuesday">1</choice>
@@ -34,6 +45,13 @@
             <choice name="Saturday">5</choice>
             <choice name="Sunday">6</choice>
         </set>
+        <set name="FuelType" encodingType="uint8">
+            <choice name="Petrol">1</choice>
+            <choice name="Diesel">2</choice>
+            <choice name="LPG">3</choice>
+            <choice name="Electric">4</choice>
+            <choice name="Hybrid">5</choice>
+        </set>
     </types>
 
     <sbe:message name="TestMessage" id="1">