Skip to content

Commit 8e18623

Browse files
brad4dcopybara-github
authored andcommitted
Report declareIcuTemplate() placeholders in non-canonical format
Placeholders are always formated in UPPER_SNAKE_CASE in XMB / XTB files and in ICU template messages. Explicitly report an error if a `declareIcuTemplate()` attempts to use a placeholder name in some other format for its `example` or `original_code` maps. NOTE: This change does not report errors for code that was accepted before. It was already the case that you would get an error message if you tried to refer to `lowerCamelCasePlaceholderName` in the `example` or `original_code` part of a `declareIcuTemplate()` message, but the error was confusing. It would say that the placeholder didn't exist, even if you actually wrote "{lowerCamelCasePlaceholderName}" in the template string. This change also adds a `JsMessage` method for getting a list of the placeholder names in canonical format. PiperOrigin-RevId: 700030842
1 parent 7513c0d commit 8e18623

File tree

3 files changed

+89
-3
lines changed

3 files changed

+89
-3
lines changed

src/com/google/javascript/jscomp/JsMessage.java

+21-3
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,21 @@ public PlaceholderFormatException(String msg) {
105105

106106
public abstract ImmutableMap<String, String> getPlaceholderNameToOriginalCodeMap();
107107

108-
/** Gets a set of the registered placeholders in this message. */
108+
/**
109+
* Gets a set of the registered placeholders in this message in lowerCamelCase format.
110+
*
111+
* <p>This is the format used in `goog.getMsg()` declarations.
112+
*/
109113
public abstract ImmutableSet<String> jsPlaceholderNames();
110114

115+
/**
116+
* Gets a set of the registered placeholders in this message in UPPER_SNAKE_CASE format.
117+
*
118+
* <p>This is the format stored into XMB / XTB files and used in `declareIcuTemplate()
119+
* declarations.
120+
*/
121+
public abstract ImmutableSet<String> canonicalPlaceholderNames();
122+
111123
public String getPlaceholderOriginalCode(PlaceholderReference placeholderReference) {
112124
return getPlaceholderNameToOriginalCodeMap()
113125
.getOrDefault(placeholderReference.getStoredPlaceholderName(), "-");
@@ -372,8 +384,12 @@ public static final class Builder {
372384
private @Nullable String alternateId;
373385

374386
private final List<Part> parts = new ArrayList<>();
375-
// Placeholder names in JS code format
387+
// Placeholder names in JS code format (lowerCamelCase),
388+
// which is used for `goog.getMsg()` messages
376389
private final Set<String> jsPlaceholderNames = new LinkedHashSet<>();
390+
// Placeholder names in canonical format (UPPER_SNAKE_CASE)
391+
// which is used in XMB / XTB files and `declareIcuTemplate()` messages.
392+
private final Set<String> canonicalPlaceholderNames = new LinkedHashSet<>();
377393
private ImmutableMap<String, String> placeholderNameToExampleMap = ImmutableMap.of();
378394
private ImmutableMap<String, String> placeholderNameToOriginalCodeMap = ImmutableMap.of();
379395

@@ -409,6 +425,7 @@ public Builder appendPart(Part part) {
409425
parts.add(part);
410426
if (part.isPlaceholder()) {
411427
jsPlaceholderNames.add(part.getJsPlaceholderName());
428+
canonicalPlaceholderNames.add(part.getCanonicalPlaceholderName());
412429
}
413430
return this;
414431
}
@@ -542,7 +559,8 @@ public JsMessage build() {
542559
meaning,
543560
placeholderNameToExampleMap,
544561
placeholderNameToOriginalCodeMap,
545-
ImmutableSet.copyOf(jsPlaceholderNames));
562+
ImmutableSet.copyOf(jsPlaceholderNames),
563+
ImmutableSet.copyOf(canonicalPlaceholderNames));
546564
}
547565
}
548566

src/com/google/javascript/jscomp/JsMessageVisitor.java

