Fixed reported Starlark error line numbers to reflect the source files.

This commit is contained in:
Mattias Hansson 2026-07-03 17:07:41 +02:00
parent a1c9e16abe
commit cf01db5e28
4 changed files with 221 additions and 45 deletions

View file

@ -1232,3 +1232,83 @@ func testExecuteScript(scriptLines []string, ctx *CompilerContext, isLibrary boo
} }
return executeScript(lines, ctx, isLibrary) 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)
}
}

View file

@ -6,6 +6,7 @@ import (
"fmt" "fmt"
"os" "os"
"path/filepath" "path/filepath"
"regexp"
"strconv" "strconv"
"strings" "strings"
@ -38,6 +39,10 @@ func mapStarlarkLine(starlarkLine int, numScriptLines int, isLibrary bool) int {
return idx 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. // starlarkPosition extracts the source position from a Starlark error.
// Returns the 1-based line number (0 if unknown). // Returns the 1-based line number (0 if unknown).
func starlarkPosition(err error) int { func starlarkPosition(err error) int {
@ -48,16 +53,43 @@ func starlarkPosition(err error) int {
return int(top.Pos.Line) 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 return 0
} }
// starlarkErrorMsg extracts just the message from a Starlark error. // 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 { func starlarkErrorMsg(err error) string {
var evalErr *starlark.EvalError var evalErr *starlark.EvalError
if errors.As(err, &evalErr) { if errors.As(err, &evalErr) {
return evalErr.Msg 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, // 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 { if err != nil {
// Map Starlark error position back to source // Map Starlark error position back to source
starLine := starlarkPosition(err) starLine := starlarkPosition(err)
idx := -1
if starLine > 0 { if starLine > 0 {
idx := mapStarlarkLine(starLine, len(scriptLines), isLibrary) idx = mapStarlarkLine(starLine, len(scriptLines), isLibrary)
if idx >= 0 { }
msg := starlarkErrorMsg(err) if idx >= 0 {
blockType := "SCRIPT" msg := starlarkErrorMsg(err)
if isLibrary { blockType := "SCRIPT"
blockType = "SCRIPT LIBRARY" if isLibrary {
} blockType = "SCRIPT LIBRARY"
printScriptErrorContext(err, msg, scriptLines, idx, blockType)
return nil, fmt.Errorf("Starlark error: %s", msg)
} }
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 // Fallback: print whatever info we can extract
printScriptErrorFallback(err) printScriptErrorFallback(err)

View file

@ -170,7 +170,21 @@ func (p *preproc) run(root string) ([]Line, error) {
p.inScript = false p.inScript = false
p.inScriptLibrary = false p.inScriptLibrary = false
p.inScriptMacro = 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 // Determine the kind based on which mode we're in
kind := Script kind := Script

View file

@ -151,9 +151,10 @@ func TestPreProcess_ScriptBlock(t *testing.T) {
t.Fatalf("PreProcess failed: %v", err) t.Fatalf("PreProcess failed: %v", err)
} }
// SCRIPT and ENDSCRIPT markers are stripped // SCRIPT and ENDSCRIPT markers are stripped, but ENDSCRIPT emits
if len(lines) != 3 { // an empty Source boundary marker
t.Fatalf("expected 3 lines, got %d", len(lines)) if len(lines) != 4 {
t.Fatalf("expected 4 lines, got %d", len(lines))
} }
// Script content should NOT be processed // 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) t.Errorf("expected Kind=Script, got %v", lines[1].Kind)
} }
// After ENDSCRIPT, defines work again // Line 2 is the ENDSCRIPT boundary marker (empty Source line)
if lines[2].Text != "LDA #100" {
t.Errorf("expected 'LDA #100', got %q", lines[2].Text)
}
if lines[2].Kind != Source { 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) t.Fatalf("PreProcess failed: %v", err)
} }
if len(lines) != 2 { // 2 script lines + 1 ENDSCRIPT boundary marker + 0 trailing source = 3 lines
t.Fatalf("expected 2 lines, got %d", len(lines)) if len(lines) != 3 {
t.Fatalf("expected 3 lines, got %d", len(lines))
} }
// Comments should be preserved in Script blocks // 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" { if lines[1].Text != " y = 2 // another one" {
t.Errorf("expected comment preserved, got %q", lines[1].Text) 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) { func TestPreProcess_RawTextPreservation(t *testing.T) {
@ -883,6 +898,7 @@ func TestPreProcess_MixedBlocksAndComments(t *testing.T) {
{"LDA #10", Source}, {"LDA #10", Source},
{" lda #X // asm comment", Assembler}, {" lda #X // asm comment", Assembler},
{" y = X // script comment", Script}, {" y = X // script comment", Script},
{"", Source}, // ENDSCRIPT boundary marker
{"STA $D020", Source}, {"STA $D020", Source},
} }
@ -938,12 +954,17 @@ func TestPreProcess_EmptyScriptBlock(t *testing.T) {
t.Fatalf("PreProcess failed: %v", err) t.Fatalf("PreProcess failed: %v", err)
} }
if len(lines) != 1 { // Empty script emits an ENDSCRIPT boundary marker + NOP
t.Fatalf("expected 1 line, got %d", len(lines)) if len(lines) != 2 {
t.Fatalf("expected 2 lines, got %d", len(lines))
} }
if lines[0].Text != "NOP" { if lines[0].Kind != Source || lines[0].Text != "" {
t.Errorf("expected 'NOP', got %q", 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) t.Fatalf("PreProcess failed: %v", err)
} }
// Should have 2 script lines + 1 source line // Should have 2 script lines + 1 ENDSCRIPT boundary + 1 source line = 4 lines
if len(lines) != 3 { if len(lines) != 4 {
t.Fatalf("expected 3 lines, got %d", len(lines)) t.Fatalf("expected 4 lines, got %d", len(lines))
} }
// Script library lines should have ScriptLibrary kind // 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) 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 { 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" { if lines[2].Text != "" {
t.Errorf("expected 'NOP', got %q", 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) t.Fatalf("PreProcess failed: %v", err)
} }
if len(lines) != 2 { // Two blocks → 1 lib line + 1 lib ENDSCRIPT boundary + 1 script line + 1 script ENDSCRIPT boundary = 4 lines
t.Fatalf("expected 2 lines, got %d", len(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 { if lines[0].Kind != ScriptLibrary {
t.Errorf("line 0: expected ScriptLibrary, got %v", lines[0].Kind) t.Errorf("line 0: expected ScriptLibrary, got %v", lines[0].Kind)
} }
// Second line is from regular SCRIPT // Line 1: ENDSCRIPT boundary for library
if lines[1].Kind != Script { if lines[1].Kind != Source || lines[1].Text != "" {
t.Errorf("line 1: expected Script, got %v", lines[1].Kind) 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) t.Fatalf("PreProcess failed: %v", err)
} }
// Should have header + body line + source line = 3 lines // Should have header + body line + ENDSCRIPT boundary + source line = 4 lines
if len(lines) != 3 { if len(lines) != 4 {
t.Fatalf("expected 3 lines, got %d", len(lines)) t.Fatalf("expected 4 lines, got %d", len(lines))
} }
// First line is the header (also ScriptMacroDef kind) // 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) 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 { 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" { if lines[2].Text != "" {
t.Errorf("line 2: expected 'NOP', got %q", 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)
} }
} }