From 8d2baa562b4b21b2a99bceb3a0668ee05b858e0f Mon Sep 17 00:00:00 2001 From: Techatrix Date: Sun, 17 Nov 2024 06:33:12 +0100 Subject: [PATCH] make autofix configurable through in-editor configuration The `force_autofix` will not work on an editor that supports code actions. --- schema.json | 10 +++--- src/Config.zig | 6 ++-- src/Server.zig | 48 ++++++++++++++++------------- src/features/diagnostics.zig | 2 +- src/tools/config.json | 12 ++++---- src/tools/config_gen.zig | 3 +- tests/lsp_features/code_actions.zig | 1 - 7 files changed, 43 insertions(+), 39 deletions(-) diff --git a/schema.json b/schema.json index ab2059337..872994bc6 100644 --- a/schema.json +++ b/schema.json @@ -27,11 +27,6 @@ }, "default": [] }, - "enable_autofix": { - "description": "Whether to automatically fix errors on save. Currently supports adding and removing discards.", - "type": "boolean", - "default": false - }, "semantic_tokens": { "description": "Set level of semantic tokens. `partial` only includes information that requires semantic analysis.", "type": "string", @@ -77,6 +72,11 @@ "type": "boolean", "default": false }, + "force_autofix": { + "description": "Work around editors that do not support 'source.fixall' code actions on save. This option may delivered a substandard user experience. Please refer to the installation guide to see which editors natively support code actions on save.", + "type": "boolean", + "default": false + }, "warn_style": { "description": "Enables warnings for style guideline mismatches", "type": "boolean", diff --git a/src/Config.zig b/src/Config.zig index 5919b5007..ea74162ab 100644 --- a/src/Config.zig +++ b/src/Config.zig @@ -18,9 +18,6 @@ enable_build_on_save: ?bool = null, /// If the `build.zig` has declared a 'check' step, it will be preferred over the default 'install' step. build_on_save_args: []const []const u8 = &.{}, -/// Whether to automatically fix errors on save. Currently supports adding and removing discards. -enable_autofix: bool = false, - /// Set level of semantic tokens. `partial` only includes information that requires semantic analysis. semantic_tokens: enum { none, @@ -49,6 +46,9 @@ inlay_hints_hide_redundant_param_names: bool = false, /// Hides inlay hints when parameter name matches the last token of a parameter node (e.g. foo: bar.foo, foo: &foo) inlay_hints_hide_redundant_param_names_last_token: bool = false, +/// Work around editors that do not support 'source.fixall' code actions on save. This option may delivered a substandard user experience. Please refer to the installation guide to see which editors natively support code actions on save. +force_autofix: bool = false, + /// Enables warnings for style guideline mismatches warn_style: bool = false, diff --git a/src/Server.zig b/src/Server.zig index 0967f4dc7..732aa0716 100644 --- a/src/Server.zig +++ b/src/Server.zig @@ -332,15 +332,19 @@ fn initAnalyser(server: *Server, handle: ?*DocumentStore.Handle) Analyser { ); } -fn getAutofixMode(server: *Server) enum { - on_save, +pub fn getAutofixMode(server: *Server) enum { + /// Autofix is implemented by providing `source.fixall` code actions. + @"source.fixall", + /// Autofix is implemented using `textDocument/willSaveWaitUntil`. + /// Requires `force_autofix` to be enabled. will_save_wait_until, - fixall, + /// Autofix is implemented by send a `workspace/applyEdit` request after receiving a `textDocument/didSave` notification. + /// Requires `force_autofix` to be enabled. + on_save, none, } { - if (!server.config.enable_autofix) return .none; - // TODO https://github.com/zigtools/zls/issues/1093 - // if (server.client_capabilities.supports_code_action_fixall) return .fixall; + if (server.client_capabilities.supports_code_action_fixall) return .@"source.fixall"; + if (!server.config.force_autofix) return .none; if (server.client_capabilities.supports_apply_edits) { if (server.client_capabilities.supports_will_save_wait_until) return .will_save_wait_until; return .on_save; @@ -397,12 +401,12 @@ fn initializeHandler(server: *Server, arena: std.mem.Allocator, request: types.I if (std.mem.eql(u8, clientInfo.name, "Sublime Text LSP")) { server.client_capabilities.max_detail_length = 256; - // TODO investigate why fixall doesn't work in sublime text + } else if (std.mem.startsWith(u8, clientInfo.name, "emacs")) { + // Assumes that `emacs` means `emacs-lsp/lsp-mode`. Eglot uses `Eglot`. + + // https://github.com/emacs-lsp/lsp-mode/issues/1842 server.client_capabilities.supports_code_action_fixall = false; skip_set_fixall = true; - } else if (std.mem.eql(u8, clientInfo.name, "Visual Studio Code")) { - server.client_capabilities.supports_code_action_fixall = true; - skip_set_fixall = true; } } @@ -479,16 +483,12 @@ fn initializeHandler(server: *Server, arena: std.mem.Allocator, request: types.I 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) |literalSupport| { - if (!skip_set_fixall) { - for (literalSupport.codeActionKind.valueSet) |code_action_kind| { - if (code_action_kind.eql(.@"source.fixAll")) { - server.client_capabilities.supports_code_action_fixall = true; - break; - } - } - } + if (textDocument.codeAction) |_| { + if (!skip_set_fixall) { + // Some clients do not specify `source.fixAll` in + // `textDocument.codeAction.?.codeActionLiteralSupport.?.codeActionKind.valueSet` + // so we assume they support it if they support code actions in general. + server.client_capabilities.supports_code_action_fixall = true; } } if (textDocument.definition) |definition| { @@ -531,6 +531,7 @@ fn initializeHandler(server: *Server, arena: std.mem.Allocator, request: types.I if (request.clientInfo) |clientInfo| { log.info("Client Info: {s}-{s}", .{ clientInfo.name, clientInfo.version orelse "" }); } + log.info("Autofix Mode: {s}", .{@tagName(server.getAutofixMode())}); log.debug("Offset Encoding: {s}", .{@tagName(server.offset_encoding)}); if (request.workspaceFolders) |workspace_folders| { @@ -879,6 +880,7 @@ pub fn updateConfiguration( const new_build_runner_path = new_config.build_runner_path != null and (server.config.build_runner_path == null or !std.mem.eql(u8, server.config.build_runner_path.?, new_config.build_runner_path.?)); + const new_force_autofix = new_config.force_autofix != null and server.config.force_autofix != new_config.force_autofix.?; inline for (std.meta.fields(Config)) |field| { if (@field(new_cfg, field.name)) |new_value| { @@ -1013,8 +1015,10 @@ pub fn updateConfiguration( } } - if (server.config.enable_autofix and server.getAutofixMode() == .none) { - log.warn("`enable_autofix` is ignored because it is not supported by {s}", .{server.client_capabilities.client_name orelse "your editor"}); + if (server.config.force_autofix and server.getAutofixMode() == .none) { + log.warn("`force_autofix` is ignored because it is not supported by {s}", .{server.client_capabilities.client_name orelse "your editor"}); + } else if (new_force_autofix) { + log.info("Autofix Mode: {s}", .{@tagName(server.getAutofixMode())}); } } diff --git a/src/features/diagnostics.zig b/src/features/diagnostics.zig index 220489980..058eee69b 100644 --- a/src/features/diagnostics.zig +++ b/src/features/diagnostics.zig @@ -53,7 +53,7 @@ pub fn generateDiagnostics(server: *Server, arena: std.mem.Allocator, handle: *D } } - if (server.config.enable_autofix and tree.mode == .zig) { + if (server.getAutofixMode() != .none and tree.mode == .zig) { try code_actions.collectAutoDiscardDiagnostics(tree, arena, &diagnostics, server.offset_encoding); } diff --git a/src/tools/config.json b/src/tools/config.json index e50cddefe..e08d60e48 100644 --- a/src/tools/config.json +++ b/src/tools/config.json @@ -24,12 +24,6 @@ "type": "[]const []const u8", "default": [] }, - { - "name": "enable_autofix", - "description": "Whether to automatically fix errors on save. Currently supports adding and removing discards.", - "type": "bool", - "default": false - }, { "name": "semantic_tokens", "description": "Set level of semantic tokens. `partial` only includes information that requires semantic analysis.", @@ -83,6 +77,12 @@ "type": "bool", "default": false }, + { + "name": "force_autofix", + "description": "Work around editors that do not support 'source.fixall' code actions on save. This option may delivered a substandard user experience. Please refer to the installation guide to see which editors natively support code actions on save.", + "type": "bool", + "default": false + }, { "name": "warn_style", "description": "Enables warnings for style guideline mismatches", diff --git a/src/tools/config_gen.zig b/src/tools/config_gen.zig index 370104047..118911c8d 100644 --- a/src/tools/config_gen.zig +++ b/src/tools/config_gen.zig @@ -261,7 +261,8 @@ fn generateVSCodeConfigFile(allocator: std.mem.Allocator, config: Config, path: }); for (config.options) |option| { - if (std.mem.eql(u8, option.name, "zig_exe_path")) continue; + if (std.mem.eql(u8, option.name, "zig_exe_path")) continue; // vscode-zig has its own option for this + if (std.mem.eql(u8, option.name, "force_autofix")) continue; // VS Code supports code actions on save without a workaround const snake_case_name = try std.fmt.allocPrint(allocator, "zig.zls.{s}", .{option.name}); defer allocator.free(snake_case_name); diff --git a/tests/lsp_features/code_actions.zig b/tests/lsp_features/code_actions.zig index e96ed73d2..12374541b 100644 --- a/tests/lsp_features/code_actions.zig +++ b/tests/lsp_features/code_actions.zig @@ -663,7 +663,6 @@ fn testDiagnostic( ) !void { var ctx = try Context.init(); defer ctx.deinit(); - ctx.server.config.enable_autofix = true; ctx.server.config.prefer_ast_check_as_child_process = !options.want_zir; const uri = try ctx.addDocument(.{ .source = before });