refactor: ban std.debug.assert (#10168)

* Ban `std.debug.assert` * Create .clangd * Update lint.yml * Update linter.ts * update * lint * Update linter.ts * Update linter.ts * update * Update linter.ts * update * Update linter.ts * more * Update install.zig * words * Remove UB
oven-sh · Apr 12, 2024 · 688844b · 688844b
1 parent 0f10d4f
commit 688844b
Show file tree

Hide file tree

Showing 149 changed files with 1,314 additions and 1,105 deletions.
diff --git a/.clangd b/.clangd
@@ -0,0 +1,3 @@
+Index:
+  Background: Skip # Disable slow background indexing of these files.
+
diff --git a/.github/workflows/bun-linux-build.yml b/.github/workflows/bun-linux-build.yml
@@ -310,7 +310,7 @@ jobs:
           mode: upsert
           create_if_not_exists: false
           message: |
-            ✅ test failures on ${{ matrix.tag }} have been resolved.
+            ✅ test failures on ${{ matrix.tag }} have been resolved. Thank you.
 
             <sup>[#${{github.sha}}](https://github.com/oven-sh/bun/commits/${{github.sha}})</sup>
       - id: fail

diff --git a/.github/workflows/bun-mac-aarch64.yml b/.github/workflows/bun-mac-aarch64.yml
@@ -472,7 +472,7 @@ jobs:
           mode: upsert
           create_if_not_exists: false
           message: |
-            ✅ test failures on ${{ matrix.tag }} have been resolved.
+            ✅ test failures on ${{ matrix.tag }} have been resolved. Thank you.
 
             <sup>[#${{github.sha}}](https://github.com/oven-sh/bun/commits/${{github.sha}})</sup>
       - id: fail

diff --git a/.github/workflows/bun-mac-x64-baseline.yml b/.github/workflows/bun-mac-x64-baseline.yml
@@ -463,7 +463,7 @@ jobs:
           mode: upsert
           create_if_not_exists: false
           message: |
-            ✅ test failures on ${{ matrix.tag }} have been resolved.
+            ✅ test failures on ${{ matrix.tag }} have been resolved. Thank you.
 
             <sup>[#${{github.sha}}](https://github.com/oven-sh/bun/commits/${{github.sha}})</sup>
       - id: fail

diff --git a/.github/workflows/bun-mac-x64.yml b/.github/workflows/bun-mac-x64.yml
@@ -458,7 +458,7 @@ jobs:
           mode: upsert
           create_if_not_exists: false
           message: |
-            ✅ test failures on ${{ matrix.tag }} have been resolved.
+            ✅ test failures on ${{ matrix.tag }} have been resolved. Thank you.
 
             <sup>[#${{github.sha}}](https://github.com/oven-sh/bun/commits/${{github.sha}})</sup>
       - id: fail

diff --git a/.github/workflows/bun-windows.yml b/.github/workflows/bun-windows.yml
@@ -501,7 +501,7 @@ jobs:
           mode: upsert
           create_if_not_exists: false
           message: |
-            ✅🪟 Test regressions on Windows ${{ matrix.arch }}${{ matrix.cpu == 'nehalem' && ' Baseline' || '' }} have been resolved.
+            ✅🪟 Test regressions on Windows ${{ matrix.arch }}${{ matrix.cpu == 'nehalem' && ' Baseline' || '' }} have been resolved. Thank you.
       - id: fail
         name: Fail the build
         if: steps.test.outputs.failing_tests != ''

diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -0,0 +1,86 @@
+name: lint
+
+permissions:
+  contents: read
+
+on:
+  workflow_dispatch:
+  pull_request:
+  push:
+    branches:
+      - main
+      - jarred/assert
+    paths:
+      - ".github/workflows/lint.yml"
+      - "src/**/*.zig"
+      - "src/*.zig"
+
+jobs:
+  format:
+    name: lint
+    runs-on: ${{ vars.RUNNER_LINUX_X64 || 'ubuntu-latest' }}
+    if: github.repository_owner == 'oven-sh'
+    permissions: write-all
+    outputs:
+      text_output: ${{ steps.lint.outputs.text_output }}
+      json_output: ${{ steps.lint.outputs.json_output }}
+      count: ${{ steps.lint.outputs.count }}
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+      - name: Setup Bun
+        uses: ./.github/actions/setup-bun
+        with:
+          bun-version: "1.1.3"
+      - name: Install Dependencies
+        run: |
+          bun --cwd=./packages/bun-internal-test install
+      - name: Lint
+        id: lint
+        run: |
+          bun ./packages/bun-internal-test/src/linter.ts || true
+      - uses: sarisia/actions-status-discord@v1
+        if: always() && steps.lint.outputs.text_output != '' && github.event_name == 'pull_request'
+        with:
+          title: ""
+          webhook: ${{ secrets.DISCORD_WEBHOOK }}
+          status: "failure"
+          noprefix: true
+          nocontext: true
+          description: |
+            Pull Request
+            ### ❌ [${{github.event.pull_request.title}}](https://github.com/oven-sh/bun/pull/${{github.event.number}})
+
+            @${{ github.actor }}, there are ${{ steps.lint.outputs.count }} lint errors on ${{ github.ref_name }}
+
+            ${{ steps.lint.outputs.text_output }}
+
+            **[View linter output](https://github.com/oven-sh/bun/actions/runs/${{github.run_id}})**
+      - name: Comment on PR
+        if: steps.lint.outputs.text_output != '' && github.event_name == 'pull_request'
+        uses: thollander/actions-comment-pull-request@v2
+        with:
+          comment_tag: lint-failures
+          message: |
+            ❌ @${{ github.actor }} ${{ steps.lint.outputs.count }} lint errors
+
+            ${{ steps.lint.outputs.text_output }}
+
+            **[View linter output](https://github.com/oven-sh/bun/actions/runs/${{github.run_id}})**
+
+            <sup>[#${{github.sha}}](https://github.com/oven-sh/bun/commits/${{github.sha}})</sup>
+      - name: Uncomment on PR
+        if: steps.lint.outputs.text_output == '' && github.event_name == 'pull_request'
+        uses: thollander/actions-comment-pull-request@v2
+        with:
+          comment_tag: lint-failures
+          mode: upsert
+          create_if_not_exists: false
+          message: |
+            ✅ lint failures have been resolved. Thank you.
+
+            <sup>[#${{github.sha}}](https://github.com/oven-sh/bun/commits/${{github.sha}})</sup>
+      - id: fail
+        name: Fail the build
+        if: steps.lint.outputs.text_output != ''
+        run: exit 1
diff --git a/packages/bun-internal-test/src/banned.json b/packages/bun-internal-test/src/banned.json
@@ -0,0 +1,3 @@
+{
+  "std.debug.assert": "Use bun.assert instead"
+}
diff --git a/packages/bun-internal-test/src/linter.ts b/packages/bun-internal-test/src/linter.ts
@@ -0,0 +1,68 @@
+import { $ } from "bun";
+import BANNED from "./banned.json";
+import * as action from "@actions/core";
+
+const IGNORED_FOLDERS = [
+  // list of folders to ignore
+  "windows-shim",
+];
+
+const ci = !!process.env["GITHUB_ACTIONS"];
+process.chdir(require("path").join(import.meta.dir, "../../../"));
+let bad = [];
+let report = "";
+const write = (text: string) => {
+  process.stdout.write(text);
+  report += text;
+};
+for (const [banned, suggestion] of Object.entries(BANNED)) {
+  // Run git grep to find occurrences of std.debug.assert in .zig files
+  let stdout = await $`git grep -n "${banned}" "src/**/**.zig"`.text();
+
+  stdout = stdout.trim();
+  if (stdout.length === 0) continue;
+
+  let lines = stdout.split("\n");
+  // Parse each line to extract filename and line number
+  const matches = lines
+    .filter(line => !IGNORED_FOLDERS.some(folder => line.includes(folder)))
+    .map(line => {
+      const [path, lineNumber, ...text] = line.split(":");
+      return { path, lineNumber, banned, suggestion, text: text.join(":") };
+    });
+  // Check if we got any output
+  // Split the output into lines
+  if (matches.length === 0) continue;
+
+  write(`Banned **'${banned}'** found in the following locations:` + "\n");
+  matches.forEach(match => {
+    write(`${match.path}:${match.lineNumber}: ${match.text.trim()}` + "\n");
+  });
+  bad = bad.concat(matches);
+}
+
+if (report.length === 0) {
+  process.exit(0);
+}
+
+function link({ path, lineNumber, suggestion, banned }) {
+  action.error(`Lint failure: ${banned} is banned, ${suggestion}`, {
+    file: path,
+    startLine: Number(lineNumber),
+    endLine: Number(lineNumber),
+  });
+  return `[\`${path}:${lineNumber}\`](https://github.com/oven-sh/bun/blob/${process.env.GITHUB_SHA}/${path}#L${lineNumber})`;
+}
+
+if (ci) {
+  if (report.length > 0) {
+    action.setFailed(`${bad.length} lint failures`);
+  }
+  action.setOutput("count", bad.length);
+  action.setOutput("text_output", bad.map(m => `- ${link(m)}: ${m.banned} is banned, ${m.suggestion}`).join("\n"));
+  action.setOutput("json_output", JSON.stringify(bad));
+  action.summary.addRaw(report);
+  await action.summary.write();
+}
+
+process.exit(1);
diff --git a/src/ArenaAllocator.zig b/src/ArenaAllocator.zig
@@ -1,5 +1,5 @@
 const std = @import("std");
-const assert = std.debug.assert;
+const assert = @import("root").bun.assert;
 const mem = std.mem;
 const Allocator = std.mem.Allocator;
 
@@ -132,7 +132,7 @@ pub const ArenaAllocator = struct {
             self.child_allocator.rawFree(alloc_buf, align_bits, @returnAddress());
             it = next_it;
         } else null;
-        std.debug.assert(maybe_first_node == null or maybe_first_node.?.next == null);
+        assert(maybe_first_node == null or maybe_first_node.?.next == null);
         // reset the state before we try resizing the buffers, so we definitely have reset the arena to 0.
         self.state.end_index = 0;
         if (maybe_first_node) |first_node| {

diff --git a/src/StandaloneModuleGraph.zig b/src/StandaloneModuleGraph.zig
@@ -487,7 +487,7 @@ pub const StandaloneModuleGraph = struct {
             var outfile_buf: bun.OSPathBuffer = undefined;
             const outfile_slice = brk: {
                 const outfile_w = bun.strings.toWPathNormalized(&outfile_buf, std.fs.path.basenameWindows(outfile));
-                std.debug.assert(outfile_w.ptr == &outfile_buf);
+                bun.assert(outfile_w.ptr == &outfile_buf);
                 const outfile_buf_u16 = bun.reinterpretSlice(u16, &outfile_buf);
                 if (!bun.strings.endsWithComptime(outfile, ".exe")) {
                     // append .exe
@@ -652,7 +652,7 @@ pub const StandaloneModuleGraph = struct {
             end -= offsets.byte_count;
             @memcpy(to_read[0..offsets.byte_count], end[0..offsets.byte_count]);
             if (comptime Environment.allow_assert) {
-                std.debug.assert(bun.strings.eqlLong(to_read, end[0..offsets.byte_count], true));
+                bun.assert(bun.strings.eqlLong(to_read, end[0..offsets.byte_count], true));
             }
         }
 

diff --git a/src/StaticHashMap.zig b/src/StaticHashMap.zig
@@ -6,7 +6,7 @@ const mem = std.mem;
 const math = std.math;
 const testing = std.testing;
 
-const assert = std.debug.assert;
+const assert = @import("root").bun.assert;
 
 pub fn AutoHashMap(comptime K: type, comptime V: type, comptime max_load_percentage: comptime_int) type {
     return HashMap(K, V, std.hash_map.AutoContext(K), max_load_percentage);

diff --git a/src/__global.zig b/src/__global.zig
@@ -162,26 +162,6 @@ pub fn panic(comptime fmt: string, args: anytype) noreturn {
     }
 }
 
-// std.debug.assert but happens at runtime
-pub fn invariant(condition: bool, comptime fmt: string, args: anytype) void {
-    if (!condition) {
-        _invariant(fmt, args);
-    }
-}
-
-inline fn _invariant(comptime fmt: string, args: anytype) noreturn {
-    @setCold(true);
-
-    if (comptime Environment.isWasm) {
-        Output.printErrorln(fmt, args);
-        Output.flush();
-        @panic(fmt);
-    } else {
-        Output.prettyErrorln(fmt, args);
-        Global.exit(1);
-    }
-}
-
 pub fn notimpl() noreturn {
     @setCold(true);
     Global.panic("Not implemented yet!!!!!", .{});

diff --git a/src/allocators.zig b/src/allocators.zig
@@ -122,7 +122,7 @@ pub fn OverflowList(comptime ValueType: type, comptime count: comptime_int) type
             }
 
             pub fn append(block: *Block, value: ValueType) *ValueType {
-                if (comptime Environment.allow_assert) std.debug.assert(block.used < count);
+                if (comptime Environment.allow_assert) bun.assert(block.used < count);
                 const index = block.used;
                 block.items[index] = value;
                 block.used +%= 1;
@@ -155,9 +155,9 @@ pub fn OverflowList(comptime ValueType: type, comptime count: comptime_int) type
             else
                 0;
 
-            if (comptime Environment.allow_assert) std.debug.assert(index.is_overflow);
-            if (comptime Environment.allow_assert) std.debug.assert(this.list.used >= block_id);
-            if (comptime Environment.allow_assert) std.debug.assert(this.list.ptrs[block_id].used > (index.index % count));
+            if (comptime Environment.allow_assert) bun.assert(index.is_overflow);
+            if (comptime Environment.allow_assert) bun.assert(this.list.used >= block_id);
+            if (comptime Environment.allow_assert) bun.assert(this.list.ptrs[block_id].used > (index.index % count));
 
             return &this.list.ptrs[block_id].items[index.index % count];
         }
@@ -168,9 +168,9 @@ pub fn OverflowList(comptime ValueType: type, comptime count: comptime_int) type
             else
                 0;
 
-            if (comptime Environment.allow_assert) std.debug.assert(index.is_overflow);
-            if (comptime Environment.allow_assert) std.debug.assert(this.list.used >= block_id);
-            if (comptime Environment.allow_assert) std.debug.assert(this.list.ptrs[block_id].used > (index.index % count));
+            if (comptime Environment.allow_assert) bun.assert(index.is_overflow);
+            if (comptime Environment.allow_assert) bun.assert(this.list.used >= block_id);
+            if (comptime Environment.allow_assert) bun.assert(this.list.ptrs[block_id].used > (index.index % count));
 
             return &this.list.ptrs[block_id].items[index.index % count];
         }

diff --git a/src/ast/base.zig b/src/ast/base.zig
@@ -262,7 +262,7 @@ pub const Ref = packed struct(u64) {
     }
 
     pub fn initSourceEnd(old: Ref) Ref {
-        std.debug.assert(old.tag != .invalid);
+        bun.assert(old.tag != .invalid);
         return init(old.inner_index, old.source_index, old.tag == .source_contents_slice);
     }