From 230844c3ae20fc9365073964bf0caeb67a36ba98 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Patrick=20L=C3=BChne?= <patrick-github@luehne.de>
Date: Sun, 18 Jun 2017 05:18:52 +0200
Subject: [PATCH] Fixed whitespace issue in typing sections and added test
 cases.

---
 .../parsing/PrimitiveTypeDeclaration.cpp      |  5 ++-
 lib/pddlparse/tests/TestIssues.cpp            | 34 +++++++++++++++++++
 lib/tokenize/include/tokenize/Tokenizer.h     |  5 +++
 lib/tokenize/src/tokenize/Stream.cpp          |  1 +
 tests/data/issues/issue-7.pddl                |  8 +++++
 tests/data/issues/issue-8.pddl                |  9 +++++
 6 files changed, 59 insertions(+), 3 deletions(-)
 create mode 100644 lib/pddlparse/tests/TestIssues.cpp
 create mode 100644 tests/data/issues/issue-7.pddl
 create mode 100644 tests/data/issues/issue-8.pddl

diff --git a/lib/pddlparse/src/pddlparse/detail/parsing/PrimitiveTypeDeclaration.cpp b/lib/pddlparse/src/pddlparse/detail/parsing/PrimitiveTypeDeclaration.cpp
index 89226a0..87cb6a5 100644
--- a/lib/pddlparse/src/pddlparse/detail/parsing/PrimitiveTypeDeclaration.cpp
+++ b/lib/pddlparse/src/pddlparse/detail/parsing/PrimitiveTypeDeclaration.cpp
@@ -59,6 +59,7 @@ void parseAndAddPrimitiveTypeDeclarations(Context &context, ast::Domain &domain)
 
 		// Skip parent type information for now
 		tokenizer.getIdentifier();
+		tokenizer.skipWhiteSpace();
 	}
 
 	tokenizer.seek(position);
@@ -83,6 +84,7 @@ void parseAndAddPrimitiveTypeDeclarations(Context &context, ast::Domain &domain)
 
 		// If existing, parse and store parent type
 		auto parentType = parsePrimitiveType(context, domain);
+		tokenizer.skipWhiteSpace();
 
 		auto &types = domain.types;
 
@@ -91,9 +93,6 @@ void parseAndAddPrimitiveTypeDeclarations(Context &context, ast::Domain &domain)
 
 		// All types up to now are labeled with their parent types
 		inheritanceIndex = i + 1;
-
-		tokenizer.skipWhiteSpace();
-
 		i++;
 	}
 }
diff --git a/lib/pddlparse/tests/TestIssues.cpp b/lib/pddlparse/tests/TestIssues.cpp
new file mode 100644
index 0000000..5a324d0
--- /dev/null
+++ b/lib/pddlparse/tests/TestIssues.cpp
@@ -0,0 +1,34 @@
+#include <catch.hpp>
+
+#include <experimental/filesystem>
+
+#include <pddlparse/AST.h>
+#include <pddlparse/Parse.h>
+
+namespace fs = std::experimental::filesystem;
+
+const pddl::Context::WarningCallback ignoreWarnings = [](const auto &, const auto &warning){std::cout << warning << std::endl;};
+
+////////////////////////////////////////////////////////////////////////////////////////////////////
+
+TEST_CASE("[PDDL parser issues] Check past issues", "[PDDL parser issues]")
+{
+	pddl::Tokenizer tokenizer;
+	pddl::Context context(std::move(tokenizer), ignoreWarnings);
+
+	// Check that no infinite loop occurs
+	SECTION("“either” in typing section")
+	{
+		const auto domainFile = fs::path("data") / "issues" / "issue-7.pddl";
+		context.tokenizer.read(domainFile);
+		CHECK_THROWS(pddl::parseDescription(context));
+	}
+
+	// Check that whitespace is correctly ignored in type section
+	SECTION("whitespace in typing section")
+	{
+		const auto domainFile = fs::path("data") / "issues" / "issue-8.pddl";
+		context.tokenizer.read(domainFile);
+		CHECK_NOTHROW(pddl::parseDescription(context));
+	}
+}
diff --git a/lib/tokenize/include/tokenize/Tokenizer.h b/lib/tokenize/include/tokenize/Tokenizer.h
index 61d013c..0edea3b 100644
--- a/lib/tokenize/include/tokenize/Tokenizer.h
+++ b/lib/tokenize/include/tokenize/Tokenizer.h
@@ -214,7 +214,12 @@ std::string Tokenizer<TokenizerPolicy>::getIdentifier()
 		const auto character = currentCharacter();
 
 		if (!TokenizerPolicy::isIdentifierCharacter(character))
+		{
+			if (value.empty())
+				throw TokenizerException(location(), "could not parse identifier");
+
 			return value;
+		}
 
 		value.push_back(character);
 		advance();
diff --git a/lib/tokenize/src/tokenize/Stream.cpp b/lib/tokenize/src/tokenize/Stream.cpp
index 6f4f5ce..06450f1 100644
--- a/lib/tokenize/src/tokenize/Stream.cpp
+++ b/lib/tokenize/src/tokenize/Stream.cpp
@@ -128,6 +128,7 @@ Location Stream::location() const
 
 char Stream::currentCharacter() const
 {
+	// TODO: check if this should be secured by check()
 	return m_stream.peek();
 }
 
diff --git a/tests/data/issues/issue-7.pddl b/tests/data/issues/issue-7.pddl
new file mode 100644
index 0000000..5ef0950
--- /dev/null
+++ b/tests/data/issues/issue-7.pddl
@@ -0,0 +1,8 @@
+(define
+	(domain mystery-typed)
+	(:requirements :typing)
+	(:types
+		foo - object
+		bar - (either foo foo)
+	)
+)
diff --git a/tests/data/issues/issue-8.pddl b/tests/data/issues/issue-8.pddl
new file mode 100644
index 0000000..446ffc6
--- /dev/null
+++ b/tests/data/issues/issue-8.pddl
@@ -0,0 +1,9 @@
+(define (domain miconic)
+	(:requirements :typing)
+	(:types
+		object
+		passenger - object
+		floor - object
+	)
+)
+