Add error checking for translatable strings
Translatable strings that have multiple substitutions should use
positional String.format() substitutions. This change makes it an error
not to use that format on translatable strings that have more than one
substitution in its text.
Change-Id: I3a19707f3804aa24e8568dc1653a11576cac5916
diff --git a/tools/aapt/ResourceTable.cpp b/tools/aapt/ResourceTable.cpp
index a2f085a..755b93b 100644
--- a/tools/aapt/ResourceTable.cpp
+++ b/tools/aapt/ResourceTable.cpp
@@ -573,6 +573,7 @@
const String16& parentIdent,
const String16& itemIdent,
int32_t curFormat,
+ bool isFormatted,
bool pseudolocalize,
const bool overwrite,
ResourceTable* outTable)
@@ -583,7 +584,7 @@
String16 str;
Vector<StringPool::entry_style_span> spans;
err = parseStyledString(bundle, in->getPrintableSource().string(),
- block, item16, &str, &spans,
+ block, item16, &str, &spans, isFormatted,
pseudolocalize);
if (err != NO_ERROR) {
return err;
@@ -616,6 +617,7 @@
const String16& curTag,
bool curIsStyled,
int32_t curFormat,
+ bool isFormatted,
bool pseudolocalize,
const bool overwrite,
ResourceTable* outTable)
@@ -626,7 +628,7 @@
Vector<StringPool::entry_style_span> spans;
err = parseStyledString(bundle, in->getPrintableSource().string(), block,
curTag, &str, curIsStyled ? &spans : NULL,
- pseudolocalize);
+ isFormatted, pseudolocalize);
if (err < NO_ERROR) {
return err;
@@ -709,12 +711,18 @@
// useful attribute names and special values
const String16 name16("name");
const String16 translatable16("translatable");
+ const String16 formatted16("formatted");
const String16 false16("false");
const String16 myPackage(assets->getPackage());
bool hasErrors = false;
-
+
+ bool fileIsTranslatable = true;
+ if (strstr(in->getPrintableSource().string(), "donottranslate") != NULL) {
+ fileIsTranslatable = false;
+ }
+
DefaultKeyedVector<String16, uint32_t> nextPublicId(0);
ResXMLTree::event_code_t code;
@@ -751,6 +759,7 @@
bool curIsBagReplaceOnOverwrite = false;
bool curIsStyled = false;
bool curIsPseudolocalizable = false;
+ bool curIsFormatted = fileIsTranslatable;
bool localHasErrors = false;
if (strcmp16(block.getElementName(&len), skip16.string()) == 0) {
@@ -1136,6 +1145,7 @@
String8 locale(rawLocale);
String16 name;
String16 translatable;
+ String16 formatted;
size_t n = block.getAttributeCount();
for (size_t i = 0; i < n; i++) {
@@ -1145,11 +1155,14 @@
name.setTo(block.getAttributeStringValue(i, &length));
} else if (strcmp16(attr, translatable16.string()) == 0) {
translatable.setTo(block.getAttributeStringValue(i, &length));
+ } else if (strcmp16(attr, formatted16.string()) == 0) {
+ formatted.setTo(block.getAttributeStringValue(i, &length));
}
}
if (name.size() > 0) {
if (translatable == false16) {
+ curIsFormatted = false;
// Untranslatable strings must only exist in the default [empty] locale
if (locale.size() > 0) {
fprintf(stderr, "aapt: warning: string '%s' in %s marked untranslatable but exists"
@@ -1167,6 +1180,10 @@
} else {
outTable->addLocalization(name, locale);
}
+
+ if (formatted == false16) {
+ curIsFormatted = false;
+ }
}
curTag = &string16;
@@ -1356,7 +1373,7 @@
block.getPosition(&parserPosition);
err = parseAndAddBag(bundle, in, &block, curParams, myPackage, curType,
- ident, parentIdent, itemIdent, curFormat,
+ ident, parentIdent, itemIdent, curFormat, curIsFormatted,
false, overwrite, outTable);
if (err == NO_ERROR) {
if (curIsPseudolocalizable && localeIsDefined(curParams)
@@ -1365,8 +1382,8 @@
#if 1
block.setPosition(parserPosition);
err = parseAndAddBag(bundle, in, &block, pseudoParams, myPackage,
- curType, ident, parentIdent, itemIdent, curFormat, true,
- overwrite, outTable);
+ curType, ident, parentIdent, itemIdent, curFormat,
+ curIsFormatted, true, overwrite, outTable);
#endif
}
}
@@ -1389,7 +1406,8 @@
block.getPosition(&parserPosition);
err = parseAndAddEntry(bundle, in, &block, curParams, myPackage, curType, ident,
- *curTag, curIsStyled, curFormat, false, overwrite, outTable);
+ *curTag, curIsStyled, curFormat, curIsFormatted,
+ false, overwrite, outTable);
if (err < NO_ERROR) { // Why err < NO_ERROR instead of err != NO_ERROR?
hasErrors = localHasErrors = true;
@@ -1400,7 +1418,8 @@
// pseudolocalize here
block.setPosition(parserPosition);
err = parseAndAddEntry(bundle, in, &block, pseudoParams, myPackage, curType,
- ident, *curTag, curIsStyled, curFormat, true, overwrite, outTable);
+ ident, *curTag, curIsStyled, curFormat,
+ curIsFormatted, true, overwrite, outTable);
if (err != NO_ERROR) {
hasErrors = localHasErrors = true;
}
diff --git a/tools/aapt/XMLNode.cpp b/tools/aapt/XMLNode.cpp
index 4c59288..57ff47a 100644
--- a/tools/aapt/XMLNode.cpp
+++ b/tools/aapt/XMLNode.cpp
@@ -68,12 +68,118 @@
return String16(namespaceUri, namespaceUri.size()-prefixSize, prefixSize);
}
+status_t hasSubstitutionErrors(const char* fileName,
+ ResXMLTree* inXml,
+ String16 str16)
+{
+ const char16_t* str = str16.string();
+ const char16_t* p = str;
+ const char16_t* end = str + str16.size();
+
+ bool nonpositional = false;
+ int argCount = 0;
+
+ while (p < end) {
+ /*
+ * Look for the start of a Java-style substitution sequence.
+ */
+ if (*p == '%' && p + 1 < end) {
+ p++;
+
+ // A literal percent sign represented by %%
+ if (*p == '%') {
+ p++;
+ continue;
+ }
+
+ argCount++;
+
+ if (*p >= '0' && *p <= '9') {
+ do {
+ p++;
+ } while (*p >= '0' && *p <= '9');
+ if (*p != '$') {
+ // This must be a size specification instead of position.
+ nonpositional = true;
+ }
+ } else if (*p == '<') {
+ // Reusing last argument; bad idea since it can be re-arranged.
+ nonpositional = true;
+ p++;
+
+ // Optionally '$' can be specified at the end.
+ if (p < end && *p == '$') {
+ p++;
+ }
+ } else {
+ nonpositional = true;
+ }
+
+ // Ignore flags and widths
+ while (p < end && (*p == '-' ||
+ *p == '#' ||
+ *p == '+' ||
+ *p == ' ' ||
+ *p == ',' ||
+ *p == '(' ||
+ (*p >= '0' && *p <= '9'))) {
+ p++;
+ }
+
+ /*
+ * This is a shortcut to detect strings that are going to Time.format()
+ * instead of String.format()
+ *
+ * Comparison of String.format() and Time.format() args:
+ *
+ * String: ABC E GH ST X abcdefgh nost x
+ * Time: DEFGHKMS W Za d hkm s w yz
+ *
+ * Therefore we know it's definitely Time if we have:
+ * DFKMWZkmwyz
+ */
+ if (p < end) {
+ switch (*p) {
+ case 'D':
+ case 'F':
+ case 'K':
+ case 'M':
+ case 'W':
+ case 'Z':
+ case 'k':
+ case 'm':
+ case 'w':
+ case 'y':
+ case 'z':
+ return NO_ERROR;
+ }
+ }
+ }
+
+ p++;
+ }
+
+ /*
+ * If we have more than one substitution in this string and any of them
+ * are not in positional form, give the user an error.
+ */
+ if (argCount > 1 && nonpositional) {
+ SourcePos(String8(fileName), inXml->getLineNumber()).error(
+ "Multiple substitutions specified in non-positional format; "
+ "did you mean to add the formatted=\"true\" attribute?\n");
+ return NOT_ENOUGH_DATA;
+ }
+
+ return NO_ERROR;
+}
+
status_t parseStyledString(Bundle* bundle,
const char* fileName,
ResXMLTree* inXml,
const String16& endTag,
String16* outString,
Vector<StringPool::entry_style_span>* outSpans,
+ bool isFormatted,
bool pseudolocalize)
{
Vector<StringPool::entry_style_span> spanStack;
@@ -101,7 +207,11 @@
std::string pseudo = pseudolocalize_string(orig);
curString.append(String16(String8(pseudo.c_str())));
} else {
- curString.append(text);
+ if (isFormatted && hasSubstitutionErrors(fileName, inXml, text) != NO_ERROR) {
+ return UNKNOWN_ERROR;
+ } else {
+ curString.append(text);
+ }
}
} else if (code == ResXMLTree::START_TAG) {
const String16 element16(inXml->getElementName(&len));
diff --git a/tools/aapt/XMLNode.h b/tools/aapt/XMLNode.h
index e9a263b..05624b7 100644
--- a/tools/aapt/XMLNode.h
+++ b/tools/aapt/XMLNode.h
@@ -25,6 +25,7 @@
const String16& endTag,
String16* outString,
Vector<StringPool::entry_style_span>* outSpans,
+ bool isFormatted,
bool isPseudolocalizable);
void printXMLBlock(ResXMLTree* block);