Avoid invalid identifiers in proc/macro/scope

This was apparently neglected and you were able to pick invalid
identifiers to identify procs, macros and scopes. Ensure this does not
happen again and provide tests for it.

Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>

5 files changed, 81 insertions, 7 deletions

diff --git a/lib/xixanta/src/assembler.rs b/lib/xixanta/src/assembler.rs
index cb992d4..136d416 100644
--- a/lib/xixanta/src/assembler.rs
+++ b/lib/xixanta/src/assembler.rs
@@ -588,12 +588,25 @@ impl<'a> Assembler<'a> {
                             // illegal definitions.
                             self.macros_seen += 1;
 
-                            // Insert a reference to this node so it can be
-                            // unrolled whenever we have to perform a macro
-                            // call.
-                            self.macros
-                                .entry(node.left.as_ref().unwrap().value.value.clone())
-                                .or_insert(node);
+                            let name = &node.left.as_ref().unwrap().value;
+                            if let Err(e) = name.is_valid_identifier(false) {
+                                errors.push(Error {
+                                    message: format!(
+                                        "'{}' is not a valid macro name: {e}",
+                                        name.value
+                                    ),
+                                    line: node.value.line,
+                                    global: false,
+                                    expanded_from: self.macro_context.clone(),
+                                    source: self.source_for(node),
+                                });
+                                continue;
+                            } else {
+                                // Insert a reference to this node so it can be
+                                // unrolled whenever we have to perform a macro
+                                // call.
+                                self.macros.entry(name.value.clone()).or_insert(node);
+                            }
                         }
                         ControlType::EndMacro => {
                             if self.macros_seen > 0 {
@@ -616,7 +629,19 @@ impl<'a> Assembler<'a> {
 
                             self.procs_seen += 1;
                             let proc_name = &node.left.as_ref().unwrap();
-                            if let Err(err) = self.define_variable(proc_name) {
+                            if let Err(err) = proc_name.value.is_valid_identifier(false) {
+                                errors.push(Error {
+                                    message: format!(
+                                        "'{}' is not a valid proc name: {err}",
+                                        proc_name.value.value,
+                                    ),
+                                    line: node.value.line,
+                                    global: false,
+                                    expanded_from: self.macro_context.clone(),
+                                    source: self.source_for(node),
+                                });
+                                continue;
+                            } else if let Err(err) = self.define_variable(proc_name) {
                                 errors.push(err);
                             }
                         }
@@ -638,6 +663,20 @@ impl<'a> Assembler<'a> {
                                 });
                                 continue;
                             }
+                            let scope_name = &node.left.as_ref().unwrap();
+                            if let Err(err) = scope_name.value.is_valid_identifier(false) {
+                                errors.push(Error {
+                                    message: format!(
+                                        "'{}' is not a valid scope name: {err}",
+                                        scope_name.value.value,
+                                    ),
+                                    line: node.value.line,
+                                    global: false,
+                                    expanded_from: self.macro_context.clone(),
+                                    source: self.source_for(node),
+                                });
+                                continue;
+                            }
                         }
                         ControlType::StartRepeat => {
                             self.repeats_seen += 1;
@@ -4758,6 +4797,18 @@ MACRO Var1
     }
 
     #[test]
+    fn bad_macro_name() {
+        assert_error(
+            r#".macro __fallthrough__ arg
+    .endmacro
+    "#,
+            1,
+            false,
+            "'__fallthrough__' is not a valid macro name: cannot use reserved name '__fallthrough__'",
+        );
+    }
+
+    #[test]
     fn bad_scope_definition_inside_of_proc() {
         assert_error(
             r#".proc Hey
diff --git a/scripts/test-e2e.sh b/scripts/test-e2e.sh
index 18dbeff..06545b8 100755
--- a/scripts/test-e2e.sh
+++ b/scripts/test-e2e.sh
@@ -88,6 +88,13 @@ exit_code=$((exit_code + $?))
 diff tests/out/fallthrough.nes tests/expected/fallthrough.nes
 exit_code=$((exit_code + $?))
 
+echo "test: custom => avoid_bad_macro.nes"
+./target/debug/nasm -c empty --asan tests/avoid_bad_macro.s -o tests/out/avoid_bad_macro.nes 2>tests/out/avoid_bad_macro.txt
+diff tests/out/avoid_bad_macro.txt tests/expected/avoid_bad_macro.txt
+exit_code=$((exit_code + $?))
+diff tests/out/avoid_bad_macro.nes tests/expected/avoid_bad_macro.nes
+exit_code=$((exit_code + $?))
+
 ##
 # code.nes
 
diff --git a/tests/avoid_bad_macro.s b/tests/avoid_bad_macro.s
new file mode 100644
index 0000000..f701c2b
--- /dev/null
+++ b/tests/avoid_bad_macro.s
@@ -0,0 +1,16 @@
+.segment "HEADER"
+    .byte 'N', 'E', 'S', $1A
+    .byte $02
+    .byte $01
+    .byte $00
+    .byte $00
+
+.segment "CODE"
+
+.ifndef __NASM__
+    .macro __fallthrough__ arg
+        lda #0
+    .endmacro
+.endif
+
+lda #1
diff --git a/tests/expected/avoid_bad_macro.nes b/tests/expected/avoid_bad_macro.nes
new file mode 100644
index 0000000..eb99eb1
--- /dev/null
+++ b/tests/expected/avoid_bad_macro.nes
diff --git a/tests/expected/avoid_bad_macro.txt b/tests/expected/avoid_bad_macro.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tests/expected/avoid_bad_macro.txt