From 4449adea1af269380163d3075ec5518e9639f271 Mon Sep 17 00:00:00 2001
From: Ezekiel Warren <zaucy@users.noreply.github.com>
Date: Fri, 28 Jun 2024 13:40:58 -0700
Subject: [PATCH] feat: check for invalid assoc with fields (#232)

---
 ecsact/interpret/eval.cc               | 39 ++++++++++++++++++++++++++
 ecsact/interpret/eval_error.h          |  3 ++
 test/errors/BUILD.bazel                |  5 ++--
 test/errors/invalid_assoc_field.cc     | 11 ++++++++
 test/errors/invalid_assoc_field.ecsact |  9 ++++++
 5 files changed, 65 insertions(+), 2 deletions(-)
 create mode 100644 test/errors/invalid_assoc_field.cc
 create mode 100644 test/errors/invalid_assoc_field.ecsact

diff --git a/ecsact/interpret/eval.cc b/ecsact/interpret/eval.cc
index d462237..5433422 100644
--- a/ecsact/interpret/eval.cc
+++ b/ecsact/interpret/eval.cc
@@ -1171,6 +1171,28 @@ static auto get_with_field_ids(
 	return with_field_ids;
 }
 
+static auto find_capabilities_for( //
+	auto sys_like_id,
+	auto id
+) -> std::optional<ecsact_system_capability> {
+	for(auto&& [comp_id, caps] : ecsact::meta::system_capabilities(sys_like_id)) {
+		if(ecsact_id_cast<decltype(id)>(comp_id) == id) {
+			return caps;
+		}
+	}
+
+	auto parent = ecsact::meta::get_parent_system_id(
+		// NOTE: this cast only works because we wrote the meta runtime for the
+		// interpreter. This is bad practice.
+		static_cast<ecsact_system_id>(sys_like_id)
+	);
+	if(parent) {
+		return find_capabilities_for(*parent, id);
+	}
+
+	return {};
+}
+
 static auto eval_system_with_statement_data_common(
 	ecsact_system_like_id                sys_like_id,
 	ecsact_component_like_id             comp_like_id,
@@ -1201,6 +1223,23 @@ static auto eval_system_with_statement_data_common(
 			};
 		}
 
+		auto field_type =
+			ecsact::meta::get_field_type(comp_like_id, *assoc_field_id);
+
+		if(field_type.kind == ECSACT_TYPE_KIND_BUILTIN) {
+			if(field_type.type.builtin != ECSACT_ENTITY_TYPE) {
+				return ecsact_eval_error{
+					.code = ECSACT_EVAL_ERR_INVALID_ASSOC_FIELD_TYPE,
+					.relevant_content = fields[i],
+				};
+			}
+		} else if(field_type.kind != ECSACT_TYPE_KIND_FIELD_INDEX) {
+			return ecsact_eval_error{
+				.code = ECSACT_EVAL_ERR_INVALID_ASSOC_FIELD_TYPE,
+				.relevant_content = fields[i],
+			};
+		}
+
 		with_field_ids.emplace_back(*assoc_field_id);
 	}
 
diff --git a/ecsact/interpret/eval_error.h b/ecsact/interpret/eval_error.h
index c52b8c4..844e413 100644
--- a/ecsact/interpret/eval_error.h
+++ b/ecsact/interpret/eval_error.h
@@ -88,6 +88,9 @@ typedef enum ecsact_eval_error_code {
 	/// Statement supports the given parameter name but the value is invalid
 	ECSACT_EVAL_ERR_INVALID_PARAMETER_VALUE,
 
+	/// Field type is not allowed in 'with' statement
+	ECSACT_EVAL_ERR_INVALID_ASSOC_FIELD_TYPE,
+
 	/// Internal error. Should not happen and is an indiciation of a bug.
 	ECSACT_EVAL_ERR_INTERNAL = 999,
 
diff --git a/test/errors/BUILD.bazel b/test/errors/BUILD.bazel
index 7241cca..e4969dc 100644
--- a/test/errors/BUILD.bazel
+++ b/test/errors/BUILD.bazel
@@ -4,6 +4,7 @@ load("@rules_cc//cc:defs.bzl", "cc_test")
 # buildifier: keep sorted
 _TESTS = [
     "duplicate_notice_components",
+    "invalid_assoc_field",
     "invalid_notify_settings",
     "no_capabilities",
     "no_package_statement_first",
@@ -15,9 +16,9 @@ _TESTS = [
 [cc_test(
     name = test,
     srcs = ["{}.cc".format(test)],
+    args = ["--gtest_catch_exceptions=0"],
     copts = copts,
     data = ["{}.ecsact".format(test)],
-    args = ["--gtest_catch_exceptions=0"],
     deps = [
         "//:test_lib",
         "@ecsact_interpret",
@@ -29,12 +30,12 @@ _TESTS = [
 cc_test(
     name = "ambiguous_field_type",
     srcs = ["ambiguous_field_type.cc"],
+    args = ["--gtest_catch_exceptions=0"],
     copts = copts,
     data = [
         "ambiguous_field_type.ecsact",
         "ambiguous_field_type_import.ecsact",
     ],
-    args = ["--gtest_catch_exceptions=0"],
     deps = [
         "//:test_lib",
         "@ecsact_interpret",
diff --git a/test/errors/invalid_assoc_field.cc b/test/errors/invalid_assoc_field.cc
new file mode 100644
index 0000000..023376e
--- /dev/null
+++ b/test/errors/invalid_assoc_field.cc
@@ -0,0 +1,11 @@
+#include "gtest/gtest.h"
+#include "ecsact/interpret/eval.h"
+
+#include "test_lib.hh"
+
+TEST(InvalidAssocField, NonEntityBuiltin) {
+	auto errs =
+		ecsact_interpret_test_files({"errors/invalid_assoc_field.ecsact"});
+	ASSERT_EQ(errs.size(), 1);
+	ASSERT_EQ(errs[0].eval_error, ECSACT_EVAL_ERR_INVALID_ASSOC_FIELD_TYPE);
+}
diff --git a/test/errors/invalid_assoc_field.ecsact b/test/errors/invalid_assoc_field.ecsact
new file mode 100644
index 0000000..d28153e
--- /dev/null
+++ b/test/errors/invalid_assoc_field.ecsact
@@ -0,0 +1,9 @@
+main package invalid_assoc_field;
+
+component A { i32 num; }
+
+system S {
+	readwrite A with num {
+		readwrite A;
+	}
+}