From b119ca33c7730fbced3a4f4262790b14d0444126 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Moritz=20H=C3=B6lting?=
 <87192362+moritz-hoelting@users.noreply.github.com>
Date: Sun, 16 Mar 2025 19:25:25 +0100
Subject: [PATCH] implement todos in scoreboard variable

---
 src/transpile/error.rs      |  53 +++++++
 src/transpile/expression.rs | 139 +++++++++++++-----
 src/transpile/lua.rs        | 283 +++++++++++++++++++++++-------------
 src/transpile/variables.rs  |  27 +++-
 4 files changed, 358 insertions(+), 144 deletions(-)

diff --git a/src/transpile/error.rs b/src/transpile/error.rs
index 4a04a97..9f3320e 100644
--- a/src/transpile/error.rs
+++ b/src/transpile/error.rs
@@ -42,6 +42,8 @@ pub enum TranspileError {
     UnknownIdentifier(#[from] UnknownIdentifier),
     #[error(transparent)]
     MissingValue(#[from] MissingValue),
+    #[error(transparent)]
+    IllegalIndexing(#[from] IllegalIndexing),
 }
 
 /// The result of a transpilation operation.
@@ -309,3 +311,54 @@ impl Display for MissingValue {
 }
 
 impl std::error::Error for MissingValue {}
+
+/// An error that occurs when an indexing operation is not permitted.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct IllegalIndexing {
+    pub reason: IllegalIndexingReason,
+    pub expression: Span,
+}
+
+impl Display for IllegalIndexing {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", Message::new(Severity::Error, &self.reason))?;
+
+        write!(
+            f,
+            "\n{}",
+            SourceCodeDisplay::new(&self.expression, Option::<u8>::None)
+        )
+    }
+}
+
+impl std::error::Error for IllegalIndexing {}
+
+/// The reason why an indexing operation is not permitted.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum IllegalIndexingReason {
+    NotIdentifier,
+    InvalidComptimeType { expected: ExpectedType },
+    IndexOutOfBounds { index: usize, length: usize },
+}
+
+impl Display for IllegalIndexingReason {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::NotIdentifier => {
+                write!(f, "The expression is not an identifier.")
+            }
+            Self::InvalidComptimeType { expected } => {
+                write!(
+                    f,
+                    "The expression can only be indexed with type {expected} that can be evaluated at compile time."
+                )
+            }
+            Self::IndexOutOfBounds { index, length } => {
+                write!(
+                    f,
+                    "The index {index} is out of bounds for the expression with length {length}."
+                )
+            }
+        }
+    }
+}
diff --git a/src/transpile/expression.rs b/src/transpile/expression.rs
index 2a94b1c..1b288a9 100644
--- a/src/transpile/expression.rs
+++ b/src/transpile/expression.rs
@@ -19,7 +19,7 @@ use shulkerbox::prelude::{Command, Condition, Execute};
 
 #[cfg(feature = "shulkerbox")]
 use super::{
-    error::{MismatchedTypes, UnknownIdentifier},
+    error::{IllegalIndexing, IllegalIndexingReason, MismatchedTypes, UnknownIdentifier},
     TranspileResult, Transpiler,
 };
 #[cfg(feature = "shulkerbox")]
@@ -31,7 +31,6 @@ use crate::{
     },
 };
 
