From cad5bc83858c66ecec4dfe433169510f7be29bfa Mon Sep 17 00:00:00 2001 From: Techatrix Date: Sun, 1 Dec 2024 16:50:43 +0100 Subject: [PATCH] publish cImport diagnostics using error bundles This also fixes a minor bug where cImport diagnostics would only be reported after a document has been modified --- src/DiagnosticsCollection.zig | 17 ++++----- src/DocumentStore.zig | 65 +++++++++++++++++++++++++++++++++++ src/Server.zig | 11 ++++-- src/features/diagnostics.zig | 63 ++------------------------------- src/main.zig | 2 +- src/translate_c.zig | 2 +- 6 files changed, 86 insertions(+), 74 deletions(-) diff --git a/src/DiagnosticsCollection.zig b/src/DiagnosticsCollection.zig index 507702714..1e9584b5e 100644 --- a/src/DiagnosticsCollection.zig +++ b/src/DiagnosticsCollection.zig @@ -15,6 +15,8 @@ tag_set: std.AutoArrayHashMapUnmanaged(Tag, struct { }) = .{}, }) = .{}, outdated_files: std.StringArrayHashMapUnmanaged(void) = .{}, +transport: ?lsp.AnyTransport = null, +offset_encoding: ?offsets.Encoding = null, const DiagnosticsCollection = @This(); @@ -55,7 +57,7 @@ pub fn pushLspDiagnostics( document_uri: []const u8, diagnostics_arena_state: std.heap.ArenaAllocator.State, diagnostics: []lsp.types.Diagnostic, -) !void { +) error{OutOfMemory}!void { collection.mutex.lock(); defer collection.mutex.unlock(); @@ -93,7 +95,7 @@ pub fn pushErrorBundle( /// The current implementation assumes that the base path is always the same for the same tag. src_base_path: ?[]const u8, error_bundle: std.zig.ErrorBundle, -) !void { +) error{OutOfMemory}!void { var new_error_bundle: std.zig.ErrorBundle.Wip = undefined; try new_error_bundle.init(collection.allocator); defer new_error_bundle.deinit(); @@ -145,7 +147,7 @@ fn collectUrisFromErrorBundle( error_bundle: std.zig.ErrorBundle, src_base_path: ?[]const u8, uri_set: *std.StringArrayHashMapUnmanaged(void), -) std.mem.Allocator.Error!void { +) error{OutOfMemory}!void { if (error_bundle.errorMessageCount() == 0) return; for (error_bundle.getMessages()) |msg_index| { const err = error_bundle.getErrorMessage(msg_index); @@ -172,11 +174,10 @@ fn pathToUri(allocator: std.mem.Allocator, base_path: ?[]const u8, src_path: []c return try URI.fromPath(allocator, absolute_src_path); } -pub fn publishDiagnostics( - collection: *DiagnosticsCollection, - transport: lsp.AnyTransport, - offset_encoding: offsets.Encoding, -) (std.mem.Allocator.Error || lsp.AnyTransport.WriteError)!void { +pub fn publishDiagnostics(collection: *DiagnosticsCollection) (std.mem.Allocator.Error || lsp.AnyTransport.WriteError)!void { + const transport = collection.transport orelse return; + const offset_encoding = collection.offset_encoding orelse return; + var arena_allocator = std.heap.ArenaAllocator.init(collection.allocator); defer arena_allocator.deinit(); diff --git a/src/DocumentStore.zig b/src/DocumentStore.zig index e07742131..2bf71da27 100644 --- a/src/DocumentStore.zig +++ b/src/DocumentStore.zig @@ -14,6 +14,7 @@ const translate_c = @import("translate_c.zig"); const AstGen = std.zig.AstGen; const Zir = std.zig.Zir; const DocumentScope = @import("DocumentScope.zig"); +const DiagnosticsCollection = @import("DiagnosticsCollection.zig"); const DocumentStore = @This(); @@ -25,6 +26,7 @@ thread_pool: if (builtin.single_threaded) void else *std.Thread.Pool, handles: std.StringArrayHashMapUnmanaged(*Handle) = .{}, build_files: std.StringArrayHashMapUnmanaged(*BuildFile) = .{}, cimports: std.AutoArrayHashMapUnmanaged(Hash, translate_c.Result) = .{}, +diagnostics_collection: *DiagnosticsCollection, pub const Uri = []const u8; @@ -1517,6 +1519,10 @@ pub fn resolveCImport(self: *DocumentStore, handle: *Handle, node: Ast.Node.Inde } } + self.publishCimportDiagnostics(handle) catch |err| { + log.err("failed to publish cImport diagnostics: {}", .{err}); + }; + switch (result) { .success => |uri| { log.debug("Translated cImport into {s}", .{uri}); @@ -1526,6 +1532,65 @@ pub fn resolveCImport(self: *DocumentStore, handle: *Handle, node: Ast.Node.Inde } } +fn publishCimportDiagnostics(self: *DocumentStore, handle: *Handle) !void { + const file_path = URI.parse(self.allocator, handle.uri) catch |err| { + log.err("failed to parse URI '{s}': {}", .{ handle.uri, err }); + return; + }; + defer self.allocator.free(file_path); + + var wip: std.zig.ErrorBundle.Wip = undefined; + try wip.init(self.allocator); + defer wip.deinit(); + + const src_path = try wip.addString(file_path); + + for (handle.cimports.items(.hash), handle.cimports.items(.node)) |hash, node| { + const result = blk: { + self.lock.lock(); + defer self.lock.unlock(); + break :blk self.cimports.get(hash) orelse continue; + }; + const error_bundle: std.zig.ErrorBundle = switch (result) { + .success => continue, + .failure => |bundle| bundle, + }; + + if (error_bundle.errorMessageCount() == 0) continue; + + const loc = offsets.nodeToLoc(handle.tree, node); + const source_loc = std.zig.findLineColumn(handle.tree.source, loc.start); + + comptime std.debug.assert(max_document_size <= std.math.maxInt(u32)); + + const src_loc = try wip.addSourceLocation(.{ + .src_path = src_path, + .line = @intCast(source_loc.line), + .column = @intCast(source_loc.column), + .span_start = @intCast(loc.start), + .span_main = @intCast(loc.start), + .span_end = @intCast(loc.end), + .source_line = try wip.addString(source_loc.source_line), + }); + + for (error_bundle.getMessages()) |err_msg_index| { + const err_msg = error_bundle.getErrorMessage(err_msg_index); + const msg = error_bundle.nullTerminatedString(err_msg.msg); + + try wip.addRootErrorMessage(.{ + .msg = try wip.addString(msg), + .src_loc = src_loc, + }); + } + } + + var error_bundle = try wip.toOwnedBundle(""); + defer error_bundle.deinit(self.allocator); + + try self.diagnostics_collection.pushErrorBundle(.cimport, handle.version, null, error_bundle); + try self.diagnostics_collection.publishDiagnostics(); +} + /// takes the string inside a @import() node (without the quotation marks) /// and returns it's uri /// caller owns the returned memory diff --git a/src/Server.zig b/src/Server.zig index 1c9440424..7adffcbe1 100644 --- a/src/Server.zig +++ b/src/Server.zig @@ -45,6 +45,7 @@ config: Config = .{}, /// will default to lookup in the system and user configuration folder provided by known-folders. config_path: ?[]const u8 = null, document_store: DocumentStore, +/// Use `setTransport` to set the Transport. transport: ?lsp.AnyTransport = null, message_tracing: bool = false, offset_encoding: offsets.Encoding = .@"utf-16", @@ -427,6 +428,7 @@ fn initializeHandler(server: *Server, arena: std.mem.Allocator, request: types.I } else { server.offset_encoding = .@"utf-16"; } + server.diagnostics_collection.offset_encoding = server.offset_encoding; } if (request.capabilities.textDocument) |textDocument| { @@ -811,7 +813,6 @@ const Workspace = struct { const zig_exe_path = server.config.zig_exe_path orelse return; const zig_lib_path = server.config.zig_lib_path orelse return; const build_runner_path = server.config.build_runner_path orelse return; - const transport = server.transport orelse return; const workspace_path = @import("uri.zig").parse(server.allocator, workspace.uri) catch |err| { log.err("failed to parse URI '{s}': {}", .{ workspace.uri, err }); @@ -829,8 +830,6 @@ const Workspace = struct { .zig_lib_path = zig_lib_path, .build_runner_path = build_runner_path, .collection = &server.diagnostics_collection, - .lsp_transport = transport, - .offset_encoding = server.offset_encoding, }) catch |err| { workspace.build_on_save = null; log.err("failed to initilize Build-On-Save for '{s}': {}", .{ workspace.uri, err }); @@ -1863,6 +1862,7 @@ pub fn create(allocator: std.mem.Allocator) !*Server { .allocator = allocator, .config = DocumentStore.Config.fromMainConfig(Config{}), .thread_pool = if (zig_builtin.single_threaded) {} else undefined, // set below + .diagnostics_collection = &server.diagnostics_collection, }, .job_queue = std.fifo.LinearFifo(Job, .Dynamic).init(allocator), .thread_pool = undefined, // set below @@ -1903,6 +1903,11 @@ pub fn destroy(server: *Server) void { server.allocator.destroy(server); } +pub fn setTransport(server: *Server, transport: lsp.AnyTransport) void { + server.transport = transport; + server.diagnostics_collection.transport = transport; +} + pub fn keepRunning(server: Server) bool { switch (server.status) { .exiting_success, .exiting_failure => return false, diff --git a/src/features/diagnostics.zig b/src/features/diagnostics.zig index 43782bbd9..6fbe2ef44 100644 --- a/src/features/diagnostics.zig +++ b/src/features/diagnostics.zig @@ -25,8 +25,6 @@ pub fn generateDiagnostics( const tracy_zone = tracy.trace(@src()); defer tracy_zone.end(); - const transport = server.transport orelse return; - { var arena_allocator = std.heap.ArenaAllocator.init(server.diagnostics_collection.allocator); errdefer arena_allocator.deinit(); @@ -61,18 +59,8 @@ pub fn generateDiagnostics( try server.diagnostics_collection.pushErrorBundle(.parse, handle.version, null, error_bundle); } - { - var arena_allocator = std.heap.ArenaAllocator.init(server.diagnostics_collection.allocator); - errdefer arena_allocator.deinit(); - const arena = arena_allocator.allocator(); - - var diagnostics: std.ArrayListUnmanaged(types.Diagnostic) = .{}; - try collectCimportDiagnostics(&server.document_store, handle, arena, &diagnostics, server.offset_encoding); - try server.diagnostics_collection.pushLspDiagnostics(.cimport, handle.uri, arena_allocator.state, diagnostics.items); - } - std.debug.assert(server.client_capabilities.supports_publish_diagnostics); - server.diagnostics_collection.publishDiagnostics(transport, server.offset_encoding) catch |err| { + server.diagnostics_collection.publishDiagnostics() catch |err| { log.err("failed to publish diagnostics: {}", .{err}); }; } @@ -182,43 +170,6 @@ fn collectWarnStyleDiagnostics( } } -fn collectCimportDiagnostics( - document_store: *DocumentStore, - handle: *DocumentStore.Handle, - arena: std.mem.Allocator, - diagnostics: *std.ArrayListUnmanaged(types.Diagnostic), - offset_encoding: offsets.Encoding, -) error{OutOfMemory}!void { - const tracy_zone = tracy.trace(@src()); - defer tracy_zone.end(); - - for (handle.cimports.items(.hash), handle.cimports.items(.node)) |hash, node| { - const result = blk: { - document_store.lock.lock(); - defer document_store.lock.unlock(); - break :blk document_store.cimports.get(hash) orelse continue; - }; - const error_bundle: std.zig.ErrorBundle = switch (result) { - .success => continue, - .failure => |bundle| bundle, - }; - - if (error_bundle.errorMessageCount() == 0) continue; // `getMessages` can't be called on an empty ErrorBundle - try diagnostics.ensureUnusedCapacity(arena, error_bundle.errorMessageCount()); - for (error_bundle.getMessages()) |err_msg_index| { - const err_msg = error_bundle.getErrorMessage(err_msg_index); - - diagnostics.appendAssumeCapacity(.{ - .range = offsets.nodeToRange(handle.tree, node, offset_encoding), - .severity = .Error, - .code = .{ .string = "cImport" }, - .source = "zls", - .message = try arena.dupe(u8, error_bundle.nullTerminatedString(err_msg.msg)), - }); - } - } -} - fn collectGlobalVarDiagnostics( tree: Ast, arena: std.mem.Allocator, @@ -440,8 +391,6 @@ pub const BuildOnSave = struct { build_runner_path: []const u8, collection: *DiagnosticsCollection, - lsp_transport: lsp.AnyTransport, - offset_encoding: offsets.Encoding, }; pub fn init(self: *BuildOnSave, options: InitOptions) !void { @@ -493,8 +442,6 @@ pub const BuildOnSave = struct { self.thread = try std.Thread.spawn(.{ .allocator = options.allocator }, loop, .{ self, options.collection, - options.lsp_transport, - options.offset_encoding, duped_workspace_path, }); } @@ -539,8 +486,6 @@ pub const BuildOnSave = struct { fn loop( self: *BuildOnSave, collection: *DiagnosticsCollection, - lsp_transport: lsp.AnyTransport, - offset_encoding: offsets.Encoding, workspace_path: []const u8, ) void { defer self.allocator.free(workspace_path); @@ -567,8 +512,6 @@ pub const BuildOnSave = struct { self, &transport, collection, - lsp_transport, - offset_encoding, workspace_path, ) catch |err| { log.err("failed to handle error bundle message from build runner: {}", .{err}); @@ -586,8 +529,6 @@ pub const BuildOnSave = struct { self: *BuildOnSave, transport: *Transport, collection: *DiagnosticsCollection, - lsp_transport: lsp.AnyTransport, - offset_encoding: offsets.Encoding, workspace_path: []const u8, ) !void { const header = try transport.reader().readStructEndian(ServerToClient.ErrorBundle, .little); @@ -607,6 +548,6 @@ pub const BuildOnSave = struct { const diagnostic_tag: DiagnosticsCollection.Tag = @enumFromInt(@as(u32, @truncate(hasher.final()))); try collection.pushErrorBundle(diagnostic_tag, header.cycle, workspace_path, error_bundle); - try collection.publishDiagnostics(lsp_transport, offset_encoding); + try collection.publishDiagnostics(); } }; diff --git a/src/main.zig b/src/main.zig index 3af665c58..df1c57f76 100644 --- a/src/main.zig +++ b/src/main.zig @@ -348,7 +348,7 @@ pub fn main() !u8 { const server = try zls.Server.create(allocator); defer server.destroy(); - server.transport = transport.any(); + server.setTransport(transport.any()); server.config_path = result.config_path; server.message_tracing = result.enable_message_tracing; diff --git a/src/translate_c.zig b/src/translate_c.zig index bc26ea7f9..0e1005d76 100644 --- a/src/translate_c.zig +++ b/src/translate_c.zig @@ -182,7 +182,7 @@ pub fn translate( var process = std.process.Child.init(argv.items, allocator); process.stdin_behavior = .Pipe; process.stdout_behavior = .Pipe; - process.stderr_behavior = .Pipe; + process.stderr_behavior = .Ignore; errdefer |err| if (!zig_builtin.is_test) reportTranslateError(allocator, process.stderr, argv.items, @errorName(err));