+9
Original file line numberDiff line numberDiff line change
@@ -1051,6 +1051,7 @@ private IcuMessageTemplateString extractIcuMessageTemplateString(Node stringExpr
10511051
private interface JsMessageOptions {
10521052
// Replace `'<'` with `'&lt;'` in the message.
10531053
boolean isEscapeLessThan();
1054+
10541055
// Replace these escaped entities with their literal characters in the message
10551056
// (Overrides escapeLessThan)
10561057
// '&lt;' -> '<'
@@ -1216,6 +1217,14 @@ private IcuTemplateOptions extractIcuTemplateOptions(Node optionsBag) throws Mal
12161217
.addAll(placeholderOriginalCodeMap.keySet())
12171218
.build();
12181219

1220+
for (String placeholderName : placeholderNames) {
1221+
if (!JsMessage.isCanonicalPlaceholderNameFormat(placeholderName)) {
1222+
throw new MalformedException(
1223+
SimpleFormat.format("Placeholder not in UPPER_SNAKE_CASE: %s", placeholderName),
1224+
optionsBag);
1225+
}
1226+
}
1227+
12191228
// NOTE: The getX() methods below should all do little to no computation.
12201229
// In particular, all checking for MalformedExceptions must be done before creating this object.
12211230
return new IcuTemplateOptions() {

test/com/google/javascript/jscomp/JsMessageVisitorTest.java

+59
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,65 @@ public void testIcuTemplatePlaceholderTypo() {
212212
assertThat(messages).isEmpty();
213213
}
214214

215+
@Test
216+
public void testIcuTemplatePlaceholderUpperSnakeCase() {
217+
// For ICU templates you must always use UPPER_SNAKE_CASE for your placeholder names.
218+
// This matches the way the placeholder names are formatted in the XMB/XTB files.
219+
// Note that `goog.getMsg()` requires lowerCamelCase for placeholder names, but
220+
// actually converts them to UPPER_SNAKE_CASE when storing them in XMB and must convert
221+
// the UPPER_SNAKE_CASE to lowerCamelCase when reading from XTB files. We want to avoid
222+
// this needless complication for `declareIcuTemplate()` message declarations.
223+
extractMessages(
224+
lines(
225+
"const MSG_ICU_EXAMPLE = declareIcuTemplate(",
226+
" 'Email: {USER_EMAIL}',",
227+
" {",
228+
" description: 'Labeled email address',",
229+
" example: {",
230+
" 'USER_EMAIL': '[email protected]',",
231+
" }",
232+
" });",
233+
""));
234+
final ImmutableList<JSError> actualErrors = compiler.getErrors();
235+
assertThat(actualErrors).isEmpty();
236+
assertThat(compiler.getWarnings()).isEmpty();
237+
assertThat(messages).hasSize(1);
238+
JsMessage jsMessage = messages.get(0);
239+
assertThat(jsMessage.canonicalPlaceholderNames()).containsExactly("USER_EMAIL");
240+
}
241+
242+
@Test
243+
public void testIcuTemplatePlaceholderLowerCamelCase() {
244+
// For ICU templates you must always use UPPER_SNAKE_CASE for your placeholder names.
245+
// This matches the way the placeholder names are formatted in the XMB/XTB files.
246+
// Note that `goog.getMsg()` requires lowerCamelCase for placeholder names, but
247+
// actually converts them to UPPER_SNAKE_CASE when storing them in XMB and must convert
248+
// the UPPER_SNAKE_CASE to lowerCamelCase when reading from XTB files. We want to avoid
249+
// this needless complication for `declareIcuTemplate()` message declarations.
250+
extractMessages(
251+
lines(
252+
"const MSG_ICU_EXAMPLE = declareIcuTemplate(",
253+
" 'Email: {userEmail}',", // should be USER_EMAIL
254+
" {",
255+
" description: 'Labeled email address',",
256+
" example: {",
257+
" 'userEmail': '[email protected]',", // should be USER_EMAIL
258+
" }",
259+
" });",
260+
""));
261+
final ImmutableList<JSError> actualErrors = compiler.getErrors();
262+
assertThat(actualErrors)
263+
.comparingElementsUsing(DIAGNOSTIC_EQUALITY)
264+
.containsExactly(MESSAGE_TREE_MALFORMED);
265+
assertThat(actualErrors)
266+
.comparingElementsUsing(DESCRIPTION_EQUALITY)
267+
.containsExactly(
268+
"Message parse tree malformed. Placeholder not in UPPER_SNAKE_CASE: userEmail");
269+
assertThat(compiler.getWarnings()).isEmpty();
270+
// The malformed message is skipped.
271+
assertThat(messages).isEmpty();
272+
}
273+
215274
@Test
216275
public void testJsMessageOnVar() {
217276
extractMessagesSafely("/** @desc Hello */ var MSG_HELLO = goog.getMsg('a')");

0 commit comments

Comments
 (0)