From e73870843cbda028098b7389455d2466ae352e9c Mon Sep 17 00:00:00 2001 From: Evgenii Tretiakov Date: Sat, 20 Jun 2026 14:06:33 +0200 Subject: [PATCH] fix: Z029 composite-type false negatives + surface silent IO/config errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WHY: A full multi-agent adversarial audit of the fork delta vs upstream (bfcb30d..b212557, 24 raw findings → 3 confirmed after verification) surfaced three real, verified defects present at HEAD: 1. Z029 (redundant @as in struct init) only compared bare-identifier field types, so `@as(*u32, ...)`, `@as(?T, ...)`, `@as([N]T, ...)` and `@as(a.b.C, ...)` on struct fields were silently skipped — genuine linter false negatives (the tool's core job). 2. lintDirectory's `walker.next(io) catch null` ended traversal silently on a permission error / symlink loop, leaving the rest of the tree un-linted with no diagnostic. 3. main's `FileConfig.load(...) catch .{}` swallowed config-discovery errors (e.g. an allocation failure joining the search path) without logging. All three are pre-existing upstream but ship in the fork; this cycle's scope was the whole fork-vs-upstream delta. WHAT: - Linter.zig: findContainerFieldTypeInTree now returns the field type's raw source (Ast.getNodeSource) instead of only `.identifier`. Added getAsTypeText (source of an `@as` cast's type node) and typeTextEql (whitespace-insensitive compare). checkRedundantAsInStructInit compares source text, so all composite field types are handled. getAsTypeName is retained (still used by call-arg / array / return / value paths). - main.zig: directory-walk errors now print a warning before stopping; config load logs the error before falling back to defaults. Both Z026-clean. - Added 4 Z029 regression tests (pointer/optional/array detection + a different-pointer-type negative case). IMPACT: src/Linter.zig, src/main.zig. No behavior change for the previously handled identifier case; new detections are strict (whitespace-normalized exact type-text match, so different types still don't trigger). Linter now catches a class of redundant casts it previously missed and no longer fails silently on unreadable directories/config. VALIDATION: TDD — the 3 composite-type tests failed (expected 1, found 0) before the fix and pass after. `zig build test --summary all` => 7/7 steps, 281/281 tests pass, zig fmt clean, self-lint (run exe ziglint) clean. The self-lint gate also caught an empty `catch {}` in the first draft of fix #2 (Z026), which was corrected to `try`. --- src/Linter.zig | 110 +++++++++++++++++++++++++++++++++++++++++++++---- src/main.zig | 17 ++++++-- 2 files changed, 115 insertions(+), 12 deletions(-) diff --git a/src/Linter.zig b/src/Linter.zig index d294353..8338f0f 100644 --- a/src/Linter.zig +++ b/src/Linter.zig @@ -2077,6 +2077,36 @@ fn getAsTypeName(self: *Linter, node: Ast.Node.Index) ?[]const u8 { return self.getTypeNodeName(params[0]); } +/// Source text of an `@as(T, ...)` cast's type node, for comparison against a +/// field's declared type. Unlike getAsTypeName this also covers composite types +/// (`*u32`, `?T`, `[N]T`, `a.b.C`); pair it with typeTextEql for whitespace- +/// insensitive comparison so Z029 detects redundant casts on all field types. +fn getAsTypeText(self: *Linter, node: Ast.Node.Index) ?[]const u8 { + const tag = self.tree.nodeTag(node); + if (tag != .builtin_call_two and tag != .builtin_call_two_comma and + tag != .builtin_call and tag != .builtin_call_comma) return null; + if (!std.mem.eql(u8, self.tree.tokenSlice(self.tree.nodeMainToken(node)), "@as")) return null; + var buf: [2]Ast.Node.Index = undefined; + const params = self.tree.builtinCallParams(&buf, node) orelse return null; + if (params.len < 1) return null; + return self.tree.getNodeSource(params[0]); +} + +/// Whitespace-insensitive equality of two type source strings, so `[3]u8` and +/// `[3] u8` (or differently-spaced field/cast types) compare equal. +fn typeTextEql(a: []const u8, b: []const u8) bool { + var ia: usize = 0; + var ib: usize = 0; + while (true) { + while (ia < a.len and std.ascii.isWhitespace(a[ia])) ia += 1; + while (ib < b.len and std.ascii.isWhitespace(b[ib])) ib += 1; + if (ia >= a.len or ib >= b.len) return ia >= a.len and ib >= b.len; + if (a[ia] != b[ib]) return false; + ia += 1; + ib += 1; + } +} + fn getTypeNodeName(self: *Linter, type_node: Ast.Node.Index) ?[]const u8 { const tag = self.tree.nodeTag(type_node); return switch (tag) { @@ -2203,7 +2233,7 @@ fn checkRedundantAsInStructInit(self: *Linter, node: Ast.Node.Index) void { const struct_type_name = self.resolveStructInitTypeName(node, struct_init) orelse return; for (struct_init.ast.fields) |field_value| { - const as_type_name = self.getAsTypeName(field_value) orelse continue; + const as_type_text = self.getAsTypeText(field_value) orelse continue; // Get field name: token before '=' before the value expression const value_main_token = self.tree.nodeMainToken(field_value); @@ -2212,10 +2242,10 @@ fn checkRedundantAsInStructInit(self: *Linter, node: Ast.Node.Index) void { if (self.tree.tokenTag(field_name_token) != .identifier) continue; const field_name = self.tree.tokenSlice(field_name_token); - const field_type_name = self.findContainerFieldType(struct_type_name, field_name) orelse continue; - if (std.mem.eql(u8, as_type_name, field_type_name)) { + const field_type_text = self.findContainerFieldType(struct_type_name, field_name) orelse continue; + if (typeTextEql(as_type_text, field_type_text)) { const loc = self.tree.tokenLocation(0, self.tree.nodeMainToken(field_value)); - self.report(loc, .Z029, as_type_name); + self.report(loc, .Z029, as_type_text); } } } @@ -2261,11 +2291,11 @@ fn findContainerFieldTypeInTree(self: *Linter, tree: *const Ast, struct_type_nam const field = tree.fullContainerField(member) orelse continue; if (!std.mem.eql(u8, tree.tokenSlice(field.ast.main_token), field_name)) continue; const type_node = field.ast.type_expr.unwrap() orelse return null; - const field_type_tag = tree.nodeTag(type_node); - return switch (field_type_tag) { - .identifier => tree.tokenSlice(tree.nodeMainToken(type_node)), - else => null, - }; + // Return the field type's raw source text. Comparison in + // checkRedundantAsInStructInit normalizes whitespace, so this + // handles composite types (`*u32`, `?T`, `[N]T`, `a.b.C`) too, + // not just bare identifiers (Z029 false negatives otherwise). + return tree.getNodeSource(type_node); } }, else => {}, @@ -5610,6 +5640,68 @@ test "Z029: detect redundant @as in struct field init" { try std.testing.expectEqual(1, linter.diagnosticCount(.Z029)); } +test "Z029: detect redundant @as for pointer-typed struct field" { + var linter: Linter = .init(std.testing.allocator, + \\const Foo = struct { + \\ p: *u32, + \\}; + \\pub fn main() void { + \\ var n: u32 = 0; + \\ const f: Foo = .{ .p = @as(*u32, &n) }; + \\ _ = f; + \\} + , "test.zig", null); + defer linter.deinit(); + linter.lint(); + try std.testing.expectEqual(1, linter.diagnosticCount(.Z029)); +} + +test "Z029: detect redundant @as for optional-typed struct field" { + var linter: Linter = .init(std.testing.allocator, + \\const Foo = struct { + \\ o: ?u32, + \\}; + \\pub fn main() void { + \\ const f: Foo = .{ .o = @as(?u32, 42) }; + \\ _ = f; + \\} + , "test.zig", null); + defer linter.deinit(); + linter.lint(); + try std.testing.expectEqual(1, linter.diagnosticCount(.Z029)); +} + +test "Z029: detect redundant @as for array-typed struct field" { + var linter: Linter = .init(std.testing.allocator, + \\const Foo = struct { + \\ a: [3]u8, + \\}; + \\pub fn main() void { + \\ const f: Foo = .{ .a = @as([3]u8, .{ 1, 2, 3 }) }; + \\ _ = f; + \\} + , "test.zig", null); + defer linter.deinit(); + linter.lint(); + try std.testing.expectEqual(1, linter.diagnosticCount(.Z029)); +} + +test "Z029: allow @as with different pointer type in struct field" { + var linter: Linter = .init(std.testing.allocator, + \\const Foo = struct { + \\ p: *u32, + \\}; + \\pub fn main() void { + \\ var n: u16 = 0; + \\ const f: Foo = .{ .p = @as(*u16, &n) }; + \\ _ = f; + \\} + , "test.zig", null); + defer linter.deinit(); + linter.lint(); + try std.testing.expectEqual(0, linter.diagnosticCount(.Z029)); +} + test "Z029: allow @as with different type in struct field init" { var linter: Linter = .init(std.testing.allocator, \\const Foo = struct { diff --git a/src/main.zig b/src/main.zig index 82075f7..027e1c8 100644 --- a/src/main.zig +++ b/src/main.zig @@ -64,9 +64,14 @@ pub fn main(init: std.process.Init) !u8 { else => return err, }; - // Load config file from first CLI path (or current directory) + // Load config file from first CLI path (or current directory). + // Log before falling back to defaults so config-discovery failures (e.g. an + // allocation error while joining the search path) aren't swallowed silently. const start_path = if (config.paths.len > 0) config.paths[0] else null; - config.file_config = FileConfig.load(cfg_alloc, io, start_path) catch .{}; + config.file_config = FileConfig.load(cfg_alloc, io, start_path) catch |err| blk: { + try stderr.interface.print("warning: failed to load config: {}\n", .{err}); + break :blk .{}; + }; applyOnlyRules(&config); @@ -305,7 +310,13 @@ fn lintDirectory(allocator: std.mem.Allocator, io: std.Io, path: []const u8, zig }; defer walker.deinit(); - while (walker.next(io) catch null) |entry| { + while (walker.next(io) catch |err| blk: { + // Report instead of silently ending traversal: a permission error or + // symlink loop mid-walk would otherwise drop the rest of the tree with + // no diagnostic, leaving files un-linted without the user knowing. + try writer.print("warning: stopped walking '{s}': {}\n", .{ path, err }); + break :blk null; + }) |entry| { if (shouldSkip(entry.path, gitignore)) continue; if (entry.kind != .file) continue; if (!std.mem.endsWith(u8, entry.basename, ".zig")) continue;