Skip to content

Commit cdc5ab9

Browse files
authored
feat(linter): add no-print rule (#318)
## What This PR Does Adds a new `restriction` level rule that bans calls to `std.debug.print`
1 parent 382277e commit cdc5ab9

File tree

7 files changed

+462
-62
lines changed

7 files changed

+462
-62
lines changed

apps/site/docs/rules/no-print.mdx

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
# `no-print`
2+
3+
<RuleBanner category="restriction" default="warning" />
4+
5+
## What This Rule Does
6+
7+
Disallows the use of `std.debug.print`.
8+
9+
`print` statements are great for debugging, but they should be removed
10+
before code gets merged. When you need debug logs in production, use
11+
`std.log` instead.
12+
13+
This rule makes a best-effort attempt to ensure `print` calls are actually
14+
from `std.debug.print`. It will not report calls to custom print functions
15+
if they are defined within the same file. If you are getting false positives
16+
because you import a custom print function, consider disabling this rule on
17+
a file-by-file basis instead of turning it off globally.
18+
19+
### Tests
20+
21+
By default, this rule ignores `print`s in test blocks and files. Files are
22+
considered to be a test file if they end with `test.zig`. You may disable
23+
this by setting `allow_tests` to `false` in the rule's metadata.
24+
25+
```json
26+
{
27+
"rules": {
28+
"no-print": ["warn", { "allow_tests": false }]
29+
}
30+
}
31+
```
32+
33+
## Examples
34+
35+
Examples of **incorrect** code for this rule:
36+
37+
```zig
38+
const std = @import("std");
39+
const debug = std.debug;
40+
const print = std.debug.print;
41+
fn main() void {
42+
std.debug.print("This should not be here: {d}\n", .{42});
43+
debug.print("This should not be here: {d}\n", .{42});
44+
print("This should not be here: {d}\n", .{42});
45+
}
46+
```
47+
48+
Examples of **correct** code for this rule:
49+
50+
```zig
51+
const std = @import("std");
52+
fn foo() u32 {
53+
std.log.debug("running foo", .{});
54+
return 1;
55+
}
56+
57+
test foo {
58+
std.debug.print("testing foo\n", .{});
59+
try std.testing.expectEqual(1, foo());
60+
}
61+
```
62+
63+
```zig
64+
fn print(comptime msg: []const u8, args: anytype) void {
65+
// ...
66+
}
67+
fn main() void {
68+
print("Staring program", .{});
69+
}
70+
```
71+
72+
## Configuration
73+
74+
This rule accepts the following options:
75+
76+
- allow_tests: boolean

src/linter/config/rules_config.zig

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,20 @@ const rules = @import("../rules.zig");
44

55
pub const RulesConfig = struct {
66
pub usingnamespace @import("./rules_config.methods.zig").RulesConfigMethods(@This());
7+
allocator_first_param: RuleConfig(rules.AllocatorFirstParam) = .{},
8+
avoid_as: RuleConfig(rules.AvoidAs) = .{},
9+
case_convention: RuleConfig(rules.CaseConvention) = .{},
10+
empty_file: RuleConfig(rules.EmptyFile) = .{},
711
homeless_try: RuleConfig(rules.HomelessTry) = .{},
812
line_length: RuleConfig(rules.LineLength) = .{},
913
must_return_ref: RuleConfig(rules.MustReturnRef) = .{},
1014
no_catch_return: RuleConfig(rules.NoCatchReturn) = .{},
15+
no_print: RuleConfig(rules.NoPrint) = .{},
1116
no_return_try: RuleConfig(rules.NoReturnTry) = .{},
1217
no_unresolved: RuleConfig(rules.NoUnresolved) = .{},
18+
returned_stack_reference: RuleConfig(rules.ReturnedStackReference) = .{},
1319
suppressed_errors: RuleConfig(rules.SuppressedErrors) = .{},
1420
unsafe_undefined: RuleConfig(rules.UnsafeUndefined) = .{},
1521
unused_decls: RuleConfig(rules.UnusedDecls) = .{},
1622
useless_error_return: RuleConfig(rules.UselessErrorReturn) = .{},
17-
empty_file: RuleConfig(rules.EmptyFile) = .{},
18-
avoid_as: RuleConfig(rules.AvoidAs) = .{},
19-
case_convention: RuleConfig(rules.CaseConvention) = .{},
20-
returned_stack_reference: RuleConfig(rules.ReturnedStackReference) = .{},
21-
allocator_first_param: RuleConfig(rules.AllocatorFirstParam) = .{},
2223
};

src/linter/rules.zig

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
1+
pub const AllocatorFirstParam = @import("./rules/allocator_first_param.zig");
2+
pub const AvoidAs = @import("./rules/avoid_as.zig");
3+
pub const CaseConvention = @import("./rules/case_convention.zig");
4+
pub const EmptyFile = @import("./rules/empty_file.zig");
15
pub const HomelessTry = @import("./rules/homeless_try.zig");
26
pub const LineLength = @import("./rules/line_length.zig");
37
pub const MustReturnRef = @import("./rules/must_return_ref.zig");
48
pub const NoCatchReturn = @import("./rules/no_catch_return.zig");
9+
pub const NoPrint = @import("./rules/no_print.zig");
510
pub const NoReturnTry = @import("./rules/no_return_try.zig");
611
pub const NoUnresolved = @import("./rules/no_unresolved.zig");
12+
pub const ReturnedStackReference = @import("./rules/returned_stack_reference.zig");
713
pub const SuppressedErrors = @import("./rules/suppressed_errors.zig");
814
pub const UnsafeUndefined = @import("./rules/unsafe_undefined.zig");
915
pub const UnusedDecls = @import("./rules/unused_decls.zig");
1016
pub const UselessErrorReturn = @import("./rules/useless_error_return.zig");
11-
pub const EmptyFile = @import("./rules/empty_file.zig");
12-
pub const AvoidAs = @import("./rules/avoid_as.zig");
13-
pub const CaseConvention = @import("./rules/case_convention.zig");
14-
pub const ReturnedStackReference = @import("./rules/returned_stack_reference.zig");
15-
pub const AllocatorFirstParam = @import("./rules/allocator_first_param.zig");

src/linter/rules/no_print.zig

Lines changed: 263 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,263 @@
1+
//! ## What This Rule Does
2+
//! Disallows the use of `std.debug.print`.
3+
//!
4+
//! `print` statements are great for debugging, but they should be removed
5+
//! before code gets merged. When you need debug logs in production, use
6+
//! `std.log` instead.
7+
//!
8+
//! This rule makes a best-effort attempt to ensure `print` calls are actually
9+
//! from `std.debug.print`. It will not report calls to custom print functions
10+
//! if they are defined within the same file. If you are getting false positives
11+
//! because you import a custom print function, consider disabling this rule on
12+
//! a file-by-file basis instead of turning it off globally.
13+
//!
14+
//! ### Tests
15+
//! By default, this rule ignores `print`s in test blocks and files. Files are
16+
//! considered to be a test file if they end with `test.zig`. You may disable
17+
//! this by setting `allow_tests` to `false` in the rule's metadata.
18+
//!
19+
//! ```json
20+
//! {
21+
//! "rules": {
22+
//! "no-print": ["warn", { "allow_tests": false }]
23+
//! }
24+
//! }
25+
//! ```
26+
//!
27+
//! ## Examples
28+
//!
29+
//! Examples of **incorrect** code for this rule:
30+
//! ```zig
31+
//! const std = @import("std");
32+
//! const debug = std.debug;
33+
//! const print = std.debug.print;
34+
//! fn main() void {
35+
//! std.debug.print("This should not be here: {d}\n", .{42});
36+
//! debug.print("This should not be here: {d}\n", .{42});
37+
//! print("This should not be here: {d}\n", .{42});
38+
//! }
39+
//! ```
40+
//!
41+
//! Examples of **correct** code for this rule:
42+
//! ```zig
43+
//! const std = @import("std");
44+
//! fn foo() u32 {
45+
//! std.log.debug("running foo", .{});
46+
//! return 1;
47+
//! }
48+
//!
49+
//! test foo {
50+
//! std.debug.print("testing foo\n", .{});
51+
//! try std.testing.expectEqual(1, foo());
52+
//! }
53+
//! ```
54+
//!
55+
//! ```zig
56+
//! fn print(comptime msg: []const u8, args: anytype) void {
57+
//! // ...
58+
//! }
59+
//! fn main() void {
60+
//! print("Staring program", .{});
61+
//! }
62+
//! ```
63+
64+
const std = @import("std");
65+
const util = @import("util");
66+
const ast_utils = @import("../ast_utils.zig");
67+
const _source = @import("../../source.zig");
68+
const _rule = @import("../rule.zig");
69+
const _span = @import("../../span.zig");
70+
71+
const Loc = std.zig.Loc;
72+
const Span = _span.Span;
73+
const LabeledSpan = _span.LabeledSpan;
74+
const LinterContext = @import("../lint_context.zig");
75+
const Rule = _rule.Rule;
76+
const NodeWrapper = _rule.NodeWrapper;
77+
78+
const Semantic = @import("../../Semantic.zig");
79+
const Ast = Semantic.Ast;
80+
const Node = Ast.Node;
81+
const TokenIndex = Ast.TokenIndex;
82+
const Symbol = Semantic.Symbol;
83+
const Scope = Semantic.Scope;
84+
85+
const Error = @import("../../Error.zig");
86+
const Cow = util.Cow(false);
87+
const eql = std.mem.eql;
88+
89+
pub const meta: Rule.Meta = .{
90+
.name = "no-print",
91+
.category = .restriction,
92+
.default = .warning,
93+
};
94+
95+
const NoPrint = @This();
96+
97+
/// Do not report print calls in test blocks or files.
98+
allow_tests: bool = true,
99+
100+
fn noPrintDiagnostic(ctx: *LinterContext, span: Span) Error {
101+
var d = ctx.diagnostic(
102+
"Using `std.debug.print` is not allowed.",
103+
.{LabeledSpan{ .span = span }},
104+
);
105+
d.help = .static("End-users don't want to see debug logs. Use `std.log` instead.");
106+
return d;
107+
}
108+
109+
pub fn runOnNode(self: *const NoPrint, wrapper: NodeWrapper, ctx: *LinterContext) void {
110+
const node = wrapper.node;
111+
112+
switch (node.tag) {
113+
.call, .call_comma => {},
114+
else => return,
115+
}
116+
if (self.allow_tests) {
117+
// check if we're inside a test block
118+
if (ast_utils.isInTest(ctx, wrapper.idx)) {
119+
return;
120+
}
121+
if (ctx.source.pathname) |pathname| {
122+
if (std.mem.endsWith(u8, pathname, "test.zig")) {
123+
return; // skip test files
124+
}
125+
}
126+
}
127+
const nodes = ctx.ast().nodes;
128+
const tags: []const Node.Tag = nodes.items(.tag);
129+
130+
const callee = node.data.lhs;
131+
// SAFETY: initialized below. if not found, or identifier is not "print",
132+
// rule returns before this is used.
133+
var print_span: Span = undefined;
134+
135+
switch (tags[callee]) {
136+
// look for `print(msg, args);`
137+
.identifier => {
138+
const ident_tok: TokenIndex = nodes.items(.main_token)[callee];
139+
std.debug.assert(ctx.ast().tokens.items(.tag)[ident_tok] == .identifier);
140+
const ident_span = ctx.semantic.tokenSpan(ident_tok);
141+
142+
if (!eql(u8, "print", ident_span.snippet(ctx.source.text()))) {
143+
return;
144+
}
145+
146+
// this may be a call to a locally-defined, custom print function
147+
check_local_print: {
148+
const decl = ctx.semantic.resolveBinding(
149+
ctx.links().getScope(callee) orelse break :check_local_print,
150+
"print",
151+
.{ .exclude = .{ .s_variable = true } },
152+
);
153+
if (decl != null) return;
154+
}
155+
// FIXME: references do fns do not appear to be working
156+
// if (ctx.links().references.get(callee)) |symbol_id| {
157+
// // is this a call to a custom print function?
158+
// const flags: Symbol.Flags = ctx.symbols().symbols.items(.flags)[symbol_id.int()];
159+
// if (flags.s_fn) {
160+
// return;
161+
// }
162+
// }
163+
164+
print_span = ident_span;
165+
},
166+
// look for `std.debug.print(msg, args);`
167+
.field_access => {
168+
const field_access: Node.Data = nodes.items(.data)[callee];
169+
170+
const ident_tok: TokenIndex = field_access.rhs;
171+
std.debug.assert(ctx.ast().tokens.items(.tag)[ident_tok] == .identifier);
172+
const field_span = ctx.semantic.tokenSpan(ident_tok);
173+
if (!eql(u8, "print", field_span.snippet(ctx.source.text()))) {
174+
return;
175+
}
176+
177+
const maybe_debug: []const u8 = ast_utils.getRightmostIdentifier(ctx, field_access.lhs) orelse {
178+
return; // not a field access with an identifier
179+
};
180+
if (!eql(u8, "debug", maybe_debug)) {
181+
return; // not `debug.print()`, probably `writer.print()` (which is fine)
182+
}
183+
184+
print_span = field_span;
185+
},
186+
else => return,
187+
}
188+
189+
ctx.report(noPrintDiagnostic(ctx, print_span));
190+
}
191+
192+
pub fn rule(self: *NoPrint) Rule {
193+
return Rule.init(self);
194+
}
195+
196+
const RuleTester = @import("../tester.zig");
197+
test NoPrint {
198+
const t = std.testing;
199+
200+
var no_print = NoPrint{};
201+
var runner = RuleTester.init(t.allocator, no_print.rule());
202+
defer runner.deinit();
203+
204+
const pass = &[_][:0]const u8{
205+
\\fn foo(Writer: type, w: *Writer) !void {
206+
\\ w.print("writers are allowed", .{});
207+
\\}
208+
,
209+
\\fn add(a: u32, b: u32) u32 {
210+
\\ return a + b;
211+
\\}
212+
\\test add {
213+
\\ const std = @import("std");
214+
\\ std.debug.print("testing add({d}, {d})\n", .{1, 2});
215+
\\ try std.testing.expectEqual(3, add(1, 2));
216+
\\}
217+
,
218+
\\fn print(comptime msg: []const u8, args: anytype) void {
219+
\\ // some custom print function
220+
\\ _ = msg;
221+
\\ _ = args;
222+
\\}
223+
\\fn main() void {
224+
\\ print("This is a custom print function", .{});
225+
\\}
226+
,
227+
\\const print = @import("custom_print.zig").print;
228+
\\fn main() void {
229+
\\ print("this has a different signature so it wont get reported");
230+
\\}
231+
,
232+
\\const println = @import("custom_print.zig").println;
233+
\\fn main() void {
234+
\\ println("std.debug has no println function so it should not get reported", .{});
235+
\\}
236+
,
237+
};
238+
239+
const fail = &[_][:0]const u8{
240+
\\const std = @import("std");
241+
\\fn foo() void {
242+
\\ std.debug.print("This should not be here: {d}\n", .{42});
243+
\\}
244+
,
245+
\\const std = @import("std");
246+
\\const debug = std.debug;
247+
\\fn foo() void {
248+
\\ debug.print("This should not be here: {d}\n", .{42});
249+
\\}
250+
,
251+
\\const std = @import("std");
252+
\\const print = std.debug.print;
253+
\\fn foo() void {
254+
\\ print("This should not be here: {d}\n", .{42});
255+
\\}
256+
,
257+
};
258+
259+
try runner
260+
.withPass(pass)
261+
.withFail(fail)
262+
.run();
263+
}

0 commit comments

Comments
 (0)