autofix: discard captures + some multi for loop support

this patch makes autofix add discards for unused loop and switch case
captures which have curlies.  it prevents adding duplicate
'remove capture' actions by checking previous action ranges.

it removes special casing of index captures now that multi for loops
have arrived.

* make getCaptureLoc() return only single Loc
* remove CaptureLocs struct which is no longer used
* add DiagnosticKind.@"switch tag capture" to handle
  'inline else => |x, tag|' discards
* add test "getCaptureLoc"
This commit is contained in:
Travis Staloch 2023-05-26 20:10:51 -07:00
parent 3500aa7a76
commit 49b679ee22
No known key found for this signature in database
GPG Key ID: 9726F5C64475E635

View File

@ -24,13 +24,11 @@ pub const Builder = struct {
.@"function parameter" => try handleUnusedFunctionParameter(builder, actions, loc), .@"function parameter" => try handleUnusedFunctionParameter(builder, actions, loc),
.@"local constant" => try handleUnusedVariableOrConstant(builder, actions, loc), .@"local constant" => try handleUnusedVariableOrConstant(builder, actions, loc),
.@"local variable" => try handleUnusedVariableOrConstant(builder, actions, loc), .@"local variable" => try handleUnusedVariableOrConstant(builder, actions, loc),
.@"loop index capture" => try handleUnusedIndexCapture(builder, actions, loc), .@"switch tag capture", .capture => try handleUnusedCapture(builder, actions, loc),
.capture => try handleUnusedCapture(builder, actions, loc),
}, },
.non_camelcase_fn => try handleNonCamelcaseFunction(builder, actions, loc), .non_camelcase_fn => try handleNonCamelcaseFunction(builder, actions, loc),
.pointless_discard => try handlePointlessDiscard(builder, actions, loc), .pointless_discard => try handlePointlessDiscard(builder, actions, loc),
.omit_discard => |id| switch (id) { .omit_discard => |id| switch (id) {
.@"index capture" => try handleUnusedIndexCapture(builder, actions, loc),
.@"error capture" => try handleUnusedCapture(builder, actions, loc), .@"error capture" => try handleUnusedCapture(builder, actions, loc),
}, },
// the undeclared identifier may be a discard // the undeclared identifier may be a discard
@ -166,64 +164,67 @@ fn handleUnusedVariableOrConstant(builder: *Builder, actions: *std.ArrayListUnma
}); });
} }
fn handleUnusedIndexCapture(builder: *Builder, actions: *std.ArrayListUnmanaged(types.CodeAction), loc: offsets.Loc) !void {
const tracy_zone = tracy.trace(@src());
defer tracy_zone.end();
const capture_locs = getCaptureLoc(builder.handle.text, loc, true) orelse return;
// TODO support discarding without modifying the capture
// by adding a discard in the block scope
const is_value_discarded = std.mem.eql(u8, offsets.locToSlice(builder.handle.text, capture_locs.value), "_");
if (is_value_discarded) {
// |_, i| ->
// TODO fix formatting
try actions.append(builder.arena, .{
.title = "remove capture",
.kind = .quickfix,
.isPreferred = true,
.edit = try builder.createWorkspaceEdit(&.{builder.createTextEditLoc(capture_locs.loc, "")}),
});
} else {
// |v, i| -> |v|
// |v, _| -> |v|
try actions.append(builder.arena, .{
.title = "remove index capture",
.kind = .quickfix,
.isPreferred = true,
.edit = try builder.createWorkspaceEdit(&.{builder.createTextEditLoc(
.{ .start = capture_locs.value.end, .end = capture_locs.loc.end - 1 },
"",
)}),
});
}
}
fn handleUnusedCapture(builder: *Builder, actions: *std.ArrayListUnmanaged(types.CodeAction), loc: offsets.Loc) !void { fn handleUnusedCapture(builder: *Builder, actions: *std.ArrayListUnmanaged(types.CodeAction), loc: offsets.Loc) !void {
const tracy_zone = tracy.trace(@src()); const tracy_zone = tracy.trace(@src());
defer tracy_zone.end(); defer tracy_zone.end();
const capture_locs = getCaptureLoc(builder.handle.text, loc, false) orelse return; const capture_loc = getCaptureLoc(builder.handle.text, loc) orelse return;
// TODO support discarding without modifying the capture // look for next non-whitespace after last '|'. if its a '{' we can insert discards.
// by adding a discard in the block scope // this means bare loop/switch captures (w/out curlies) aren't supported.
if (capture_locs.index != null) { var block_start = capture_loc.end + 1;
// |v, i| -> |_, i| while (block_start < builder.handle.text.len and
try actions.append(builder.arena, .{ std.ascii.isWhitespace(builder.handle.text[block_start])) : (block_start += 1)
{}
if (builder.handle.text[block_start] != '{') {
return;
}
const block_start_loc = offsets.Loc{ .start = block_start + 1, .end = block_start + 1 };
const identifier_name = builder.handle.text[loc.start..loc.end];
const new_text = try createDiscardText(builder, identifier_name, block_start, true);
const action1 = .{
.title = "discard capture", .title = "discard capture",
.kind = .quickfix, .kind = .@"source.fixAll",
.isPreferred = true, .isPreferred = true,
.edit = try builder.createWorkspaceEdit(&.{builder.createTextEditLoc(capture_locs.value, "_")}), .edit = try builder.createWorkspaceEdit(&.{builder.createTextEditLoc(block_start_loc, new_text)}),
}); };
} else { const action2 = .{
// |v| -> .title = "discard capture name",
// TODO fix formatting .kind = .quickfix,
try actions.append(builder.arena, .{ .isPreferred = false,
.edit = try builder.createWorkspaceEdit(&.{builder.createTextEditLoc(loc, "_")}),
};
// prevent adding duplicate 'remove capture' action.
// search for a matching action by comparing ranges.
const remove_cap_loc = builder.createTextEditLoc(capture_loc, "");
const found_remove_capture_action = outer: for (actions.items) |item| {
const edit = item.edit orelse continue;
const changes = edit.changes orelse continue;
var iter = changes.iterator();
while (iter.next()) |entry| {
const t_edits = entry.value_ptr.*;
for (t_edits) |t_edit| {
if (remove_cap_loc.range.start.line == t_edit.range.start.line and
remove_cap_loc.range.start.character == t_edit.range.start.character and
remove_cap_loc.range.end.line == t_edit.range.end.line and
remove_cap_loc.range.end.character == t_edit.range.end.character)
break :outer true;
}
}
} else false;
if (found_remove_capture_action)
try actions.appendSlice(builder.arena, &.{ action1, action2 })
else {
const action0 = types.CodeAction{
.title = "remove capture", .title = "remove capture",
.kind = .quickfix, .kind = .quickfix,
.isPreferred = true, .isPreferred = false,
.edit = try builder.createWorkspaceEdit(&.{builder.createTextEditLoc(capture_locs.loc, "")}), .edit = try builder.createWorkspaceEdit(&.{remove_cap_loc}),
}); };
try actions.appendSlice(builder.arena, &.{ action0, action1, action2 });
} }
} }
@ -374,13 +375,11 @@ const DiagnosticKind = union(enum) {
@"function parameter", @"function parameter",
@"local constant", @"local constant",
@"local variable", @"local variable",
@"loop index capture", @"switch tag capture",
capture, capture,
}; };
const DiscardCat = enum { const DiscardCat = enum {
// "discard of index capture; omit it instead"
@"index capture",
// "discard of error capture; omit it instead" // "discard of error capture; omit it instead"
@"error capture", @"error capture",
}; };
@ -478,126 +477,42 @@ fn getDiscardLoc(text: []const u8, loc: offsets.Loc) ?offsets.Loc {
}; };
} }
const CaptureLocs = struct { /// takes the location of a capture ie `value` from `...|value...|...`.
loc: offsets.Loc, /// returns the location from '|' until '|'
value: offsets.Loc, fn getCaptureLoc(text: []const u8, loc: offsets.Loc) ?offsets.Loc {
index: ?offsets.Loc, const start_pipe_position = blk: {
var i = loc.start;
while (true) : (i -= 1) {
if (text[i] == '|' or i == 0) break;
}
break :blk i;
}; };
/// takes the location of an identifier which is part of a payload `|value, index|` const end_pipe_position = (std.mem.indexOfScalarPos(u8, text, start_pipe_position + 1, '|') orelse
/// and returns the location from '|' until '|' or null on failure return null) + 1;
/// use `is_index_payload` to indicate whether `loc` points to `value` or `index`
fn getCaptureLoc(text: []const u8, loc: offsets.Loc, is_index_payload: bool) ?CaptureLocs {
const value_end = if (!is_index_payload) loc.end else found: {
// move back until we find a comma
const comma_position = found_comma: {
var i = loc.start - 1;
while (i != 0) : (i -= 1) {
switch (text[i]) {
' ' => continue,
',' => break :found_comma i,
else => return null,
}
} else return null;
};
// trim space return .{ .start = start_pipe_position, .end = end_pipe_position };
var i = comma_position - 1;
while (i != 0) : (i -= 1) {
switch (text[i]) {
' ' => continue,
else => {
if (!isSymbolChar(text[i])) return null;
break :found i + 1;
},
}
} else return null;
};
const value_start = if (!is_index_payload) loc.start else found: {
// move back until we find a non identifier character
var i = value_end - 1;
while (i != 0) : (i -= 1) {
if (isSymbolChar(text[i])) continue;
switch (text[i]) {
' ', '|', '*' => break :found i + 1,
else => return null,
}
} else return null;
};
var index: ?offsets.Loc = null;
if (is_index_payload) {
index = loc;
} else blk: {
// move forward until we find a comma
const comma_position = found_comma: {
var i = value_end;
while (i < text.len) : (i += 1) {
switch (text[i]) {
' ' => continue,
',' => break :found_comma i,
else => break :blk,
}
}
break :blk;
};
// trim space
const index_start = found_start: {
var i = comma_position + 1;
while (i < text.len) : (i += 1) {
switch (text[i]) {
' ' => continue,
else => {
if (!isSymbolChar(text[i])) break :blk;
break :found_start i;
},
}
}
break :blk;
};
// move forward until we find a non identifier character
var i = index_start + 1;
while (i < text.len) : (i += 1) {
if (isSymbolChar(text[i])) continue;
index = offsets.Loc{
.start = index_start,
.end = i,
};
break;
}
} }
const start_pipe_position = found: { test "getCaptureLoc" {
var i = value_start - 1; {
while (i != 0) : (i -= 1) { const text = "|i|";
switch (text[i]) { const caploc = getCaptureLoc(text, .{ .start = 1, .end = 2 }) orelse
' ' => continue, return std.testing.expect(false);
'|' => break :found i, const captext = text[caploc.start..caploc.end];
else => return null, try std.testing.expectEqualStrings(text, captext);
} }
} else return null; {
}; const text = "|i, jjj, foobar|";
const caploc = getCaptureLoc(text, .{ .start = 1, .end = 17 }) orelse
const end_pipe_position = found: { return std.testing.expect(false);
var i: usize = if (index) |index_loc| index_loc.end else value_end; const captext = text[caploc.start..caploc.end];
while (i < text.len) : (i += 1) { try std.testing.expectEqualStrings(text, captext);
switch (text[i]) { }
' ' => continue, {
'|' => break :found i + 1, const caploc = getCaptureLoc("||", .{ .start = 1, .end = 2 });
else => return null, try std.testing.expect(caploc == null);
} }
} else return null;
};
return CaptureLocs{
.loc = .{ .start = start_pipe_position, .end = end_pipe_position },
.value = .{ .start = value_start, .end = value_end },
.index = index,
};
} }
fn isSymbolChar(char: u8) bool { fn isSymbolChar(char: u8) bool {