added return value in addFieldDescriptor related methods to indicate whether the order of fields had to be changed.

4 files changed, 86 insertions, 56 deletions

diff --git a/src/core/model/Domain.cpp b/src/core/model/Domain.cpp
index ac0699e..f6c3956 100644
--- a/src/core/model/Domain.cpp
+++ b/src/core/model/Domain.cpp
@@ -558,7 +558,7 @@ Rooted<FieldDescriptor> Descriptor::getFieldDescriptor(
 	}
 }
 
-void Descriptor::addAndSortFieldDescriptor(Handle<FieldDescriptor> fd,
+bool Descriptor::addAndSortFieldDescriptor(Handle<FieldDescriptor> fd,
                                            Logger &logger)
 {
 	// only add it if we need to.
@@ -571,37 +571,25 @@ void Descriptor::addAndSortFieldDescriptor(Handle<FieldDescriptor> fd,
 		    fd->getFieldType() != FieldDescriptor::FieldType::TREE) {
 			// if so we add the new field before the TREE field.
 			fieldDescriptors.insert(fieldDescriptors.end() - 1, fd);
-
-			// if the new field was from the same domain we warn the user
-			// because that is bad coding style.
-			if (fd->getParent() != nullptr &&
-			    fd->getParent().cast<Descriptor>()->getParent() ==
-			        getParent()) {
-				logger.warning(
-				    std::string("Field \"") + fd->getName() +
-				        "\" was declared after main field \"" +
-				        fds.back()->getName() +
-				        "\". The order of fields was changed to make the "
-				        "main field the last field.",
-				    *fd);
-			}
+			return true;
 		} else {
 			fieldDescriptors.push_back(fd);
 		}
 	}
+	return false;
 }
 
-void Descriptor::addFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger)
+bool Descriptor::addFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger)
 {
-	addAndSortFieldDescriptor(fd, logger);
 	if (fd->getParent() == nullptr) {
 		fd->setParent(this);
 	}
+	return addAndSortFieldDescriptor(fd, logger);
 }
 
-void Descriptor::moveFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger)
+bool Descriptor::moveFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger)
 {
-	addAndSortFieldDescriptor(fd, logger);
+	bool sorted = addAndSortFieldDescriptor(fd, logger);
 	Handle<Managed> par = fd->getParent();
 	if (par != this) {
 		if (par != nullptr) {
@@ -610,9 +598,10 @@ void Descriptor::moveFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger)
 		}
 		fd->setParent(this);
 	}
+	return sorted;
 }
 
-void Descriptor::copyFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger)
+bool Descriptor::copyFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger)
 {
 	Rooted<FieldDescriptor> copy;
 	if (fd->isPrimitive()) {
@@ -631,7 +620,7 @@ void Descriptor::copyFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger)
 			copy->addChild(c);
 		}
 	}
-	addFieldDescriptor(copy, logger);
+	return addFieldDescriptor(copy, logger);
 }
 
 bool Descriptor::removeFieldDescriptor(Handle<FieldDescriptor> fd)
@@ -646,25 +635,27 @@ bool Descriptor::removeFieldDescriptor(Handle<FieldDescriptor> fd)
 	return false;
 }
 
-Rooted<FieldDescriptor> Descriptor::createPrimitiveFieldDescriptor(
-    Handle<Type> primitiveType, Logger &logger,
-    FieldDescriptor::FieldType fieldType, std::string name, bool optional)
+std::pair<Rooted<FieldDescriptor>, bool>
+Descriptor::createPrimitiveFieldDescriptor(Handle<Type> primitiveType,
+                                           Logger &logger,
+                                           FieldDescriptor::FieldType fieldType,
+                                           std::string name, bool optional)
 {
 	Rooted<FieldDescriptor> fd{new FieldDescriptor(getManager(), primitiveType,
 	                                               this, fieldType,
 	                                               std::move(name), optional)};
-	addFieldDescriptor(fd, logger);
-	return fd;
+	bool sorted = addFieldDescriptor(fd, logger);
+	return std::make_pair(fd, sorted);
 }
 
