Fully implemented and tested begin/end handling

3 files changed, 483 insertions, 32 deletions

diff --git a/src/plugins/plain/PlainFormatStreamReader.cpp b/src/plugins/plain/PlainFormatStreamReader.cpp
index 1bff24b..9c35bb0 100644
--- a/src/plugins/plain/PlainFormatStreamReader.cpp
+++ b/src/plugins/plain/PlainFormatStreamReader.cpp
@@ -197,23 +197,126 @@ Variant PlainFormatStreamReader::parseIdentifier(size_t start)
 	return res;
 }
 
-void PlainFormatStreamReader::parseCommand(size_t start)
+PlainFormatStreamReader::State PlainFormatStreamReader::parseBeginCommand()
 {
-	// Parse the commandName as a first identifier
-	Variant commandName = parseIdentifier(start);
+	// Expect a '{' after the command
+	reader.consumeWhitespace();
+	if (!reader.expect('{')) {
+		logger.error("Expected \"{\" after \\begin", reader);
+		return State::NONE;
+	}
+
+	// Parse the name of the command that should be opened
+	Variant commandName = parseIdentifier(reader.getOffset());
+	if (commandName.asString().empty()) {
+		logger.error("Expected identifier", commandName);
+		return State::ERROR;
+	}
 
 	// Check whether the next character is a '#', indicating the start of the
 	// command name
 	Variant commandArgName;
-	start = reader.getOffset();
+	SourceOffset start = reader.getOffset();
 	if (reader.expect('#')) {
 		commandArgName = parseIdentifier(start);
 		if (commandArgName.asString().empty()) {
-			logger.error("Expected identifier after '#'", commandArgName);
+			logger.error("Expected identifier after \"#\"", commandArgName);
+		}
+	}
+
+	if (!reader.expect('}')) {
+		logger.error("Expected \"}\"", reader);
+		return State::ERROR;
+	}
+
+	// Parse the arguments
+	Variant commandArguments = parseCommandArguments(std::move(commandArgName));
+
+	// Push the command onto the command stack
+	pushCommand(std::move(commandName), std::move(commandArguments), true);
+
+	return State::COMMAND;
+}
+
+static bool checkStillInField(const PlainFormatStreamReader::Command &cmd,
+                              const Variant &endName, Logger &logger)
+{
+	if (cmd.inField && !cmd.inRangeField) {
+		logger.error(std::string("\\end in open field of command \"") +
+		                 cmd.name.asString() + std::string("\""),
+		             endName);
+		logger.note(std::string("Open command started here:"), cmd.name);
+		return true;
+	}
+	return false;
+}
+
+PlainFormatStreamReader::State PlainFormatStreamReader::parseEndCommand()
+{
+	// Expect a '{' after the command
+	if (!reader.expect('{')) {
+		logger.error("Expected \"{\" after \\end", reader);
+		return State::NONE;
+	}
+
+	// Fetch the name of the command that should be ended here
+	Variant name = parseIdentifier(reader.getOffset());
+
+	// Make sure the given command name is not empty
+	if (name.asString().empty()) {
+		logger.error("Expected identifier", name);
+		return State::ERROR;
+	}
+
+	// Make sure the command name is terminated with a '}'
+	if (!reader.expect('}')) {
+		logger.error("Expected \"}\"", reader);
+		return State::ERROR;
+	}
+
+	// Unroll the command stack up to the last range command
+	while (!commands.top().hasRange) {
+		if (checkStillInField(commands.top(), name, logger)) {
+			return State::ERROR;
 		}
+		commands.pop();
+	}
+
+	// Make sure we're not in an open field of this command
+	if (checkStillInField(commands.top(), name, logger)) {
+		return State::ERROR;
+	}
+
+	// Special error message if the top-level command is reached
+	if (commands.size() == 1) {
+		logger.error(std::string("Cannot end command \"") + name.asString() +
+		                 std::string("\" here, no command open"),
+		             name);
+		return State::ERROR;
 	}
 
-	// Read the arguments (if they are available), otherwise reset them
+	// Inform the about command mismatches
+	const Command &cmd = commands.top();
+	if (commands.top().name.asString() != name.asString()) {
+		logger.error(std::string("Trying to end command \"") +
+		                 cmd.name.asString() +
+		                 std::string(", but open command is \"") +
+		                 name.asString() + std::string("\""),
+		             name);
+		logger.note("Last command was opened here:", cmd.name);
+		return State::ERROR;
+	}
+
+	// Set the location to the location of the command that was ended, then end
+	// the current command
+	location = name.getLocation();
+	commands.pop();
+	return cmd.inRangeField ? State::FIELD_END : State::NONE;
+}
+
+Variant PlainFormatStreamReader::parseCommandArguments(Variant commandArgName)
+{
+	// Parse the arguments using the universal VariantReader
 	Variant commandArguments;
 	if (reader.expect('[')) {
 		auto res = VariantReader::parseObject(reader, logger, ']');
@@ -225,7 +328,8 @@ void PlainFormatStreamReader::parseCommand(size_t start)
 	// Insert the parsed name, make sure "name" was not specified in the
 	// arguments
 	if (commandArgName.isString()) {
-		auto res = commandArguments.asMap().emplace("name", commandArgName);
+		auto res =
+		    commandArguments.asMap().emplace("name", std::move(commandArgName));
 		if (!res.second) {
 			logger.error("Name argument specified multiple times",
 			             SourceLocation{}, MessageMode::NO_CONTEXT);
@@ -233,13 +337,56 @@ void PlainFormatStreamReader::parseCommand(size_t start)
 			logger.note("Second occurance is here: ", res.first->second);
 		}
 	}
+	return commandArguments;
+}
+
+void PlainFormatStreamReader::pushCommand(Variant commandName,
+                                          Variant commandArguments,
+                                          bool hasRange)
+{
+	// Store the location on the stack
+	location = commandName.getLocation();
 
 	// Place the command on the command stack, remove the last commands if we're
 	// not currently inside a field of these commands
 	while (!commands.top().inField) {
 		commands.pop();
 	}
-	commands.push(Command{commandName, commandArguments, false, false, false});
+	commands.push(Command{std::move(commandName), std::move(commandArguments),
+	                      hasRange, false, false});
+}
+
+PlainFormatStreamReader::State PlainFormatStreamReader::parseCommand(
+    size_t start)
+{
+	// Parse the commandName as a first identifier
+	Variant commandName = parseIdentifier(start);
+
+	// Handle the special "begin" and "end" commands
+	if (commandName.asString() == "begin") {
+		return parseBeginCommand();
+	} else if (commandName.asString() == "end") {
+		return parseEndCommand();
+	}
+
+	// Check whether the next character is a '#', indicating the start of the
+	// command name
+	Variant commandArgName;
+	start = reader.getOffset();
+	if (reader.expect('#')) {
+		commandArgName = parseIdentifier(start);
+		if (commandArgName.asString().empty()) {
+			logger.error("Expected identifier after \"#\"", commandArgName);
+		}
+	}
+
+	// Parse the arugments
+	Variant commandArguments = parseCommandArguments(std::move(commandArgName));
+
+	// Push the command onto the command stack
+	pushCommand(std::move(commandName), std::move(commandArguments), false);
+
+	return State::COMMAND;
 }
 
 void PlainFormatStreamReader::parseBlockComment()
@@ -293,6 +440,7 @@ bool PlainFormatStreamReader::checkIssueFieldStart()
 		// this command -- we'll have to issue a field start command!
 		if (cmd.hasRange) {
 			cmd.inField = true;
+			cmd.inRangeField = true;
 			reader.resetPeek();
 			return true;
 		}
@@ -319,6 +467,14 @@ PlainFormatStreamReader::State PlainFormatStreamReader::parse()
 
 		// Special handling for Backslash and Text
 		if (type == Tokens.Backslash) {
+			// Before appending anything to the output data or starting a new
+			// command, check whether FIELD_START has to be issued, as the
+			// current command is a command with range
+			if (checkIssueFieldStart()) {
+				location = token.location;
+				return State::FIELD_START;
+			}
+
 			// Check whether a command starts now, without advancing the peek
 			// cursor
 			char c;
@@ -330,20 +486,23 @@ PlainFormatStreamReader::State PlainFormatStreamReader::parse()
 
 			// Try to parse a command
 			if (Utils::isIdentifierStartCharacter(c)) {
-				parseCommand(token.location.getStart());
+				// Make sure to issue any data before it is to late
 				if (checkIssueData(handler)) {
 					return State::DATA;
 				}
-				location = commands.top().name.getLocation();
-				return State::COMMAND;
-			}
 
-			// Before appending anything to the output data, check whether
-			// FIELD_START has to be issued, as the current command is a command
-			// with range
-			if (checkIssueFieldStart()) {
-				location = token.location;
-				return State::FIELD_START;
+				// Parse the actual command
+				State res = parseCommand(token.location.getStart());
+				switch (res) {
+					case State::ERROR:
+						throw LoggableException(
+						    "Last error was irrecoverable, ending parsing "
+						    "process");
+					case State::NONE:
+						continue;
+					default:
+						return res;
+				}
 			}
 
 			// This was not a special character, just append the given character
@@ -393,7 +552,7 @@ PlainFormatStreamReader::State PlainFormatStreamReader::parse()
 			}
 			logger.error(
 			    "Got field start token \"{\", but no command for which to "
-			    "start the field. Did you mean to write \"\\{\"?",
+			    "start the field. Did you mean \"\\{\"?",
 			    token);
 		} else if (token.type == Tokens.FieldEnd) {
 			// Try to end an open field of the current command -- if the current
@@ -412,8 +571,8 @@ PlainFormatStreamReader::State PlainFormatStreamReader::parse()
 				}
 			}
 			logger.error(
-			    "Got field end token \"}\" but there is no field to end. Did you "
-			    "mean to write \"\\}\"?",
+			    "Got field end token \"}\" but there is no field to end. Did "
+			    "you mean \"\\}\"?",
 			    token);
 		} else {
 			logger.error("Unexpected token \"" + token.content + "\"", token);
@@ -425,6 +584,18 @@ PlainFormatStreamReader::State PlainFormatStreamReader::parse()
 		return State::DATA;
 	}
 
