Goto definition works when the cursor is at the start of the identifier.
Before, the code lexed only a prefix of the line up to cursor position. Now, we lex the entire line, and do look at the token just after the cursor. This subtly changes sematncih of `getPostionContext`: now it is becomes oblivious of the _exact_ position of the cursor and returns the whole token at cursor's position. I believe this is semantically right approach -- _most_ of the callsite should not worry at all about such details. Something like completion _might_ want to know more, but it's better to make that call site's problem. It might be the case that some existing code relies on the past behavior. It's hard to tell though -- we don't have a lot of tests for _features_, and changes to unit-tests don't explain if the changes are meaningful for user-observable behavior or not. In general, for LSP-shaped thing, I feel that the bulk of testing should be focused on end-to-end behaviors....
This commit is contained in:
Aleksey Kladov
Lee Cannon
parent
1ed8d49b30
commit
ea05916e69
|
|
@ -1553,9 +1553,15 @@ fn tokenLocAppend(prev: offsets.Loc, token: std.zig.Token) offsets.Loc {
|
|||
};
|
||||
}
|
||||
|
||||
/// Given a byte index in a document (typically cursor offset), classify what kind of entity is at that index.
|
||||
///
|
||||
/// Classification is based on the lexical structure -- we fetch the line containin index, tokenize it,
|
||||
/// and look at the sequence of tokens just before the cursor. Due to the nice way zig is designed (only line
|
||||
/// comments, etc) lexing just a single line is always correct.
|
||||
pub fn getPositionContext(allocator: std.mem.Allocator, text: []const u8, doc_index: usize) !PositionContext {
|
||||
const line_loc = offsets.lineLocUntilIndex(text, doc_index);
|
||||
const line_loc = offsets.lineLocAtIndex(text, doc_index);
|
||||
const line = offsets.locToSlice(text, line_loc);
|
||||
const prev_char = if (doc_index > 0) text[doc_index - 1] else 0;
|
||||
|
||||
const is_comment = std.mem.startsWith(u8, std.mem.trimLeft(u8, line, " \t"), "//");
|
||||
if (is_comment) return .comment;
|
||||
|
|
@ -1576,10 +1582,16 @@ pub fn getPositionContext(allocator: std.mem.Allocator, text: []const u8, doc_in
|
|||
while (true) {
|
||||
const tok = tokenizer.next();
|
||||
// Early exits.
|
||||
if (tok.loc.start > doc_index) break;
|
||||
if (tok.loc.start == doc_index) {
|
||||
// Tie-breaking, the curosr is exactly between two tokens, and
|
||||
// `tok` is the latter of the two.
|
||||
if (tok.tag != .identifier) break;
|
||||
}
|
||||
switch (tok.tag) {
|
||||
.invalid => {
|
||||
// Single '@' do not return a builtin token so we check this on our own.
|
||||
if (line[line.len - 1] == '@') {
|
||||
if (prev_char == '@') {
|
||||
return PositionContext{
|
||||
.builtin = .{
|
||||
.start = line_loc.end - 1,
|
||||
|
|
@ -1685,7 +1697,7 @@ pub fn getPositionContext(allocator: std.mem.Allocator, text: []const u8, doc_in
|
|||
.label => |filled| {
|
||||
// We need to check this because the state could be a filled
|
||||
// label if only a space follows it
|
||||
if (!filled or line[line.len - 1] != ' ') {
|
||||
if (!filled or prev_char != ' ') {
|
||||
return state.ctx;
|
||||
}
|
||||
},
|
||||
|
|
|
|||
|
|
@ -28,15 +28,10 @@ test "definition - cursor is at the end of an identifier" {
|
|||
}
|
||||
|
||||
test "definition - cursor is at the start of an identifier" {
|
||||
testDefinition(
|
||||
try testDefinition(
|
||||
\\fn main() void { <>foo(); }
|
||||
\\fn <def>foo</def>() void {}
|
||||
) catch |err| switch (err) {
|
||||
error.UnresolvedDefinition => {
|
||||
// TODO: #891
|
||||
},
|
||||
else => return err,
|
||||
};
|
||||
);
|
||||
}
|
||||
|
||||
fn testDefinition(source: []const u8) !void {
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ test "position context - var access" {
|
|||
\\const this_var = id<cursor>entifier;
|
||||
,
|
||||
.var_access,
|
||||
"id",
|
||||
"identifier",
|
||||
);
|
||||
try testContext(
|
||||
\\const this_var = identifier<cursor>;
|
||||
|
|
@ -40,37 +40,37 @@ test "position context - field access" {
|
|||
\\if (foo.<cursor>field == foo) {
|
||||
,
|
||||
.field_access,
|
||||
"foo.",
|
||||
"foo.field",
|
||||
);
|
||||
try testContext(
|
||||
\\if (foo.member.<cursor>field == foo) {
|
||||
,
|
||||
.field_access,
|
||||
"foo.member.",
|
||||
"foo.member.field",
|
||||
);
|
||||
try testContext(
|
||||
\\if (foo.*.?.<cursor>field == foo) {
|
||||
,
|
||||
.field_access,
|
||||
"foo.*.?.",
|
||||
"foo.*.?.field",
|
||||
);
|
||||
try testContext(
|
||||
\\if (foo[0].<cursor>field == foo) {
|
||||
,
|
||||
.field_access,
|
||||
"foo[0].",
|
||||
"foo[0].field",
|
||||
);
|
||||
try testContext(
|
||||
\\if (foo.<cursor>@"field" == foo) {
|
||||
,
|
||||
.field_access,
|
||||
"foo.",
|
||||
"foo.@\"field\"",
|
||||
);
|
||||
try testContext(
|
||||
\\const arr = std.ArrayList(SomeStruct(a, b, c, d)).in<cursor>it(allocator);
|
||||
,
|
||||
.field_access,
|
||||
"std.ArrayList(SomeStruct(a, b, c, d)).in",
|
||||
"std.ArrayList(SomeStruct(a, b, c, d)).init",
|
||||
);
|
||||
try testContext(
|
||||
\\fn foo() !Foo.b<cursor> {
|
||||
|
|
@ -122,13 +122,13 @@ test "position context - import/embedfile string literal" {
|
|||
\\const std = @import("st<cursor>");
|
||||
,
|
||||
.import_string_literal,
|
||||
"\"st", // maybe report just "st"
|
||||
"\"st\"", // maybe report just "st"
|
||||
);
|
||||
try testContext(
|
||||
\\const std = @embedFile("file.<cursor>");
|
||||
,
|
||||
.embedfile_string_literal,
|
||||
"\"file.", // maybe report just "file."
|
||||
"\"file.\"", // maybe report just "file."
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -137,13 +137,13 @@ test "position context - string literal" {
|
|||
\\var foo = "he<cursor>llo world!";
|
||||
,
|
||||
.string_literal,
|
||||
"\"he", // maybe report just "he"
|
||||
"\"hello world!\"", // maybe report just "hello world!"
|
||||
);
|
||||
try testContext(
|
||||
\\var foo = \\hello<cursor>;
|
||||
,
|
||||
.string_literal,
|
||||
"\\\\hello", // maybe report just "hello"
|
||||
"\\\\hello;", // maybe report just "hello;"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -237,7 +237,7 @@ fn testContext(line: []const u8, tag: std.meta.Tag(analysis.PositionContext), ma
|
|||
const final_line = try std.mem.concat(allocator, u8, &.{ line[0..cursor_idx], line[cursor_idx + "<cursor>".len ..] });
|
||||
defer allocator.free(final_line);
|
||||
|
||||
const ctx = try analysis.getPositionContext(allocator, line, cursor_idx);
|
||||
const ctx = try analysis.getPositionContext(allocator, final_line, cursor_idx);
|
||||
|
||||
if (std.meta.activeTag(ctx) != tag) {
|
||||
std.debug.print("Expected tag `{s}`, got `{s}`\n", .{ @tagName(tag), @tagName(std.meta.activeTag(ctx)) });
|
||||
|
|
@ -253,7 +253,7 @@ fn testContext(line: []const u8, tag: std.meta.Tag(analysis.PositionContext), ma
|
|||
|
||||
const expected_range = maybe_range orelse {
|
||||
std.debug.print("Expected null range, got `{s}`\n", .{
|
||||
line[actual_loc.start..actual_loc.end],
|
||||
final_line[actual_loc.start..actual_loc.end],
|
||||
});
|
||||
return error.DifferentRange;
|
||||
};
|
||||
|
|
@ -263,8 +263,8 @@ fn testContext(line: []const u8, tag: std.meta.Tag(analysis.PositionContext), ma
|
|||
|
||||
if (expected_range_start != actual_loc.start or expected_range_end != actual_loc.end) {
|
||||
std.debug.print("Expected range `{s}` ({}..{}), got `{s}` ({}..{})\n", .{
|
||||
line[expected_range_start..expected_range_end], expected_range_start, expected_range_end,
|
||||
line[actual_loc.start..actual_loc.end], actual_loc.start, actual_loc.end,
|
||||
final_line[expected_range_start..expected_range_end], expected_range_start, expected_range_end,
|
||||
final_line[actual_loc.start..actual_loc.end], actual_loc.start, actual_loc.end,
|
||||
});
|
||||
return error.DifferentRange;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user