Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 101 additions & 9 deletions src/Linter.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment on lines +2101 to +2102

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid concatenating type tokens when normalizing

When the field type contains whitespace that separates tokens, this byte-level normalization can turn a different type into the same text. For example, a field declared as *const u8 and a cast written as @as(*constu8, ...) become identical after stripping spaces if the user has a constu8 type alias, so Z029 reports the cast as redundant even though the cast type is not the field's context type. Normalize trivia without merging tokens, or compare the parsed type structure instead of deleting all whitespace.

Useful? React with 👍 / 👎.

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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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 => {},
Expand Down Expand Up @@ -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 {
Expand Down
17 changes: 14 additions & 3 deletions src/main.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down
Loading