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 6, 2024
1 parent 1b31950 commit bbb731f
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 37 deletions.
4 changes: 2 additions & 2 deletions schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
},
"default": []
},
"enable_autofix": {
"description": "Whether to automatically fix errors on save. Currently supports adding and removing discards.",
"force_autofix": {
"description": "TODO",
"type": "boolean",
"default": false
},
Expand Down
4 changes: 2 additions & 2 deletions src/Config.zig
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ 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,
/// TODO
force_autofix: bool = false,

/// Set level of semantic tokens. `partial` only includes information that requires semantic analysis.
semantic_tokens: enum {
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 @@ -392,19 +396,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 @@ -481,17 +477,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 @@ -1026,8 +1013,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) {
if (server.getAutofixMode() != .none) {
try code_actions.collectAutoDiscardDiagnostics(tree, arena, &diagnostics, server.offset_encoding);
}

Expand Down
4 changes: 2 additions & 2 deletions src/tools/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
"default": []
},
{
"name": "enable_autofix",
"description": "Whether to automatically fix errors on save. Currently supports adding and removing discards.",
"name": "force_autofix",
"description": "TODO",
"type": "bool",
"default": false
},
Expand Down
2 changes: 0 additions & 2 deletions tests/lsp_features/code_actions.zig
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,6 @@ fn testAutofix(before: []const u8, after: []const u8) !void {
fn testAutofixOptions(before: []const u8, after: []const u8, want_zir: bool) !void {
var ctx = try Context.init();
defer ctx.deinit();
ctx.server.config.enable_autofix = true;
ctx.server.config.prefer_ast_check_as_child_process = !want_zir;

const uri = try ctx.addDocument(.{ .source = before });
Expand Down Expand Up @@ -641,7 +640,6 @@ fn testAutofixOptions(before: []const u8, after: []const u8, want_zir: bool) !vo
fn testDiagnostic(before: []const u8, after: []const u8) !void {
var ctx = try Context.init();
defer ctx.deinit();
ctx.server.config.enable_autofix = true;

const uri = try ctx.addDocument(.{ .source = before });
const handle = ctx.server.document_store.getHandle(uri).?;
Expand Down

0 comments on commit bbb731f

Please sign in to comment.