Skip to content

Commit c848904

Browse files
rockoragerampagent
andcommitted
add Z028: inline import rule
Detect @import calls that are not assigned to a top-level const. The rule flags inline imports in switch prongs, function bodies, and expressions. It allows: - Top-level const assignments: const X = @import(...) - Field access: const X = @import(...).Y - Imports in test blocks - Discard imports for pulling in tests: _ = @import(...) Amp-Thread-ID: https://ampcode.com/threads/T-019c144e-1ab8-77d5-951d-bb541fe7d1f4 Co-authored-by: Amp <amp@ampcode.com>
1 parent a0ca75e commit c848904

6 files changed

Lines changed: 298 additions & 7 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ Directories are scanned recursively for `.zig` files.
4848
| Z025 | Redundant `catch |err| return err`; use `try` instead |
4949
| Z026 | Empty `catch` block suppresses errors |
5050
| Z027 | Access declaration through type instead of instance |
51+
| Z028 | Inline `@import`; assign to a top-level `const` |
5152
| Z029 | Redundant `@as` cast; type already known from context |
5253

5354
## Configuration

docs/rules/Z013.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,12 @@ Public re-exports are not considered unused:
4040
pub const utils = @import("utils.zig");
4141
```
4242

43-
Imports used only in tests are allowed when the import is in a test block:
43+
Imports used only in tests should be at the top level. If the file only contains tests, the import will be considered used:
4444

4545
```zig
46+
const testing = @import("testing.zig");
47+
4648
test "example" {
47-
const testing = @import("testing.zig");
4849
testing.check();
4950
}
5051
```

docs/rules/Z028.md

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
---
2+
rule: Z028
3+
title: Inline import
4+
enabled: true
5+
---
6+
7+
# Z028: Inline import
8+
9+
Detects `@import` calls that are not assigned to a top-level `const`. Imports should be declared at the top of the file, not inline in expressions.
10+
11+
## Examples
12+
13+
### Bad
14+
15+
```zig
16+
// expect: Z028
17+
// expect: Z028
18+
const builtin = @import("builtin");
19+
const Backend = switch (builtin.os.tag) {
20+
.linux => @import("linux.zig"),
21+
.macos => @import("macos.zig"),
22+
else => @compileError("unsupported"),
23+
};
24+
```
25+
26+
```zig
27+
// expect: Z028
28+
fn foo() void {
29+
const x = @import("bar.zig").baz;
30+
_ = x;
31+
}
32+
```
33+
34+
```zig
35+
// expect: Z028
36+
pub fn main() void {
37+
const std = @import("std");
38+
_ = std;
39+
}
40+
```
41+
42+
### Good
43+
44+
Assign imports to top-level constants:
45+
46+
```zig
47+
const builtin = @import("builtin");
48+
const linux = @import("linux.zig");
49+
const macos = @import("macos.zig");
50+
51+
const Backend = switch (builtin.os.tag) {
52+
.linux => linux,
53+
.macos => macos,
54+
else => @compileError("unsupported"),
55+
};
56+
```
57+
58+
Field access on imports is allowed:
59+
60+
```zig
61+
const Rule = @import("rules.zig").Rule;
62+
```
63+
64+
Imports in test blocks are allowed:
65+
66+
```zig
67+
test "example" {
68+
const testing = @import("testing.zig");
69+
testing.check();
70+
}
71+
```
72+
73+
Discard imports for pulling in tests are allowed:
74+
75+
```zig
76+
test {
77+
_ = @import("other.zig");
78+
}
79+
```
80+
81+
## Rationale
82+
83+
Inline imports make code harder to read and maintain:
84+
85+
1. **Discoverability**: All imports at the top of a file make dependencies immediately visible
86+
2. **Consistency**: Standard Zig style places imports at the top
87+
3. **Deduplication**: Top-level imports naturally prevent duplicate imports of the same module
88+
4. **Tooling**: Static analysis tools work better with predictable import locations

src/Linter.zig

Lines changed: 200 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ pub fn lint(self: *Linter) void {
143143

144144
self.checkUnusedImports();
145145
self.checkThisBuiltin();
146+
self.checkInlineImports();
146147
self.checkCatchReturnAll();
147148
self.checkEmptyCatchAll();
148149
self.checkInstanceDeclAccess();
@@ -373,6 +374,19 @@ fn getNodeChildren(self: *Linter, node: Ast.Node.Index) ChildList {
373374
children.append(data[1]);
374375
},
375376

377+
// field_access: node_and_token = [lhs, field_token]
378+
.field_access => {
379+
const data = self.tree.nodeData(node).node_and_token;
380+
children.append(data[0]);
381+
},
382+
383+
// assign: node_and_node = [lhs, rhs]
384+
.assign => {
385+
const data = self.tree.nodeData(node).node_and_node;
386+
children.append(data[0]);
387+
children.append(data[1]);
388+
},
389+
376390
else => {},
377391
}
378392

@@ -461,6 +475,87 @@ fn checkThisBuiltin(self: *Linter) void {
461475
}
462476
}
463477

478+
fn checkInlineImports(self: *Linter) void {
479+
if (!self.config.isRuleEnabled(.Z028)) return;
480+
481+
for (0..self.tree.nodes.len) |i| {
482+
const node: Ast.Node.Index = @enumFromInt(i);
483+
const tag = self.tree.nodeTag(node);
484+
485+
// Look for @import() calls
486+
if (tag != .builtin_call_two and tag != .builtin_call_two_comma) continue;
487+
const main_token = self.tree.nodeMainToken(node);
488+
const builtin_name = self.tree.tokenSlice(main_token);
489+
if (!std.mem.eql(u8, builtin_name, "@import")) continue;
490+
491+
// Walk up through field_access chain to find var decl
492+
// e.g., `const Rule = @import("rules.zig").Rule;`
493+
var current = node;
494+
while (true) {
495+
const parent = self.parent_map[@intFromEnum(current)].unwrap() orelse {
496+
const loc = self.tree.tokenLocation(0, main_token);
497+
self.report(loc, .Z028, "");
498+
break;
499+
};
500+
501+
const parent_tag = self.tree.nodeTag(parent);
502+
switch (parent_tag) {
503+
.field_access => {
504+
// Continue walking up through field access chain
505+
current = parent;
506+
continue;
507+
},
508+
.simple_var_decl, .aligned_var_decl, .local_var_decl, .global_var_decl => {
509+
const var_decl = self.tree.fullVarDecl(parent) orelse {
510+
const loc = self.tree.tokenLocation(0, main_token);
511+
self.report(loc, .Z028, "");
512+
break;
513+
};
514+
const mut_token = self.tree.tokenSlice(var_decl.ast.mut_token);
515+
const init_node = var_decl.ast.init_node.unwrap() orelse {
516+
const loc = self.tree.tokenLocation(0, main_token);
517+
self.report(loc, .Z028, "");
518+
break;
519+
};
520+
// Must be `const` and the init must be our current node (or an ancestor)
521+
if (!std.mem.eql(u8, mut_token, "const") or init_node != current) {
522+
const loc = self.tree.tokenLocation(0, main_token);
523+
self.report(loc, .Z028, "");
524+
break;
525+
}
526+
// Check that the var decl is at file level (not inside a function)
527+
// Allow imports in test blocks
528+
if (!self.isAtFileLevel(parent) and !self.isInTestBlock(parent)) {
529+
const loc = self.tree.tokenLocation(0, main_token);
530+
self.report(loc, .Z028, "");
531+
}
532+
break;
533+
},
534+
.assign => {
535+
// Allow `_ = @import(...)` pattern for pulling in tests
536+
const data = self.tree.nodeData(parent).node_and_node;
537+
const lhs = data[0];
538+
if (self.tree.nodeTag(lhs) == .identifier) {
539+
const lhs_name = self.tree.tokenSlice(self.tree.nodeMainToken(lhs));
540+
if (std.mem.eql(u8, lhs_name, "_")) {
541+
// Discarding import is allowed
542+
break;
543+
}
544+
}
545+
const loc = self.tree.tokenLocation(0, main_token);
546+
self.report(loc, .Z028, "");
547+
break;
548+
},
549+
else => {
550+
const loc = self.tree.tokenLocation(0, main_token);
551+
self.report(loc, .Z028, "");
552+
break;
553+
},
554+
}
555+
}
556+
}
557+
}
558+
464559
fn hasTopLevelFields(self: *Linter) bool {
465560
for (self.tree.rootDecls()) |node| {
466561
const tag = self.tree.nodeTag(node);
@@ -503,6 +598,21 @@ fn isAtFileLevel(self: *Linter, start_node: Ast.Node.Index) bool {
503598
}
504599
}
505600

601+
fn isInTestBlock(self: *Linter, start_node: Ast.Node.Index) bool {
602+
var current = start_node;
603+
604+
while (true) {
605+
const parent_opt = self.parent_map[@intFromEnum(current)];
606+
const parent = parent_opt.unwrap() orelse return false;
607+
const parent_tag = self.tree.nodeTag(parent);
608+
609+
if (parent_tag == .test_decl) return true;
610+
if (parent_tag == .fn_decl) return false; // Functions block test scope
611+
612+
current = parent;
613+
}
614+
}
615+
506616
fn findEnclosingStructName(self: *Linter, start_node: Ast.Node.Index) ?[]const u8 {
507617
var current = start_node;
508618
var enclosing_container: ?Ast.Node.Index = null;
@@ -3650,7 +3760,8 @@ test "Z023: argument order - aliased Allocator" {
36503760
const path = try tmp_dir.dir.realpathAlloc(std.testing.allocator, "test.zig");
36513761
defer std.testing.allocator.free(path);
36523762

3653-
var graph = try @import("ModuleGraph.zig").init(std.testing.allocator, path, null);
3763+
const ModuleGraph = @import("ModuleGraph.zig");
3764+
var graph = try ModuleGraph.init(std.testing.allocator, path, null);
36543765
defer graph.deinit();
36553766

36563767
var resolver: TypeResolver = .init(std.testing.allocator, &graph);
@@ -3683,7 +3794,8 @@ test "Z023: argument order - aliased Io" {
36833794
const path = try tmp_dir.dir.realpathAlloc(std.testing.allocator, "test.zig");
36843795
defer std.testing.allocator.free(path);
36853796

3686-
var graph = try @import("ModuleGraph.zig").init(std.testing.allocator, path, null);
3797+
const ModuleGraph = @import("ModuleGraph.zig");
3798+
var graph = try ModuleGraph.init(std.testing.allocator, path, null);
36873799
defer graph.deinit();
36883800

36893801
var resolver: TypeResolver = .init(std.testing.allocator, &graph);
@@ -3750,7 +3862,8 @@ test "Z023: receiver param with struct name is ok (semantic)" {
37503862
const path = try tmp_dir.dir.realpathAlloc(std.testing.allocator, "test.zig");
37513863
defer std.testing.allocator.free(path);
37523864

3753-
var graph = try @import("ModuleGraph.zig").init(std.testing.allocator, path, null);
3865+
const ModuleGraph = @import("ModuleGraph.zig");
3866+
var graph = try ModuleGraph.init(std.testing.allocator, path, null);
37543867
defer graph.deinit();
37553868

37563869
var resolver: TypeResolver = .init(std.testing.allocator, &graph);
@@ -3781,7 +3894,8 @@ test "Z023: file-as-struct receiver is ok (semantic)" {
37813894
const path = try tmp_dir.dir.realpathAlloc(std.testing.allocator, "Terminal.zig");
37823895
defer std.testing.allocator.free(path);
37833896

3784-
var graph = try @import("ModuleGraph.zig").init(std.testing.allocator, path, null);
3897+
const ModuleGraph = @import("ModuleGraph.zig");
3898+
var graph = try ModuleGraph.init(std.testing.allocator, path, null);
37853899
defer graph.deinit();
37863900

37873901
var resolver: TypeResolver = .init(std.testing.allocator, &graph);
@@ -4706,3 +4820,85 @@ test "Z029: mixed match and mismatch in call args" {
47064820
linter.lint();
47074821
try std.testing.expectEqual(1, linter.diagnosticCount(.Z029));
47084822
}
4823+
4824+
test "Z028: inline import in switch" {
4825+
var linter: Linter = .init(std.testing.allocator,
4826+
\\const builtin = @import("builtin");
4827+
\\const Backend = switch (builtin.os.tag) {
4828+
\\ .linux => @import("linux.zig"),
4829+
\\ .macos => @import("macos.zig"),
4830+
\\ else => @compileError("unsupported"),
4831+
\\};
4832+
, "test.zig", null);
4833+
defer linter.deinit();
4834+
linter.lint();
4835+
try std.testing.expectEqual(2, linter.diagnosticCount(.Z028));
4836+
}
4837+
4838+
test "Z028: inline import in function call" {
4839+
var linter: Linter = .init(std.testing.allocator,
4840+
\\fn foo(x: anytype) void {
4841+
\\ _ = x;
4842+
\\}
4843+
\\pub fn main() void {
4844+
\\ foo(@import("bar.zig").baz);
4845+
\\}
4846+
, "test.zig", null);
4847+
defer linter.deinit();
4848+
linter.lint();
4849+
try std.testing.expectEqual(1, linter.diagnosticCount(.Z028));
4850+
}
4851+
4852+
test "Z028: allow top-level const import" {
4853+
var linter: Linter = .init(std.testing.allocator,
4854+
\\const std = @import("std");
4855+
\\const other = @import("other.zig");
4856+
, "test.zig", null);
4857+
defer linter.deinit();
4858+
linter.lint();
4859+
try std.testing.expectEqual(0, linter.diagnosticCount(.Z028));
4860+
}
4861+
4862+
test "Z028: disallow import inside function" {
4863+
var linter: Linter = .init(std.testing.allocator,
4864+
\\pub fn main() void {
4865+
\\ const std = @import("std");
4866+
\\ _ = std;
4867+
\\}
4868+
, "test.zig", null);
4869+
defer linter.deinit();
4870+
linter.lint();
4871+
try std.testing.expectEqual(1, linter.diagnosticCount(.Z028));
4872+
}
4873+
4874+
test "Z028: allow discard import for pulling in tests" {
4875+
var linter: Linter = .init(std.testing.allocator,
4876+
\\test {
4877+
\\ _ = @import("other.zig");
4878+
\\}
4879+
, "test.zig", null);
4880+
defer linter.deinit();
4881+
linter.lint();
4882+
try std.testing.expectEqual(0, linter.diagnosticCount(.Z028));
4883+
}
4884+
4885+
test "Z028: allow const import in test block" {
4886+
var linter: Linter = .init(std.testing.allocator,
4887+
\\test "example" {
4888+
\\ const testing = @import("testing.zig");
4889+
\\ testing.check();
4890+
\\}
4891+
, "test.zig", null);
4892+
defer linter.deinit();
4893+
linter.lint();
4894+
try std.testing.expectEqual(0, linter.diagnosticCount(.Z028));
4895+
}
4896+
4897+
test "Z028: allow field access on import" {
4898+
var linter: Linter = .init(std.testing.allocator,
4899+
\\const Rule = @import("rules.zig").Rule;
4900+
, "test.zig", null);
4901+
defer linter.deinit();
4902+
linter.lint();
4903+
try std.testing.expectEqual(0, linter.diagnosticCount(.Z028));
4904+
}

src/main.zig

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//! ziglint - A linter for Zig source code
22

33
const std = @import("std");
4+
const builtin = @import("builtin");
45
const build_options = @import("build_options");
56
pub const version = build_options.version;
67

@@ -65,7 +66,7 @@ pub fn main() !u8 {
6566
}
6667

6768
fn detectColorSupport(file: std.fs.File) bool {
68-
const native = @import("builtin").os.tag;
69+
const native = builtin.os.tag;
6970
// NO_COLOR takes precedence (https://no-color.org/)
7071
if (native == .windows) {
7172
if (std.process.getenvW(std.unicode.utf8ToUtf16LeStringLiteral("NO_COLOR"))) |_| return false;

0 commit comments

Comments
 (0)