-// TODO: fix this leading to compile errors without 'shulkerbox' feature
 /// Compile-time evaluated value
 #[allow(missing_docs)]
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -287,27 +286,29 @@ impl Primary {
                 }
             },
             Self::Indexed(indexed) => {
-                let Self::Identifier(ident) = indexed.object().as_ref() else {
-                    todo!("throw error: cannot index anything except identifiers")
-                };
-                scope
-                    .get_variable(ident.span.str())
-                    .map_or(false, |variable| match r#type {
-                        ValueType::Boolean => {
-                            matches!(
-                                variable.as_ref(),
-                                VariableData::Tag { .. } | VariableData::BooleanStorageArray { .. }
-                            )
-                        }
-                        ValueType::Integer => {
-                            matches!(
-                                variable.as_ref(),
-                                VariableData::Scoreboard { .. }
-                                    | VariableData::ScoreboardArray { .. }
-                            )
-                        }
-                        ValueType::String => false,
-                    })
+                if let Self::Identifier(ident) = indexed.object().as_ref() {
+                    scope
+                        .get_variable(ident.span.str())
+                        .map_or(false, |variable| match r#type {
+                            ValueType::Boolean => {
+                                matches!(
+                                    variable.as_ref(),
+                                    VariableData::Tag { .. }
+                                        | VariableData::BooleanStorageArray { .. }
+                                )
+                            }
+                            ValueType::Integer => {
+                                matches!(
+                                    variable.as_ref(),
+                                    VariableData::Scoreboard { .. }
+                                        | VariableData::ScoreboardArray { .. }
+                                )
+                            }
+                            ValueType::String => false,
+                        })
+                } else {
+                    false
+                }
             }
             #[cfg_attr(not(feature = "lua"), expect(unused_variables))]
             Self::Lua(lua) => {
@@ -785,9 +786,16 @@ impl Transpiler {
                 }
             }
             Primary::Indexed(indexed) => {
-                let Primary::Identifier(ident) = indexed.object().as_ref() else {
-                    todo!("can only index identifier")
-                };
+                let ident = if let Primary::Identifier(ident) = indexed.object().as_ref() {
+                    Ok(ident)
+                } else {
+                    let err = TranspileError::IllegalIndexing(IllegalIndexing {
+                        reason: IllegalIndexingReason::NotIdentifier,
+                        expression: indexed.object().span(),
+                    });
+                    handler.receive(err.clone());
+                    Err(err)
+                }?;
                 let variable = scope.get_variable(ident.span.str());
                 #[expect(clippy::option_if_let_else)]
                 if let Some(variable) = variable.as_deref() {
@@ -801,7 +809,14 @@ impl Transpiler {
                                     target,
                                 })
                             } else {
-                                todo!("can only index scoreboard with comptime string")
+                                let err = TranspileError::IllegalIndexing(IllegalIndexing {
+                                    reason: IllegalIndexingReason::InvalidComptimeType {
+                                        expected: ExpectedType::String,
+                                    },
+                                    expression: indexed.index().span(),
+                                });
+                                handler.receive(err.clone());
+                                Err(err)
                             }
                         }
                         VariableData::ScoreboardArray { objective, targets } => {
@@ -818,10 +833,25 @@ impl Transpiler {
                                         target,
                                     })
                                 } else {
-                                    todo!("index out of bounds")
+                                    let err = TranspileError::IllegalIndexing(IllegalIndexing {
+                                        reason: IllegalIndexingReason::IndexOutOfBounds {
+                                            length: targets.len(),
+                                            index: usize::try_from(index).unwrap_or(usize::MAX),
+                                        },
+                                        expression: indexed.index().span(),
+                                    });
+                                    handler.receive(err.clone());
+                                    Err(err)
                                 }
                             } else {
-                                todo!("can only index array with comptime integer")
+                                let err = TranspileError::IllegalIndexing(IllegalIndexing {
+                                    reason: IllegalIndexingReason::InvalidComptimeType {
+                                        expected: ExpectedType::Integer,
+                                    },
+                                    expression: indexed.index().span(),
+                                });
+                                handler.receive(err.clone());
+                                Err(err)
                             }
                         }
                         VariableData::BooleanStorageArray {
@@ -842,10 +872,25 @@ impl Transpiler {
                                         r#type: StorageType::Boolean,
                                     })
                                 } else {
-                                    todo!("index out of bounds")
+                                    let err = TranspileError::IllegalIndexing(IllegalIndexing {
+                                        reason: IllegalIndexingReason::IndexOutOfBounds {
+                                            length: paths.len(),
+                                            index: usize::try_from(index).unwrap_or(usize::MAX),
+                                        },
+                                        expression: indexed.index().span(),
+                                    });
+                                    handler.receive(err.clone());
+                                    Err(err)
                                 }
                             } else {
-                                todo!("can only index array with comptime integer")
+                                let err = TranspileError::IllegalIndexing(IllegalIndexing {
+                                    reason: IllegalIndexingReason::InvalidComptimeType {
+                                        expected: ExpectedType::Integer,
+                                    },
+                                    expression: indexed.index().span(),
+                                });
+                                handler.receive(err.clone());
+                                Err(err)
                             }
                         }
                         _ => {
@@ -1022,9 +1067,16 @@ impl Transpiler {
                 self.transpile_expression_as_condition(parenthesized.expression(), scope, handler)
             }
             Primary::Indexed(indexed) => {
-                let Primary::Identifier(ident) = indexed.object().as_ref() else {
-                    todo!("can only index identifier")
-                };
+                let ident = if let Primary::Identifier(ident) = indexed.object().as_ref() {
+                    Ok(ident)
+                } else {
+                    let err = TranspileError::IllegalIndexing(IllegalIndexing {
+                        reason: IllegalIndexingReason::NotIdentifier,
+                        expression: indexed.object().span(),
+                    });
+                    handler.receive(err.clone());
+                    Err(err)
+                }?;
                 #[expect(clippy::option_if_let_else)]
                 if let Some(variable) = scope.get_variable(ident.span.str()).as_deref() {
                     #[expect(clippy::single_match_else)]
@@ -1049,10 +1101,25 @@ impl Transpiler {
                                         )),
                                     ))
                                 } else {
-                                    todo!("index out of bounds")
+                                    let err = TranspileError::IllegalIndexing(IllegalIndexing {
+                                        reason: IllegalIndexingReason::IndexOutOfBounds {
+                                            length: paths.len(),
+                                            index: usize::try_from(index).unwrap_or(usize::MAX),
+                                        },
+                                        expression: indexed.index().span(),
+                                    });
+                                    handler.receive(err.clone());
+                                    Err(err)
                                 }
                             } else {
-                                todo!("can only index array with comptime integer")
+                                let err = TranspileError::IllegalIndexing(IllegalIndexing {
+                                    reason: IllegalIndexingReason::InvalidComptimeType {
+                                        expected: ExpectedType::Integer,
+                                    },
+                                    expression: indexed.index().span(),
+                                });
+                                handler.receive(err.clone());
+                                Err(err)
                             }
                         }
                         _ => {
diff --git a/src/transpile/lua.rs b/src/transpile/lua.rs
index f67b553..6f282df 100644
--- a/src/transpile/lua.rs
+++ b/src/transpile/lua.rs
@@ -4,13 +4,17 @@
 mod enabled {
     use std::sync::Arc;
 
-    use mlua::{Lua, Value};
+    use mlua::{Lua, Table, Value};
 
     use crate::{
         base::{self, source_file::SourceElement, Handler},
+        lexical::token::Identifier,
         syntax::syntax_tree::expression::LuaCode,
         transpile::{
-            error::{LuaRuntimeError, MismatchedTypes, TranspileError, TranspileResult},
+            error::{
+                LuaRuntimeError, MismatchedTypes, TranspileError, TranspileResult,
+                UnknownIdentifier,
+            },
             expression::{ComptimeValue, ExpectedType},
             Scope, VariableData,
         },
@@ -53,14 +57,8 @@ mod enabled {
                 )
             };
 
-            if let Err(err) = self.add_globals(&lua, scope) {
-                let err = TranspileError::LuaRuntimeError(LuaRuntimeError::from_lua_err(
-                    &err,
-                    self.span(),
-                ));
-                handler.receive(crate::Error::from(err.clone()));
-                return Err(err);
-            }
+            self.add_globals(&lua, scope)
+                .inspect_err(|err| handler.receive(err.clone()))?;
 
             let res = lua
                 .load(self.code())
@@ -95,111 +93,188 @@ mod enabled {
             self.handle_lua_result(lua_result, handler)
         }
 
-        fn add_globals(&self, lua: &Lua, scope: &Arc<Scope>) -> mlua::Result<()> {
+        fn add_globals(&self, lua: &Lua, scope: &Arc<Scope>) -> TranspileResult<()> {
             let globals = lua.globals();
 
-            let shulkerscript_globals = {
-                let table = lua.create_table()?;
-
-                let (location_path, location_start, location_end) = {
-                    let span = self.span();
-                    let file = span.source_file();
-                    let path = file.path().to_owned();
-                    let start_location = span.start_location();
-                    let end_location = span.end_location().unwrap_or_else(|| {
-                        let line_amount = file.line_amount();
-                        let column = file.get_line(line_amount).expect("line amount used").len();
-
-                        crate::base::source_file::Location {
-                            line: line_amount,
-                            column,
-                        }
-                    });
-
-                    (path, start_location, end_location)
-                };
-
-                table.set("file_path", location_path.to_string_lossy())?;
-                table.set("start_line", location_start.line)?;
-                table.set("start_column", location_start.column)?;
-                table.set("end_line", location_end.line)?;
-                table.set("end_column", location_end.column)?;
-
-                table.set("version", crate::VERSION)?;
-
-                table
-            };
+            let shulkerscript_globals = self
+                .get_std_library(lua)
+                .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
 
             if let Some(inputs) = self.inputs() {
-                for x in inputs.elements() {
-                    let name = x.span.str();
-                    let value = match scope.get_variable(name).as_deref() {
-                        Some(VariableData::MacroParameter { macro_name, .. }) => {
-                            Value::String(lua.create_string(format!("$({macro_name})"))?)
-                        }
-                        Some(VariableData::Scoreboard { objective }) => {
-                            let table = lua.create_table()?;
-                            table.set("objective", objective.as_str())?;
-                            Value::Table(table)
-                        }
-                        Some(VariableData::ScoreboardValue { objective, target }) => {
-                            let table = lua.create_table()?;
-                            table.set("objective", lua.create_string(objective)?)?;
-                            table.set("target", lua.create_string(target)?)?;
-                            Value::Table(table)
-                        }
-                        Some(VariableData::ScoreboardArray { objective, targets }) => {
-                            let table = lua.create_table()?;
-                            table.set("objective", objective.as_str())?;
-                            let values = lua.create_table_from(
-                                targets
-                                    .iter()
-                                    .enumerate()
-                                    .map(|(i, target)| (i + 1, target.as_str())),
-                            )?;
-                            table.set("targets", values)?;
-                            Value::Table(table)
-                        }
-                        Some(VariableData::BooleanStorage { storage_name, path }) => {
-                            let table = lua.create_table()?;
-                            table.set("storage", lua.create_string(storage_name)?)?;
-                            table.set("path", lua.create_string(path)?)?;
-                            Value::Table(table)
-                        }
-                        Some(VariableData::BooleanStorageArray {
-                            storage_name,
-                            paths,
-                        }) => {
-                            let table = lua.create_table()?;
-                            table.set("storage", storage_name.as_str())?;
-                            let values = lua.create_table_from(
-                                paths
-                                    .iter()
-                                    .enumerate()
-                                    .map(|(i, path)| (i + 1, path.as_str())),
-                            )?;
-                            table.set("paths", values)?;
-                            Value::Table(table)
-                        }
-                        Some(VariableData::Tag { tag_name }) => {
-                            let table = lua.create_table()?;
-                            table.set("name", tag_name.as_str())?;
-                            Value::Table(table)
-                        }
-                        Some(VariableData::Function { .. }) => {
-                            todo!("allow function variable type")
-                        }
-                        None => todo!("throw correct error"),
-                    };
-                    globals.set(name, value)?;
+                for identifier in inputs.elements() {
+                    let (name, value) = self.add_input_to_globals(identifier, lua, scope)?;
+                    globals
+                        .set(name, value)
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
                 }
             }
 
-            globals.set("shulkerscript", shulkerscript_globals)?;
+            globals
+                .set("shulkerscript", shulkerscript_globals)
+                .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
 
             Ok(())
         }
 
+        fn get_std_library(&self, lua: &Lua) -> mlua::Result<Table> {
+            let table = lua.create_table()?;
+
+            let (location_path, location_start, location_end) = {
+                let span = self.span();
+                let file = span.source_file();
+                let path = file.path().to_owned();
+                let start_location = span.start_location();
+                let end_location = span.end_location().unwrap_or_else(|| {
+                    let line_amount = file.line_amount();
+                    let column = file.get_line(line_amount).expect("line amount used").len();
+
+                    crate::base::source_file::Location {
+                        line: line_amount,
+                        column,
+                    }
+                });
+
+                (path, start_location, end_location)
+            };
+
+            table.set("file_path", location_path.to_string_lossy())?;
+            table.set("start_line", location_start.line)?;
+            table.set("start_column", location_start.column)?;
+            table.set("end_line", location_end.line)?;
+            table.set("end_column", location_end.column)?;
+
+            table.set("version", crate::VERSION)?;
+
+            Ok(table)
+        }
+
+        #[expect(clippy::too_many_lines)]
+        fn add_input_to_globals<'a>(
+            &self,
+            identifier: &'a Identifier,
+            lua: &Lua,
+            scope: &Arc<Scope>,
+        ) -> TranspileResult<(&'a str, Value)> {
+            let name = identifier.span.str();
+            let value = match scope.get_variable(name).as_deref() {
+                Some(VariableData::MacroParameter { macro_name, .. }) => Value::String(
+                    lua.create_string(format!("$({macro_name})"))
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?,
+                ),
+                Some(VariableData::Scoreboard { objective }) => {
+                    let table = lua
+                        .create_table()
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    table
+                        .set("objective", objective.as_str())
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    Value::Table(table)
+                }
+                Some(VariableData::ScoreboardValue { objective, target }) => {
+                    let table = lua
+                        .create_table()
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    table
+                        .set(
+                            "objective",
+                            lua.create_string(objective)
+                                .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?,
+                        )
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    table
+                        .set(
+                            "target",
+                            lua.create_string(target)
+                                .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?,
+                        )
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    Value::Table(table)
+                }
+                Some(VariableData::ScoreboardArray { objective, targets }) => {
+                    let table = lua
+                        .create_table()
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    table
+                        .set("objective", objective.as_str())
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    let values = lua
+                        .create_table_from(
+                            targets
+                                .iter()
+                                .enumerate()
+                                .map(|(i, target)| (i + 1, target.as_str())),
+                        )
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    table
+                        .set("targets", values)
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    Value::Table(table)
+                }
+                Some(VariableData::BooleanStorage { storage_name, path }) => {
+                    let table = lua
+                        .create_table()
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    table
+                        .set(
+                            "storage",
+                            lua.create_string(storage_name)
+                                .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?,
+                        )
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    table
+                        .set(
+                            "path",
+                            lua.create_string(path)
+                                .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?,
+                        )
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    Value::Table(table)
+                }
+                Some(VariableData::BooleanStorageArray {
+                    storage_name,
+                    paths,
+                }) => {
+                    let table = lua
+                        .create_table()
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    table
+                        .set("storage", storage_name.as_str())
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    let values = lua
+                        .create_table_from(
+                            paths
+                                .iter()
+                                .enumerate()
+                                .map(|(i, path)| (i + 1, path.as_str())),
+                        )
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    table
+                        .set("paths", values)
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    Value::Table(table)
+                }
+                Some(VariableData::Tag { tag_name }) => {
+                    let table = lua
+                        .create_table()
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    table
+                        .set("name", tag_name.as_str())
+                        .map_err(|err| LuaRuntimeError::from_lua_err(&err, self.span()))?;
+                    Value::Table(table)
+                }
+                Some(VariableData::Function { .. }) => {
+                    todo!("functions are not supported yet");
+                }
+                None => {
+                    return Err(TranspileError::UnknownIdentifier(UnknownIdentifier {
+                        identifier: identifier.span(),
+                    }));
+                }
+            };
+
+            Ok((name, value))
+        }
+
         fn handle_lua_result(
             &self,
             value: Value,
diff --git a/src/transpile/variables.rs b/src/transpile/variables.rs
index 489a4d7..368367e 100644
--- a/src/transpile/variables.rs
+++ b/src/transpile/variables.rs
@@ -27,7 +27,10 @@ use crate::{
 };
 
 use super::{
-    error::{AssignmentError, IllegalAnnotationContent, MismatchedTypes},
+    error::{
+        AssignmentError, IllegalAnnotationContent, IllegalIndexing, IllegalIndexingReason,
+        MismatchedTypes,
+    },
     expression::{ComptimeValue, DataLocation, ExpectedType, StorageType},
     FunctionData, TranspileAnnotationValue, TranspileError, TranspileResult,
 };
@@ -244,7 +247,9 @@ impl Transpiler {
                 scope,
                 handler,
             ),
-            _ => todo!("declarations other than single not supported yet: {declaration:?}"),
+            _ => todo!(
+                "declarations other than single and scoreboard not supported yet: {declaration:?}"
+            ),
         }
     }
 
@@ -388,9 +393,23 @@ impl Transpiler {
                     Some(ComptimeValue::MacroString(s)) => {
                         todo!("indexing scoreboard with macro string: {s}")
                     }
-                    Some(_) => todo!("invalid indexing value"),
+                    Some(_) => {
+                        let err = TranspileError::IllegalIndexing(IllegalIndexing {
+                            expression: expression.span(),
+                            reason: IllegalIndexingReason::InvalidComptimeType {
+                                expected: ExpectedType::String,
+                            },
+                        });
+                        handler.receive(err.clone());
+                        return Err(err);
+                    }
                     None => {
-                        todo!("cannot assign to scoreboard without indexing")
+                        let err = TranspileError::AssignmentError(AssignmentError {
+                            identifier: identifier.span(),
+                            message: "Cannot assign to a scoreboard without indexing".to_string(),
+                        });
+                        handler.receive(err.clone());
+                        return Err(err);
                     }
                 },
                 VariableData::Function { .. } | VariableData::MacroParameter { .. } => {