From 5944db49b35839278435289cc182bd51151ad42b Mon Sep 17 00:00:00 2001 From: Techatrix <19954306+Techatrix@users.noreply.github.com> Date: Tue, 21 Mar 2023 01:33:56 +0100 Subject: [PATCH] add multi-file support to ErrorBuilder colorize ErrorBuilder output --- tests/ErrorBuilder.zig | 164 +++++++++++++++++-------- tests/lsp_features/completion.zig | 12 +- tests/lsp_features/inlay_hints.zig | 12 +- tests/lsp_features/references.zig | 12 +- tests/lsp_features/semantic_tokens.zig | 12 +- tests/utility/ast.zig | 13 +- 6 files changed, 152 insertions(+), 73 deletions(-) diff --git a/tests/ErrorBuilder.zig b/tests/ErrorBuilder.zig index 01e0b60..945f81a 100644 --- a/tests/ErrorBuilder.zig +++ b/tests/ErrorBuilder.zig @@ -7,88 +7,156 @@ const offsets = zls.offsets; const ErrorBuilder = @This(); allocator: std.mem.Allocator, -items: std.ArrayListUnmanaged(MsgItem) = .{}, -source: []const u8, +files: std.StringArrayHashMapUnmanaged(File) = .{}, +message_count: usize = 0, -pub fn init(allocator: std.mem.Allocator, source: []const u8) ErrorBuilder { - return ErrorBuilder{ - .allocator = allocator, - .source = source, - }; +pub fn init(allocator: std.mem.Allocator) ErrorBuilder { + return ErrorBuilder{ .allocator = allocator }; } pub fn deinit(builder: *ErrorBuilder) void { - for (builder.items.items) |item| { - builder.allocator.free(item.message); + for (builder.files.values()) |*file| { + for (file.messages.items) |item| { + builder.allocator.free(item.message); + } + file.messages.deinit(builder.allocator); } - builder.items.deinit(builder.allocator); + builder.files.deinit(builder.allocator); } -pub fn msgAtLoc(builder: *ErrorBuilder, comptime fmt: []const u8, loc: offsets.Loc, level: std.log.Level, args: anytype) !void { - try builder.items.append(builder.allocator, .{ +/// assumes `name` and `source` outlives the `ErrorBuilder` +pub fn addFile(builder: *ErrorBuilder, name: []const u8, source: []const u8) error{OutOfMemory}!void { + const gop = try builder.files.getOrPutValue(builder.allocator, name, .{ .source = source }); + if (gop.found_existing) + std.debug.panic("file '{s}' already exists", .{name}); +} + +pub fn removeFile(builder: *ErrorBuilder, name: []const u8) error{OutOfMemory}!void { + const found = builder.files.remove(name); + if (!found) + std.debug.panic("file '{s}' doesn't exist", .{name}); + builder.message_count -= found.messages.items.len; +} + +pub fn msgAtLoc( + builder: *ErrorBuilder, + comptime fmt: []const u8, + file_name: []const u8, + loc: offsets.Loc, + level: std.log.Level, + args: anytype, +) error{OutOfMemory}!void { + if (loc.start > loc.end) + std.debug.panic("invalid source location {}", .{loc}); + const file = builder.files.getPtr(file_name) orelse + std.debug.panic("file '{s}' doesn't exist", .{file_name}); + if (loc.end > file.source.len) + std.debug.panic("source location {} is outside file source (len: {d})", .{ loc, file.source.len }); + const message = try std.fmt.allocPrint(builder.allocator, fmt, args); + errdefer builder.allocator.free(message); + + try file.messages.append(builder.allocator, .{ .loc = loc, .level = level, - .message = try std.fmt.allocPrint(builder.allocator, fmt, args), + .message = message, }); + builder.message_count += 1; } -pub fn msgAtIndex(builder: *ErrorBuilder, comptime fmt: []const u8, index: usize, level: std.log.Level, args: anytype) !void { - return msgAtLoc(builder, fmt, .{ .start = index, .end = index }, level, args); +pub fn msgAtIndex( + builder: *ErrorBuilder, + comptime fmt: []const u8, + file: []const u8, + source_index: usize, + level: std.log.Level, + args: anytype, +) error{OutOfMemory}!void { + return msgAtLoc(builder, fmt, file, .{ .start = source_index, .end = source_index }, level, args); } pub fn hasMessages(builder: *ErrorBuilder) bool { - return builder.items.items.len != 0; + return builder.message_count != 0; } -pub fn write(builder: *ErrorBuilder, writer: anytype) !void { - if (!builder.hasMessages()) return; - - std.sort.sort(MsgItem, builder.items.items, builder, ErrorBuilder.lessThan); - - try writer.writeByte('\n'); - - var start: usize = 0; - for (builder.items.items) |item| { - const line = offsets.lineLocAtIndex(builder.source, item.loc.start); - defer start = line.end; - - try writer.writeAll(builder.source[start..line.end]); - try writer.writeByte('\n'); - { - var i: usize = line.start; - while (i < item.loc.start) : (i += 1) try writer.writeByte(' '); - while (i < item.loc.end) : (i += 1) try writer.writeByte('^'); - if (item.loc.start == item.loc.end) try writer.writeByte('^'); +pub fn clearMessages(builder: *ErrorBuilder) void { + for (builder.files.values()) |*file| { + for (file.messages.items) |item| { + builder.allocator.free(item.message); } - const level_txt: []const u8 = switch (item.level) { - .err => "error", - .warn => "warning", - .info => "info", - .debug => "debug", - }; - try writer.print(" {s}: {s}", .{ level_txt, item.message }); + file.messages.clearAndFree(builder.allocator); } + builder.message_count = 0; +} - try writer.writeAll(builder.source[start..builder.source.len]); - try writer.writeByte('\n'); +pub fn write(builder: *ErrorBuilder, writer: anytype, tty_config: std.debug.TTY.Config) !void { + for (builder.files.keys(), builder.files.values()) |file_name, file| { + if (file.messages.items.len == 0) continue; + + std.sort.sort(MsgItem, file.messages.items, file.source, ErrorBuilder.lessThan); + + try writer.writeByte('\n'); + if (builder.files.count() > 1) { + try writer.print("{s}:\n", .{file_name}); + } + + var start: usize = 0; + for (file.messages.items) |item| { + const line = offsets.lineLocAtIndex(file.source, item.loc.start); + defer start = line.end; + + try writer.writeAll(file.source[start..line.end]); + try writer.writeByte('\n'); + for (line.start..item.loc.start) |_| try writer.writeByte(' '); + for (item.loc.start..item.loc.end) |_| try writer.writeByte('^'); + if (item.loc.start == item.loc.end) try writer.writeByte('^'); + + const level_txt: []const u8 = switch (item.level) { + .err => "error", + .warn => "warning", + .info => "info", + .debug => "debug", + }; + const color: std.debug.TTY.Color = switch (item.level) { + .err => .Red, + .warn => .Yellow, + .info => .White, + .debug => .White, + }; + try tty_config.setColor(writer, color); + try writer.print(" {s}: ", .{level_txt}); + try tty_config.setColor(writer, .Reset); + try writer.writeAll(item.message); + } + + try writer.writeAll(file.source[start..file.source.len]); + try writer.writeByte('\n'); + } } pub fn writeDebug(builder: *ErrorBuilder) void { - if (!builder.hasMessages()) return; std.debug.getStderrMutex().lock(); defer std.debug.getStderrMutex().unlock(); - nosuspend builder.write(std.io.getStdErr().writer()) catch return; + const stderr = std.io.getStdErr(); + const tty_config = std.debug.detectTTYConfig(stderr); + // does zig trim the output or why is this needed? + stderr.writeAll(" ") catch return; + nosuspend builder.write(stderr.writer(), tty_config) catch return; } +const File = struct { + source: []const u8, + messages: std.ArrayListUnmanaged(MsgItem) = .{}, +}; + const MsgItem = struct { loc: offsets.Loc, level: std.log.Level, message: []const u8, }; -fn lessThan(builder: *ErrorBuilder, lhs: MsgItem, rhs: MsgItem) bool { +fn lessThan(source: []const u8, lhs: MsgItem, rhs: MsgItem) bool { const is_less = lhs.loc.start < rhs.loc.start; - const text = if (is_less) builder.source[lhs.loc.start..rhs.loc.start] else builder.source[rhs.loc.start..lhs.loc.start]; + const text = if (is_less) source[lhs.loc.start..rhs.loc.start] else source[rhs.loc.start..lhs.loc.start]; // report messages on the same line in reverse order if (std.mem.indexOfScalar(u8, text, '\n') == null) { diff --git a/tests/lsp_features/completion.zig b/tests/lsp_features/completion.zig index 05171aa..a54b0a0 100644 --- a/tests/lsp_features/completion.zig +++ b/tests/lsp_features/completion.zig @@ -190,7 +190,7 @@ test "completion - captures" { , &.{ .{ .label = "alpha", .kind = .Field, .detail = "alpha: u32" }, }); - + try testCompletion( \\const S = struct { alpha: u32 }; \\fn foo(items: [2]S) void { @@ -452,10 +452,12 @@ fn testCompletion(source: []const u8, expected_completions: []const Completion) var unexpected = try set_difference(actual, expected); defer unexpected.deinit(allocator); - var error_builder = ErrorBuilder.init(allocator, text); + var error_builder = ErrorBuilder.init(allocator); defer error_builder.deinit(); errdefer error_builder.writeDebug(); + try error_builder.addFile(test_uri, text); + for (found.keys()) |label| { const actual_completion: types.CompletionItem = blk: { for (completion_list.items) |item| { @@ -472,7 +474,7 @@ fn testCompletion(source: []const u8, expected_completions: []const Completion) }; if (actual_completion.kind == null or expected_completion.kind != actual_completion.kind.?) { - try error_builder.msgAtIndex("label '{s}' should be of kind '{s}' but was '{?s}'!", cursor_idx, .err, .{ + try error_builder.msgAtIndex("label '{s}' should be of kind '{s}' but was '{?s}'!", test_uri, cursor_idx, .err, .{ label, @tagName(expected_completion.kind), if (actual_completion.kind) |kind| @tagName(kind) else null, @@ -483,7 +485,7 @@ fn testCompletion(source: []const u8, expected_completions: []const Completion) if (expected_completion.detail == null) continue; if (actual_completion.detail != null and std.mem.eql(u8, expected_completion.detail.?, actual_completion.detail.?)) continue; - try error_builder.msgAtIndex("label '{s}' should have detail '{?s}' but was '{?s}'!", cursor_idx, .err, .{ + try error_builder.msgAtIndex("label '{s}' should have detail '{?s}' but was '{?s}'!", test_uri, cursor_idx, .err, .{ label, expected_completion.detail, actual_completion.detail, @@ -499,7 +501,7 @@ fn testCompletion(source: []const u8, expected_completions: []const Completion) try printLabels(out, found, "found"); try printLabels(out, missing, "missing"); try printLabels(out, unexpected, "unexpected"); - try error_builder.msgAtIndex("invalid completions\n{s}", cursor_idx, .err, .{buffer.items}); + try error_builder.msgAtIndex("invalid completions\n{s}", test_uri, cursor_idx, .err, .{buffer.items}); return error.InvalidCompletions; } } diff --git a/tests/lsp_features/inlay_hints.zig b/tests/lsp_features/inlay_hints.zig index 1afaf2e..2b23418 100644 --- a/tests/lsp_features/inlay_hints.zig +++ b/tests/lsp_features/inlay_hints.zig @@ -98,10 +98,12 @@ fn testInlayHints(source: []const u8) !void { return error.InvalidResponse; }; - var error_builder = ErrorBuilder.init(allocator, phr.new_source); + var error_builder = ErrorBuilder.init(allocator); defer error_builder.deinit(); errdefer error_builder.writeDebug(); + try error_builder.addFile(test_uri, phr.new_source); + var i: usize = 0; outer: while (i < phr.locations.len) : (i += 1) { const old_loc = phr.locations.items(.old)[i]; @@ -116,20 +118,20 @@ fn testInlayHints(source: []const u8) !void { if (position.line != hint.position.line or position.character != hint.position.character) continue; if (!std.mem.endsWith(u8, hint.label, ":")) { - try error_builder.msgAtLoc("label `{s}` must end with a colon!", new_loc, .err, .{hint.label}); + try error_builder.msgAtLoc("label `{s}` must end with a colon!", test_uri, new_loc, .err, .{hint.label}); } const actual_label = hint.label[0 .. hint.label.len - 1]; if (!std.mem.eql(u8, expected_label, actual_label)) { - try error_builder.msgAtLoc("expected label `{s}` here but got `{s}`!", new_loc, .err, .{ expected_label, actual_label }); + try error_builder.msgAtLoc("expected label `{s}` here but got `{s}`!", test_uri, new_loc, .err, .{ expected_label, actual_label }); } if (hint.kind != types.InlayHintKind.Parameter) { - try error_builder.msgAtLoc("hint kind should be `{s}` but got `{s}`!", new_loc, .err, .{ @tagName(types.InlayHintKind.Parameter), @tagName(hint.kind) }); + try error_builder.msgAtLoc("hint kind should be `{s}` but got `{s}`!", test_uri, new_loc, .err, .{ @tagName(types.InlayHintKind.Parameter), @tagName(hint.kind) }); } continue :outer; } - try error_builder.msgAtLoc("expected hint `{s}` here", new_loc, .err, .{expected_label}); + try error_builder.msgAtLoc("expected hint `{s}` here", test_uri, new_loc, .err, .{expected_label}); } if (error_builder.hasMessages()) return error.InvalidResponse; diff --git a/tests/lsp_features/references.zig b/tests/lsp_features/references.zig index dcfb466..51be046 100644 --- a/tests/lsp_features/references.zig +++ b/tests/lsp_features/references.zig @@ -148,14 +148,16 @@ fn testReferences(source: []const u8) !void { const response = try ctx.requestGetResponse(?[]types.Location, "textDocument/references", params); - var error_builder = ErrorBuilder.init(allocator, phr.new_source); + var error_builder = ErrorBuilder.init(allocator); defer error_builder.deinit(); errdefer { const note_loc = phr.locations.items(.new)[i]; - error_builder.msgAtLoc("asked for references here", note_loc, .info, .{}) catch {}; + error_builder.msgAtLoc("asked for references here", file_uri, note_loc, .info, .{}) catch {}; error_builder.writeDebug(); } + try error_builder.addFile(file_uri, phr.new_source); + const locations: []types.Location = response.result orelse { std.debug.print("Server returned `null` as the result\n", .{}); return error.InvalidResponse; @@ -200,12 +202,12 @@ fn testReferences(source: []const u8) !void { if (expected_loc.end != actual_loc.end) continue; break :found_index idx; } - try error_builder.msgAtLoc("server returned unexpected reference!", actual_loc, .err, .{}); + try error_builder.msgAtLoc("server returned unexpected reference!", file_uri, actual_loc, .err, .{}); return error.UnexpectedReference; }; if (visited.isSet(index)) { - try error_builder.msgAtLoc("server returned duplicate reference!", actual_loc, .err, .{}); + try error_builder.msgAtLoc("server returned duplicate reference!", file_uri, actual_loc, .err, .{}); return error.DuplicateReference; } else { visited.set(index); @@ -215,7 +217,7 @@ fn testReferences(source: []const u8) !void { var has_unvisited = false; var unvisited_it = visited.iterator(.{ .kind = .unset }); while (unvisited_it.next()) |index| { - try error_builder.msgAtLoc("expected reference here!", expected_locs[index], .err, .{}); + try error_builder.msgAtLoc("expected reference here!", file_uri, expected_locs[index], .err, .{}); has_unvisited = true; } diff --git a/tests/lsp_features/semantic_tokens.zig b/tests/lsp_features/semantic_tokens.zig index 84a3ec9..f090f3a 100644 --- a/tests/lsp_features/semantic_tokens.zig +++ b/tests/lsp_features/semantic_tokens.zig @@ -975,17 +975,19 @@ fn testSemanticTokens(source: [:0]const u8, expected_tokens: []const TokenData) const actual = response.result.data; try std.testing.expect(actual.len % 5 == 0); // every token is represented by 5 integers - var error_builder = ErrorBuilder.init(allocator, source); + var error_builder = ErrorBuilder.init(allocator); defer error_builder.deinit(); errdefer error_builder.writeDebug(); + try error_builder.addFile(uri, source); + var token_it = std.mem.window(u32, actual, 5, 5); var position: types.Position = .{ .line = 0, .character = 0 }; var last_token_end: usize = 0; for (expected_tokens) |expected_token| { const token_data = token_it.next() orelse { - try error_builder.msgAtIndex("expected a `{s}` token here", last_token_end, .err, .{expected_token.@"0"}); + try error_builder.msgAtIndex("expected a `{s}` token here", uri, last_token_end, .err, .{expected_token.@"0"}); return error.ExpectedToken; }; @@ -1012,13 +1014,13 @@ fn testSemanticTokens(source: [:0]const u8, expected_tokens: []const TokenData) const expected_token_modifiers = expected_token.@"2"; if (!std.mem.eql(u8, expected_token_source, token_source)) { - try error_builder.msgAtLoc("expected `{s}` as the next token but got `{s}` here", token_loc, .err, .{ expected_token_source, token_source }); + try error_builder.msgAtLoc("expected `{s}` as the next token but got `{s}` here", uri, token_loc, .err, .{ expected_token_source, token_source }); return error.UnexpectedTokenContent; } else if (expected_token_type != token_type) { - try error_builder.msgAtLoc("expected token type `{s}` but got `{s}`", token_loc, .err, .{ @tagName(expected_token_type), @tagName(token_type) }); + try error_builder.msgAtLoc("expected token type `{s}` but got `{s}`", uri, token_loc, .err, .{ @tagName(expected_token_type), @tagName(token_type) }); return error.UnexpectedTokenType; } else if (!std.meta.eql(expected_token_modifiers, token_modifiers)) { - try error_builder.msgAtLoc("expected token modifiers `{}` but got `{}`", token_loc, .err, .{ expected_token_modifiers, token_modifiers }); + try error_builder.msgAtLoc("expected token modifiers `{}` but got `{}`", uri, token_loc, .err, .{ expected_token_modifiers, token_modifiers }); return error.UnexpectedTokenModifiers; } } diff --git a/tests/utility/ast.zig b/tests/utility/ast.zig index b01bd0d..95d2978 100644 --- a/tests/utility/ast.zig +++ b/tests/utility/ast.zig @@ -162,19 +162,22 @@ fn testNodesAtLoc(source: []const u8) !void { .end = offsets.nodeToLoc(tree, nodes[nodes.len - 1]).end, }; - var error_builder = ErrorBuilder.init(allocator, new_source); + const uri = "file.zig"; + var error_builder = ErrorBuilder.init(allocator); defer error_builder.deinit(); errdefer error_builder.writeDebug(); + try error_builder.addFile(uri, new_source); + if (outer_loc.start != actual_loc.start) { - try error_builder.msgAtIndex("actual start here", actual_loc.start, .err, .{}); - try error_builder.msgAtIndex("expected start here", outer_loc.start, .err, .{}); + try error_builder.msgAtIndex("actual start here", uri, actual_loc.start, .err, .{}); + try error_builder.msgAtIndex("expected start here", uri, outer_loc.start, .err, .{}); return error.LocStartMismatch; } if (outer_loc.end != actual_loc.end) { - try error_builder.msgAtIndex("actual end here", actual_loc.end, .err, .{}); - try error_builder.msgAtIndex("expected end here", outer_loc.end, .err, .{}); + try error_builder.msgAtIndex("actual end here", uri, actual_loc.end, .err, .{}); + try error_builder.msgAtIndex("expected end here", uri, outer_loc.end, .err, .{}); return error.LocEndMismatch; } }