[Java] Tidy up handling of reporting warnings on non-standard header and replace use of String.format.

Author: mjpt777

sbe-tool/src/main/java/uk/co/real_logic/sbe/xml/CompositeType.java

@@ -195,9 +195,9 @@ else if (!isUnsigned(numInGroupType.primitiveType()))
  }
  else if (numInGroupMinValue.longValue() < 0)
  {
- XmlSchemaParser.handleError(node, String.format(
- "\"numInGroup\" minValue=%s must be greater than zero " +
- "for signed \"numInGroup\" types", numInGroupMinValue));
+ XmlSchemaParser.handleError(node,
+ "\"numInGroup\" minValue=" + numInGroupMinValue + " must be greater than zero " +
+ "for signed \"numInGroup\" types");
  }
  }
  else
@@ -218,8 +218,8 @@ else if (numInGroupMinValue.longValue() < 0)
 
  if (numInGroupMinValue.longValue() > max)
  {
- XmlSchemaParser.handleError(node, String.format(
- "\"numInGroup\" minValue=%s greater than maxValue=%d", numInGroupMinValue, max));
+ XmlSchemaParser.handleError(
+ node, "\"numInGroup\" minValue=" + numInGroupMinValue + " greater than maxValue=" + max);
  }
  }
  }
@@ -275,22 +275,22 @@ private static void validateGroupMaxValue(
  final long allowedValue = primitiveType.maxValue().longValue();
  if (longValue > allowedValue)
  {
- XmlSchemaParser.handleError(node, String.format(
- "maxValue greater than allowed for type: maxValue=%d allowed=%d", longValue, allowedValue));
+ XmlSchemaParser.handleError(
+ node, "maxValue greater than allowed for type: maxValue=" + longValue + " allowed=" + allowedValue);
  }
 
  final long maxInt = INT32.maxValue().longValue();
  if (primitiveType == UINT32 && longValue > maxInt)
  {
- XmlSchemaParser.handleError(node, String.format(
- "maxValue greater than allowed for type: maxValue=%d allowed=%d", longValue, maxInt));
+ XmlSchemaParser.handleError(
+ node, "maxValue greater than allowed for type: maxValue=" + longValue + " allowed=" + maxInt);
  }
  }
  else if (primitiveType == UINT32)
  {
  final long maxInt = INT32.maxValue().longValue();
- XmlSchemaParser.handleError(node, String.format(
- "maxValue must be set for varData UINT32 type: max value allowed=%d", maxInt));
+ XmlSchemaParser.handleError(
+ node, "maxValue must be set for varData UINT32 type: max value allowed=" + maxInt);
  }
  }
 