-Rooted<FieldDescriptor> Descriptor::createFieldDescriptor(
+std::pair<Rooted<FieldDescriptor>, bool> Descriptor::createFieldDescriptor(
     Logger &logger, FieldDescriptor::FieldType fieldType, std::string name,
     bool optional)
 {
 	Rooted<FieldDescriptor> fd{new FieldDescriptor(
 	    getManager(), this, fieldType, std::move(name), optional)};
-	addFieldDescriptor(fd, logger);
-	return fd;
+	bool sorted = addFieldDescriptor(fd, logger);
+	return std::make_pair(fd, sorted);
 }
 
 /* Class StructuredClass */
diff --git a/src/core/model/Domain.hpp b/src/core/model/Domain.hpp
index 350c7ba..476a38c 100644
--- a/src/core/model/Domain.hpp
+++ b/src/core/model/Domain.hpp
@@ -469,7 +469,7 @@ private:
 	Owned<StructType> attributesDescriptor;
 	NodeVector<FieldDescriptor> fieldDescriptors;
 
-	void addAndSortFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger);
+	bool addAndSortFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger);
 
 protected:
 	void doResolve(ResolutionState &state) override;
@@ -557,8 +557,11 @@ public:
 	 * parent of the given FieldDescriptor if it is not set yet.
 	 *
 	 * @param fd is a FieldDescriptor.
+	 * @return   returns true if the given FieldDescriptor was not added at the
+	 *           end one place before because a TREE field already existed and
+	 *           the TREE field has to be at the end.
 	 */
-	void addFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger);
+	bool addFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger);
 
 	/**
 	 * Adds the given FieldDescriptor to this Descriptor. This also sets the
@@ -566,16 +569,22 @@ public:
 	 * already and removes it from the old parent Descriptor.
 	 *
 	 * @param fd is a FieldDescriptor.
+	 * @return   returns true if the given FieldDescriptor was not added at the
+	 *           end one place before because a TREE field already existed and
+	 *           the TREE field has to be at the end.
 	 */
-	void moveFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger);
+	bool moveFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger);
 
 	/**
 	 * Copies a FieldDescriptor that belongs to another Descriptor to this
 	 * Descriptor.
 	 *
 	 * @param fd some FieldDescriptor belonging to another Descriptor.
+	 * @return   returns true if the given FieldDescriptor was not added at the
+	 *           end one place before because a TREE field already existed and
+	 *           the TREE field has to be at the end.
 	 */
-	void copyFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger);
+	bool copyFieldDescriptor(Handle<FieldDescriptor> fd, Logger &logger);
 
 	/**
 	 * Removes the given FieldDescriptor from this Descriptor. This also sets
@@ -598,9 +607,12 @@ public:
 	 *                      filled in order for an instance of the parent
 	 *                      Descriptor to be valid.
 	 *
-	 * @return              the newly created FieldDescriptor.
+	 * @return              the newly created FieldDescriptor and a bool
+	 *                      indicating whether the order of FieldDescriptors had
+	 *                      to be changed for the TREE field to be in the last
+	 *                      spot.
 	 */
-	Rooted<FieldDescriptor> createPrimitiveFieldDescriptor(
+	std::pair<Rooted<FieldDescriptor>, bool> createPrimitiveFieldDescriptor(
 	    Handle<Type> primitiveType, Logger &logger,
 	    FieldDescriptor::FieldType fieldType = FieldDescriptor::FieldType::TREE,
 	    std::string name = "", bool optional = false);
@@ -617,9 +629,12 @@ public:
 	 *                      filled in order for an instance of the parent
 	 *                      Descriptor to be valid.
 	 *
-	 * @return              the newly created FieldDescriptor.
+	 * @return              the newly created FieldDescriptor and a bool
+	 *                      indicating whether the order of FieldDescriptors had
+	 *                      to be changed for the TREE field to be in the last
+	 *                      spot.
 	 */
-	Rooted<FieldDescriptor> createFieldDescriptor(
+	std::pair<Rooted<FieldDescriptor>, bool> createFieldDescriptor(
 	    Logger &logger,
 	    FieldDescriptor::FieldType fieldType = FieldDescriptor::FieldType::TREE,
 	    std::string name = "", bool optional = false);
diff --git a/src/core/parser/stack/DocumentHandler.cpp b/src/core/parser/stack/DocumentHandler.cpp
index d514701..41ca8ec 100644
--- a/src/core/parser/stack/DocumentHandler.cpp
+++ b/src/core/parser/stack/DocumentHandler.cpp
@@ -348,4 +348,4 @@ namespace RttiTypes {
 const Rtti DocumentField = RttiBuilder<ousia::parser_stack::DocumentField>(
                                "DocumentField").parent(&Node);
 }
-}
+}
\ No newline at end of file
diff --git a/src/core/parser/stack/DomainHandler.cpp b/src/core/parser/stack/DomainHandler.cpp
index a2c8eec..ddec1ee 100644
--- a/src/core/parser/stack/DomainHandler.cpp
+++ b/src/core/parser/stack/DomainHandler.cpp
@@ -133,11 +133,18 @@ bool DomainFieldHandler::start(Variant::mapType &args)
 
 	Rooted<Descriptor> parent = scope().selectOrThrow<Descriptor>();
 
