Skip to content

Commit

Permalink
make autofix configurable through in-editor configuration
Browse files Browse the repository at this point in the history
VS Code and Sublime Text offer their own setting to configure code actions on save. `editor.codeActionsOnSave` and `lsp_code_actions_on_save` respectively.
These options should be preferred over `enable_autofix` if possible. The `enable_autofix` gets renamed to `force_autofix` to reflect that it should only be used on editors that don't support code actions on save. Some further research is necessary to identify which editors support these features to make the transition as smooth as possible for all users.

See #1093
  • Loading branch information
Techatrix committed Nov 12, 2024
1 parent 43725e9 commit 1de821b
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 45 deletions.
10 changes: 5 additions & 5 deletions schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
6 changes: 3 additions & 3 deletions src/Config.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,

Expand Down
43 changes: 15 additions & 28 deletions src/Server.zig
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ const ClientCapabilities = struct {
supports_will_save: bool = false,
supports_will_save_wait_until: bool = false,
supports_publish_diagnostics: bool = false,
supports_code_action_fixall: bool = false,
supports_code_actions: bool = false,
hover_supports_md: bool = false,
signature_help_supports_md: bool = false,
completion_doc_supports_md: bool = false,
Expand Down Expand Up @@ -332,15 +332,19 @@ fn initAnalyser(server: *Server, handle: ?*DocumentStore.Handle) Analyser {
);
}

fn getAutofixMode(server: *Server) enum {
on_save,
will_save_wait_until,
pub fn getAutofixMode(server: *Server) enum {
/// Autofix is implemented by providing `source.fixall` code actions.
fixall,
/// Autofix is implemented using `textDocument/willSaveWaitUntil`.
/// Requires `force_autofix` to be enabled.
will_save_wait_until,
/// 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_actions) return .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;
Expand Down Expand Up @@ -393,19 +397,11 @@ fn autofix(server: *Server, arena: std.mem.Allocator, handle: *DocumentStore.Han
}

fn initializeHandler(server: *Server, arena: std.mem.Allocator, request: types.InitializeParams) Error!types.InitializeResult {
var skip_set_fixall = false;

if (request.clientInfo) |clientInfo| {
server.client_capabilities.client_name = try server.allocator.dupe(u8, clientInfo.name);

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
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;
}
}

Expand Down Expand Up @@ -482,17 +478,8 @@ 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) |_| {
server.client_capabilities.supports_code_actions = true;
}
if (textDocument.definition) |definition| {
server.client_capabilities.supports_textDocument_definition_linkSupport = definition.linkSupport orelse false;
Expand Down Expand Up @@ -1027,8 +1014,8 @@ 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"});
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/features/diagnostics.zig
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub fn generateDiagnostics(server: *Server, arena: std.mem.Allocator, handle: *D
try getAstCheckDiagnostics(server, arena, handle, &diagnostics);
}

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);
}

Expand Down
12 changes: 6 additions & 6 deletions src/tools/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion src/tools/config_gen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion tests/lsp_features/code_actions.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down

0 comments on commit 1de821b

Please sign in to comment.