Skip to content

Commit

Permalink
publish cImport diagnostics using error bundles
Browse files Browse the repository at this point in the history 
This also fixes a minor bug where cImport diagnostics would only be reported after a document has been modified
  • Loading branch information
@Techatrix
Techatrix committed Dec 1, 2024
1 parent 254e96e commit cad5bc8
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 74 deletions.
17 changes: 9 additions & 8 deletions src/DiagnosticsCollection.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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();

Expand Down
65 changes: 65 additions & 0 deletions src/DocumentStore.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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;

Expand Down Expand Up @@ -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});
Expand All @@ -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
Expand Down
11 changes: 8 additions & 3 deletions src/Server.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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 });
Expand All @@ -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 });
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
63 changes: 2 additions & 61 deletions src/features/diagnostics.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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});
};
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
});
}
Expand Down Expand Up @@ -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);
Expand All @@ -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});
Expand All @@ -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);
Expand All @@ -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();
}
};
2 changes: 1 addition & 1 deletion src/main.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion src/translate_c.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down

0 comments on commit cad5bc8

Please sign in to comment.