Skip to content

Commit 2873400

Browse files
committed
Fix trait-codegen failure when string literals contain dollar signs
Add defer() and writeLazy() to AbstractCodeWriter, enabling lazy resolution of format expressions at toString() time without re-parsing the entire output. Update TraitCodegenWriter to use defer() for type name placeholders instead of a second format() pass that would misinterpret literal $ characters in generated code.
1 parent a876699 commit 2873400

6 files changed

Lines changed: 150 additions & 5 deletions

File tree

smithy-trait-codegen/src/it/java/software/amazon/smithy/traitcodegen/test/LoadsFromModelTest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,9 @@ static Stream<Arguments> loadsModelTests() {
484484
Arguments.of("defaults/defaults.smithy",
485485
StructDefaultsTrait.class,
486486
MapUtils.of("getDefaultString", "default")),
487+
Arguments.of("defaults/defaults.smithy",
488+
StructDefaultsTrait.class,
489+
MapUtils.of("getDefaultDollarString", "def$ault")),
487490
Arguments.of("defaults/defaults.smithy",
488491
StructDefaultsTrait.class,
489492
MapUtils.of("getDefaultByte", (byte) 1)),

smithy-trait-codegen/src/main/java/software/amazon/smithy/traitcodegen/integrations/javadoc/DocumentationTraitInterceptor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ final class DocumentationTraitInterceptor implements CodeInterceptor<JavaDocSect
1919
@Override
2020
public void write(TraitCodegenWriter writer, String previousText, JavaDocSection section) {
2121
String docs = section.shape().expectTrait(DocumentationTrait.class).getValue();
22-
writer.write(escapeAt(docs));
22+
writer.writeWithNoFormatting(escapeAt(docs));
2323

2424
if (!previousText.isEmpty()) {
2525
// Add spacing if tags have been added to the javadoc

smithy-trait-codegen/src/main/java/software/amazon/smithy/traitcodegen/writer/TraitCodegenWriter.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,9 @@ public String toString() {
135135
builder.append(getPackageHeader()).append(getNewline());
136136
builder.append(getImportContainer().toString()).append(getNewline());
137137

138-
// Handle duplicates that may need to use full name
138+
// Resolve type name conflicts before toString() resolves deferred values
139139
resolveNameContext();
140-
builder.append(format(super.toString()));
140+
builder.append(super.toString());
141141

142142
return builder.toString();
143143
}
@@ -242,8 +242,9 @@ private String getPlaceholder(Symbol symbol) {
242242
Set<Symbol> nameSet = symbolNames.computeIfAbsent(normalizedSymbol.getName(), n -> new HashSet<>());
243243
nameSet.add(normalizedSymbol);
244244

245-
// Return a placeholder value that will be filled when toString is called
246-
return format("$${$L:L}", normalizedSymbol.getFullName());
245+
// Return a deferred value that resolves the type name at toString() time
246+
String fullName = normalizedSymbol.getFullName();
247+
return defer(() -> format("${" + fullName + ":L}"));
247248
}
248249

249250
// Recursively process the symbols using Java Type.

smithy-trait-codegen/src/test/resources/META-INF/smithy/defaults/defaults.smithy

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ structure StructDefaults {
1616
@default("default")
1717
defaultString: String
1818

19+
/// Doc comment that states that this member has a default value of `def$ault`.
20+
@default("def$ault")
21+
defaultDollarString: String
22+
1923
@default(1)
2024
defaultByte: Byte
2125

smithy-utils/src/main/java/software/amazon/smithy/utils/AbstractCodeWriter.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.function.BiFunction;
2020
import java.util.function.Consumer;
2121
import java.util.function.Function;
22+
import java.util.function.Supplier;
2223
import java.util.regex.Pattern;
2324

2425
/**
@@ -620,6 +621,8 @@ public abstract class AbstractCodeWriter<T extends AbstractCodeWriter<T>> {
620621
private boolean trailingNewline = true;
621622
private int trimBlankLines = -1;
622623
private boolean enableStackTraceComments;
624+
private final Map<String, Supplier<String>> deferredValues = new HashMap<>();
625+
private int deferredCounter = 0;
623626

624627
/**
625628
* Creates a new SimpleCodeWriter that uses "\n" for a newline, four spaces
@@ -784,6 +787,13 @@ public char getExpressionStart() {
784787
public String toString() {
785788
String result = currentState.toString();
786789

790+
// Resolve any deferred values.
791+
if (!deferredValues.isEmpty()) {
792+
for (Map.Entry<String, Supplier<String>> entry : deferredValues.entrySet()) {
793+
result = result.replace(entry.getKey(), entry.getValue().get());
794+
}
795+
}
796+
787797
// Trim excessive blank lines.
788798
if (trimBlankLines > -1) {
789799
StringBuilder builder = new StringBuilder(result.length());
@@ -1756,6 +1766,45 @@ public final String format(Object content, Object... args) {
17561766
return result.toString();
17571767
}
17581768

1769+
/**
1770+
* Registers a deferred value that will be resolved when {@link #toString()} is called.
1771+
*
1772+
* <p>This method returns a sentinel string that can be embedded in the writer's output
1773+
* (for example, as the return value of a custom formatter). When the writer's content is
1774+
* materialized via {@code toString()}, all sentinels are replaced with the result of
1775+
* invoking their corresponding supplier.
1776+
*
1777+
* <p>This mechanism allows code generators to defer decisions (such as whether to use
1778+
* a short or fully-qualified type name) until all content has been written, without
1779+
* requiring a second formatting pass that would re-interpret expression characters
1780+
* in the output.
1781+
*
1782+
* @param supplier A supplier that produces the final string value at resolution time.
1783+
* @return A sentinel string to embed in the output.
1784+
*/
1785+
protected final String defer(Supplier<String> supplier) {
1786+
String id = "\u0000\u0000" + (deferredCounter++) + "\u0000\u0000";
1787+
deferredValues.put(id, supplier);
1788+
return id;
1789+
}
1790+
1791+
/**
1792+
* Writes a lazily-evaluated value to the writer.
1793+
*
1794+
* <p>The supplier is not invoked immediately. Instead, a sentinel is written to the
1795+
* output, and the supplier is invoked when {@link #toString()} is called. This is
1796+
* useful for writing content that depends on information not yet available at write
1797+
* time.
1798+
*
1799+
* @param supplier A supplier that produces the string value at resolution time.
1800+
* @return Returns self.
1801+
*/
1802+
@SuppressWarnings("unchecked")
1803+
public T writeLazy(Supplier<String> supplier) {
1804+
writeInlineWithNoFormatting(defer(supplier));
1805+
return (T) this;
1806+
}
1807+
17591808
/**
17601809
* A simple helper method that makes it easier to invoke the built-in {@code C}
17611810
* (call) formatter using a {@link Consumer} where {@code T} is the specific type

smithy-utils/src/test/java/software/amazon/smithy/utils/SimpleCodeWriterTest.java

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,4 +1204,92 @@ public void ensuresNewlineIsPresent() {
12041204

12051205
assertThat(writer.toString(), equalTo("Foo\nBar\nBaz\nBam\n"));
12061206
}
1207+
1208+
@Test
1209+
public void writeLazyResolvesAtToStringTime() {
1210+
SimpleCodeWriter writer = new SimpleCodeWriter();
1211+
String[] value = {"initial"};
1212+
writer.writeInline("Hello ");
1213+
writer.writeLazy(() -> value[0]);
1214+
value[0] = "world";
1215+
1216+
assertThat(writer.toString(), equalTo("Hello world\n"));
1217+
}
1218+
1219+
@Test
1220+
public void writeLazyWorksWithSurroundingContent() {
1221+
SimpleCodeWriter writer = new SimpleCodeWriter();
1222+
writer.write("before");
1223+
writer.writeLazy(() -> "lazy");
1224+
writer.write("after");
1225+
1226+
assertThat(writer.toString(), equalTo("before\nlazyafter\n"));
1227+
}
1228+
1229+
@Test
1230+
public void writeLazyDoesNotReinterpretDollarSigns() {
1231+
SimpleCodeWriter writer = new SimpleCodeWriter();
1232+
writer.writeWithNoFormatting("String s = \"$100\";");
1233+
writer.writeLazy(() -> "resolved");
1234+
1235+
assertThat(writer.toString(), equalTo("String s = \"$100\";\nresolved\n"));
1236+
}
1237+
1238+
@Test
1239+
public void deferCanBeUsedInCustomFormatter() {
1240+
MyWriter writer = new MyWriter();
1241+
writer.putFormatter('T', (value, indent) -> writer.defer(() -> "Resolved<" + value + ">"));
1242+
writer.write("Type: $T", "Foo");
1243+
1244+
assertThat(writer.toString(), equalTo("Type: Resolved<Foo>\n"));
1245+
}
1246+
1247+
@Test
1248+
public void deferredValuesResolveBeforeBlankLineTrimming() {
1249+
SimpleCodeWriter writer = new SimpleCodeWriter();
1250+
writer.trimBlankLines();
1251+
writer.write("before");
1252+
writer.writeLazy(() -> "lazy");
1253+
writer.write("after");
1254+
1255+
assertThat(writer.toString(), equalTo("before\nlazyafter\n"));
1256+
}
1257+
1258+
@Test
1259+
public void multipleDeferredValuesResolveIndependently() {
1260+
SimpleCodeWriter writer = new SimpleCodeWriter();
1261+
writer.writeInline("A=");
1262+
writer.writeLazy(() -> "1");
1263+
writer.writeInline(" B=");
1264+
writer.writeLazy(() -> "2");
1265+
1266+
assertThat(writer.toString(), equalTo("A=1 B=2\n"));
1267+
}
1268+
1269+
@Test
1270+
public void deferredFormatDoesNotReinterpretDollarSignsInOutput() {
1271+
// This test reproduces the TraitCodegenWriter problem: a custom $T formatter
1272+
// uses defer() to lazily resolve type names via context variables. The output
1273+
// also contains literal $ characters (e.g., from string constants in generated code).
1274+
// Without defer(), the old approach would re-format the entire output, causing
1275+
// the literal $ to be misinterpreted as an expression start.
1276+
MyWriter writer = new MyWriter();
1277+
1278+
// Register a formatter that defers type name resolution (like TraitCodegenWriter's $T)
1279+
writer.putFormatter('T', (value, indent) -> {
1280+
String fullName = value.toString();
1281+
return writer.defer(() -> writer.format("${" + fullName + ":L}"));
1282+
});
1283+
1284+
// Set up context that maps full name to short name (like resolveNameContext does)
1285+
writer.putContext("com.example.MyType", "MyType");
1286+
1287+
// Write code that includes both a $T reference AND a literal dollar sign
1288+
writer.write("public $T getValue() {", "com.example.MyType");
1289+
writer.writeWithNoFormatting(" return \"costs $100\";");
1290+
writer.write("}");
1291+
1292+
String result = writer.toString();
1293+
assertThat(result, equalTo("public MyType getValue() {\n return \"costs $100\";\n}\n"));
1294+
}
12071295
}

0 commit comments

Comments
 (0)