Commit 56dea612 authored by Karandeep Bhatia's avatar Karandeep Bhatia Committed by Commit Bot

JsonSchemaCompiler: Ensure we never concatenate different errors.

After https://chromium-review.googlesource.com/c/chromium/src/+/2519173,
we raise a hard error on failing to parse optional properties. As a
result, we should never be concatenating unrelated errors. Modify the
code to reflect the same.

BUG=1113513

Change-Id: If7a4220435378207d84e32e9dd093d63059e7a29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2547422Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828993}
parent d18e5d25
...@@ -1274,9 +1274,8 @@ class _Generator(object): ...@@ -1274,9 +1274,8 @@ class _Generator(object):
c = Code() c = Code()
if not self._generate_error_messages: if not self._generate_error_messages:
return c return c
(c.Append('if (error->length())') c.Append('DCHECK(error->empty());')
.Append(' error->append(UTF8ToUTF16("; "));') c.Append('*error = %s;' % error16)
.Append('error->append(%s);' % error16))
return c return c
def _GenerateError(self, body): def _GenerateError(self, body):
......
...@@ -46,6 +46,7 @@ void PopulateInvalidEnumValueError( ...@@ -46,6 +46,7 @@ void PopulateInvalidEnumValueError(
base::string16* error, base::string16* error,
std::vector<base::StringPiece>* error_path_reversed) { std::vector<base::StringPiece>* error_path_reversed) {
DCHECK(error); DCHECK(error);
DCHECK(error->empty());
DCHECK(error_path_reversed); DCHECK(error_path_reversed);
DCHECK(error_path_reversed->empty()); DCHECK(error_path_reversed->empty());
...@@ -76,6 +77,7 @@ const base::Value* FindKeyOfType( ...@@ -76,6 +77,7 @@ const base::Value* FindKeyOfType(
base::string16* error, base::string16* error,
std::vector<base::StringPiece>* error_path_reversed) { std::vector<base::StringPiece>* error_path_reversed) {
DCHECK(error); DCHECK(error);
DCHECK(error->empty());
DCHECK(error_path_reversed); DCHECK(error_path_reversed);
DCHECK(error_path_reversed->empty()); DCHECK(error_path_reversed->empty());
......
...@@ -309,25 +309,6 @@ TEST(JsonSchemaCompilerErrorTest, OptionalUnableToPopulateArray) { ...@@ -309,25 +309,6 @@ TEST(JsonSchemaCompilerErrorTest, OptionalUnableToPopulateArray) {
} }
} }
TEST(JsonSchemaCompilerErrorTest, MultiplePopulationErrors) {
{
std::unique_ptr<base::DictionaryValue> value =
Dictionary("TheArray", std::make_unique<Value>(5));
errors::ArrayObject out;
base::string16 error;
EXPECT_FALSE(errors::ArrayObject::Populate(*value, &out, &error));
EXPECT_TRUE(EqualsUtf16("'TheArray': expected list, got integer",
error));
EXPECT_EQ(NULL, out.the_array.get());
EXPECT_FALSE(errors::ArrayObject::Populate(*value, &out, &error));
EXPECT_TRUE(EqualsUtf16("'TheArray': expected list, got integer; "
"'TheArray': expected list, got integer",
error));
EXPECT_EQ(NULL, out.the_array.get());
}
}
TEST(JsonSchemaCompilerErrorTest, TooManyKeys) { TEST(JsonSchemaCompilerErrorTest, TooManyKeys) {
{ {
std::unique_ptr<base::DictionaryValue> value = std::unique_ptr<base::DictionaryValue> value =
......
...@@ -16,11 +16,10 @@ namespace { ...@@ -16,11 +16,10 @@ namespace {
bool ReportError(const base::Value& from, bool ReportError(const base::Value& from,
base::Value::Type expected, base::Value::Type expected,
base::string16* error) { base::string16* error) {
if (!error->empty()) DCHECK(error->empty());
error->append(base::ASCIIToUTF16("; ")); *error = base::ASCIIToUTF16(base::StringPrintf(
error->append(base::ASCIIToUTF16(base::StringPrintf(
"expected %s, got %s", base::Value::GetTypeName(expected), "expected %s, got %s", base::Value::GetTypeName(expected),
base::Value::GetTypeName(from.type())))); base::Value::GetTypeName(from.type())));
return false; // Always false on purpose. return false; // Always false on purpose.
} }
......
...@@ -95,11 +95,10 @@ bool PopulateArrayFromList(const base::ListValue& list_value, ...@@ -95,11 +95,10 @@ bool PopulateArrayFromList(const base::ListValue& list_value,
const auto& list = list_value.GetList(); const auto& list = list_value.GetList();
for (size_t i = 0; i < list.size(); ++i) { for (size_t i = 0; i < list.size(); ++i) {
if (!PopulateItem(list[i], &item, &item_error)) { if (!PopulateItem(list[i], &item, &item_error)) {
if (!error->empty()) DCHECK(error->empty());
error->append(base::ASCIIToUTF16("; ")); *error = base::ASCIIToUTF16(
error->append(base::ASCIIToUTF16(
base::StringPrintf("Parsing array failed at index %" PRIuS ": %s", i, base::StringPrintf("Parsing array failed at index %" PRIuS ": %s", i,
base::UTF16ToASCII(item_error).c_str()))); base::UTF16ToASCII(item_error).c_str()));
return false; return false;
} }
out->push_back(std::move(item)); out->push_back(std::move(item));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment