Explorar o código

Fix false-positive conflicts with append/appendItems syntax

Should no longer result in false positives. True positives when entire
list is being replaced (1.6-style) or if same item is modified should
still be reported
Ivan Savenko hai 6 meses
pai
achega
b28fc7c096
Modificáronse 1 ficheiros con 45 adicións e 23 borrados
  1. 45 23
      lib/json/JsonUtils.cpp

+ 45 - 23
lib/json/JsonUtils.cpp

@@ -19,6 +19,26 @@ VCMI_LIB_USING_NAMESPACE
 
 static const JsonNode nullNode;
 
+static std::optional<int> getIndexSafe(const JsonNode & node, const std::string & keyName)
+{
+	try {
+		int index = std::stoi(keyName);
+		if (index <= 0 || index > node.Vector().size())
+			throw std::out_of_range("dummy");
+		return index - 1; // 1-based index -> 0-based index
+	}
+	catch(const std::invalid_argument &)
+	{
+		logMod->warn("Failed to interpret key '%s' when replacing individual items in array. Expected 'appendItem', 'appendItems', 'modify@NUM' or 'insert@NUM", keyName);
+		return std::nullopt;
+	}
+	catch(const std::out_of_range & )
+	{
+		logMod->warn("Failed to replace index when replacing individual items in array. Value '%s' does not exists in targeted array of %d items", keyName, node.Vector().size());
+		return std::nullopt;
+	}
+};
+
 static JsonNode getDefaultValue(const JsonNode & schema, std::string fieldName)
 {
 	const JsonNode & fieldProps = schema["properties"][fieldName];
@@ -218,26 +238,6 @@ void JsonUtils::merge(JsonNode & dest, JsonNode & source, bool ignoreOverride, b
 				}
 				if (dest.isVector())
 				{
-					auto getIndexSafe = [&dest](const std::string & keyName) -> std::optional<int>
-					{
-						try {
-							int index = std::stoi(keyName);
-							if (index <= 0 || index > dest.Vector().size())
-								throw std::out_of_range("dummy");
-							return index - 1; // 1-based index -> 0-based index
-						}
-						catch(const std::invalid_argument &)
-						{
-							logMod->warn("Failed to interpret key '%s' when replacing individual items in array. Expected 'appendItem', 'appendItems', 'modify@NUM' or 'insert@NUM", keyName);
-							return std::nullopt;
-						}
-						catch(const std::out_of_range & )
-						{
-							logMod->warn("Failed to replace index when replacing individual items in array. Value '%s' does not exists in targeted array of %d items", keyName, dest.Vector().size());
-							return std::nullopt;
-						}
-					};
-
 					for(auto & node : source.Struct())
 					{
 						if (node.first == "append")
@@ -253,14 +253,14 @@ void JsonUtils::merge(JsonNode & dest, JsonNode & source, bool ignoreOverride, b
 						else if (boost::algorithm::starts_with(node.first, "insert@"))
 						{
 							constexpr int numberPosition = std::char_traits<char>::length("insert@");
-							auto index = getIndexSafe(node.first.substr(numberPosition));
+							auto index = getIndexSafe(node.second, node.first.substr(numberPosition));
 							if (index)
 								dest.Vector().insert(dest.Vector().begin() + index.value(), std::move(node.second));
 						}
 						else if (boost::algorithm::starts_with(node.first, "modify@"))
 						{
 							constexpr int numberPosition = std::char_traits<char>::length("modify@");
-							auto index = getIndexSafe(node.first.substr(numberPosition));
+							auto index = getIndexSafe(node.second,	node.first.substr(numberPosition));
 							if (index)
 								merge(dest.Vector().at(index.value()), node.second, ignoreOverride);
 						}
@@ -363,12 +363,34 @@ void JsonUtils::detectConflicts(JsonNode & result, const JsonNode & left, const
 		case JsonNode::JsonType::DATA_FLOAT:
 		case JsonNode::JsonType::DATA_INTEGER:
 		case JsonNode::JsonType::DATA_STRING:
-		case JsonNode::JsonType::DATA_VECTOR: // NOTE: comparing vectors as whole - since merge will overwrite it in its entirety
 		{
 			result[keyName][left.getModScope()] = left;
 			result[keyName][right.getModScope()] = right;
 			return;
 		}
+		case JsonNode::JsonType::DATA_VECTOR:
+		{
+			if (right.isStruct())
+			{
+				for(const auto & node : right.Struct())
+				{
+					if (boost::algorithm::starts_with(node.first, "modify@"))
+					{
+						constexpr int numberPosition = std::char_traits<char>::length("modify@");
+						auto index = getIndexSafe(node.second, node.first.substr(numberPosition));
+						if (index)
+							detectConflicts(result, left[*index], node.second, keyName + "/" + node.first.substr(numberPosition));
+					}
+				}
+			}
+			else
+			{
+				// NOTE: comparing vectors as whole - since merge will overwrite it in its entirety
+				result[keyName][left.getModScope()] = left;
+				result[keyName][right.getModScope()] = right;
+			}
+			return;
+		}
 		case JsonNode::JsonType::DATA_STRUCT:
 		{
 			for(const auto & node : left.Struct())