+	// Make sure all open commands and fields have been ended at the end of the
+	// stream
+	while (commands.size() > 1) {
+		Command &cmd = commands.top();
+		if (cmd.inField || cmd.hasRange) {
+			logger.error("Reached end of stream, but command \"" +
+			                 cmd.name.asString() + "\" has not been ended",
+			             cmd.name);
+		}
+		commands.pop();
+	}
+
 	location = SourceLocation{reader.getSourceId(), reader.getOffset()};
 	return State::END;
 }
diff --git a/src/plugins/plain/PlainFormatStreamReader.hpp b/src/plugins/plain/PlainFormatStreamReader.hpp
index 4a11b8e..a14ca10 100644
--- a/src/plugins/plain/PlainFormatStreamReader.hpp
+++ b/src/plugins/plain/PlainFormatStreamReader.hpp
@@ -107,7 +107,17 @@ public:
 		/**
 	     * The end of the stream has been reached.
 	     */
-		END
+		END,
+
+		/**
+		 * Returned from internal functions if nothing should be done.
+		 */
+		NONE,
+
+		/**
+		 * Returned from internal function to indicate irrecoverable errors.
+		 */
+		ERROR
 	};
 
 	/**
@@ -159,10 +169,10 @@ public:
 		 * @param inRangeField is set to true if we currently inside the outer
 		 * field of the command.
 		 */
