Fix for issue #85 -- only allowing explicit fields if no structure elements or data have been given beforehand. Added unit tests.

5 files changed, 107 insertions, 8 deletions

diff --git a/src/core/parser/ParserScope.hpp b/src/core/parser/ParserScope.hpp
index fa78c17..e27c81e 100644
--- a/src/core/parser/ParserScope.hpp
+++ b/src/core/parser/ParserScope.hpp
@@ -286,7 +286,13 @@ enum class ParserFlag {
      * Set to the boolean value "true" if the head section of a file has passed.
      * This happens once the first non-import tag is reached.
      */
-	POST_HEAD
+	POST_HEAD,
+
+	/**
+	 * Set to the boolean value "true" if explicit fields may no longer be
+	 * defined inside a structure element.
+	 */
+	POST_EXPLICIT_FIELDS
 };
 
 /**
@@ -797,6 +803,7 @@ public:
 	bool resolveFieldDescriptor(const std::string &name, Handle<Node> owner,
 	                            Logger &logger,
 	                            ResolutionResultCallback resultCallback);
+
 	/**
 	 * Tries to resolve all currently deferred resolution steps. The list of
 	 * pending deferred resolutions is cleared after this function has run.
diff --git a/src/core/parser/stack/DocumentHandler.cpp b/src/core/parser/stack/DocumentHandler.cpp
index e959b9a..1df3cb3 100644
--- a/src/core/parser/stack/DocumentHandler.cpp
+++ b/src/core/parser/stack/DocumentHandler.cpp
@@ -98,6 +98,9 @@ void DocumentChildHandler::createPath(const NodeVector<Node> &path,
 	    manager(), scope().getLeaf(),
 	    parent->getDescriptor()->getFieldDescriptorIndex(), true)};
 	scope().push(field);
+
+	// Generally allow explicit fields in the new field
+	scope().setFlag(ParserFlag::POST_EXPLICIT_FIELDS, false);
 }
 
 void DocumentChildHandler::createPath(const size_t &firstFieldIdx,
@@ -113,6 +116,9 @@ void DocumentChildHandler::createPath(const size_t &firstFieldIdx,
 	parent = static_cast<DocumentEntity *>(transparent.get());
 
 	createPath(path, parent, 2);
+
+	// Generally allow explicit fields in the new field
+	scope().setFlag(ParserFlag::POST_EXPLICIT_FIELDS, false);
 }
 
 bool DocumentChildHandler::start(Variant::mapType &args)
@@ -170,12 +176,25 @@ bool DocumentChildHandler::start(Variant::mapType &args)
 				ssize_t newFieldIdx =
 				    parent->getDescriptor()->getFieldDescriptorIndex(name());
 				if (newFieldIdx != -1) {
-					Rooted<DocumentField> field{new DocumentField(
-					    manager(), parentNode, newFieldIdx, false)};
-					field->setLocation(location());
-					scope().push(field);
-					isExplicitField = true;
-					return true;
+					// Check whether explicit fields are allowed here, if not
+					if (scope().getFlag(ParserFlag::POST_EXPLICIT_FIELDS)) {
+						logger().note(
+						    std::string(
+						        "Data or structure commands have already been "
+						        "given, command \"") +
+						        name() + std::string(
+						                     "\" is not interpreted explicit "
+						                     "field. Move explicit field "
+						                     "references to the beginning."),
+						    location());
+					} else {
+						Rooted<DocumentField> field{new DocumentField(
+						    manager(), parentNode, newFieldIdx, false)};
+						field->setLocation(location());
+						scope().push(field);
+						isExplicitField = true;
+						return true;
+					}
 				}
 			}
 
@@ -218,11 +237,17 @@ bool DocumentChildHandler::start(Variant::mapType &args)
 				    parent->getDescriptor()->getFieldDescriptorIndex();
 			}
 			// create the entity for the new element at last.
-			//TODO: REMOVE
+			// TODO: REMOVE
 			strct_name = strct->getName();
 			entity = parent->createChildStructuredEntity(strct, lastFieldIdx,
 			                                             args, nameAttr);
 		}
+
+		// We're past the region in which explicit fields can be defined in the
+		// parent structure element
+		scope().setFlag(ParserFlag::POST_EXPLICIT_FIELDS, true);
+
+		// Bush the entity onto the stack
 		entity->setLocation(location());
 		scope().push(entity);
 		return true;
@@ -271,6 +296,10 @@ bool DocumentChildHandler::fieldStart(bool &isDefault, size_t fieldIdx)
 	    new DocumentField(manager(), parentNode, fieldIdx, false)};
 	field->setLocation(location());
 	scope().push(field);
+
+	// Generally allow explicit fields in the new field
+	scope().setFlag(ParserFlag::POST_EXPLICIT_FIELDS, false);
+
 	return true;
 }
 
@@ -334,6 +363,10 @@ bool DocumentChildHandler::convertData(Handle<FieldDescriptor> field,
 
 bool DocumentChildHandler::data(Variant &data)
 {
+	// We're past the region in which explicit fields can be defined in the
+	// parent structure element
+	scope().setFlag(ParserFlag::POST_EXPLICIT_FIELDS, true);
+
 	Rooted<Node> parentField = scope().getLeaf();
 	assert(parentField->isa(&RttiTypes::DocumentField));
 
diff --git a/test/formats/osml/OsmlParserTest.cpp b/test/formats/osml/OsmlParserTest.cpp
index 4cda20a..5127b32 100644
--- a/test/formats/osml/OsmlParserTest.cpp
+++ b/test/formats/osml/OsmlParserTest.cpp
@@ -172,5 +172,32 @@ TEST(OsmlParser, structWithNoField)
 	ASSERT_TRUE(node->isa(&RttiTypes::Document));
 }
 
+TEST(OsmlParser, invalidExplicitFields)
+{
+	OsmlStandaloneEnvironment env(logger);
+	logger.reset();
+
+	ASSERT_FALSE(logger.hasError());
+	Rooted<Node> node = env.parse("invalid_explicit_fields.osml", "", "",
+	                              RttiSet{&RttiTypes::Node});
+	ASSERT_TRUE(logger.hasError());
+
+	ASSERT_TRUE(node != nullptr);
+	ASSERT_TRUE(node->isa(&RttiTypes::Document));
+}
+
+TEST(OsmlParser, explicitFields)
+{
+	OsmlStandaloneEnvironment env(logger);
+	logger.reset();
+
+	Rooted<Node> node = env.parse("explicit_fields.osml", "", "",
+	                              RttiSet{&RttiTypes::Node});
+	ASSERT_FALSE(logger.hasError());
+
+	ASSERT_TRUE(node != nullptr);
+	ASSERT_TRUE(node->isa(&RttiTypes::Document));
+}
+
 }
 
diff --git a/testdata/osmlparser/explicit_fields.osml b/testdata/osmlparser/explicit_fields.osml
new file mode 100644
index 0000000..a9ba1a3
--- /dev/null
+++ b/testdata/osmlparser/explicit_fields.osml
@@ -0,0 +1,16 @@
+\document
+
+\domain#test
+	\struct#a[isRoot=true]
+		\primitive#b[type=string,isSubtree=true]
+		\primitive#c[type=string,isSubtree=true]
+		\primitive#d[type=string,isSubtree=false]
+
+
+\a{!
+	\b{test}
+	\c{test}
+	test
+	test
+	test
+}
diff --git a/testdata/osmlparser/invalid_explicit_fields.osml b/testdata/osmlparser/invalid_explicit_fields.osml
new file mode 100644
index 0000000..9986204
--- /dev/null
+++ b/testdata/osmlparser/invalid_explicit_fields.osml
@@ -0,0 +1,16 @@
+\document
+
+\domain#test
+	\struct#a[isRoot=true]
+		\primitive#b[type=string,isSubtree=true]
+		\primitive#c[type=string,isSubtree=true]
+		\primitive#d[type=string,isSubtree=false]
+
+
+\a{!
+	\b{test}
+	test
+	\c{test}
+	test
+	test
+}