From c718e12d16cabb422671bc0b2472565f5e84ad13 Mon Sep 17 00:00:00 2001 From: Techatrix <19954306+Techatrix@users.noreply.github.com> Date: Mon, 2 Jan 2023 19:59:01 +0000 Subject: [PATCH] Autofix improvements (#879) * improve autofix stability and client support * run zig fmt --- src/DocumentStore.zig | 4 +- src/Header.zig | 4 +- src/Server.zig | 104 ++++++++++++++++++++-------------- src/analysis.zig | 2 +- src/ast.zig | 2 +- src/config_gen/config_gen.zig | 8 +-- src/data/master.zig | 2 +- 7 files changed, 71 insertions(+), 55 deletions(-) diff --git a/src/DocumentStore.zig b/src/DocumentStore.zig index 7e5d2cd..d78d809 100644 --- a/src/DocumentStore.zig +++ b/src/DocumentStore.zig @@ -653,14 +653,14 @@ fn createDocument(self: *DocumentStore, uri: Uri, text: [:0]u8, open: bool) erro self.build_files.swapRemoveAt(gop.index); log.debug("Failed to load build file {s}: (error: {})", .{ uri, err }); } - if(!gop.found_existing) { + if (!gop.found_existing) { const duped_uri = try self.allocator.dupe(u8, uri); gop.value_ptr.* = try self.createBuildFile(duped_uri); gop.key_ptr.* = gop.value_ptr.uri; } } else if (self.config.zig_exe_path != null and !isBuiltinFile(handle.uri) and !isInStd(handle.uri)) blk: { // log.debug("Going to walk down the tree towards: {s}", .{uri}); - + // walk down the tree towards the uri. When we hit build.zig files // determine if the uri we're interested in is involved with the build. // This ensures that _relevant_ build.zig files higher in the diff --git a/src/Header.zig b/src/Header.zig index 669948b..16b6600 100644 --- a/src/Header.zig +++ b/src/Header.zig @@ -49,9 +49,9 @@ pub fn parse(allocator: std.mem.Allocator, include_carriage_return: bool, reader pub fn write(header: Header, include_carriage_return: bool, writer: anytype) @TypeOf(writer).Error!void { const seperator: []const u8 = if (include_carriage_return) "\r\n" else "\n"; - try writer.print("Content-Length: {}{s}", .{header.content_length, seperator}); + try writer.print("Content-Length: {}{s}", .{ header.content_length, seperator }); if (header.content_type) |content_type| { - try writer.print("Content-Type: {s}{s}", .{content_type, seperator}); + try writer.print("Content-Type: {s}{s}", .{ content_type, seperator }); } try writer.writeAll(seperator); } diff --git a/src/Server.zig b/src/Server.zig index 04353e0..5f750c4 100644 --- a/src/Server.zig +++ b/src/Server.zig @@ -53,13 +53,13 @@ status: enum { // Code was based off of https://github.com/andersfr/zig-lsp/blob/master/server.zig -const ClientCapabilities = struct { +const ClientCapabilities = packed struct { supports_snippets: bool = false, - supports_semantic_tokens: bool = false, - supports_inlay_hints: bool = false, + supports_apply_edits: bool = false, supports_will_save: bool = false, supports_will_save_wait_until: bool = false, supports_publish_diagnostics: bool = false, + supports_code_action_fixall: bool = false, hover_supports_md: bool = false, completion_doc_supports_md: bool = false, label_details_support: bool = false, @@ -469,8 +469,26 @@ fn getAstCheckDiagnostics( } } +fn getAutofixMode(server: *Server) enum { + on_save, + will_save_wait_until, + fixall, + none, +} { + if (!server.config.enable_autofix) return .none; + if (server.client_capabilities.supports_code_action_fixall) return .fixall; + if (server.client_capabilities.supports_apply_edits) { + if (server.client_capabilities.supports_will_save_wait_until) return .will_save_wait_until; + return .on_save; + } + return .none; +} + /// caller owns returned memory. fn autofix(server: *Server, allocator: std.mem.Allocator, handle: *const DocumentStore.Handle) !std.ArrayListUnmanaged(types.TextEdit) { + if (!server.config.enable_ast_check_diagnostics) return .{}; + + if (handle.tree.errors.len != 0) return .{}; var diagnostics = std.ArrayListUnmanaged(types.Diagnostic){}; try getAstCheckDiagnostics(server, handle.*, &diagnostics); @@ -1564,11 +1582,16 @@ fn initializeHandler(server: *Server, request: types.InitializeParams) !types.In const tracy_zone = tracy.trace(@src()); defer tracy_zone.end(); + var skip_set_fixall = false; + if (request.clientInfo) |clientInfo| { log.info("client is '{s}-{s}'", .{ clientInfo.name, clientInfo.version orelse "" }); if (std.mem.eql(u8, clientInfo.name, "Sublime Text LSP")) blk: { server.config.max_detail_length = 256; + // TODO investigate why fixall doesn't work in sublime text + server.client_capabilities.supports_code_action_fixall = false; + skip_set_fixall = true; const version_str = clientInfo.version orelse break :blk; const version = std.SemanticVersion.parse(version_str) catch break :blk; @@ -1577,6 +1600,9 @@ fn initializeHandler(server: *Server, request: types.InitializeParams) !types.In if (version.major == 0) { server.config.include_at_in_builtins = true; } + } else if (std.mem.eql(u8, clientInfo.name, "Visual Studio Code")) { + server.client_capabilities.supports_code_action_fixall = true; + skip_set_fixall = true; } } @@ -1604,8 +1630,6 @@ fn initializeHandler(server: *Server, request: types.InitializeParams) !types.In } if (request.capabilities.textDocument) |textDocument| { - server.client_capabilities.supports_semantic_tokens = textDocument.semanticTokens != null; - server.client_capabilities.supports_inlay_hints = textDocument.inlayHint != null; server.client_capabilities.supports_publish_diagnostics = textDocument.publishDiagnostics != null; if (textDocument.hover) |hover| { if (hover.contentFormat) |content_format| { @@ -1635,6 +1659,14 @@ fn initializeHandler(server: *Server, request: types.InitializeParams) !types.In server.client_capabilities.supports_will_save = synchronization.willSave orelse false; server.client_capabilities.supports_will_save_wait_until = synchronization.willSaveWaitUntil orelse false; } + if (textDocument.codeAction) |codeaction| { + if (codeaction.codeActionLiteralSupport) |literlSupport| { + if (!skip_set_fixall) { + const fixall = std.mem.indexOfScalar(types.CodeActionKind, literlSupport.codeActionKind.valueSet, .@"source.fixAll") != null; + server.client_capabilities.supports_code_action_fixall = fixall; + } + } + } } // NOTE: everything is initialized, we got the client capabilities @@ -1646,6 +1678,7 @@ fn initializeHandler(server: *Server, request: types.InitializeParams) !types.In } if (request.capabilities.workspace) |workspace| { + server.client_capabilities.supports_apply_edits = workspace.applyEdit orelse false; server.client_capabilities.supports_configuration = workspace.configuration orelse false; if (workspace.didChangeConfiguration) |did_change| { if (did_change.dynamicRegistration orelse false) { @@ -1943,25 +1976,21 @@ fn saveDocumentHandler(server: *Server, notification: types.DidSaveTextDocumentP const handle = server.document_store.getHandle(uri) orelse return; try server.document_store.applySave(handle); - if (handle.tree.errors.len != 0) return; - if (!server.config.enable_ast_check_diagnostics) return; - if (!server.config.enable_autofix) return; - if (server.client_capabilities.supports_will_save) return; - if (server.client_capabilities.supports_will_save_wait_until) return; + if (server.getAutofixMode() == .on_save) { + var text_edits = try server.autofix(allocator, handle); - var text_edits = try server.autofix(allocator, handle); + var workspace_edit = types.WorkspaceEdit{ .changes = .{} }; + try workspace_edit.changes.?.putNoClobber(allocator, uri, try text_edits.toOwnedSlice(allocator)); - var workspace_edit = types.WorkspaceEdit{ .changes = .{} }; - try workspace_edit.changes.?.putNoClobber(allocator, uri, try text_edits.toOwnedSlice(allocator)); - - server.sendRequest( - .{ .string = "apply_edit" }, - "workspace/applyEdit", - types.ApplyWorkspaceEditParams{ - .label = "autofix", - .edit = workspace_edit, - }, - ); + server.sendRequest( + .{ .string = "apply_edit" }, + "workspace/applyEdit", + types.ApplyWorkspaceEditParams{ + .label = "autofix", + .edit = workspace_edit, + }, + ); + } } fn closeDocumentHandler(server: *Server, notification: types.DidCloseTextDocumentParams) error{}!void { @@ -1971,27 +2000,15 @@ fn closeDocumentHandler(server: *Server, notification: types.DidCloseTextDocumen server.document_store.closeDocument(notification.textDocument.uri); } -fn willSaveHandler(server: *Server, request: types.WillSaveTextDocumentParams) !?[]types.TextEdit { - const tracy_zone = tracy.trace(@src()); - defer tracy_zone.end(); - - if (server.client_capabilities.supports_will_save_wait_until) return null; - return try willSaveWaitUntilHandler(server, request); -} - fn willSaveWaitUntilHandler(server: *Server, request: types.WillSaveTextDocumentParams) !?[]types.TextEdit { const tracy_zone = tracy.trace(@src()); defer tracy_zone.end(); const allocator = server.arena.allocator(); - if (!server.config.enable_ast_check_diagnostics) return null; - if (!server.config.enable_autofix) return null; + if (server.getAutofixMode() != .will_save_wait_until) return null; - const uri = request.textDocument.uri; - - const handle = server.document_store.getHandle(uri) orelse return null; - if (handle.tree.errors.len != 0) return null; + const handle = server.document_store.getHandle(request.textDocument.uri) orelse return null; var text_edits = try server.autofix(allocator, handle); @@ -2422,15 +2439,15 @@ fn codeActionHandler(server: *Server, request: types.CodeActionParams) !?[]types .offset_encoding = server.offset_encoding, }; - var actions = std.ArrayListUnmanaged(types.CodeAction){}; - - for (request.context.diagnostics) |diagnostic| { - try builder.generateCodeAction(diagnostic, &actions); + // as of right now, only ast-check errors may get a code action + var diagnostics = std.ArrayListUnmanaged(types.Diagnostic){}; + if (server.config.enable_ast_check_diagnostics and handle.tree.errors.len == 0) { + try getAstCheckDiagnostics(server, handle.*, &diagnostics); } - for (actions.items) |*action| { - // TODO query whether SourceFixAll is supported by the server - if (action.kind.? == .@"source.fixAll") action.kind = .quickfix; + var actions = std.ArrayListUnmanaged(types.CodeAction){}; + for (diagnostics.items) |diagnostic| { + try builder.generateCodeAction(diagnostic, &actions); } return actions.items; @@ -2981,7 +2998,6 @@ fn processMessage(server: *Server, message: Message) Error!void { .{ "textDocument/didChange", changeDocumentHandler }, .{ "textDocument/didSave", saveDocumentHandler }, .{ "textDocument/didClose", closeDocumentHandler }, - .{ "textDocument/willSave", willSaveHandler }, .{ "textDocument/willSaveWaitUntil", willSaveWaitUntilHandler }, .{ "textDocument/semanticTokens/full", semanticTokensFullHandler }, .{ "textDocument/inlayHint", inlayHintHandler }, diff --git a/src/analysis.zig b/src/analysis.zig index ee34216..7c1879f 100644 --- a/src/analysis.zig +++ b/src/analysis.zig @@ -1466,7 +1466,7 @@ pub fn getImportStr(tree: Ast, node: Ast.Node.Index, source_index: usize) ?[]con if (params.len != 1) return null; - if(node_tags[params[0]] != .string_literal) return null; + if (node_tags[params[0]] != .string_literal) return null; const import_str = tree.tokenSlice(tree.nodes.items(.main_token)[params[0]]); return import_str[1 .. import_str.len - 1]; diff --git a/src/ast.zig b/src/ast.zig index 48db2f4..5c7f5ce 100644 --- a/src/ast.zig +++ b/src/ast.zig @@ -539,7 +539,7 @@ pub fn lastToken(tree: Ast, node: Ast.Node.Index) Ast.TokenIndex { .container_decl_arg_trailing, .switch_comma, => { - if(datas[n].rhs != 0) { + if (datas[n].rhs != 0) { const members = tree.extraData(datas[n].rhs, Node.SubRange); std.debug.assert(members.end - members.start > 0); end_offset += 2; // for the comma + rbrace diff --git a/src/config_gen/config_gen.zig b/src/config_gen/config_gen.zig index bb3249d..8939274 100644 --- a/src/config_gen/config_gen.zig +++ b/src/config_gen/config_gen.zig @@ -188,14 +188,14 @@ fn generateVSCodeConfigFile(allocator: std.mem.Allocator, config: Config, path: configuration.putAssumeCapacityNoClobber("trace.server", .{ .scope = "window", .type = "string", - .@"enum" = &.{"off", "message", "verbose"}, + .@"enum" = &.{ "off", "message", "verbose" }, .description = "Traces the communication between VS Code and the language server.", - .default = .{.String = "off"}, + .default = .{ .String = "off" }, }); configuration.putAssumeCapacityNoClobber("check_for_update", .{ .type = "boolean", .description = "Whether to automatically check for new updates", - .default = .{.Bool = true}, + .default = .{ .Bool = true }, }); configuration.putAssumeCapacityNoClobber("path", .{ .type = "string", @@ -214,7 +214,7 @@ fn generateVSCodeConfigFile(allocator: std.mem.Allocator, config: Config, path: .type = try zigTypeToTypescript(option.type), .description = option.description, .format = if (std.mem.indexOf(u8, option.name, "path") != null) "path" else null, - .default = if(default == .Null) null else default, + .default = if (default == .Null) null else default, }); } diff --git a/src/data/master.zig b/src/data/master.zig index b116201..8896e78 100644 --- a/src/data/master.zig +++ b/src/data/master.zig @@ -1779,4 +1779,4 @@ pub const builtins = [_]Builtin{ "Element: type", }, }, -}; \ No newline at end of file +};