Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linter: ergonomics and new rules #10197

Merged
merged 8 commits into from
Apr 12, 2024
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
6 changes: 5 additions & 1 deletion packages/bun-internal-test/src/banned.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
{
"std.debug.assert": "Use bun.assert instead"
"std.debug.assert": "Use bun.assert instead",
"@import(\"root\").bun.": "Only import 'bun' once",
"std.mem.indexOfAny": "Use bun.strings.indexAny or bun.strings.indexAnyComptime",
"std.debug.print": "Don't let this be committed",
"": ""
}
6 changes: 5 additions & 1 deletion packages/bun-internal-test/src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,12 @@ const write = (text: string) => {
report += text;
};
for (const [banned, suggestion] of Object.entries(BANNED)) {
if (banned.length === 0) continue;
// Run git grep to find occurrences of std.debug.assert in .zig files
let stdout = await $`git grep -n "${banned}" "src/**/**.zig"`.text();
// .nothrow() is here since git will exit with non-zero if no matches are found.
let stdout = await $`git grep -n -F "${banned}" "src/**/**.zig" | grep -v -F '//' | grep -v -F bench`
.nothrow()
.text();

stdout = stdout.trim();
if (stdout.length === 0) continue;
Expand Down
10 changes: 5 additions & 5 deletions src/bun.js/bindings/bindings-generator.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ const Exports = @import("exports.zig");
const HeaderGen = @import("./header-gen.zig").HeaderGen;
const std = @import("std");
const builtin = @import("builtin");
const bun = @import("root").bun;
const io = std.io;
const fs = std.fs;
const process = std.process;
const ChildProcess = std.ChildProcess;
const Progress = std.Progress;
const print = std.debug.print;
const mem = std.mem;
const testing = std.testing;
const Allocator = std.mem.Allocator;
Expand All @@ -20,7 +20,7 @@ const JSC = bun.JSC;
const Classes = JSC.GlobalClasses;

pub fn main() anyerror!void {
var allocator = std.heap.c_allocator;
const allocator = std.heap.c_allocator;
const src: std.builtin.SourceLocation = @src();
const src_path = comptime bun.Environment.base_path ++ std.fs.path.dirname(src.file).?;
{
Expand All @@ -46,16 +46,16 @@ pub fn main() anyerror!void {
inline while (i < Classes.len) : (i += 1) {
const Class = Classes[i];
const paths = [_][]const u8{ src_path, Class.name ++ ".generated.h" };
var headerFilePath = try std.fs.path.join(
const headerFilePath = try std.fs.path.join(
allocator,
&paths,
);
var implFilePath = try std.fs.path.join(
const implFilePath = try std.fs.path.join(
allocator,
&[_][]const u8{ std.fs.path.dirname(src.file) orelse return error.BadPath, Class.name ++ ".generated.cpp" },
);
var headerFile = try std.fs.createFileAbsolute(headerFilePath, .{});
var header_writer = headerFile.writer();
const header_writer = headerFile.writer();
var implFile = try std.fs.createFileAbsolute(implFilePath, .{});
try Class.@"generateC++Header"(header_writer);
try Class.@"generateC++Class"(implFile.writer());
Expand Down
3 changes: 2 additions & 1 deletion src/bun.js/unbounded_queue.zig
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const std = @import("std");
const bun = @import("root").bun;

const os = std.os;
const mem = std.mem;
Expand All @@ -7,7 +8,7 @@ const atomic = std.atomic;
const builtin = std.builtin;
const testing = std.testing;

const assert = @import("root").bun.assert;
const assert = bun.assert;

const mpsc = @This();

Expand Down
3 changes: 1 addition & 2 deletions src/cli/bunx_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,7 @@ pub const BunxCommand = struct {
else => ":",
};

const has_banned_char = std.mem.indexOfAny(u8, update_request.name, banned_path_chars) != null or
std.mem.indexOfAny(u8, display_version, banned_path_chars) != null;
const has_banned_char = bun.strings.indexAnyComptime(update_request.name, banned_path_chars) != null or bun.strings.indexAnyComptime(display_version, banned_path_chars) != null;

break :brk try if (has_banned_char)
// This branch gets hit usually when a URL is requested as the package
Expand Down
3 changes: 2 additions & 1 deletion src/cli/upgrade_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ pub const UpgradeCheckerThread = struct {
fn run(env_loader: *DotEnv.Loader) void {
_run(env_loader) catch |err| {
if (Environment.isDebug) {
std.debug.print("\n[UpgradeChecker] ERROR: {s}\n", .{@errorName(err)});
Output.prettyError("\n[UpgradeChecker] ERROR: {s}\n", .{@errorName(err)});
Output.flush();
}
};
}
Expand Down
20 changes: 10 additions & 10 deletions src/deps/diffz/DiffMatchPatch.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1398,15 +1398,15 @@ fn diffCommonOverlap(text1_in: []const u8, text2_in: []const u8) usize {
}

// pub fn main() void {
// var arena = @import("root").bun.ArenaAllocator.init(std.heap.page_allocator);
// var arena = bun.ArenaAllocator.init(std.heap.page_allocator);
// defer arena.deinit();

// var bruh = default.diff(arena.allocator(), "Hello World.", "Goodbye World.", true);
// std.log.err("{any}", .{bruh});
// }

// test {
// var arena = @import("root").bun.ArenaAllocator.init(testing.allocator);
// var arena = bun.ArenaAllocator.init(testing.allocator);
// defer arena.deinit();

// var bruh = try default.diff(arena.allocator(), "Hello World.", "Goodbye World.", true);
Expand Down Expand Up @@ -1455,7 +1455,7 @@ test diffCommonOverlap {
}

test diffHalfMatch {
var arena = @import("root").bun.ArenaAllocator.init(testing.allocator);
var arena = bun.ArenaAllocator.init(testing.allocator);
defer arena.deinit();

var one_timeout = DiffMatchPatch{};
Expand Down Expand Up @@ -1549,7 +1549,7 @@ test diffHalfMatch {
}

test diffLinesToChars {
var arena = @import("root").bun.ArenaAllocator.init(testing.allocator);
var arena = bun.ArenaAllocator.init(testing.allocator);
defer arena.deinit();

// Convert lines down to characters.
Expand Down Expand Up @@ -1611,7 +1611,7 @@ test diffLinesToChars {
}

test diffCharsToLines {
var arena = @import("root").bun.ArenaAllocator.init(testing.allocator);
var arena = bun.ArenaAllocator.init(testing.allocator);
defer arena.deinit();

try testing.expect((Diff.init(.equal, "a")).eql(Diff.init(.equal, "a")));
Expand Down Expand Up @@ -1640,7 +1640,7 @@ test diffCharsToLines {
}

test diffCleanupMerge {
var arena = @import("root").bun.ArenaAllocator.init(testing.allocator);
var arena = bun.ArenaAllocator.init(testing.allocator);
defer arena.deinit();

// Cleanup a messy diff.
Expand Down Expand Up @@ -1828,7 +1828,7 @@ test diffCleanupMerge {
}

test diffCleanupSemanticLossless {
var arena = @import("root").bun.ArenaAllocator.init(testing.allocator);
var arena = bun.ArenaAllocator.init(testing.allocator);
defer arena.deinit();

var diffs = DiffList{};
Expand Down Expand Up @@ -1953,7 +1953,7 @@ fn rebuildtexts(allocator: std.mem.Allocator, diffs: DiffList) ![2][]const u8 {
}

test diffBisect {
var arena = @import("root").bun.ArenaAllocator.init(talloc);
var arena = bun.ArenaAllocator.init(talloc);
defer arena.deinit();

// Normal.
Expand Down Expand Up @@ -1987,7 +1987,7 @@ test diffBisect {

const talloc = testing.allocator;
test diff {
var arena = @import("root").bun.ArenaAllocator.init(talloc);
var arena = bun.ArenaAllocator.init(talloc);
defer arena.deinit();

// Perform a trivial diff.
Expand Down Expand Up @@ -2094,7 +2094,7 @@ test diff {
}

test diffCleanupSemantic {
var arena = @import("root").bun.ArenaAllocator.init(talloc);
var arena = bun.ArenaAllocator.init(talloc);
defer arena.deinit();

// Cleanup semantically trivial equalities.
Expand Down
42 changes: 0 additions & 42 deletions src/deps/picohttp.zig
Original file line number Diff line number Diff line change
Expand Up @@ -199,30 +199,6 @@ pub const Response = struct {
}
};

test "pico_http: parse response" {
const RES = "HTTP/1.1 200 OK\r\n" ++
"Date: Mon, 22 Mar 2021 08:15:54 GMT\r\n" ++
"Content-Type: text/html; charset=utf-8\r\n" ++
"Content-Length: 9593\r\n" ++
"Connection: keep-alive\r\n" ++
"Server: gunicorn/19.9.0\r\n" ++
"Access-Control-Allow-Origin: *\r\n" ++
"Access-Control-Allow-Credentials: true\r\n" ++
"\r\n";

var headers: [32]Header = undefined;

const res = try Response.parse(RES, &headers);

std.debug.print("Minor Version: {}\n", .{res.minor_version});
std.debug.print("Status Code: {}\n", .{res.status_code});
std.debug.print("Status: {s}\n", .{res.status});

for (res.headers) |header| {
std.debug.print("{}\n", .{header});
}
}

pub const Headers = struct {
headers: []const Header,

Expand Down Expand Up @@ -253,22 +229,4 @@ pub const Headers = struct {
}
};

test "pico_http: parse headers" {
const HEADERS = "Date: Mon, 22 Mar 2021 08:15:54 GMT\r\n" ++
"Content-Type: text/html; charset=utf-8\r\n" ++
"Content-Length: 9593\r\n" ++
"Connection: keep-alive\r\n" ++
"Server: gunicorn/19.9.0\r\n" ++
"Access-Control-Allow-Origin: *\r\n" ++
"Access-Control-Allow-Credentials: true\r\n" ++
"\r\n";

var headers: [32]Header = undefined;

const result = try Headers.parse(HEADERS, &headers);
for (result.headers) |header| {
std.debug.print("{}\n", .{header});
}
}

pub usingnamespace c;
5 changes: 3 additions & 2 deletions src/deps/zig-clap/clap.zig
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const std = @import("std");
const bun = @import("root").bun;

const debug = std.debug;
const heap = std.heap;
Expand Down Expand Up @@ -251,7 +252,7 @@ fn testDiag(diag: Diagnostic, err: anyerror, expected: []const u8) void {

pub fn Args(comptime Id: type, comptime params: []const Param(Id)) type {
return struct {
arena: @import("root").bun.ArenaAllocator,
arena: bun.ArenaAllocator,
clap: ComptimeClap(Id, params),
exe_arg: ?[]const u8,

Expand Down Expand Up @@ -484,7 +485,7 @@ pub fn simpleHelp(
if (desc_text.len == 0) continue;

// create a string with spaces_len spaces
const default_allocator = @import("root").bun.default_allocator;
const default_allocator = bun.default_allocator;

const flags_len = if (param.names.long) |l| l.len else 0;
const num_spaces_after = max_spacing - flags_len;
Expand Down
8 changes: 4 additions & 4 deletions src/deps/zig-clap/clap/args.zig
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const bun = @import("root").bun;
pub const OsIterator = struct {
const Error = process.ArgIterator.InitError;

arena: @import("root").bun.ArenaAllocator,
arena: bun.ArenaAllocator,
remain: [][:0]const u8,

/// The executable path (this is the first argument passed to the program)
Expand All @@ -59,7 +59,7 @@ pub const OsIterator = struct {

pub fn init(allocator: mem.Allocator) OsIterator {
var res = OsIterator{
.arena = @import("root").bun.ArenaAllocator.init(allocator),
.arena = bun.ArenaAllocator.init(allocator),
.exe_arg = undefined,
.remain = bun.argv(),
};
Expand Down Expand Up @@ -90,12 +90,12 @@ pub const ShellIterator = struct {
QuoteNotClosed,
} || mem.Allocator.Error;

arena: @import("root").bun.ArenaAllocator,
arena: bun.ArenaAllocator,
str: []const u8,

pub fn init(allocator: mem.Allocator, str: []const u8) ShellIterator {
return .{
.arena = @import("root").bun.ArenaAllocator.init(allocator),
.arena = bun.ArenaAllocator.init(allocator),
.str = str,
};
}
Expand Down
3 changes: 2 additions & 1 deletion src/deps/zig-clap/clap/streaming.zig
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
const builtin = @import("builtin");
const clap = @import("../clap.zig");
const std = @import("std");
const Output = @import("root").bun.Output;
const bun = @import("root").bun;
const Output = bun.Output;

const args = clap.args;
const debug = std.debug;
Expand Down
18 changes: 9 additions & 9 deletions src/install/install.zig
Original file line number Diff line number Diff line change
Expand Up @@ -8861,15 +8861,15 @@ pub const PackageManager = struct {
LifecycleScriptSubprocess.alive_count.load(.Monotonic) < this.manager.options.max_concurrent_lifecycle_scripts;
}

pub fn printTreeDeps(this: *PackageInstaller) void {
for (this.tree_ids_to_trees_the_id_depends_on, 0..) |deps, j| {
std.debug.print("tree #{d:3}: ", .{j});
for (0..this.lockfile.buffers.trees.items.len) |tree_id| {
std.debug.print("{d} ", .{@intFromBool(deps.isSet(tree_id))});
}
std.debug.print("\n", .{});
}
}
// pub fn printTreeDeps(this: *PackageInstaller) void {
// for (this.tree_ids_to_trees_the_id_depends_on, 0..) |deps, j| {
// std.debug.print("tree #{d:3}: ", .{j});
// for (0..this.lockfile.buffers.trees.items.len) |tree_id| {
// std.debug.print("{d} ", .{@intFromBool(deps.isSet(tree_id))});
// }
// std.debug.print("\n", .{});
// }
// }

pub fn deinit(this: *PackageInstaller) void {
const allocator = this.manager.allocator;
Expand Down
2 changes: 1 addition & 1 deletion src/install/lockfile.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1450,7 +1450,7 @@ pub const Printer = struct {
needs_comma = false;
}
const version_name = dependency_version.literal.slice(string_buf);
const needs_quote = always_needs_quote or std.mem.indexOfAny(u8, version_name, " |\t-/!") != null or strings.hasPrefixComptime(version_name, "npm:");
const needs_quote = always_needs_quote or bun.strings.indexAnyComptime(version_name, " |\t-/!") != null or strings.hasPrefixComptime(version_name, "npm:");

if (needs_quote) {
try writer.writeByte('"');
Expand Down
3 changes: 2 additions & 1 deletion src/io/fifo.zig
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const std = @import("std");
const assert = @import("root").bun.assert;
const bun = @import("root").bun;
const assert = bun.assert;

/// An intrusive first in/first out linked list.
/// The element type T must have a field called "next" of type ?*T
Expand Down
3 changes: 2 additions & 1 deletion src/io/heap.zig
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const std = @import("std");
const assert = @import("root").bun.assert;
const bun = @import("root").bun;
const assert = bun.assert;

/// An intrusive heap implementation backed by a pairing heap[1] implementation.
///
Expand Down
3 changes: 2 additions & 1 deletion src/io/time.zig
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const std = @import("std");
const assert = @import("root").bun.assert;
const bun = @import("root").bun;
const assert = bun.assert;
const is_darwin = @import("builtin").target.isDarwin();

pub const Time = struct {
Expand Down
Loading
Loading