diff --git a/CHANGELOG.md b/CHANGELOG.md index bc9bfb9..5fc1d34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,16 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `check`/`fix` output is no longer corrupted when redirected to a file: stdout/stderr writers now append at the current offset instead of the positional default, which overwrote earlier output from offset 0 +- `const X = @This();` is now treated as an alias and sorts first in the + alias band instead of splitting the import block +- aliases stranded below the import block are now hoisted into the alias + band instead of being left unsorted in the body +- hoisting a stray import no longer swallows the blank line separating it + from the code above +- the sorted import block is now always separated from the body by a blank + line (doc comments stay attached to the decls they document) +- the maximum readable file size was raised from 10 MiB to 32 MiB, so large + generated files are processed instead of failing with `FileTooBig` ## [0.5.0] - 2026-08-05 diff --git a/README.md b/README.md index aaca9d8..b628e10 100644 --- a/README.md +++ b/README.md @@ -155,7 +155,9 @@ changed `.zig` files are passed to the hook. - **Plain import** — `const x = @import("path");` - **Member import** — `const X = @import("path").Member;` (a member access after the import; the base path is what gets sorted) -- **Alias** — `const X = module.Member;` (a dotted name with no `@import`) +- **Alias** — `const X = module.Member;` (a dotted name with no `@import`). + `const X = @This();` is treated as an alias too; since it references the + current module it has no resolvable path and sorts first in the alias band. Imports are emitted in this order: @@ -204,8 +206,10 @@ the same output. `@cImport` blocks count as third-party imports. - Sorts and groups top-of-file imports - Organizes typed imports (`const x: SomeType = @import(...)`) like plain ones - Hoists imports that appear later in the file into their proper group +- Hoists aliases stranded below the import block into the alias band - Keeps comments that immediately precede an import attached to it (blank lines do not travel with an import) +- Ensures a blank line separates the import block from the following code - Preserves `//!` module documentation and the file's line endings (CRLF kept) ### Escape hatches diff --git a/src/ast_scan.zig b/src/ast_scan.zig index 72ad4d1..836ae94 100644 --- a/src/ast_scan.zig +++ b/src/ast_scan.zig @@ -6,6 +6,8 @@ const std = @import("std"); +const Ast = std.zig.Ast; + pub const class_std_builtin: u2 = 0; pub const class_third_party: u2 = 1; pub const class_local: u2 = 2; @@ -85,8 +87,6 @@ pub fn findCommentStart(source: []const u8, line_start: usize) ?usize { return comment_start; } -const Ast = std.zig.Ast; - pub const Analysis = struct { imports: std.ArrayListUnmanaged(Import) = .empty, aliases: std.ArrayListUnmanaged(Import) = .empty, @@ -162,15 +162,9 @@ pub fn analyze(allocator: std.mem.Allocator, source: [:0]const u8) !Analysis { } } - // Aliases outside the block are left alone (never hoisted). - var j: usize = 0; - while (j < result.aliases.items.len) { - if (result.aliases.items[j].start >= result.block_end) { - _ = result.aliases.orderedRemove(j); - } else { - j += 1; - } - } + // Aliases below the block are marked stray so they can be hoisted into + // the alias band, mirroring how stray imports are hoisted. + for (result.aliases.items) |*alias| alias.stray = alias.start >= result.block_end; for (result.imports.items) |*imp| imp.stray = imp.start >= result.block_end; std.sort.pdq(Import, result.imports.items, {}, Import.lessThan); @@ -321,6 +315,20 @@ fn classifyDecl(allocator: std.mem.Allocator, owned: *std.ArrayListUnmanaged([]c }, .alias = false }; } + if (std.mem.eql(u8, base_slice, "@This") and !member) { + // `const X = @This();` references the current module, so it has no + // resolvable path; the sentinel sorts it first in the alias band. + return .{ .imp = .{ + .start = start, + .end = end, + .path = "@This()", + .class = class_std_builtin, + .comment_start = comment_start, + .name = name, + .text = text, + }, .alias = true }; + } + if (member and isAliasChain(tree, init)) { const first = tree.firstToken(init); const last = tree.lastToken(init); diff --git a/src/ast_scan_test.zig b/src/ast_scan_test.zig index ea88986..0a03baa 100644 --- a/src/ast_scan_test.zig +++ b/src/ast_scan_test.zig @@ -202,7 +202,26 @@ test "analyze: spaced-dot alias stops the block" { try std.testing.expectEqual("const std = @import(\"std\");\n".len, analysis.block_end); } -test "analyze: alias after the block is left alone" { +test "analyze: @This() decl collected as alias" { + const source = "const IP = @This();\n"; + var analysis = try ast_scan.analyze(std.testing.allocator, source); + defer analysis.deinit(std.testing.allocator); + try std.testing.expectEqual(@as(usize, 0), analysis.imports.items.len); + try std.testing.expectEqual(@as(usize, 1), analysis.aliases.items.len); + try std.testing.expectEqualStrings("@This()", analysis.aliases.items[0].path); + try std.testing.expect(!analysis.aliases.items[0].member); + try std.testing.expectEqual(ast_scan.class_std_builtin, analysis.aliases.items[0].class); +} + +test "analyze: @This() member chain is not an alias" { + const source = "const Foo = @This().Foo;\n"; + var analysis = try ast_scan.analyze(std.testing.allocator, source); + defer analysis.deinit(std.testing.allocator); + try std.testing.expectEqual(@as(usize, 0), analysis.aliases.items.len); + try std.testing.expectEqual(@as(usize, 0), analysis.imports.items.len); +} + +test "analyze: alias after the block is marked stray" { const source = \\const std = @import("std"); \\pub fn main() {} @@ -210,7 +229,8 @@ test "analyze: alias after the block is left alone" { ; var analysis = try ast_scan.analyze(std.testing.allocator, source); defer analysis.deinit(std.testing.allocator); - try std.testing.expectEqual(@as(usize, 0), analysis.aliases.items.len); + try std.testing.expectEqual(@as(usize, 1), analysis.aliases.items.len); + try std.testing.expect(analysis.aliases.items[0].stray); } test "analyze: nested re-export import is not collected" { diff --git a/src/zsort.zig b/src/zsort.zig index 0e2f3ac..9232537 100644 --- a/src/zsort.zig +++ b/src/zsort.zig @@ -485,6 +485,32 @@ pub fn walkDir( } } +/// Start of the first non-blank line at or after `start`, up to `limit`; +/// blank lines are skipped so a stray import's removal window starts at its +/// first comment line rather than the blank run above it. +fn firstNonBlankLineStart(source: []const u8, start: usize, limit: usize) usize { + var pos = start; + while (pos < limit) { + const le = findLineEnd(source, pos); + if (std.mem.trim(u8, source[pos..le], " \t\r\n").len == 0) { + pos = le; + } else { + return pos; + } + } + return limit; +} + +/// True when `text`'s last line is blank (works for LF and CRLF endings). +fn endsWithBlankLine(text: []const u8) bool { + if (text.len == 0 or text[text.len - 1] != '\n') return false; + var end = text.len - 1; + if (end > 0 and text[end - 1] == '\r') end -= 1; + var start = end; + while (start > 0 and text[start - 1] != '\n') : (start -= 1) {} + return start == end; +} + fn hasSkipComment(source: []const u8) bool { var pos: usize = 0; while (pos < source.len) { @@ -506,6 +532,8 @@ const ProcessResult = struct { banned: bool, banned_msg: ?[]const u8, stray_count: usize, + /// A blank line was inserted between the sorted block and the body. + junction_blank: bool = false, }; pub fn processSource( @@ -554,6 +582,11 @@ pub fn processSource( try stray_imports.append(allocator, imp); } } + for (aliases) |alias| { + if (alias.stray) { + try stray_imports.append(allocator, alias); + } + } var rest: []const u8 = source[block_end..]; var rest_owned: ?[]const u8 = null; @@ -568,7 +601,11 @@ pub fn processSource( defer buf.deinit(allocator); var pos: usize = block_end; for (stray_imports.items) |imp| { - const raw_start = if (imp.comment_start) |cs| cs else imp.start; + // The comment window starts at the first non-blank line so that + // blank lines separating the import from the code above survive + // the removal instead of being swallowed with it. + var raw_start = imp.start; + if (imp.comment_start) |cs| raw_start = firstNonBlankLineStart(source, cs, imp.start); const removal_start = @max(raw_start, pos); if (imp.end <= pos) continue; try buf.appendSlice(allocator, source[pos..removal_start]); @@ -584,9 +621,31 @@ pub fn processSource( errdefer allocator.free(new_imports); const original_block = source[0..block_end]; - const changed = !std.mem.eql(u8, original_block, new_imports) or stray_imports.items.len > 0; + // The sorted block is separated from the body by a blank line unless the + // block already ends with a blank line or a trailing comment (which stays + // attached to the decl it documents). + var last_span_end: usize = 0; + for (imports) |imp| last_span_end = @max(last_span_end, imp.end); + for (aliases) |alias| last_span_end = @max(last_span_end, alias.end); + var has_trailing_comment = false; + var scan_pos = last_span_end; + while (scan_pos < block_end) { + const le = findLineEnd(source, scan_pos); + const line = std.mem.trim(u8, source[scan_pos..le], " \t\r\n"); + if (std.mem.startsWith(u8, line, "//")) { + has_trailing_comment = true; + break; + } + if (line.len != 0) break; + scan_pos = le; + } + const junction_blank = !has_trailing_comment and !endsWithBlankLine(new_imports) and rest.len > 0 and rest[0] != '\n' and rest[0] != '\r'; + const changed = !std.mem.eql(u8, original_block, new_imports) or stray_imports.items.len > 0 or junction_blank; - const full_new = try std.mem.concat(allocator, u8, &.{ new_imports, rest }); + const full_new = if (junction_blank) + try std.mem.concat(allocator, u8, &.{ new_imports, detectNewline(source), rest }) + else + try std.mem.concat(allocator, u8, &.{ new_imports, rest }); if (rest_owned) |owned| allocator.free(owned); return .{ @@ -597,6 +656,7 @@ pub fn processSource( .banned = banned_msg != null, .banned_msg = banned_msg, .stray_count = stray_imports.items.len, + .junction_blank = junction_blank, }; } @@ -792,7 +852,7 @@ const FileJob = struct { fn processFileJob(job: *const FileJob, io: compat.Io) void { const allocator = job.slot.arena.allocator(); - const source = compat.readFileAllocZ(io, compat.cwd(), job.path, allocator, 10 * 1024 * 1024) catch |err| { + const source = compat.readFileAllocZ(io, compat.cwd(), job.path, allocator, 32 * 1024 * 1024) catch |err| { job.slot.read_err = @errorName(err); return; }; @@ -980,7 +1040,7 @@ fn runMain(allocator: std.mem.Allocator, args: []const []const u8, io: compat.Io printStdout(io, " {s}Fixed:{s} {s}{s}{s}\n", .{ out_green, out_reset, out_yellow, esc_path, out_reset }); } } else { - if (result.stray_count > 0) { + if (result.stray_count > 0 or result.junction_blank) { showDiff(io, allocator, file_path, slot.source, result.new_text, color); } else { showDiff(io, allocator, file_path, slot.source[0..result.block_end], result.new_block, color); diff --git a/src/zsort_test.zig b/src/zsort_test.zig index 53a57f6..f16c416 100644 --- a/src/zsort_test.zig +++ b/src/zsort_test.zig @@ -189,6 +189,55 @@ test "processSource: hoists multiline stray import intact" { try std.testing.expect(std.mem.indexOf(u8, result.new_text, "late.zig").? < std.mem.indexOf(u8, result.new_text, "pub fn main").?); } +test "processSource: blank line inserted between block and body" { + const source = + \\pub fn main() void {} + \\ + \\const build_options = @import("build_options"); + ; + const result = try zsort.processSource(std.testing.allocator, source, &.{}); + defer std.testing.allocator.free(result.new_text); + defer std.testing.allocator.free(result.new_block); + try std.testing.expect(result.changed); + const expected = "const build_options = @import(\"build_options\");\n\npub fn main() void {}\n\n"; + try std.testing.expectEqualStrings(expected, result.new_text); +} + +test "processSource: clean block abutting body gains a blank line" { + const source = "const std = @import(\"std\");\npub fn main() {}\n"; + const result = try zsort.processSource(std.testing.allocator, source, &.{}); + defer std.testing.allocator.free(result.new_text); + defer std.testing.allocator.free(result.new_block); + try std.testing.expect(result.changed); + const expected = "const std = @import(\"std\");\n\npub fn main() {}\n"; + try std.testing.expectEqualStrings(expected, result.new_text); +} + +test "processSource: no double blank when rest starts blank" { + const source = + \\const std = @import("std"); + \\ + \\pub fn main() {} + ; + const result = try zsort.processSource(std.testing.allocator, source, &.{}); + defer std.testing.allocator.free(result.new_text); + defer std.testing.allocator.free(result.new_block); + try std.testing.expect(!result.changed); +} + +test "processSource: doc comment stays attached to the following decl" { + const source = + \\const std = @import("std"); + \\/// Docs for the decl. + \\pub fn main() {} + ; + const result = try zsort.processSource(std.testing.allocator, source, &.{}); + defer std.testing.allocator.free(result.new_text); + defer std.testing.allocator.free(result.new_block); + try std.testing.expect(result.changed); + try std.testing.expect(std.mem.indexOf(u8, result.new_text, "/// Docs for the decl.\npub fn main") != null); +} + test "processSource: division-deref is not mistaken for a block comment" { const source = \\const v = a/*b; @@ -201,7 +250,7 @@ test "processSource: division-deref is not mistaken for a block comment" { }; try std.testing.expect(result.changed); try std.testing.expectEqualStrings( - "const std = @import(\"std\");\nconst v = a/*b;\n", + "const std = @import(\"std\");\n\nconst v = a/*b;\n", result.new_text, ); } @@ -233,7 +282,7 @@ test "processSource: unterminated import at EOF terminates" { const result = try zsort.processSource(std.testing.allocator, source, &.{}); defer std.testing.allocator.free(result.new_text); defer std.testing.allocator.free(result.new_block); - try std.testing.expect(!result.changed); + try std.testing.expect(result.changed); try std.testing.expectEqual(@as(usize, 1), std.mem.count(u8, result.new_text, "late.zig")); } @@ -264,7 +313,7 @@ test "processSource: import expression ending in brace terminates" { defer std.testing.allocator.free(result.new_block); defer if (result.banned_msg) |msg| std.testing.allocator.free(msg); try std.testing.expect(result.banned); - try std.testing.expect(!result.changed); + try std.testing.expect(result.changed); try std.testing.expectEqual(@as(usize, 1), std.mem.count(u8, result.new_text, "const x = @import")); } @@ -642,6 +691,25 @@ test "buildSortedImportText: alias imports hoisted after imports" { try std.testing.expect(bar_pos < debug_pos); } +test "buildSortedImportText: @This() sorts first in the alias band" { + const source = + \\const std = @import("std"); + \\const Io = std.Io; + \\const IP = @This(); + \\ + \\const rest = 1; + ; + const block_end = blockEndForTest(source); + var imports = try collectImportsForTest(source, block_end); + defer imports.deinit(std.testing.allocator); + var aliases = try collectAliasesForTest(source); + defer aliases.deinit(std.testing.allocator); + const result = try zsort.buildSortedImportText(std.testing.allocator, source, imports.items, aliases.items, block_end); + defer std.testing.allocator.free(result); + const expected = "const std = @import(\"std\");\n\nconst IP = @This();\nconst Io = std.Io;\n\n"; + try std.testing.expectEqualStrings(expected, result); +} + test "buildSortedImportText: full band order with members and aliases" { const source = \\const std = @import("std"); @@ -754,6 +822,41 @@ test "processSource: hoisted stray import lands in its group" { try std.testing.expect(std.mem.indexOf(u8, result.new_text, "pub fn main") != null); } +test "processSource: alias stranded below block is hoisted into the alias band" { + const source = + \\const std = @import("std"); + \\const log = std.log.scoped(.x); + \\const Allocator = std.mem.Allocator; + \\ + \\pub fn main() {} + ; + const result = try zsort.processSource(std.testing.allocator, source, &.{}); + defer std.testing.allocator.free(result.new_text); + defer std.testing.allocator.free(result.new_block); + try std.testing.expect(result.changed); + const std_pos = std.mem.indexOf(u8, result.new_text, "const std") orelse return error.TestUnexpectedResult; + const alloc_pos = std.mem.indexOf(u8, result.new_text, "const Allocator") orelse return error.TestUnexpectedResult; + const log_pos = std.mem.indexOf(u8, result.new_text, "const log") orelse return error.TestUnexpectedResult; + try std.testing.expect(std_pos < alloc_pos); + try std.testing.expect(alloc_pos < log_pos); +} + +test "processSource: blank above hoisted stray import is preserved" { + const source = + \\const std = @import("std"); + \\const log = std.log.scoped(.x); + \\ + \\const late = @import("late.zig"); + \\ + \\pub fn main() {} + ; + const result = try zsort.processSource(std.testing.allocator, source, &.{}); + defer std.testing.allocator.free(result.new_text); + defer std.testing.allocator.free(result.new_block); + try std.testing.expect(result.changed); + try std.testing.expect(std.mem.indexOf(u8, result.new_text, "const log = std.log.scoped(.x);\n\n\npub fn main") != null); +} + test "processSource: output independent of input order" { const forward = \\const std = @import("std"); @@ -886,8 +989,15 @@ test "processSource: cimport block kept intact" { const result = try zsort.processSource(std.testing.allocator, source, &.{}); defer std.testing.allocator.free(result.new_text); defer std.testing.allocator.free(result.new_block); - try std.testing.expect(!result.changed); - try std.testing.expectEqualStrings(source, result.new_text); + try std.testing.expect(result.changed); + const expected = + \\const c = @cImport({ + \\ #include + \\}); // c trailing + \\ + \\const rest = 1; + ; + try std.testing.expectEqualStrings(expected, result.new_text); } test "processSource: stray cimport hoisted" {