Skip to content

Commit bb45e17

Browse files
vitautfacebook-github-bot
authored andcommitted
Improve string generation in Java
Summary: Remove all the code for dealing with escape sequences from Java codegens. It is no longer needed because escape sequences are now handled in the lexer/parser. Instead add previously missing escaping of control characters. In particular, this fixes handling of multiline strings. Before: ``` public static final String multi_line_string = "This is a multi line string. "; ``` After: ``` public static final String multi_line_string = "This\nis a\nmulti line string.\n"; ``` Reviewed By: bahadird Differential Revision: D44559991 fbshipit-source-id: c079aad9ba69d08fd0576bd14d73bd2ea93b5147
1 parent 17bd0f7 commit bb45e17

File tree

4 files changed

+30
-79
lines changed

4 files changed

+30
-79
lines changed

thrift/compiler/generate/t_java_deprecated_generator.cc

Lines changed: 13 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -422,62 +422,30 @@ string t_java_deprecated_generator::render_const_value(
422422
t_base_type::t_base tbase = ((t_base_type*)type)->get_base();
423423
switch (tbase) {
424424
case t_base_type::TYPE_STRING:
425-
case t_base_type::TYPE_BINARY: {
425+
case t_base_type::TYPE_BINARY:
426426
render << '"';
427-
auto& rawValue = value->get_string();
428-
for (std::string::size_type i = 0; i < rawValue.size();) {
429-
switch (rawValue[i]) {
430-
case '\\': {
431-
render << rawValue[i];
432-
++i;
433-
assert(i <= rawValue.size());
434-
if (i == rawValue.size()) {
435-
throw std::string(
436-
"compiler error: leading backslash missing "
437-
"escape sequence: ") +
438-
rawValue;
439-
}
440-
if (rawValue[i] == 'x') {
441-
auto end =
442-
rawValue.find_first_not_of("0123456789abcdefABCDEF", ++i);
443-
if (end == std::string::npos) {
444-
end = rawValue.size();
445-
}
446-
if (end == i) {
447-
throw std::string(
448-
"compiler error: missing hexadecimal "
449-
"character code in escape sequence: ") +
450-
rawValue;
451-
}
452-
assert(i < end);
453-
if (end > i + 2) {
454-
end = i + 2;
455-
}
456-
render << 'u';
457-
for (auto n = 4 - (end - i); n--;) {
458-
render << '0';
459-
}
460-
render.write(std::next(rawValue.data(), i), end - i);
461-
i = end;
462-
} else {
463-
render << rawValue[i++];
464-
}
427+
for (unsigned char c : value->get_string()) {
428+
switch (c) {
429+
case '\n':
430+
render << "\\n";
465431
break;
466-
}
467432
case '"':
468-
render << '\\';
469-
// intentional fallback
433+
render << "\\\"";
434+
break;
470435
default:
471-
render << rawValue[i];
472-
++i;
436+
if (c < 0x20) {
437+
render << fmt::format("\\x{:02x}", c);
438+
} else {
439+
render << c;
440+
}
441+
break;
473442
}
474443
}
475444
render << '"';
476445
if (tbase == t_base_type::TYPE_BINARY) {
477446
render << ".getBytes()";
478447
}
479448
break;
480-
}
481449
case t_base_type::TYPE_BOOL:
482450
render << ((value->get_integer() > 0) ? "true" : "false");
483451
break;

thrift/compiler/lib/java/util.cc

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -80,33 +80,22 @@ std::string mangle_java_constant_name(const std::string& ref) {
8080
std::string quote_java_string(const std::string& unescaped) {
8181
std::ostringstream quoted;
8282
quoted << '\"';
83-
for (std::string::size_type i = 0; i < unescaped.size();) {
84-
switch (unescaped[i]) {
85-
case '\\': {
86-
quoted << unescaped[i];
87-
++i;
88-
assert(i <= unescaped.size());
89-
if (i == unescaped.size()) {
90-
throw std::runtime_error(
91-
"compiler error: leading backslash missing escape sequence: " +
92-
unescaped);
93-
}
94-
assert(unescaped[i] != 'x');
95-
quoted << unescaped[i++];
83+
for (unsigned char c : unescaped) {
84+
switch (c) {
85+
case '\n':
86+
quoted << "\\n";
9687
break;
97-
}
9888
case '"':
99-
quoted << '\\' << unescaped[i];
100-
++i;
89+
quoted << "\\\"";
10190
break;
10291
default: {
103-
unsigned char c = unescaped[i];
104-
if (c >= 0xf8) {
92+
if (c < 0x20) {
93+
quoted << fmt::format("\\x{:02x}", c);
94+
} else if (c >= 0xf8) {
10595
quoted << fmt::format("\\u{:04x}", c);
10696
} else {
107-
quoted << unescaped[i];
97+
quoted << c;
10898
}
109-
++i;
11099
break;
111100
}
112101
}

thrift/compiler/test/fixtures/constants/gen-android/Constants.java

Lines changed: 4 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

thrift/compiler/test/fixtures/constants/gen-java/test/fixtures/constants/ModuleConstants.java

Lines changed: 4 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)