Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/features/code_actions.zig
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const DocumentStore = @import("../DocumentStore.zig");
const DocumentScope = @import("../DocumentScope.zig");
const Analyser = @import("../analysis.zig");
const ast = @import("../ast.zig");
const diff = @import("../diff.zig");
const types = @import("lsp").types;
const offsets = @import("../offsets.zig");
const tracy = @import("tracy");
Expand Down Expand Up @@ -682,6 +683,9 @@ fn handleUnorganizedImport(builder: *Builder) error{OutOfMemory}!void {
}
}

const resulting_source = try diff.applyTextEdits(builder.arena, tree.source, edits.items, builder.offset_encoding);
if (std.mem.eql(u8, resulting_source, tree.source)) return;

const workspace_edit = try builder.createWorkspaceEdit(edits.items);

try builder.actions.append(builder.arena, .{
Expand Down
61 changes: 61 additions & 0 deletions tests/lsp_features/code_actions.zig
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,31 @@ test "organize imports - edge cases" {
);
}

test "organize imports - no action when already organized" {
// Single import plus the trailing blank line that organize would normalize to.
// https://github.com/zigtools/zls/issues/2523
try testOrganizeImportsNoAction(
\\const a = @import("a");
\\
\\
);
// Sorted imports from different kinds with the expected group separator and trailing blank line.
try testOrganizeImportsNoAction(
\\const std = @import("std");
\\
\\const abc = @import("abc.zig");
\\
\\
);
// Imports already sorted, separated from following decls by the expected blank line.
try testOrganizeImportsNoAction(
\\const std = @import("std");
\\
\\fn main() void {}
\\
);
}

test "convert multiline string literal" {
try testConvertString(
\\const foo = \\Hell<cursor>o
Expand Down Expand Up @@ -960,6 +985,42 @@ fn testOrganizeImports(before: []const u8, after: []const u8) !void {
try testDiagnostic(before, after, .{ .filter_kind = .@"source.organizeImports" });
}

fn testOrganizeImportsNoAction(source: []const u8) !void {
var ctx: Context = try .init();
defer ctx.deinit();

var phr = try helper.collectClearPlaceholders(allocator, source);
defer phr.deinit(allocator);
const clean_source = phr.new_source;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests do not use placeholders so source can just be used directly.

Suggested change
var phr = try helper.collectClearPlaceholders(allocator, source);
defer phr.deinit(allocator);
const clean_source = phr.new_source;


const range: types.Range = .{
.start = .{ .line = 0, .character = 0 },
.end = offsets.indexToPosition(clean_source, clean_source.len, ctx.server.offset_encoding),
};

const uri = try ctx.addDocument(.{ .source = clean_source });

const params: types.CodeAction.Params = .{
.textDocument = .{ .uri = uri.raw },
.range = range,
.context = .{
.diagnostics = &.{},
.only = &.{.@"source.organizeImports"},
},
};

@setEvalBranchQuota(5000);
const response = try ctx.server.sendRequestSync(ctx.arena.allocator(), "textDocument/codeAction", params) orelse return;

for (response) |action| {
const kind = action.code_action.kind orelse continue;
if (kind == .@"source.organizeImports") {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too permissive. Because only source.organizeImports have been requested, if the response contains any actions, the test should fail.

std.debug.print("expected no organize-imports code action for already-organized source, got: {s}\n", .{action.code_action.title});
return error.UnexpectedOrganizeImportsAction;
}
}
}

fn testConvertString(before: []const u8, after: []const u8) !void {
try testDiagnostic(before, after, .{ .filter_kind = .refactor });
}
Expand Down