-	Rooted<FieldDescriptor> field = parent->createFieldDescriptor(
+	auto res = parent->createFieldDescriptor(
 	    logger(), type, args["name"].asString(), args["optional"].asBool());
-	field->setLocation(location());
+	res.first->setLocation(location());
+	if (res.second) {
+		logger().warning(
+		    std::string("Field \"") + res.first->getName() +
+		        "\" was declared after main field. The order of fields "
+		        "was changed to make the main field the last field.",
+		    *res.first);
+	}
 
-	scope().push(field);
+	scope().push(res.first);
 	return true;
 }
 
@@ -150,14 +157,24 @@ bool DomainFieldRefHandler::start(Variant::mapType &args)
 	Rooted<Descriptor> parent = scope().selectOrThrow<Descriptor>();
 
 	const std::string &name = args["ref"].asString();
-	scope().resolveFieldDescriptor(
-	    name, parent, logger(),
-	    [](Handle<Node> field, Handle<Node> parent, Logger &logger) {
-		    if (field != nullptr) {
-			    parent.cast<StructuredClass>()->addFieldDescriptor(
-			        field.cast<FieldDescriptor>(), logger);
-		    }
-		});
+
+	auto loc = location();
+
+	scope().resolveFieldDescriptor(name, parent, logger(),
+	                               [loc](Handle<Node> field,
+	                                     Handle<Node> parent, Logger &logger) {
+		if (field != nullptr) {
+			if (parent.cast<StructuredClass>()->addFieldDescriptor(
+			        field.cast<FieldDescriptor>(), logger)) {
+				logger.warning(
+				    std::string("Field \"") + field->getName() +
+				        "\" was referenced after main field was declared. The "
+				        "order of fields was changed to make the main field "
+				        "the last field.",
+				    loc);
+			}
+		}
+	});
 	return true;
 }
 
@@ -176,13 +193,20 @@ bool DomainPrimitiveHandler::start(Variant::mapType &args)
 		fieldType = FieldDescriptor::FieldType::TREE;
 	}
 
-	Rooted<FieldDescriptor> field = parent->createPrimitiveFieldDescriptor(
+	auto res = parent->createPrimitiveFieldDescriptor(
 	    new UnknownType(manager()), logger(), fieldType,
 	    args["name"].asString(), args["optional"].asBool());
-	field->setLocation(location());
+	res.first->setLocation(location());
+	if (res.second) {
+		logger().warning(
+		    std::string("Field \"") + res.first->getName() +
+		        "\" was declared after main field. The order of fields "
+		        "was changed to make the main field the last field.",
+		    *res.first);
+	}
 
 	const std::string &type = args["type"].asString();
-	scope().resolve<Type>(type, field, logger(),
+	scope().resolve<Type>(type, res.first, logger(),
 	                      [](Handle<Node> type, Handle<Node> field,
 	                         Logger &logger) {
 		if (type != nullptr) {
@@ -190,7 +214,7 @@ bool DomainPrimitiveHandler::start(Variant::mapType &args)
 		}
 	});
 
-	scope().push(field);
+	scope().push(res.first);
 	return true;
 }
 
@@ -254,8 +278,8 @@ bool DomainParentFieldHandler::start(Variant::mapType &args)
 	                           Logger &logger) {
 		    if (parent != nullptr) {
 			    Rooted<FieldDescriptor> field =
-			        parent.cast<Descriptor>()->createFieldDescriptor(
-			            logger, type, name, optional);
+			        (parent.cast<Descriptor>()->createFieldDescriptor(
+			             logger, type, name, optional)).first;
 			    field->addChild(strct.cast<StructuredClass>());
 		    }
 		});
@@ -390,4 +414,4 @@ namespace RttiTypes {
 const Rtti DomainParent = RttiBuilder<ousia::parser_stack::DomainParent>(
                               "DomainParent").parent(&Node);
 }
-}
+}
\ No newline at end of file