From cf01db5e28711a1094c3631d2f9e81550ba551d5 Mon Sep 17 00:00:00 2001 From: Mattias Hansson Date: Fri, 3 Jul 2026 17:07:41 +0200 Subject: [PATCH] Fixed reported Starlark error line numbers to reflect the source files. --- internal/compiler/compiler_test.go | 80 ++++++++++++++++++++ internal/compiler/scriptexec.go | 54 +++++++++++--- internal/preproc/preproc.go | 16 +++- internal/preproc/preproc_test.go | 116 ++++++++++++++++++++--------- 4 files changed, 221 insertions(+), 45 deletions(-) diff --git a/internal/compiler/compiler_test.go b/internal/compiler/compiler_test.go index d9496e4..d1c2c71 100644 --- a/internal/compiler/compiler_test.go +++ b/internal/compiler/compiler_test.go @@ -1232,3 +1232,83 @@ func testExecuteScript(scriptLines []string, ctx *CompilerContext, isLibrary boo } return executeScript(lines, ctx, isLibrary) } + +func TestCompile_MultipleScriptBlocks_Success(t *testing.T) { + // Two consecutive SCRIPT blocks should execute independently + pragma := preproc.NewPragma() + comp := NewCompiler(pragma) + + lines := []preproc.Line{ + // Block 1: print("hello") + {Text: "print('hello')", Filename: "test.c65", LineNo: 2, Kind: preproc.Script}, + // ENDSCRIPT boundary (empty Source) + {Text: "", Filename: "test.c65", LineNo: 3, Kind: preproc.Source}, + // Block 2: print("world") + {Text: "print('world')", Filename: "test.c65", LineNo: 5, Kind: preproc.Script}, + // ENDSCRIPT boundary (empty Source) + {Text: "", Filename: "test.c65", LineNo: 6, Kind: preproc.Source}, + } + + output, err := comp.Compile(lines) + if err != nil { + t.Fatalf("Compile failed: %v", err) + } + + // Output should contain both "hello" and "world" in that order + foundHello := false + foundWorld := false + helloBeforeWorld := false + for _, line := range output { + if strings.Contains(line, "hello") && !foundHello { + foundHello = true + } + if foundHello && strings.Contains(line, "world") && !foundWorld { + foundWorld = true + helloBeforeWorld = true + } + } + if !foundHello { + t.Error("expected 'hello' from block 1 in output") + } + if !foundWorld { + t.Error("expected 'world' from block 2 in output") + } + if !helloBeforeWorld { + t.Error("expected 'hello' before 'world' in output") + } +} + +func TestCompile_MultipleScriptBlocks_ErrorInBlock2(t *testing.T) { + // Error in second SCRIPT block should not be confused with block 1 + pragma := preproc.NewPragma() + comp := NewCompiler(pragma) + + lines := []preproc.Line{ + // Block 1: no error + {Text: "x = 1", Filename: "test.c65", LineNo: 2, Kind: preproc.Script}, + {Text: "print(x)", Filename: "test.c65", LineNo: 3, Kind: preproc.Script}, + // ENDSCRIPT boundary + {Text: "", Filename: "test.c65", LineNo: 4, Kind: preproc.Source}, + // Block 2: error at line 8 (division by zero) + {Text: "y = 2", Filename: "test.c65", LineNo: 6, Kind: preproc.Script}, + {Text: "z = y + 1", Filename: "test.c65", LineNo: 7, Kind: preproc.Script}, + {Text: "1 / 0", Filename: "test.c65", LineNo: 8, Kind: preproc.Script}, + // ENDSCRIPT boundary + {Text: "", Filename: "test.c65", LineNo: 9, Kind: preproc.Source}, + } + + _, err := comp.Compile(lines) + if err == nil { + t.Fatal("expected error from block 2, got none") + } + + errMsg := err.Error() + if !strings.Contains(errMsg, "Starlark error") { + t.Errorf("expected Starlark error, got: %s", errMsg) + } + // Verify error references the correct source line in block 2 (line 8 = division by zero) + if !strings.Contains(errMsg, ":8:") { + t.Errorf("error should reference source line 8 in block 2, got: %s", errMsg) + } +} + diff --git a/internal/compiler/scriptexec.go b/internal/compiler/scriptexec.go index c9a8152..b83aa96 100644 --- a/internal/compiler/scriptexec.go +++ b/internal/compiler/scriptexec.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "strconv" "strings" @@ -38,6 +39,10 @@ func mapStarlarkLine(starlarkLine int, numScriptLines int, isLibrary bool) int { return idx } +// starlarkErrorMsgLineMatch matches Starlark error messages of the form +// "filename:LINE:COL: message" produced by resolve errors. +var starlarkErrorMsgLineMatch = regexp.MustCompile(`^(.+):(\d+):(\d+): (.*)$`) + // starlarkPosition extracts the source position from a Starlark error. // Returns the 1-based line number (0 if unknown). func starlarkPosition(err error) int { @@ -48,16 +53,43 @@ func starlarkPosition(err error) int { return int(top.Pos.Line) } } + // For non-EvalError (resolve errors), extract line from the message + return parseStarlarkLineFromMsg(err.Error()) +} + +// parseStarlarkLineFromMsg attempts to extract the Starlark line number from +// a non-EvalError error message string. Starlark resolve errors follow the +// format "filename:LINE:COL: message". +func parseStarlarkLineFromMsg(msg string) int { + matches := starlarkErrorMsgLineMatch.FindStringSubmatch(msg) + if len(matches) >= 3 { + line, err := strconv.Atoi(matches[2]) + if err == nil && line > 0 { + return line + } + } return 0 } // starlarkErrorMsg extracts just the message from a Starlark error. +// For EvalError, it returns the Msg field directly. +// For non-EvalError (resolve errors), it strips the "filename:LINE:COL: " prefix. func starlarkErrorMsg(err error) string { var evalErr *starlark.EvalError if errors.As(err, &evalErr) { return evalErr.Msg } - return err.Error() + return stripStarlarkPositionFromMsg(err.Error()) +} + +// stripStarlarkPositionFromMsg removes the "filename:LINE:COL: " prefix from +// a Starlark error message if present. +func stripStarlarkPositionFromMsg(msg string) string { + matches := starlarkErrorMsgLineMatch.FindStringSubmatch(msg) + if len(matches) >= 5 { + return matches[4] + } + return msg } // printScriptErrorContext prints a Starlark error with script-block-bounded source context, @@ -230,17 +262,19 @@ func executeScript(scriptLines []preproc.Line, ctx *CompilerContext, isLibrary b if err != nil { // Map Starlark error position back to source starLine := starlarkPosition(err) + idx := -1 if starLine > 0 { - idx := mapStarlarkLine(starLine, len(scriptLines), isLibrary) - if idx >= 0 { - msg := starlarkErrorMsg(err) - blockType := "SCRIPT" - if isLibrary { - blockType = "SCRIPT LIBRARY" - } - printScriptErrorContext(err, msg, scriptLines, idx, blockType) - return nil, fmt.Errorf("Starlark error: %s", msg) + idx = mapStarlarkLine(starLine, len(scriptLines), isLibrary) + } + if idx >= 0 { + msg := starlarkErrorMsg(err) + blockType := "SCRIPT" + if isLibrary { + blockType = "SCRIPT LIBRARY" } + printScriptErrorContext(err, msg, scriptLines, idx, blockType) + srcLine := scriptLines[idx] + return nil, fmt.Errorf("Starlark error: %s:%d: %s", srcLine.Filename, srcLine.LineNo, msg) } // Fallback: print whatever info we can extract printScriptErrorFallback(err) diff --git a/internal/preproc/preproc.go b/internal/preproc/preproc.go index 7e683fa..6d69ebc 100644 --- a/internal/preproc/preproc.go +++ b/internal/preproc/preproc.go @@ -170,7 +170,21 @@ func (p *preproc) run(root string) ([]Line, error) { p.inScript = false p.inScriptLibrary = false p.inScriptMacro = false - continue // don't emit ENDSCRIPT marker + // Emit an empty Source line as a block boundary marker so the + // compiler can distinguish consecutive SCRIPT blocks via kind + // transitions. The compiler skips empty Source lines, so this + // serves purely as a transition trigger. + if includeSource { + out = append(out, Line{ + RawText: raw, + Text: "", + Filename: currFrame.path, + LineNo: currFrame.line, + Kind: Source, + PragmaSetIndex: p.pragma.GetCurrentPragmaSetIndex(), + }) + } + continue } // Determine the kind based on which mode we're in kind := Script diff --git a/internal/preproc/preproc_test.go b/internal/preproc/preproc_test.go index f21d475..9bb39ca 100644 --- a/internal/preproc/preproc_test.go +++ b/internal/preproc/preproc_test.go @@ -151,9 +151,10 @@ func TestPreProcess_ScriptBlock(t *testing.T) { t.Fatalf("PreProcess failed: %v", err) } - // SCRIPT and ENDSCRIPT markers are stripped - if len(lines) != 3 { - t.Fatalf("expected 3 lines, got %d", len(lines)) + // SCRIPT and ENDSCRIPT markers are stripped, but ENDSCRIPT emits + // an empty Source boundary marker + if len(lines) != 4 { + t.Fatalf("expected 4 lines, got %d", len(lines)) } // Script content should NOT be processed @@ -171,12 +172,20 @@ func TestPreProcess_ScriptBlock(t *testing.T) { t.Errorf("expected Kind=Script, got %v", lines[1].Kind) } - // After ENDSCRIPT, defines work again - if lines[2].Text != "LDA #100" { - t.Errorf("expected 'LDA #100', got %q", lines[2].Text) - } + // Line 2 is the ENDSCRIPT boundary marker (empty Source line) if lines[2].Kind != Source { - t.Errorf("expected Kind=Source, got %v", lines[2].Kind) + t.Errorf("line 2: expected Kind=Source (ENDSCRIPT boundary), got %v", lines[2].Kind) + } + if lines[2].Text != "" { + t.Errorf("line 2: expected empty text, got %q", lines[2].Text) + } + + // After ENDSCRIPT, defines work again + if lines[3].Text != "LDA #100" { + t.Errorf("expected 'LDA #100', got %q", lines[3].Text) + } + if lines[3].Kind != Source { + t.Errorf("expected Kind=Source, got %v", lines[3].Kind) } } @@ -262,8 +271,9 @@ func TestPreProcess_CommentInScriptBlock(t *testing.T) { t.Fatalf("PreProcess failed: %v", err) } - if len(lines) != 2 { - t.Fatalf("expected 2 lines, got %d", len(lines)) + // 2 script lines + 1 ENDSCRIPT boundary marker + 0 trailing source = 3 lines + if len(lines) != 3 { + t.Fatalf("expected 3 lines, got %d", len(lines)) } // Comments should be preserved in Script blocks @@ -273,6 +283,11 @@ func TestPreProcess_CommentInScriptBlock(t *testing.T) { if lines[1].Text != " y = 2 // another one" { t.Errorf("expected comment preserved, got %q", lines[1].Text) } + + // Line 2 is ENDSCRIPT boundary marker + if lines[2].Kind != Source || lines[2].Text != "" { + t.Errorf("line 2: expected empty Source (ENDSCRIPT boundary), got Kind=%v Text=%q", lines[2].Kind, lines[2].Text) + } } func TestPreProcess_RawTextPreservation(t *testing.T) { @@ -883,6 +898,7 @@ func TestPreProcess_MixedBlocksAndComments(t *testing.T) { {"LDA #10", Source}, {" lda #X // asm comment", Assembler}, {" y = X // script comment", Script}, + {"", Source}, // ENDSCRIPT boundary marker {"STA $D020", Source}, } @@ -938,12 +954,17 @@ func TestPreProcess_EmptyScriptBlock(t *testing.T) { t.Fatalf("PreProcess failed: %v", err) } - if len(lines) != 1 { - t.Fatalf("expected 1 line, got %d", len(lines)) + // Empty script emits an ENDSCRIPT boundary marker + NOP + if len(lines) != 2 { + t.Fatalf("expected 2 lines, got %d", len(lines)) } - if lines[0].Text != "NOP" { - t.Errorf("expected 'NOP', got %q", lines[0].Text) + if lines[0].Kind != Source || lines[0].Text != "" { + t.Errorf("line 0: expected empty Source (ENDSCRIPT boundary), got Kind=%v Text=%q", lines[0].Kind, lines[0].Text) + } + + if lines[1].Text != "NOP" { + t.Errorf("expected 'NOP', got %q", lines[1].Text) } } @@ -963,9 +984,9 @@ func TestPreProcess_ScriptLibraryBlock(t *testing.T) { t.Fatalf("PreProcess failed: %v", err) } - // Should have 2 script lines + 1 source line - if len(lines) != 3 { - t.Fatalf("expected 3 lines, got %d", len(lines)) + // Should have 2 script lines + 1 ENDSCRIPT boundary + 1 source line = 4 lines + if len(lines) != 4 { + t.Fatalf("expected 4 lines, got %d", len(lines)) } // Script library lines should have ScriptLibrary kind @@ -980,12 +1001,20 @@ func TestPreProcess_ScriptLibraryBlock(t *testing.T) { t.Errorf("expected Kind=ScriptLibrary, got %v", lines[1].Kind) } - // Source line after ENDSCRIPT + // Line 2 is the ENDSCRIPT boundary marker if lines[2].Kind != Source { - t.Errorf("expected Kind=Source, got %v", lines[2].Kind) + t.Errorf("line 2: expected Kind=Source (ENDSCRIPT boundary), got %v", lines[2].Kind) } - if lines[2].Text != "NOP" { - t.Errorf("expected 'NOP', got %q", lines[2].Text) + if lines[2].Text != "" { + t.Errorf("line 2: expected empty text, got %q", lines[2].Text) + } + + // Source line after ENDSCRIPT + if lines[3].Kind != Source { + t.Errorf("expected Kind=Source, got %v", lines[3].Kind) + } + if lines[3].Text != "NOP" { + t.Errorf("expected 'NOP', got %q", lines[3].Text) } } @@ -1006,18 +1035,29 @@ func TestPreProcess_ScriptVsScriptLibrary(t *testing.T) { t.Fatalf("PreProcess failed: %v", err) } - if len(lines) != 2 { - t.Fatalf("expected 2 lines, got %d", len(lines)) + // Two blocks → 1 lib line + 1 lib ENDSCRIPT boundary + 1 script line + 1 script ENDSCRIPT boundary = 4 lines + if len(lines) != 4 { + t.Fatalf("expected 4 lines, got %d", len(lines)) } - // First line is from SCRIPT LIBRARY + // Line 0: SCRIPT LIBRARY content if lines[0].Kind != ScriptLibrary { t.Errorf("line 0: expected ScriptLibrary, got %v", lines[0].Kind) } - // Second line is from regular SCRIPT - if lines[1].Kind != Script { - t.Errorf("line 1: expected Script, got %v", lines[1].Kind) + // Line 1: ENDSCRIPT boundary for library + if lines[1].Kind != Source || lines[1].Text != "" { + t.Errorf("line 1: expected empty Source (ENDSCRIPT boundary), got Kind=%v Text=%q", lines[1].Kind, lines[1].Text) + } + + // Line 2: regular SCRIPT content + if lines[2].Kind != Script { + t.Errorf("line 2: expected Script, got %v", lines[2].Kind) + } + + // Line 3: ENDSCRIPT boundary for script + if lines[3].Kind != Source || lines[3].Text != "" { + t.Errorf("line 3: expected empty Source (ENDSCRIPT boundary), got Kind=%v Text=%q", lines[3].Kind, lines[3].Text) } } @@ -1036,9 +1076,9 @@ func TestPreProcess_ScriptMacroBlock(t *testing.T) { t.Fatalf("PreProcess failed: %v", err) } - // Should have header + body line + source line = 3 lines - if len(lines) != 3 { - t.Fatalf("expected 3 lines, got %d", len(lines)) + // Should have header + body line + ENDSCRIPT boundary + source line = 4 lines + if len(lines) != 4 { + t.Fatalf("expected 4 lines, got %d", len(lines)) } // First line is the header (also ScriptMacroDef kind) @@ -1054,12 +1094,20 @@ func TestPreProcess_ScriptMacroBlock(t *testing.T) { t.Errorf("line 1: expected ScriptMacroDef, got %v", lines[1].Kind) } - // Third line is source + // Third line is ENDSCRIPT boundary marker if lines[2].Kind != Source { - t.Errorf("line 2: expected Source, got %v", lines[2].Kind) + t.Errorf("line 2: expected Source (ENDSCRIPT boundary), got %v", lines[2].Kind) } - if lines[2].Text != "NOP" { - t.Errorf("line 2: expected 'NOP', got %q", lines[2].Text) + if lines[2].Text != "" { + t.Errorf("line 2: expected empty text, got %q", lines[2].Text) + } + + // Fourth line is source + if lines[3].Kind != Source { + t.Errorf("line 3: expected Source, got %v", lines[3].Kind) + } + if lines[3].Text != "NOP" { + t.Errorf("line 3: expected 'NOP', got %q", lines[3].Text) } }