fix(diagnostics): locate import parse errors in the imported file
A parse error raised while resolving an `#import` was rendered against the ROOT file's source — the caret landed on an unrelated line (often a comment) even though the message named the correct imported file. Two compounding causes: - core.zig wired `diagnostics.import_sources` only AFTER import resolution returned, but a parse error aborts mid-resolution (before that wiring), so the renderer had no imported sources and fell back to the root file. Wire it (and seed the main-file source) BEFORE resolving. - imports.zig emitted the diagnostic at the importer's `#import` span instead of the parser's actual error offset inside the imported file, and didn't pin the diagnostic's source_file to that file. parser.zig now records `err_end` alongside `err_offset` for a proper caret width. New `DiagnosticList.addFmtInFile` renders against an explicit source file; imports.zig uses it with `importErrSpan(&p)`. Regression test: examples/1176-diagnostics-import-parse-error-location (importer + deliberately-broken companion; caret must land in the companion).
This commit is contained in:
13
examples/1176-diagnostics-import-parse-error-location.sx
Normal file
13
examples/1176-diagnostics-import-parse-error-location.sx
Normal file
@@ -0,0 +1,13 @@
|
||||
// A parse error in an IMPORTED file must be located in THAT file, not the
|
||||
// importer. Regression: `import_sources` was wired to the diagnostics only
|
||||
// AFTER import resolution finished, so a parse error raised MID-resolution
|
||||
// (which aborts before that wiring) could not resolve the imported file's
|
||||
// source — the caret fell back to the root file and landed on an unrelated
|
||||
// line. The fix wires `import_sources` before resolving and pins the
|
||||
// diagnostic's `source_file` + offset to the imported file.
|
||||
//
|
||||
// The companion's error sits several lines down (after comments) so a caret
|
||||
// mislocated against THIS importer would be unmistakable.
|
||||
#import "1176-diagnostics-import-parse-error-location/broken.sx";
|
||||
|
||||
main :: () {}
|
||||
@@ -0,0 +1,6 @@
|
||||
// Deliberately broken: exercises import parse-error LOCATION reporting.
|
||||
// These leading comment lines push the parse error down so its line number
|
||||
// differs from the importer's — a mislocated caret would point here-or-wrong
|
||||
// instead of at the real offending token below.
|
||||
//
|
||||
broken :: 1 2;
|
||||
@@ -0,0 +1 @@
|
||||
1
|
||||
@@ -0,0 +1,5 @@
|
||||
error: parse error in 'examples/1176-diagnostics-import-parse-error-location/broken.sx': expected ';'
|
||||
--> examples/1176-diagnostics-import-parse-error-location/broken.sx:6:13
|
||||
|
|
||||
6 | broken :: 1 2;
|
||||
| ^
|
||||
@@ -0,0 +1 @@
|
||||
|
||||
17
src/core.zig
17
src/core.zig
@@ -106,6 +106,16 @@ pub const Compilation = struct {
|
||||
var chain = std.StringHashMap(void).init(self.allocator);
|
||||
var cache = imports.ModuleCache.init(self.allocator);
|
||||
const base_dir = imports.dirName(self.file_path);
|
||||
|
||||
// Wire import_sources to diagnostics BEFORE resolving imports, so a parse
|
||||
// error in an imported file (reported mid-resolution, which then aborts
|
||||
// before the post-resolution wiring below) can resolve its caret against
|
||||
// the imported file's OWN source. The map pointer is stable; per-file
|
||||
// entries fill in as imports load. The main-file source is seeded here
|
||||
// too so a root-file diagnostic resolves identically.
|
||||
self.import_sources.put(self.file_path, self.source) catch {};
|
||||
self.diagnostics.import_sources = &self.import_sources;
|
||||
|
||||
const mod = imports.resolveImports(
|
||||
self.allocator,
|
||||
self.io,
|
||||
@@ -145,11 +155,8 @@ pub const Compilation = struct {
|
||||
// `namespace_edges` in place to record each target's member ids.
|
||||
self.decl_table = try imports.buildDeclTable(self.allocator, self.file_path, mod, &cache, &self.module_decls, &self.namespace_edges);
|
||||
|
||||
// Store main file source in import_sources so error reporting can find it
|
||||
self.import_sources.put(self.file_path, self.source) catch {};
|
||||
|
||||
// Wire import_sources to diagnostics for file-aware error rendering
|
||||
self.diagnostics.import_sources = &self.import_sources;
|
||||
// (import_sources ↔ diagnostics wiring + main-file seed now done before
|
||||
// resolution, above, so mid-resolution parse errors render correctly.)
|
||||
|
||||
// Build a root node from the resolved module's decls
|
||||
const new_root = try self.allocator.create(Node);
|
||||
|
||||
@@ -179,6 +179,18 @@ pub const DiagnosticList = struct {
|
||||
_ = self.addFmtId(level, span, fmt, args);
|
||||
}
|
||||
|
||||
/// Like `addFmt`, but renders against an EXPLICIT source file (resolved via
|
||||
/// `import_sources`) instead of the ambient `current_source_file`. Used to
|
||||
/// pin a diagnostic whose `span` is an offset into a NON-current file — e.g.
|
||||
/// a parse error raised while resolving an `#import`, where the span belongs
|
||||
/// to the imported file, not the importer.
|
||||
pub fn addFmtInFile(self: *DiagnosticList, level: Level, source_file: []const u8, span: ?Span, comptime fmt: []const u8, args: anytype) void {
|
||||
const saved = self.current_source_file;
|
||||
self.current_source_file = source_file;
|
||||
defer self.current_source_file = saved;
|
||||
_ = self.addFmtId(level, span, fmt, args);
|
||||
}
|
||||
|
||||
pub fn addFmtId(self: *DiagnosticList, level: Level, span: ?Span, comptime fmt: []const u8, args: anytype) usize {
|
||||
const message = std.fmt.allocPrint(self.allocator, fmt, args) catch "diagnostic format error";
|
||||
return self.addId(level, message, span);
|
||||
|
||||
@@ -796,6 +796,17 @@ fn reportDuplicateName(diagnostics: ?*errors.DiagnosticList, added: bool, name:
|
||||
diags.addFmt(.err, span, "duplicate top-level declaration '{s}'", .{name});
|
||||
}
|
||||
|
||||
/// Build the diagnostic span for a failed import parse, from the parser's
|
||||
/// ACTUAL error location INSIDE the imported file (`err_offset`/`err_end`) —
|
||||
/// not the importing file's `#import` span. Pair with `addFmtInFile` so the
|
||||
/// caret resolves against the imported file's own source (the source is already
|
||||
/// registered in `import_sources`); otherwise it falls back to the root file
|
||||
/// and the caret lands on an unrelated line.
|
||||
fn importErrSpan(p: *const parser.Parser) ast.Span {
|
||||
const start = p.err_offset orelse 0;
|
||||
return .{ .start = start, .end = p.err_end orelse start };
|
||||
}
|
||||
|
||||
/// Stamp the DEFINING module path onto a function body node, so a later
|
||||
/// pack/comptime monomorphization can pin `current_source_file` to the body's
|
||||
/// own module and resolve its bare names in that module's visibility context
|
||||
@@ -1062,7 +1073,7 @@ pub fn resolveImports(
|
||||
var p = parser.Parser.init(allocator, imp_source);
|
||||
const imp_root = p.parse() catch {
|
||||
if (diagnostics) |diags| {
|
||||
diags.addFmt(.err, decl.span, "parse error in '{s}': {s}", .{ resolved_path, p.err_msg orelse "unknown" });
|
||||
diags.addFmtInFile(.err, resolved_path, importErrSpan(&p), "parse error in '{s}': {s}", .{ resolved_path, p.err_msg orelse "unknown" });
|
||||
}
|
||||
return error.ImportError;
|
||||
};
|
||||
@@ -1207,7 +1218,7 @@ fn resolveDirectoryImport(
|
||||
var p = parser.Parser.init(allocator, imp_source);
|
||||
const imp_root = p.parse() catch {
|
||||
if (diagnostics) |diags| {
|
||||
diags.addFmt(.err, span, "parse error in '{s}': {s}", .{ file_path, p.err_msg orelse "unknown" });
|
||||
diags.addFmtInFile(.err, file_path, importErrSpan(&p), "parse error in '{s}': {s}", .{ file_path, p.err_msg orelse "unknown" });
|
||||
}
|
||||
return error.ImportError;
|
||||
};
|
||||
|
||||
@@ -15,6 +15,7 @@ pub const Parser = struct {
|
||||
allocator: std.mem.Allocator,
|
||||
err_msg: ?[]const u8,
|
||||
err_offset: ?u32 = null,
|
||||
err_end: ?u32 = null,
|
||||
prev_end: u32 = 0,
|
||||
diagnostics: ?*errors.DiagnosticList = null,
|
||||
/// Type param names from enclosing generic struct (set while parsing methods)
|
||||
@@ -3976,6 +3977,7 @@ pub const Parser = struct {
|
||||
fn fail(self: *Parser, msg: []const u8) error{ParseError} {
|
||||
self.err_msg = msg;
|
||||
self.err_offset = self.current.loc.start;
|
||||
self.err_end = self.current.loc.end;
|
||||
if (self.diagnostics) |diags| {
|
||||
diags.add(.err, msg, .{ .start = self.current.loc.start, .end = self.current.loc.end });
|
||||
}
|
||||
@@ -3988,6 +3990,7 @@ pub const Parser = struct {
|
||||
fn failAt(self: *Parser, loc: anytype, msg: []const u8) error{ParseError} {
|
||||
self.err_msg = msg;
|
||||
self.err_offset = loc.start;
|
||||
self.err_end = loc.end;
|
||||
if (self.diagnostics) |diags| {
|
||||
diags.add(.err, msg, .{ .start = loc.start, .end = loc.end });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user