diff --git a/java/protopoet/OptionSpec.java b/java/protopoet/OptionSpec.java index aed6b88..4c2bff8 100644 --- a/java/protopoet/OptionSpec.java +++ b/java/protopoet/OptionSpec.java @@ -27,8 +27,8 @@ import java.util.List; /** - * Models an Option in the Protocol Buffers language. - * Learn more: https://developers.google.com/protocol-buffers/docs/proto#options + * Models an Option in the Protocol Buffers language. Learn more: + * https://developers.google.com/protocol-buffers/docs/proto#options */ public final class OptionSpec implements Buildable, Emittable { @@ -71,7 +71,7 @@ public static Builder oneofOption(String optionName) { public static Builder methodOption(String optionName) { return builder(OptionType.METHOD, optionName); } - + /** Creates a builder for an {@link OptionSpec}. */ static Builder builder(OptionType optionType, String optionName) { checkNotNull(optionType, "option type may not be null"); @@ -80,13 +80,13 @@ static Builder builder(OptionType optionType, String optionName) { } // Snippet of resuable logic for rendering field-based options. - static ProtoWriter emitFieldOptions(List options, - ProtoWriter writer) throws IOException { + static ProtoWriter emitFieldOptions(List options, ProtoWriter writer) + throws IOException { if (!options.isEmpty()) { writer.emit(" ["); for (int i = 0; i < options.size(); i++) { options.get(i).emit(writer); - if (i+1 < options.size()) { + if (i + 1 < options.size()) { writer.emit(", "); } } @@ -94,54 +94,55 @@ static ProtoWriter emitFieldOptions(List options, } return writer; } - + // Field option types have specialized formatting that are inconsistent with non-field options // such as Message, Service, File, etc. - private static ImmutableSet FIELD_OPTION_TYPES = ImmutableSet.of(OptionType.FIELD, - OptionType.ENUM_VALUE); + private static ImmutableSet FIELD_OPTION_TYPES = + ImmutableSet.of(OptionType.FIELD, OptionType.ENUM_VALUE); // Protobufs have some well known option names that require special formatting to // disambiguate from custom options. This is largely a rendering implementation // detail so we keep a list sycned with this file: // https://github.com/google/protobuf/blob/master/src/google/protobuf/descriptor.proto private static ImmutableMap> WELL_KNOWN_OPTIONS; + static { - WELL_KNOWN_OPTIONS = ImmutableMap.>builder() - .put(OptionType.FILE, ImmutableSet.of("java_package", - "java_outer_classname", - "java_multiple_files", - "java_generate_equals_and_hash", - "java_string_check_utf8", - "optimize_for", - "go_package", - "cc_generic_services", - "java_generic_services", - "py_generic_services", - "php_generic_services", - "deprecated", - "cc_enable_arenas", - "objc_class_prefix", - "csharp_namespace", - "swift_prefix", - "php_class_prefix", - "php_namespace")) - .put(OptionType.MESSAGE, ImmutableSet.of("message_set_wire_format", - "no_standard_descriptor_accessor", - "deprecated")) - .put(OptionType.SERVICE, ImmutableSet.of("deprecated")) - .put(OptionType.ENUM, ImmutableSet.of("allow_alias", - "deprecated")) - .put(OptionType.ONEOF, ImmutableSet.of()) - .put(OptionType.FIELD, ImmutableSet.of("ctype", - "packed", - "jstype", - "lazy", - "deprecated", - "weak")) - .put(OptionType.ENUM_VALUE, ImmutableSet.of("deprecated")) - .put(OptionType.METHOD, ImmutableSet.of("deprecated", - "idempotency_level")) - .build(); + WELL_KNOWN_OPTIONS = + ImmutableMap.>builder() + .put( + OptionType.FILE, + ImmutableSet.of( + "java_package", + "java_outer_classname", + "java_multiple_files", + "java_generate_equals_and_hash", + "java_string_check_utf8", + "optimize_for", + "go_package", + "cc_generic_services", + "java_generic_services", + "py_generic_services", + "php_generic_services", + "deprecated", + "cc_enable_arenas", + "objc_class_prefix", + "csharp_namespace", + "swift_prefix", + "php_class_prefix", + "php_namespace")) + .put( + OptionType.MESSAGE, + ImmutableSet.of( + "message_set_wire_format", "no_standard_descriptor_accessor", "deprecated")) + .put(OptionType.SERVICE, ImmutableSet.of("deprecated")) + .put(OptionType.ENUM, ImmutableSet.of("allow_alias", "deprecated")) + .put(OptionType.ONEOF, ImmutableSet.of()) + .put( + OptionType.FIELD, + ImmutableSet.of("ctype", "packed", "jstype", "lazy", "deprecated", "weak")) + .put(OptionType.ENUM_VALUE, ImmutableSet.of("deprecated")) + .put(OptionType.METHOD, ImmutableSet.of("deprecated", "idempotency_level")) + .build(); } private final OptionType optionType; @@ -182,16 +183,13 @@ public void emit(ProtoWriter writer) throws IOException { // different approach for both field and non-field use cases. if (optionValueType == FieldType.MESSAGE) { if (!isFieldOptionType) { - writer - .emit(String.format("option %s = {\n", formattedOptionName)) - .indent(); + writer.emit(String.format("option %s = {\n", formattedOptionName)).indent(); // Cast is safe here because the builder below enforces type safety. for (FieldValue fieldValue : (Iterable) optionValue) { - writer.emit(String.format("%s: %s\n", fieldValue.fieldName(), fieldValue.formattedValue())); + writer.emit( + String.format("%s: %s\n", fieldValue.fieldName(), fieldValue.formattedValue())); } - writer - .unindent() - .emit("};\n"); + writer.unindent().emit("};\n"); return; } @@ -214,9 +212,12 @@ OptionType optionType() { } private static String formatOptionName(OptionType optionType, String optionName) { - checkState(WELL_KNOWN_OPTIONS.containsKey(optionType), - String.format("unexpected option type: %s", optionType)); - return WELL_KNOWN_OPTIONS.get(optionType).contains(optionName) ? optionName : "(" + optionName + ")"; + checkState( + WELL_KNOWN_OPTIONS.containsKey(optionType), + String.format("unexpected option type: %s", optionType)); + return WELL_KNOWN_OPTIONS.get(optionType).contains(optionName) || optionName.startsWith("(") + ? optionName + : "(" + optionName + ")"; } /** Builder for producing new instances of {@link OptionSpec}. */ @@ -227,21 +228,26 @@ public static final class Builder implements Buildable { private ImmutableList optionComment = ImmutableList.of(); private FieldType optionValueType; private Object optionValue; - + private Builder(OptionType optionType, String optionName) { this.optionType = optionType; this.optionName = optionName; } - /** Declares a top-level comment for the option. Note, this only renders for non-field options. */ + /** + * Declares a top-level comment for the option. Note, this only renders for non-field options. + */ public Builder setOptionComment(Iterable lines) { - checkState(!OptionSpec.FIELD_OPTION_TYPES.contains(optionType), - "comments aren't available for field options"); + checkState( + !OptionSpec.FIELD_OPTION_TYPES.contains(optionType), + "comments aren't available for field options"); optionComment = ImmutableList.copyOf(lines); return this; } - /** Declares a top-level comment for the option. Note, this only renders for non-field options. */ + /** + * Declares a top-level comment for the option. Note, this only renders for non-field options. + */ public Builder setOptionComment(String... lines) { return setOptionComment(ImmutableList.copyOf(lines)); } @@ -249,40 +255,40 @@ public Builder setOptionComment(String... lines) { /** Sets an integer-based value (eg: int32, uint32, fixed32, sfixed32). */ public Builder setValue(FieldType valueType, int intValue) { switch (valueType) { - case INT32: - case UINT32: - case FIXED32: - case SFIXED32: - optionValueType = valueType; - optionValue = intValue; - return this; - default: - throw new IllegalArgumentException(String.format("'%s' invalid type for an int value", - valueType)); + case INT32: + case UINT32: + case FIXED32: + case SFIXED32: + optionValueType = valueType; + optionValue = intValue; + return this; + default: + throw new IllegalArgumentException( + String.format("'%s' invalid type for an int value", valueType)); } } /** Sets a long-based value (eg: int64, uint64, fixed64, sfixed64). */ public Builder setValue(FieldType valueType, long longValue) { switch (valueType) { - case INT64: - case UINT64: - case FIXED64: - case SFIXED64: - optionValueType = valueType; - optionValue = longValue; - return this; - default: - throw new IllegalArgumentException(String.format("'%s' invalid type for a long value", - valueType)); + case INT64: + case UINT64: + case FIXED64: + case SFIXED64: + optionValueType = valueType; + optionValue = longValue; + return this; + default: + throw new IllegalArgumentException( + String.format("'%s' invalid type for a long value", valueType)); } } /** Sets a float-based value. */ public Builder setValue(FieldType valueType, float floatValue) { - checkArgument(valueType == FieldType.FLOAT, - String.format("'%s' invalid type for a float value", - valueType)); + checkArgument( + valueType == FieldType.FLOAT, + String.format("'%s' invalid type for a float value", valueType)); optionValueType = valueType; optionValue = floatValue; return this; @@ -290,9 +296,9 @@ public Builder setValue(FieldType valueType, float floatValue) { /** Sets a double-based value. */ public Builder setValue(FieldType valueType, double doubleValue) { - checkArgument(valueType == FieldType.DOUBLE, - String.format("'%s' invalid type for a double value", - valueType)); + checkArgument( + valueType == FieldType.DOUBLE, + String.format("'%s' invalid type for a double value", valueType)); optionValueType = valueType; optionValue = doubleValue; return this; @@ -302,32 +308,32 @@ public Builder setValue(FieldType valueType, double doubleValue) { public Builder setValue(FieldType valueType, String stringValue) { checkNotNull(stringValue, "value must not be null"); switch (valueType) { - case ENUM: - case STRING: - case BYTES: - optionValueType = valueType; - optionValue = stringValue; - return this; - default: - throw new IllegalArgumentException(String.format("'%s' invalid type for a string value", - valueType)); + case ENUM: + case STRING: + case BYTES: + optionValueType = valueType; + optionValue = stringValue; + return this; + default: + throw new IllegalArgumentException( + String.format("'%s' invalid type for a string value", valueType)); } } /** Sets a boolean value. */ public Builder setValue(FieldType valueType, boolean boolValue) { - checkArgument(valueType == FieldType.BOOL, - String.format("'%s' invalid type for a bool value", - valueType)); + checkArgument( + valueType == FieldType.BOOL, + String.format("'%s' invalid type for a bool value", valueType)); optionValueType = valueType; optionValue = boolValue; return this; } public Builder setValue(FieldType valueType, Iterable fieldValues) { - checkArgument(valueType == FieldType.MESSAGE, - String.format("'%s' invalid type for a message value", - valueType)); + checkArgument( + valueType == FieldType.MESSAGE, + String.format("'%s' invalid type for a message value", valueType)); optionValueType = valueType; optionValue = fieldValues; return this; diff --git a/javatests/protopoet/OptionSpecTest.java b/javatests/protopoet/OptionSpecTest.java index 4540b00..29cf913 100644 --- a/javatests/protopoet/OptionSpecTest.java +++ b/javatests/protopoet/OptionSpecTest.java @@ -28,188 +28,214 @@ public final class OptionSpecTest { @Test public void testWritingNonFieldScalarOption_integer() { output - .expects("// comment\noption (foo) = 2147483647;\n") - .produce(() -> - OptionSpec.builder(OptionType.MESSAGE, "foo") - .setOptionComment("comment") - .setValue(FieldType.INT32, Integer.MAX_VALUE) - .build()); + .expects("// comment\noption (foo) = 2147483647;\n") + .produce( + () -> + OptionSpec.builder(OptionType.MESSAGE, "foo") + .setOptionComment("comment") + .setValue(FieldType.INT32, Integer.MAX_VALUE) + .build()); } @Test public void testWritingFieldScalarOption_integer() { output - .expects("(foo) = 2147483647") - .produce(() -> - OptionSpec.builder(OptionType.FIELD, "foo") - .setValue(FieldType.INT32, Integer.MAX_VALUE) - .build()); + .expects("(foo) = 2147483647") + .produce( + () -> + OptionSpec.builder(OptionType.FIELD, "foo") + .setValue(FieldType.INT32, Integer.MAX_VALUE) + .build()); } @Test public void testWritingNonFieldScalarOption_long() { output - .expects("// comment\noption (foo) = 9223372036854775807;\n") - .produce(() -> - OptionSpec.builder(OptionType.MESSAGE, "foo") - .setOptionComment("comment") - .setValue(FieldType.INT64, Long.MAX_VALUE) - .build()); + .expects("// comment\noption (foo) = 9223372036854775807;\n") + .produce( + () -> + OptionSpec.builder(OptionType.MESSAGE, "foo") + .setOptionComment("comment") + .setValue(FieldType.INT64, Long.MAX_VALUE) + .build()); } @Test public void testWritingFieldScalarOption_long() { output - .expects("(foo) = 9223372036854775807") - .produce(() -> - OptionSpec.builder(OptionType.FIELD, "foo") - .setValue(FieldType.INT64, Long.MAX_VALUE) - .build()); + .expects("(foo) = 9223372036854775807") + .produce( + () -> + OptionSpec.builder(OptionType.FIELD, "foo") + .setValue(FieldType.INT64, Long.MAX_VALUE) + .build()); } - + @Test public void testWritingNonFieldScalarOption_float() { output - .expects("// comment\noption (foo) = 3.4028235E38;\n") - .produce(() -> - OptionSpec.builder(OptionType.MESSAGE, "foo") - .setOptionComment("comment") - .setValue(FieldType.FLOAT, Float.MAX_VALUE) - .build()); + .expects("// comment\noption (foo) = 3.4028235E38;\n") + .produce( + () -> + OptionSpec.builder(OptionType.MESSAGE, "foo") + .setOptionComment("comment") + .setValue(FieldType.FLOAT, Float.MAX_VALUE) + .build()); } @Test public void testWritingFieldScalarOption_float() { output - .expects("(foo) = 3.4028235E38") - .produce(() -> - OptionSpec.builder(OptionType.FIELD, "foo") - .setValue(FieldType.FLOAT, Float.MAX_VALUE) - .build()); + .expects("(foo) = 3.4028235E38") + .produce( + () -> + OptionSpec.builder(OptionType.FIELD, "foo") + .setValue(FieldType.FLOAT, Float.MAX_VALUE) + .build()); } - + @Test public void testWritingNonFieldScalarOption_double() { output - .expects("// comment\noption (foo) = 1.7976931348623157E308;\n") - .produce(() -> - OptionSpec.builder(OptionType.MESSAGE, "foo") - .setOptionComment("comment") - .setValue(FieldType.DOUBLE, Double.MAX_VALUE) - .build()); + .expects("// comment\noption (foo) = 1.7976931348623157E308;\n") + .produce( + () -> + OptionSpec.builder(OptionType.MESSAGE, "foo") + .setOptionComment("comment") + .setValue(FieldType.DOUBLE, Double.MAX_VALUE) + .build()); } @Test public void testWritingFieldScalarOption_double() { output - .expects("(foo) = 1.7976931348623157E308") - .produce(() -> - OptionSpec.builder(OptionType.FIELD, "foo") - .setValue(FieldType.DOUBLE, Double.MAX_VALUE) - .build()); + .expects("(foo) = 1.7976931348623157E308") + .produce( + () -> + OptionSpec.builder(OptionType.FIELD, "foo") + .setValue(FieldType.DOUBLE, Double.MAX_VALUE) + .build()); } - @Test public void testWritingNonFieldScalarOption_enum() { output - .expects("// comment\noption (foo) = FOO;\n") - .produce(() -> - OptionSpec.builder(OptionType.MESSAGE, "foo") - .setOptionComment("comment") - .setValue(FieldType.ENUM, "FOO") - .build()); + .expects("// comment\noption (foo) = FOO;\n") + .produce( + () -> + OptionSpec.builder(OptionType.MESSAGE, "foo") + .setOptionComment("comment") + .setValue(FieldType.ENUM, "FOO") + .build()); } - @Test public void testWritingFieldScalarOption_enum() { output - .expects("(foo) = FOO") - .produce(() -> - OptionSpec.builder(OptionType.FIELD, "foo") - .setValue(FieldType.ENUM, "FOO") - .build()); + .expects("(foo) = FOO") + .produce( + () -> + OptionSpec.builder(OptionType.FIELD, "foo") + .setValue(FieldType.ENUM, "FOO") + .build()); } - @Test public void testWritingNonFieldScalarOption_string() { output - .expects("// comment\noption (foo) = \"hello\";\n") - .produce(() -> - OptionSpec.builder(OptionType.MESSAGE, "foo") - .setOptionComment("comment") - .setValue(FieldType.STRING, "hello") - .build()); + .expects("// comment\noption (foo) = \"hello\";\n") + .produce( + () -> + OptionSpec.builder(OptionType.MESSAGE, "foo") + .setOptionComment("comment") + .setValue(FieldType.STRING, "hello") + .build()); } @Test public void testWritinFieldScalarOption_string() { output - .expects("(foo) = \"hello\"") - .produce(() -> - OptionSpec.builder(OptionType.FIELD, "foo") - .setValue(FieldType.STRING, "hello") - .build()); + .expects("(foo) = \"hello\"") + .produce( + () -> + OptionSpec.builder(OptionType.FIELD, "foo") + .setValue(FieldType.STRING, "hello") + .build()); } - + @Test public void testWritingNonFieldScalarOption_boolean() { output - .expects("// comment\noption (foo) = true;\n") - .produce(() -> - OptionSpec.builder(OptionType.MESSAGE, "foo") - .setOptionComment("comment") - .setValue(FieldType.BOOL, true) - .build()); + .expects("// comment\noption (foo) = true;\n") + .produce( + () -> + OptionSpec.builder(OptionType.MESSAGE, "foo") + .setOptionComment("comment") + .setValue(FieldType.BOOL, true) + .build()); } @Test public void testWritingFieldScalarOption_boolean() { output - .expects("(foo) = true") - .produce(() -> - OptionSpec.builder(OptionType.FIELD, "foo") - .setValue(FieldType.BOOL, true) - .build()); + .expects("(foo) = true") + .produce( + () -> + OptionSpec.builder(OptionType.FIELD, "foo").setValue(FieldType.BOOL, true).build()); } @Test public void testWellKnownFieldFormatting() { output - .expects("// comment\noption java_package = \"com.whatever\";\n") - .produce(() -> - OptionSpec.builder(OptionType.FILE, "java_package") - .setOptionComment("comment") - .setValue(FieldType.STRING, "com.whatever") - .build()); + .expects("// comment\noption java_package = \"com.whatever\";\n") + .produce( + () -> + OptionSpec.builder(OptionType.FILE, "java_package") + .setOptionComment("comment") + .setValue(FieldType.STRING, "com.whatever") + .build()); } @Test public void testMessageFieldValuesScalars() { output - .expectsTestData() - .produce(() -> - OptionSpec.builder(OptionType.METHOD, "foo") - .setOptionComment("comment") - .setValue(FieldType.MESSAGE, - FieldValue.of("bar", FieldType.STRING, "hello"), - FieldValue.of("baz", FieldType.BOOL, true)) - .build()); + .expectsTestData() + .produce( + () -> + OptionSpec.builder(OptionType.METHOD, "foo") + .setOptionComment("comment") + .setValue( + FieldType.MESSAGE, + FieldValue.of("bar", FieldType.STRING, "hello"), + FieldValue.of("baz", FieldType.BOOL, true)) + .build()); } @Test public void testMessageFieldFieldValuesScalars() { output - .expects("(foo) = { bar: \"hello\" baz: true }") - .produce(() -> - OptionSpec.builder(OptionType.ENUM_VALUE, "foo") - .setValue(FieldType.MESSAGE, - FieldValue.of("bar", FieldType.STRING, "hello"), - FieldValue.of("baz", FieldType.BOOL, true)) - .build()); + .expects("(foo) = { bar: \"hello\" baz: true }") + .produce( + () -> + OptionSpec.builder(OptionType.ENUM_VALUE, "foo") + .setValue( + FieldType.MESSAGE, + FieldValue.of("bar", FieldType.STRING, "hello"), + FieldValue.of("baz", FieldType.BOOL, true)) + .build()); } + @Test + public void testMessageFieldNameParensExplicit() { + output + .expects("(foo) = { bar: \"hello\" baz: true }") + .produce( + () -> + OptionSpec.builder(OptionType.ENUM_VALUE, "(foo)") + .setValue( + FieldType.MESSAGE, + FieldValue.of("bar", FieldType.STRING, "hello"), + FieldValue.of("baz", FieldType.BOOL, true)) + .build()); + } } -