diff --git a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/CppLiterals.java b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/CppLiterals.java index 15fcd7bd..eb2d192c 100644 --- a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/CppLiterals.java +++ b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/CppLiterals.java @@ -12,7 +12,17 @@ private CppLiterals() {} * as three-digit octal escapes (never ambiguous before a following digit, unlike {@code \x}). */ static String stringLiteral(String text) { - StringBuilder out = new StringBuilder("\""); + return "\"" + escapeStringBody(text) + "\""; + } + + /** + * Escapes arbitrary text for inclusion inside a double-quoted C++ string literal, + * without the surrounding quotes — for splicing model-controlled text into a larger literal. + * Non-ASCII and control bytes become three-digit octal escapes (never ambiguous before a + * following digit). + */ + static String escapeStringBody(String text) { + StringBuilder out = new StringBuilder(); for (byte raw : text.getBytes(StandardCharsets.UTF_8)) { int b = raw & 0xFF; switch (b) { @@ -30,7 +40,19 @@ static String stringLiteral(String text) { } } } - return out.append('"').toString(); + return out.toString(); + } + + /** + * A C++ literal for a 64-bit integer. {@code Long.MIN_VALUE} cannot be written as {@code + * -9223372036854775808} — C++ parses that as negation of a value one past {@code int64_t} max, + * which is ill-formed — so it is emitted with the header-free {@code INT64_MIN} idiom. + */ + static String int64Literal(long value) { + // Only the minimum needs special handling; every other int64 magnitude is a + // valid decimal literal that promotes to a wide enough type on assignment or + // comparison, so those are left byte-for-byte unchanged. + return value == Long.MIN_VALUE ? "(-9223372036854775807LL - 1)" : String.valueOf(value); } /** Text of a double literal; guarantees a decimal point or exponent so the type stays double. */ diff --git a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/MemberDefaults.java b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/MemberDefaults.java index 38750aec..533b09cf 100644 --- a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/MemberDefaults.java +++ b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/MemberDefaults.java @@ -85,8 +85,8 @@ static String literal(CppContext context, MemberShape member) { + CppLiterals.stringLiteral(value.expectStringNode().getValue()) + ")"; case BOOLEAN -> value.expectBooleanNode().getValue() ? "true" : "false"; - case BYTE, SHORT, INTEGER, LONG -> - String.valueOf(value.expectNumberNode().getValue().longValue()); + case BYTE, SHORT, INTEGER -> String.valueOf(value.expectNumberNode().getValue().longValue()); + case LONG -> CppLiterals.int64Literal(value.expectNumberNode().getValue().longValue()); case INT_ENUM -> "static_cast<" + type + ">(" + value.expectNumberNode().getValue().longValue() + ")"; case FLOAT -> "static_cast(" + value.expectNumberNode().getValue().doubleValue() + ")"; diff --git a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/TypeGenerators.java b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/TypeGenerators.java index 4ed84493..27bdecc4 100644 --- a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/TypeGenerators.java +++ b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/TypeGenerators.java @@ -47,6 +47,53 @@ static String enumConstant(String memberName) { return "k" + CaseUtils.toPascalCase(memberName.toLowerCase().replace('-', '_')); } + /** + * Fails generation when two members of the same shape fold to one C++ name, or a member collides + * with a reserved synthetic name. Enum-constant and union-factory naming lower-case, strip + * separators, or PascalCase the member name, so distinct Smithy members ({@code fooBar} and + * {@code foo_bar}, or a member literally named {@code unknown}) can produce a duplicate + * enumerator/method that no longer compiles. Catching it here names both members and the fix + * instead of surfacing a C++ redefinition error in the generated output. + */ + private static void requireDistinctNames( + String kind, + Object shapeId, + java.util.LinkedHashMap foldedByMember, + java.util.Set reserved) { + java.util.Map owner = new java.util.HashMap<>(); + for (var entry : foldedByMember.entrySet()) { + String member = entry.getKey(); + String folded = entry.getValue(); + if (reserved.contains(folded)) { + throw new software.amazon.smithy.codegen.core.CodegenException( + "cpp-codegen: " + + kind + + " " + + shapeId + + " member '" + + member + + "' maps to the reserved generated name '" + + folded + + "'; rename the member"); + } + String prior = owner.putIfAbsent(folded, member); + if (prior != null) { + throw new software.amazon.smithy.codegen.core.CodegenException( + "cpp-codegen: " + + kind + + " " + + shapeId + + " members '" + + prior + + "' and '" + + member + + "' both map to the generated name '" + + folded + + "'; rename one so their generated names differ"); + } + } + } + /** * Forward declarations for recursive member targets: on a cycle, the target's definition may come * later in types.h. Boxed members and std::vector elements only need the name declared; duplicate @@ -121,6 +168,11 @@ void generateStructure(StructureShape shape) { void generateEnum(EnumShape shape) { String name = typeName(shape); Map values = shape.getEnumValues(); + java.util.LinkedHashMap folded = new java.util.LinkedHashMap<>(); + for (String memberName : values.keySet()) { + folded.put(memberName, enumConstant(memberName)); + } + requireDistinctNames("enum", shape.getId(), folded, java.util.Set.of("kUnknown")); writer.addInclude("").addInclude(""); writeDocs(shape); @@ -179,6 +231,11 @@ void generateEnum(EnumShape shape) { void generateIntEnum(IntEnumShape shape) { String name = typeName(shape); + java.util.LinkedHashMap folded = new java.util.LinkedHashMap<>(); + for (String memberName : shape.getEnumValues().keySet()) { + folded.put(memberName, enumConstant(memberName)); + } + requireDistinctNames("intEnum", shape.getId(), folded, java.util.Set.of()); writer.addInclude(""); writeDocs(shape); writer.openBlock("enum class $L : std::int32_t {", name); @@ -192,6 +249,12 @@ void generateIntEnum(IntEnumShape shape) { void generateUnion(UnionShape shape) { String name = typeName(shape); List members = List.copyOf(shape.members()); + java.util.LinkedHashMap folded = new java.util.LinkedHashMap<>(); + for (MemberShape member : members) { + folded.put( + member.getMemberName(), "From" + CaseUtils.toPascalCase(symbols().toMemberName(member))); + } + requireDistinctNames("union", shape.getId(), folded, java.util.Set.of()); writer.addInclude("").addInclude(""); writeDocs(shape); diff --git a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/ValidationGenerator.java b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/ValidationGenerator.java index a9d75f47..53051b70 100644 --- a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/ValidationGenerator.java +++ b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/ValidationGenerator.java @@ -371,6 +371,18 @@ private static String plainNumber(BigDecimal value) { return value.stripTrailingZeros().toPlainString(); } + /** + * The C++ literal for a @range bound used in the *comparison* (the failure message keeps the + * plain decimal via {@link #plainNumber}). Identical to plainNumber except int64 min, which is + * not a writable decimal literal in C++. + */ + private static String rangeBoundExpr(BigDecimal value) { + if (value.scale() <= 0 && value.compareTo(BigDecimal.valueOf(Long.MIN_VALUE)) == 0) { + return CppLiterals.int64Literal(Long.MIN_VALUE); + } + return plainNumber(value); + } + private void writeLengthCheck( CppWriter w, Shape target, @@ -429,11 +441,11 @@ private void writeRangeCheck(CppWriter w, RangeTrait range, String valueExpr, St condition = valueExpr + " < " - + plainNumber(min.get()) + + rangeBoundExpr(min.get()) + " || " + valueExpr + " > " - + plainNumber(max.get()); + + rangeBoundExpr(max.get()); constraintText = "Member must be between " + plainNumber(min.get()) @@ -441,10 +453,10 @@ private void writeRangeCheck(CppWriter w, RangeTrait range, String valueExpr, St + plainNumber(max.get()) + ", inclusive"; } else if (min.isPresent()) { - condition = valueExpr + " < " + plainNumber(min.get()); + condition = valueExpr + " < " + rangeBoundExpr(min.get()); constraintText = "Member must be greater than or equal to " + plainNumber(min.get()); } else { - condition = valueExpr + " > " + plainNumber(max.get()); + condition = valueExpr + " > " + rangeBoundExpr(max.get()); constraintText = "Member must be less than or equal to " + plainNumber(max.get()); } w.openBlock("if ($L) {", condition); @@ -493,6 +505,18 @@ private void writePatternCheck( */ private static void rejectUnsupportedPattern(PatternTrait pattern) { String value = pattern.getValue(); + // The pattern is emitted verbatim inside R"__smithy(...)__smithy". A value + // containing the closing sequence would terminate the raw literal early and + // produce uncompilable code; reject it rather than emit broken output. + if (value.contains(")__smithy\"")) { + throw new software.amazon.smithy.codegen.core.CodegenException( + "@pattern " + + value + + " (at " + + pattern.getSourceLocation() + + ") contains the raw-string delimiter sequence )__smithy\", which cannot be emitted" + + " safely; rewrite the pattern to avoid that literal sequence"); + } boolean inClass = false; for (int i = 0; i < value.length(); i++) { char c = value.charAt(i); @@ -570,7 +594,13 @@ private void writeEnumCheck(CppWriter w, Shape target, String valueExpr, String .expectTrait(software.amazon.smithy.model.traits.EnumValueTrait.class) .expectStringValue()) .toList(); - String set = String.join(", ", values); + // The wire values are model-controlled, so escape them for the literal — a + // value containing " or \ would otherwise break the emitted string. Escaping + // in place keeps the message byte-identical for the common (safe) case. + String set = + values.stream() + .map(CppLiterals::escapeStringBody) + .collect(java.util.stream.Collectors.joining(", ")); String type = context.cppSymbols().toSymbol(target).getName(); w.openBlock("if ($L.value() == $L::Value::kUnknown) {", valueExpr, type); w.write( diff --git a/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/CppCodegenPluginTest.java b/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/CppCodegenPluginTest.java index 9116e2e6..372a3584 100644 --- a/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/CppCodegenPluginTest.java +++ b/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/CppCodegenPluginTest.java @@ -1,6 +1,7 @@ package io.smithycpp.codegen; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -256,4 +257,176 @@ void rejectsRecursionThroughUnionMembers() { assertTrue(error.getMessage().contains("union member")); assertTrue(error.getMessage().contains("TreeValue")); } + + // A jsonRpc2 server generated from an inline model; returns the manifest so a + // test can assert on the emitted C++. + private static MockManifest generateJsonRpc2(String filename, String modelText, String service) { + Model model = + Model.assembler() + .discoverModels(CppCodegenPluginTest.class.getClassLoader()) + .addUnparsedModel(filename, modelText) + .assemble() + .unwrap(); + MockManifest manifest = new MockManifest(); + new CppCodegenPlugin() + .execute( + PluginContext.builder() + .fileManifest(manifest) + .model(model) + .settings( + Node.objectNodeBuilder() + .withMember("service", service) + .withMember("namespace", "test::gen") + .build()) + .build()); + return manifest; + } + + private static CodegenException assertJsonRpc2Rejected( + String filename, String modelText, String service) { + Model model = + Model.assembler() + .discoverModels(CppCodegenPluginTest.class.getClassLoader()) + .addUnparsedModel(filename, modelText) + .assemble() + .unwrap(); + PluginContext context = + PluginContext.builder() + .fileManifest(new MockManifest()) + .model(model) + .settings( + Node.objectNodeBuilder() + .withMember("service", service) + .withMember("namespace", "test::gen") + .build()) + .build(); + return assertThrows(CodegenException.class, () -> new CppCodegenPlugin().execute(context)); + } + + @Test + void escapesEnumValueSetInTheValidationMessage() { + // An enum wire value containing a quote/backslash must be escaped in the + // generated value-set message, not emitted raw (which would not compile). + MockManifest manifest = + generateJsonRpc2( + "enum-escape.smithy", + """ + $version: "2.0" + namespace test.gen + use smithy.cpp.protocols#jsonRpc2 + + @jsonRpc2 + service Svc { version: "1", operations: [Op] } + operation Op { input := { grade: Grade } } + enum Grade { + TRICKY = "a\\"b\\\\c" + PLAIN = "plain" + } + """, + "test.gen#Svc"); + String server = manifest.expectFileString("/src/server.cc"); + assertTrue(server.contains("enum value set:")); + // The quote and backslash are escaped; the raw form never appears. + assertTrue(server.contains("a\\\"b\\\\c")); + assertFalse(server.contains("[a\"b")); + } + + @Test + void rejectsPatternContainingTheRawStringDelimiter() { + // A valid regex (balanced group) that nonetheless contains the raw-string + // closing sequence )__smithy" — emitting it verbatim would break the literal. + CodegenException error = + assertJsonRpc2Rejected( + "delim.smithy", + """ + $version: "2.0" + namespace test.gen + use smithy.cpp.protocols#jsonRpc2 + + @jsonRpc2 + service Svc { version: "1", operations: [Op] } + operation Op { input := { @pattern("(a)__smithy\\".*") s: String } } + """, + "test.gen#Svc"); + assertTrue(error.getMessage().contains("raw-string delimiter")); + } + + @Test + void rejectsEnumMemberNameCollision() { + CodegenException error = + assertJsonRpc2Rejected( + "enum-collide.smithy", + """ + $version: "2.0" + namespace test.gen + use smithy.cpp.protocols#jsonRpc2 + + @jsonRpc2 + service Svc { version: "1", operations: [Op] } + operation Op { input := { e: E } } + enum E { + foo_bar = "1" + foo__bar = "2" + } + """, + "test.gen#Svc"); + assertTrue(error.getMessage().contains("generated name")); + assertTrue(error.getMessage().contains("foo_bar")); + assertTrue(error.getMessage().contains("foo__bar")); + } + + @Test + void rejectsEnumMemberCollidingWithTheUnknownSentinel() { + CodegenException error = + assertJsonRpc2Rejected( + "enum-unknown.smithy", + """ + $version: "2.0" + namespace test.gen + use smithy.cpp.protocols#jsonRpc2 + + @jsonRpc2 + service Svc { version: "1", operations: [Op] } + operation Op { input := { e: E } } + enum E { + unknown = "1" + known = "2" + } + """, + "test.gen#Svc"); + assertTrue(error.getMessage().contains("reserved generated name")); + assertTrue(error.getMessage().contains("kUnknown")); + } + + @Test + void emitsInt64MinRangeBoundAndDefaultWithoutOverflow() { + // -9223372036854775808 is not a writable C++ decimal literal; it must be + // emitted via the INT64_MIN idiom in both the @range comparison and the + // @default initializer. + MockManifest manifest = + generateJsonRpc2( + "int64min.smithy", + """ + $version: "2.0" + namespace test.gen + use smithy.cpp.protocols#jsonRpc2 + + @jsonRpc2 + service Svc { version: "1", operations: [Op] } + operation Op { + input := { + @range(min: -9223372036854775808) + bounded: Long + + @default(-9223372036854775808) + defaulted: Long + } + } + """, + "test.gen#Svc"); + String server = manifest.expectFileString("/src/server.cc"); + assertTrue(server.contains("(-9223372036854775807LL - 1)")); + // The comparison must not emit the bare, ill-formed decimal literal. + assertFalse(server.contains("< -9223372036854775808")); + } }