@@ -333,33 +333,25 @@ private void validateHeaderField(
  {
  if (actualType == null)
  {
- XmlSchemaParser.handleError(
- node,
- String.format("composite for message header must have \"%s\"", fieldName));
+ XmlSchemaParser.handleError(node, "composite for message header must have \"" + fieldName + "\"");
  }
  else if (actualType.primitiveType() != expectedType)
  {
- XmlSchemaParser.handleWarning(node, String.format("\"%s\" should be %s", fieldName, expectedType.name()));
+ XmlSchemaParser.handleWarning(node, "\"" + fieldName + "\" should be " + expectedType.name());
 
  if (shouldGenerateInterfaces)
  {
  if (actualType.primitiveType().size() > expectedType.size())
  {
- XmlSchemaParser.handleError(
- node,
- String.format("\"%s\" must be less than %s bytes to use %s",
- fieldName,
- expectedType.size(),
- JAVA_GENERATE_INTERFACES));
+ final String msg = "\"" + fieldName + "\" must be less than " + expectedType.size() +
+ " bytes to use " + JAVA_GENERATE_INTERFACES;
+ XmlSchemaParser.handleError(node, msg);
  }
  else
  {
- XmlSchemaParser.handleWarning(
- node,
- String.format("\"%s\" will be cast to %s to use %s",
- fieldName,
- expectedType.name(),
- JAVA_GENERATE_INTERFACES));
+ final String msg = "\"" + fieldName + "\" will be cast to " + expectedType.name() +
+ " to use " + JAVA_GENERATE_INTERFACES;
+ XmlSchemaParser.handleWarning(node, msg);
  }
  }
  }
@@ -383,8 +375,7 @@ public void checkForValidOffsets(final Node node)
  if (offsetAttribute < offset)
  {
  XmlSchemaParser.handleError(
- node,
- String.format("composite element \"%s\" has incorrect offset specified", edt.name()));
+ node, "composite element \"" + edt.name() + "\" has incorrect offset specified");
  }
 
  offset = offsetAttribute;
@@ -434,8 +425,8 @@ private Type processType(
  {
  final XPath xPath = XPathFactory.newInstance().newXPath();
  final String refTypeName = XmlSchemaParser.getAttributeValue(subTypeNode, "type");
- final Node refTypeNode = (Node)xPath.compile(
-  "/*[local-name() = 'messageSchema']/types/*[@name='" + refTypeName + "']")
+ final String expression = "/*[local-name() = 'messageSchema']/types/*[@name='" + refTypeName + "']";
+ final Node refTypeNode = (Node)xPath.compile(expression)
  .evaluate(subTypeNode.getOwnerDocument(), XPathConstants.NODE);
 
  if (refTypeNode == null)

sbe-tool/src/main/java/uk/co/real_logic/sbe/xml/MessageSchema.java

@@ -52,8 +52,7 @@ public class MessageSchema
  private final Map<String, Type> typeByNameMap;
  private final Map<Long, Message> messageByIdMap;
 
- MessageSchema(
- final Node schemaNode, final Map<String, Type> typeByNameMap, final Map<Long, Message> messageByIdMap)
+ MessageSchema(final Node schemaNode, final Map<String, Type> typeByNameMap, final Map<Long, Message> messageByIdMap)
  {
  this.packageName = getAttributeValue(schemaNode, "package");
  this.description = getAttributeValueOrNull(schemaNode, "description");
@@ -371,9 +370,9 @@ private static Node findNode(final Node contextNode, final String path)
  return (Node)XPathFactory.newInstance().newXPath()
  .evaluate(path, contextNode, XPathConstants.NODE);
  }
- catch (final XPathExpressionException e)
+ catch (final XPathExpressionException ex)
  {
- throw new IllegalArgumentException("Unable to locate node with path=" + path, e);
+ throw new IllegalArgumentException("Unable to locate node with path=" + path, ex);
  }
  }
 }

sbe-tool/src/main/java/uk/co/real_logic/sbe/xml/XmlSchemaParser.java

@@ -176,9 +176,10 @@ public static MessageSchema parse(final InputStream in, final ParserOptions opti
  * @param document for the XML parsing.
  * @param xPath for XPath expression reuse.
  * @return {@link java.util.Map} of name {@link java.lang.String} to {@link Type}.
- * @throws Exception on parsing error.
+ * @throws XPathExpressionException on parsing error.
  */
- public static Map<String, Type> findTypes(final Document document, final XPath xPath) throws Exception
+ public static Map<String, Type> findTypes(final Document document, final XPath xPath)
+ throws XPathExpressionException
  {
  final Map<String, Type> typeByNameMap = new HashMap<>();
 
@@ -216,10 +217,11 @@ public static Map<String, Type> findTypes(final Document document, final XPath x
  * @param xPath for XPath expression reuse.
  * @param typeByNameMap to use for Type objects.
  * @return {@link java.util.Map} of schemaId to {@link Message}.
- * @throws Exception on parsing error.
+ * @throws XPathExpressionException on parsing error.
  */
  public static Map<Long, Message> findMessages(
- final Document document, final XPath xPath, final Map<String, Type> typeByNameMap) throws Exception
+ final Document document, final XPath xPath, final Map<String, Type> typeByNameMap)
+ throws XPathExpressionException
  {
  final Map<Long, Message> messageByIdMap = new HashMap<>();
  final ObjectHashSet<String> distinctNames = new ObjectHashSet<>();

sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/java/WarningFormattingTest.java

@@ -24,27 +24,31 @@
 import java.io.PrintStream;
 import java.nio.charset.StandardCharsets;
 
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
 import static uk.co.real_logic.sbe.Tests.getLocalResource;
 import static uk.co.real_logic.sbe.xml.XmlSchemaParser.parse;
 
-public class WarningFormattingTest
+class WarningFormattingTest
 {
  @Test
  void shouldGenerateCorrectWarning() throws Exception
  {
  final ByteArrayOutputStream out = new ByteArrayOutputStream();
- final ParserOptions options = new ParserOptions.Builder().errorPrintStream(new PrintStream(out)).build();
+ final ParserOptions options = new ParserOptions.Builder()
+ .stopOnError(true)
+ .errorPrintStream(new PrintStream(out))
+ .build();
  final MessageSchema schema = parse(getLocalResource("npe-small-header.xml"), options);
  final IrGenerator irg = new IrGenerator();
  irg.generate(schema);
 
  final String warnings = out.toString(StandardCharsets.US_ASCII.name());
- assertTrue(warnings.contains(
+ assertThat(warnings, containsString(
  "WARNING: at <types><composite name=\"messageHeader\"> \"blockLength\" should be UINT16"));
- assertTrue(warnings.contains(
+ assertThat(warnings, containsString(
  "WARNING: at <types><composite name=\"messageHeader\"> \"templateId\" should be UINT16"));
- assertTrue(warnings.contains(
+ assertThat(warnings, containsString(
  "WARNING: at <types><composite name=\"messageHeader\"> \"version\" should be UINT16"));
  }
 }

sbe-tool/src/test/resources/npe-small-header.xml

@@ -7,16 +7,19 @@
  description="Message format for XXXXXX"
  byteOrder="littleEndian">
  <types>
- <!-- SBE standard-ish MessageHeader:
- We think we can use uint8 for most of these fields.
- Part of the argument is to acknowledge XXXXXXXX does use complex/large msgs, but they are/should-be config/protobuffs, not SBE -->
- <composite name="messageHeader" description="Header included at the start of every SBE message">
- <type name="schemaId" primitiveType="uint16" description="ID for the schema"/>
- <type name="version" primitiveType="uint8" description="Version number of the schema"/>
- <type name="templateId" primitiveType="uint8" description="ID of the msg type. 0-127 = outbound (inc Ping, etc), 128-255 = inbound(Pong, etc)"/>
- <type name="blockLength" primitiveType="uint8" description="Length of fixed-size root-block part, excludes this header, repeating-groups and VarData"/>
- <type name="numGroups" primitiveType="uint8" description="Number of repeating-groups"/>
- <type name="numVarDataFields" primitiveType="uint8" description="Number of variable-length fields"/>
+ <composite name="messageHeader" description="Header included before every SBE message">
+ <type name="schemaId" primitiveType="uint16"
+ description="ID for the schema"/>
+ <type name="version" primitiveType="uint8"
+ description="Version number of the schema"/>
+ <type name="templateId" primitiveType="uint8"
+ description="ID of the msg type."/>
+ <type name="blockLength" primitiveType="uint8"
+ description="Length of fixed-size root-block, excludes this header, repeating-groups and var-data"/>
+ <type name="numGroups" primitiveType="uint8"
+ description="Number of repeating-groups"/>
+ <type name="numVarDataFields" primitiveType="uint8"
+ description="Number of variable-length fields"/>
  </composite>
  </types>
  <sbe:message name="Ping" id="0"> </sbe:message>