From 4a340b4a67b060e68a6bf42963d8a564836ccb96 Mon Sep 17 00:00:00 2001 From: Evgenii Tretiakov Date: Sat, 13 Jun 2026 01:36:38 +0200 Subject: [PATCH 1/3] fix: harden resolver, config ownership, and doc-test strictness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WHY: CodeRabbit review of zigdoc#2 surfaced findings against ziglint's vendored copy; the canonical home for those fixes is this repo. WHAT: - TypeResolver: cap alias-following at 32 hops (cyclic aliases like 'const a = b; const b = a;' no longer recurse unbounded); stop coercing field access on user types into .std_type — return .unknown so user namespaces cannot masquerade as stdlib ones. - Config: add pub deinit(allocator) for the paths slice ownership; re-export Rule publicly (Z012 — pub fns take it); deinit ends with self.* = undefined per Z030. - doc_comments: skip the full qualifier set (inline/extern/export/...) when scanning backward, so 'pub inline fn' keeps its doc comment. - doc_tests: reject unexpected diagnostics even when expectations are present; run all fixtures before failing; declare the incidental second rule in 5 affected fixtures (Z002+Z031, Z007+Z013, Z011+Z020, Z021+Z009, Z031+Z002). NOTE: ConfigType/RuleConfig stay PascalCase — type-returning functions follow Zig's type-constructor naming convention. IMPACT: linter precision (the user_type fix immediately exposed a real Z012 in our own Config.zig, fixed here); public Config API gains deinit. VALIDATION: mise x zig@0.16.0 zig build test --summary all => 7/7 steps, 274/274 tests (125 doc fixtures now strict), fmt clean, self-lint gate clean. --- docs/rules/Z002.md | 2 +- docs/rules/Z007.md | 2 +- docs/rules/Z011.md | 2 +- docs/rules/Z021.md | 2 +- docs/rules/Z031.md | 2 +- src/Config.zig | 20 ++++++++++++++------ src/TypeResolver.zig | 23 +++++++++++++++++++---- src/doc_comments.zig | 17 +++++++++++++++-- src/doc_tests.zig | 32 ++++++++++++++++++++++---------- 9 files changed, 75 insertions(+), 27 deletions(-) diff --git a/docs/rules/Z002.md b/docs/rules/Z002.md index a2504f9..9a52cb9 100644 --- a/docs/rules/Z002.md +++ b/docs/rules/Z002.md @@ -13,7 +13,7 @@ Detects variables that are assigned a value but never used. If you don't need th ### Bad ```zig -// expect: Z002 +// expect: Z002, Z031 const _unused = getValue(); ``` diff --git a/docs/rules/Z007.md b/docs/rules/Z007.md index 232270f..65f674d 100644 --- a/docs/rules/Z007.md +++ b/docs/rules/Z007.md @@ -13,7 +13,7 @@ Detects when the same module is imported multiple times in the same file. ### Bad ```zig -// expect: Z007 +// expect: Z007, Z013 const std = @import("std"); const std2 = @import("std"); ``` diff --git a/docs/rules/Z011.md b/docs/rules/Z011.md index 149e966..9ee520c 100644 --- a/docs/rules/Z011.md +++ b/docs/rules/Z011.md @@ -21,7 +21,7 @@ const MyType = struct { } }; const instance: MyType = .{}; -// expect: Z011 +// expect: Z011, Z020 pub fn main() void { instance.oldMethod(); } diff --git a/docs/rules/Z021.md b/docs/rules/Z021.md index a112c65..4a8d380 100644 --- a/docs/rules/Z021.md +++ b/docs/rules/Z021.md @@ -15,7 +15,7 @@ In file-as-struct patterns (files with top-level fields), the `@This()` alias sh In a file named `Config.zig`: ```zig -// expect: Z021 +// expect: Z021, Z009 const Wrong = @This(); value: u32, diff --git a/docs/rules/Z031.md b/docs/rules/Z031.md index bdbccb4..fa6bada 100644 --- a/docs/rules/Z031.md +++ b/docs/rules/Z031.md @@ -18,7 +18,7 @@ fn _privateHelper() void {} ``` ```zig -// expect: Z031 +// expect: Z031, Z002 const _internalValue: u32 = undefined; ``` diff --git a/src/Config.zig b/src/Config.zig index 7a6e0a9..2bd4bb5 100644 --- a/src/Config.zig +++ b/src/Config.zig @@ -3,7 +3,9 @@ const std = @import("std"); -const Rule = @import("rules.zig").Rule; +/// Public re-export: `isRuleEnabled`/`setRuleEnabled` take a `Rule`, so the +/// type must be reachable from this namespace by callers (ziglint Z012). +pub const Rule = @import("rules.zig").Rule; const Config = @This(); @@ -28,6 +30,15 @@ pub fn isRuleEnabled(self: *const Config, rule: Rule) bool { return true; } +/// Frees the `paths` entries allocated by `parseConfigSource` and leaves +/// the config invalidated. Safe to call on a default-initialized `Config` +/// (the default `paths` slice is empty and never freed). +pub fn deinit(self: *Config, allocator: std.mem.Allocator) void { + for (self.paths) |p| allocator.free(p); + if (self.paths.len > 0) allocator.free(self.paths); + self.* = undefined; +} + /// Set whether a rule is enabled. pub fn setRuleEnabled(self: *Config, rule: Rule, enabled: bool) void { inline for (@typeInfo(Rule).@"enum".fields) |field| { @@ -194,11 +205,8 @@ test "parse paths config" { \\ }, \\} ; - const config = try parseConfigSource(std.testing.allocator, source); - defer { - for (config.paths) |item| std.testing.allocator.free(item); - std.testing.allocator.free(config.paths); - } + var config = try parseConfigSource(std.testing.allocator, source); + defer config.deinit(std.testing.allocator); try std.testing.expectEqual(2, config.paths.len); try std.testing.expectEqualStrings("src", config.paths[0]); try std.testing.expectEqualStrings("lib", config.paths[1]); diff --git a/src/TypeResolver.zig b/src/TypeResolver.zig index 8ca283b..c558b06 100644 --- a/src/TypeResolver.zig +++ b/src/TypeResolver.zig @@ -491,8 +491,22 @@ fn findMethodInModule(self: *TypeResolver, module_path: []const u8, type_name: [ return self.findMethodInType(tree, type_node, method_name, mod.path); } +/// Alias chains (`const a = b;`) are followed at most this many hops so +/// cyclic aliases (`const a = b; const b = a;`) cannot recurse unbounded. +const max_alias_depth = 32; + /// For file-as-struct modules (like fs/File.zig), look for methods in root declarations. fn findMethodInFileAsStruct(self: *TypeResolver, module_path: []const u8, method_name: []const u8) ?MethodDef { + return self.findMethodInFileAsStructDepth(module_path, method_name, 0); +} + +fn findMethodInFileAsStructDepth( + self: *TypeResolver, + module_path: []const u8, + method_name: []const u8, + depth: u32, +) ?MethodDef { + if (depth >= max_alias_depth) return null; self.graph.addModulePublic(module_path); const mod = self.graph.getModule(module_path) orelse return null; const tree = &mod.tree; @@ -530,7 +544,7 @@ fn findMethodInFileAsStruct(self: *TypeResolver, module_path: []const u8, method // It's an alias like `const ArrayListUnmanaged = ArrayList;` // Follow the alias to find the actual function const alias_target = tree.tokenSlice(tree.nodeMainToken(init_node)); - return self.findMethodInFileAsStruct(module_path, alias_target); + return self.findMethodInFileAsStructDepth(module_path, alias_target, depth + 1); } else if (init_tag == .fn_decl) { // Inline function definition var buf: [1]Ast.Node.Index = undefined; @@ -822,9 +836,10 @@ fn resolveFieldAccess(self: *TypeResolver, tree: *const Ast, node: Ast.Node.Inde return .{ .std_type = .{ .path = full_path } }; }, .user_type => { - // Accessing a field on a user type - could be a nested type - const full_path = self.buildStdTypePath(tree, node); - return .{ .std_type = .{ .path = full_path } }; + // A field access on a user-defined type is not a stdlib type. + // Coercing it into `.std_type` conflated user namespaces with + // `std` ones and misrouted downstream semantic resolution. + return .unknown; }, .unknown => { const lhs_tag = tree.nodeTag(lhs_node); diff --git a/src/doc_comments.zig b/src/doc_comments.zig index 0f5a687..5f73b76 100644 --- a/src/doc_comments.zig +++ b/src/doc_comments.zig @@ -23,8 +23,21 @@ pub fn getDocComment(allocator: std.mem.Allocator, tree: *const Ast, node: Ast.N if (tag == .doc_comment) { doc_tokens.append(allocator, token) catch return null; } else { - // Stop at non-doc-comment tokens (skip pub keyword) - if (tag != .keyword_pub) break; + // Stop at non-doc-comment tokens, skipping declaration + // qualifiers (`pub inline fn`, `pub extern "c" fn`, ...) that + // may sit between the doc comment and the declaration itself. + switch (tag) { + .keyword_pub, + .keyword_inline, + .keyword_noinline, + .keyword_extern, + .keyword_export, + .keyword_threadlocal, + .keyword_comptime, + .string_literal, + => {}, + else => break, + } } if (token == 0) break; token -= 1; diff --git a/src/doc_tests.zig b/src/doc_tests.zig index 5f75cab..e43589f 100644 --- a/src/doc_tests.zig +++ b/src/doc_tests.zig @@ -138,19 +138,21 @@ fn runDocTest(allocator: std.mem.Allocator, doc_path: []const u8, doc_test: DocT } } - // If no expectations, should have no diagnostics - if (doc_test.expected_rules.len == 0) { - const total = linter.diagnostics.items.len; - if (total > 0) { - std.debug.print("\n{s}:{d}: expected no diagnostics but got {d}\n", .{ + // Reject diagnostics for rules that were not expected. Extra findings + // must not ride along silently just because one expectation matched — + // and with no expectations at all, any diagnostic is unexpected. + for (linter.diagnostics.items) |d| { + const expected = for (doc_test.expected_rules) |expected_rule| { + if (expected_rule == d.rule) break true; + } else false; + if (!expected) { + std.debug.print("\n{s}:{d}: unexpected {s} diagnostic: {s}\n", .{ doc_path, doc_test.line_in_doc, - total, + d.rule.code(), + d.context, }); std.debug.print("Code:\n{s}\n", .{doc_test.code}); - for (linter.diagnostics.items) |d| { - std.debug.print(" - {s}: {s}\n", .{ d.rule.code(), d.context }); - } return error.UnexpectedDiagnostic; } } @@ -175,6 +177,7 @@ pub fn runAllDocTests(allocator: std.mem.Allocator) !void { var file_count: usize = 0; var test_count: usize = 0; + var failure_count: usize = 0; var iter = dir.iterate(); while (try iter.next(io)) |entry| { if (entry.kind != .file) continue; @@ -193,11 +196,20 @@ pub fn runAllDocTests(allocator: std.mem.Allocator) !void { defer allocator.free(full_path); for (doc.tests) |doc_test| { - try runDocTest(allocator, full_path, doc_test, &tmp_dir); + // Run every fixture before failing so one gate run reports all + // offending docs instead of stopping at the first. + runDocTest(allocator, full_path, doc_test, &tmp_dir) catch { + failure_count += 1; + }; test_count += 1; } file_count += 1; } + + if (failure_count > 0) { + std.debug.print("\n{d}/{d} doc tests failed\n", .{ failure_count, test_count }); + return error.DocTestsFailed; + } } test "doc tests" { From 960baf21ffaeed87a387556385242162f99098c6 Mon Sep 17 00:00:00 2001 From: Evgenii Tretiakov Date: Sat, 13 Jun 2026 01:40:16 +0200 Subject: [PATCH 2/3] fix(doc-tests): only count assertion errors as fixture failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WHY: PR #4 review — the collect-all catch swallowed infrastructure errors (I/O, OOM) along with assertion failures, hiding root causes. WHAT: catch-switch counts MissingExpectedDiagnostic/UnexpectedDiagnostic toward failure_count and rethrows everything else. IMPACT: doc-test runner; no behavior change for passing suites. VALIDATION: zig build test --summary all => 274/274, self-lint clean. --- src/doc_tests.zig | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/doc_tests.zig b/src/doc_tests.zig index e43589f..ff5b5cf 100644 --- a/src/doc_tests.zig +++ b/src/doc_tests.zig @@ -197,9 +197,14 @@ pub fn runAllDocTests(allocator: std.mem.Allocator) !void { for (doc.tests) |doc_test| { // Run every fixture before failing so one gate run reports all - // offending docs instead of stopping at the first. - runDocTest(allocator, full_path, doc_test, &tmp_dir) catch { - failure_count += 1; + // offending docs instead of stopping at the first. Only the two + // assertion errors count as fixture failures; infrastructure + // errors (I/O, OOM, ...) propagate immediately. + runDocTest(allocator, full_path, doc_test, &tmp_dir) catch |err| switch (err) { + error.MissingExpectedDiagnostic, + error.UnexpectedDiagnostic, + => failure_count += 1, + else => return err, }; test_count += 1; } From 96bf7e40cacffcc717549a35d015ac930c831cdd Mon Sep 17 00:00:00 2001 From: Evgenii Tretiakov Date: Sat, 13 Jun 2026 01:46:18 +0200 Subject: [PATCH 3/3] docs: align AGENTS.md constant naming with Z006 (snake_case) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WHY: PR #4 review asked for SCREAMING_SNAKE_CASE per AGENTS.md, but the repo's own Z006 rule (and Zig stdlib convention, e.g. max_path_bytes) enforces snake_case for value constants — the prose was stale, the machine rule is canonical. WHAT: max_alias_depth keeps snake_case (now with explicit u32 type); AGENTS.md naming section now documents snake_case value constants, PascalCase type-returning functions, and cites Z006. IMPACT: docs only; no behavior change. VALIDATION: zig build test => 274/274, self-lint clean (Z006 satisfied). --- AGENTS.md | 6 +++--- src/TypeResolver.zig | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0cfa40c..0a4bd0b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,10 +60,10 @@ b.addExecutable(.{ ## Zig Code Style **Naming:** -- `camelCase` for functions and methods -- `snake_case` for variables and parameters +- `camelCase` for functions and methods (`PascalCase` when returning a type) +- `snake_case` for variables, parameters, and value constants — Zig stdlib + convention (e.g. `std.fs.max_path_bytes`), enforced by Z006 - `PascalCase` for types, structs, and enums -- `SCREAMING_SNAKE_CASE` for constants **Struct initialization:** Prefer explicit type annotation with anonymous literals: ```zig diff --git a/src/TypeResolver.zig b/src/TypeResolver.zig index c558b06..31d5a80 100644 --- a/src/TypeResolver.zig +++ b/src/TypeResolver.zig @@ -493,7 +493,7 @@ fn findMethodInModule(self: *TypeResolver, module_path: []const u8, type_name: [ /// Alias chains (`const a = b;`) are followed at most this many hops so /// cyclic aliases (`const a = b; const b = a;`) cannot recurse unbounded. -const max_alias_depth = 32; +const max_alias_depth: u32 = 32; /// For file-as-struct modules (like fs/File.zig), look for methods in root declarations. fn findMethodInFileAsStruct(self: *TypeResolver, module_path: []const u8, method_name: []const u8) ?MethodDef {