-		Command(const Variant &name, const Variant &arguments, bool hasRange,
+		Command(Variant name, Variant arguments, bool hasRange,
 		        bool inField, bool inRangeField)
-		    : name(name),
-		      arguments(arguments),
+		    : name(std::move(name)),
+		      arguments(std::move(arguments)),
 		      hasRange(hasRange),
 		      inField(inField),
 		      inRangeField(inRangeField)
@@ -217,12 +227,33 @@ private:
 	Variant parseIdentifier(size_t start);
 
 	/**
+	 * Function used internally to handle the special "\begin" command.
+	 */
+	State parseBeginCommand();
+
+	/**
+	 * Function used internally to handle the special "\end" command.
+	 */
+	State parseEndCommand();
+
+	/**
+	 * Pushes the parsed command onto the command stack.
+	 */
+	void pushCommand(Variant commandName, Variant commandArguments, bool hasRange);
+
+	/**
+	 * Parses the command arguments.
+	 */
+	Variant parseCommandArguments(Variant commandArgName);
+
+	/**
 	 * Function used internally to parse a command.
 	 *
 	 * @param start is the start byte offset of the command (including the
 	 * backslash)
+	 * @return true if a command was actuall parsed, false otherwise.
 	 */
-	void parseCommand(size_t start);
+	State parseCommand(size_t start);
 
 	/**
 	 * Function used internally to parse a block comment.
diff --git a/test/plugins/plain/PlainFormatStreamReaderTest.cpp b/test/plugins/plain/PlainFormatStreamReaderTest.cpp
index 423bc45..4aa7fad 100644
--- a/test/plugins/plain/PlainFormatStreamReaderTest.cpp
+++ b/test/plugins/plain/PlainFormatStreamReaderTest.cpp
@@ -362,6 +362,15 @@ static void assertCommand(PlainFormatStreamReader &reader,
 	}
 }
 
+static void assertCommand(PlainFormatStreamReader &reader,
+                          const std::string &name, const Variant::mapType &args,
+                          SourceOffset start = InvalidSourceOffset,
+                          SourceOffset end = InvalidSourceOffset)
+{
+	assertCommand(reader, name, start, end);
+	EXPECT_EQ(args, reader.getCommandArguments());
+}
+
 static void assertData(PlainFormatStreamReader &reader, const std::string &data,
                        SourceOffset start = InvalidSourceOffset,
                        SourceOffset end = InvalidSourceOffset)
@@ -379,8 +388,8 @@ static void assertData(PlainFormatStreamReader &reader, const std::string &data,
 }
 
 static void assertFieldStart(PlainFormatStreamReader &reader,
-                       SourceOffset start = InvalidSourceOffset,
-                       SourceOffset end = InvalidSourceOffset)
+                             SourceOffset start = InvalidSourceOffset,
+                             SourceOffset end = InvalidSourceOffset)
 {
 	ASSERT_EQ(PlainFormatStreamReader::State::FIELD_START, reader.parse());
 	if (start != InvalidSourceOffset) {
@@ -392,8 +401,8 @@ static void assertFieldStart(PlainFormatStreamReader &reader,
 }
 
 static void assertFieldEnd(PlainFormatStreamReader &reader,
-                       SourceOffset start = InvalidSourceOffset,
-                       SourceOffset end = InvalidSourceOffset)
+                           SourceOffset start = InvalidSourceOffset,
+                           SourceOffset end = InvalidSourceOffset)
 {
 	ASSERT_EQ(PlainFormatStreamReader::State::FIELD_END, reader.parse());
 	if (start != InvalidSourceOffset) {
@@ -405,8 +414,8 @@ static void assertFieldEnd(PlainFormatStreamReader &reader,
 }
 
 static void assertEnd(PlainFormatStreamReader &reader,
-                       SourceOffset start = InvalidSourceOffset,
-                       SourceOffset end = InvalidSourceOffset)
+                      SourceOffset start = InvalidSourceOffset,
+                      SourceOffset end = InvalidSourceOffset)
 {
 	ASSERT_EQ(PlainFormatStreamReader::State::END, reader.parse());
 	if (start != InvalidSourceOffset) {
@@ -634,5 +643,245 @@ TEST(PlainFormatStreamReader, errorNoFieldEndNestedData)
 	ASSERT_TRUE(logger.hasError());
 }
 
+TEST(PlainFormatStreamReader, beginEnd)
+{
+	const char *testString = "\\begin{book}\\end{book}";
+	//                         012345678901 2345678901
+	//                         0         1          2
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	assertCommand(reader, "book", 7, 11);
+	assertFieldStart(reader, 12, 13);
+	assertFieldEnd(reader, 17, 21);
+	assertEnd(reader, 22, 22);
+}
+
+TEST(PlainFormatStreamReader, beginEndWithName)
+{
+	const char *testString = "\\begin{book#a}\\end{book}";
+	//                         01234567890123 4567890123
+	//                         0         1          2
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	assertCommand(reader, "book", {{"name", "a"}}, 7, 11);
+	assertFieldStart(reader, 14, 15);
+	assertFieldEnd(reader, 19, 23);
+	assertEnd(reader, 24, 24);
+}
+
+TEST(PlainFormatStreamReader, beginEndWithNameAndArgs)
+{
+	const char *testString = "\\begin{book#a}[a=1,b=2,c=\"test\"]\\end{book}";
+	//                         0123456789012345678901234 56789 01 2345678901
+	//                         0         1         2           3          4
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	assertCommand(reader, "book",
+	              {{"name", "a"}, {"a", 1}, {"b", 2}, {"c", "test"}}, 7, 11);
+	assertFieldStart(reader, 32, 33);
+	assertFieldEnd(reader, 37, 41);
+	assertEnd(reader, 42, 42);
+}
+
+TEST(PlainFormatStreamReader, beginEndWithNameAndArgsMultipleFields)
+{
+	const char *testString =
+	    "\\begin{book#a}[a=1,b=2,c=\"test\"]{a \\test}{b \\test{}}\\end{book}";
+	//    0123456789012345678901234 56789 01234 567890123 45678901 2345678901
+	//    0         1         2           3          4          5          6
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	assertCommand(reader, "book",
+	              {{"name", "a"}, {"a", 1}, {"b", 2}, {"c", "test"}}, 7, 11);
+	assertFieldStart(reader, 32, 33);
+	assertData(reader, "a", 33, 34);
+	assertCommand(reader, "test", Variant::mapType{}, 35, 40);
+	assertFieldEnd(reader, 40, 41);
+	assertFieldStart(reader, 41, 42);
+	assertData(reader, "b", 42, 43);
+	assertCommand(reader, "test", Variant::mapType{}, 44, 49);
+	assertFieldStart(reader, 49, 50);
+	assertFieldEnd(reader, 50, 51);
+	assertFieldEnd(reader, 51, 52);
+	assertFieldStart(reader, 52, 53);
+	assertFieldEnd(reader, 57, 61);
+	assertEnd(reader, 62, 62);
+}
+
+TEST(PlainFormatStreamReader, beginEndWithData)
+{
+	const char *testString = "\\begin{book}a\\end{book}";
+	//                         0123456789012 3456789012
+	//                         0         1          2
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	assertCommand(reader, "book", 7, 11);
+	assertFieldStart(reader, 12, 13);
+	assertData(reader, "a", 12, 13);
+	assertFieldEnd(reader, 18, 22);
+	assertEnd(reader, 23, 23);
+}
+
+TEST(PlainFormatStreamReader, beginEndWithCommand)
+{
+	const char *testString = "\\begin{book}\\a{test}\\end{book}";
+	//                         012345678901 23456789 0123456789
+	//                         0         1           2
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	assertCommand(reader, "book", 7, 11);
+	assertFieldStart(reader, 12, 13);
+	assertCommand(reader, "a", 12, 14);
+	assertFieldStart(reader, 14, 15);
+	assertData(reader, "test", 15, 19);
+	assertFieldEnd(reader, 19, 20);
+	assertFieldEnd(reader, 25, 29);
+	assertEnd(reader, 30, 30);
+}
+
+TEST(PlainFormatStreamReader, errorBeginNoBraceOpen)
+{
+	const char *testString = "\\begin a";
+	//                         01234567
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	logger.reset();
+	ASSERT_FALSE(logger.hasError());
+	assertData(reader, "a", 7, 8);
+	ASSERT_TRUE(logger.hasError());
+}
+
+TEST(PlainFormatStreamReader, errorBeginNoIdentifier)
+{
+	const char *testString = "\\begin{!";
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	logger.reset();
+	ASSERT_FALSE(logger.hasError());
+	ASSERT_THROW(reader.parse(), LoggableException);
+	ASSERT_TRUE(logger.hasError());
+}
+
+TEST(PlainFormatStreamReader, errorBeginNoBraceClose)
+{
+	const char *testString = "\\begin{a";
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	logger.reset();
+	ASSERT_FALSE(logger.hasError());
+	ASSERT_THROW(reader.parse(), LoggableException);
+	ASSERT_TRUE(logger.hasError());
+}
+
+TEST(PlainFormatStreamReader, errorBeginNoName)
+{
+	const char *testString = "\\begin{a#}";
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	logger.reset();
+	ASSERT_FALSE(logger.hasError());
+	assertCommand(reader, "a");
+	ASSERT_TRUE(logger.hasError());
+	logger.reset();
+	ASSERT_FALSE(logger.hasError());
+	assertEnd(reader);
+	ASSERT_TRUE(logger.hasError());
+}
+
+TEST(PlainFormatStreamReader, errorEndNoBraceOpen)
+{
+	const char *testString = "\\end a";
+	//                         012345
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	logger.reset();
+	ASSERT_FALSE(logger.hasError());
+	assertData(reader, "a", 5, 6);
+	ASSERT_TRUE(logger.hasError());
+}
+
+TEST(PlainFormatStreamReader, errorEndNoIdentifier)
+{
+	const char *testString = "\\end{!";
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	logger.reset();
+	ASSERT_FALSE(logger.hasError());
+	ASSERT_THROW(reader.parse(), LoggableException);
+	ASSERT_TRUE(logger.hasError());
+}
+
+TEST(PlainFormatStreamReader, errorEndNoBraceClose)
+{
+	const char *testString = "\\end{a";
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	logger.reset();
+	ASSERT_FALSE(logger.hasError());
+	ASSERT_THROW(reader.parse(), LoggableException);
+	ASSERT_TRUE(logger.hasError());
+}
+
+TEST(PlainFormatStreamReader, errorEndNoBegin)
+{
+	const char *testString = "\\end{a}";
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	logger.reset();
+	ASSERT_FALSE(logger.hasError());
+	ASSERT_THROW(reader.parse(), LoggableException);
+	ASSERT_TRUE(logger.hasError());
+}
+
+
+TEST(PlainFormatStreamReader, errorBeginEndMismatch)
+{
+	const char *testString = "\\begin{a} \\begin{b} test \\end{a}";
+	//                         0123456789 012345678901234 5678901
+	//                         0          1         2          3
+	CharReader charReader(testString);
+
+	PlainFormatStreamReader reader(charReader, logger);
+
+	logger.reset();
+	assertCommand(reader, "a", 7, 8);
+	assertFieldStart(reader, 10, 11);
+	assertCommand(reader, "b", 17, 18);
+	assertFieldStart(reader, 20, 24);
+	assertData(reader, "test", 20, 24);
+	ASSERT_FALSE(logger.hasError());
+	ASSERT_THROW(reader.parse(), LoggableException);
+	ASSERT_TRUE(logger.hasError());
+}
+
+
 }