Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in identifier detection #2035

Open
Sekky61 opened this issue Sep 21, 2024 · 3 comments
Open

Bug in identifier detection #2035

Sekky61 opened this issue Sep 21, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@Sekky61
Copy link
Contributor

Sekky61 commented Sep 21, 2024

Zig Version

0.14.0-dev.1573+4d81e8ee9

ZLS Version

0.14.0-dev.160+cf9b49a

Client / Code Editor / Extensions

NVIM v0.10.1 and lspconfig

Steps to Reproduce and Observed Behavior

const std = @import("std");
const print = std.debug.print;

const Enum = enum {
    source,
    @"source.fixAll",
};

pub fn main() void {
    const @"identifier with spaces in it" = Enum.source;

    switch (@"identifier with spaces in it") {
        .source => print("abc\n", .{}),
        .@"source.fixAll" => print("abcd--\n", .{}),
    }
}

Reproduction:

  1. Hover over the @"identifier with spaces in it" and perform Go to definition
  2. Hover over the word source in .@"source.fixAll" and perform Go to definition
  3. Hover over the word fixAll in .@"source.fixAll" and perform Go to definition

Notice that:

  • The go to definition does not work for the variable in the switch statement.
  • The go to definition on the .@"source.fixAll" jumps to the source definition and only works before the dot.

Expected Behavior

I expected the cursor to go to the definition of the variable/enum variant.

Relevant log output

No response

@Sekky61 Sekky61 added the bug Something isn't working label Sep 21, 2024
@Sekky61
Copy link
Contributor Author

Sekky61 commented Sep 21, 2024

As for the fix, the problem seems to be in the identifierLocFromIndex function.
It does not work with identifiers with special characters (the @"" syntax).

I propose that identifierLocFromIndex should handle the @"" syntax.
If you agree, I would like to take a look at this issue

@llogick
Copy link
Member

llogick commented Sep 21, 2024

I do agree that it is an issue, see

Spoilers ahead

Apparently one can get pretty close with

diff --git a/src/analysis.zig b/src/analysis.zig
index edf0b8c5..aa144af1 100644
--- a/src/analysis.zig
+++ b/src/analysis.zig
@@ -602,16 +602,26 @@ pub fn identifierLocFromIndex(tree: Ast, source_index: usize) ?offsets.Loc {
     std.debug.assert(source_index <= tree.source.len);
 
     var start = source_index;
-    while (start > 0 and isSymbolChar(tree.source[start - 1])) {
+    while (start > 0 and (isSymbolChar(tree.source[start - 1]) or switch (tree.source[start - 1]) {
+        ' ', '.' => true,
+        else => false,
+    })) {
         start -= 1;
     }
 
     var end = source_index;
-    while (end < tree.source.len and isSymbolChar(tree.source[end])) {
+    while (end < tree.source.len and (isSymbolChar(tree.source[end]) or switch (tree.source[end]) {
+        ' ', '.' => true,
+        else => false,
+    })) {
         end += 1;
     }
 
     if (start == end) return null;
+
+    while (start < end and std.mem.indexOfScalar(u8, " .", tree.source[start]) != null) : (start += 1) {}
+    while (end > start and std.mem.indexOfScalar(u8, " .", tree.source[end - 1]) != null) : (end -= 1) {}
+
     return .{ .start = start, .end = end };
 }

and that'd cover ~99% of the use cases..

But,

const std = @import("std");
const print = std.debug.print;

const Enum = enum {
    source,
    @"source 😃 fixAll",
};

pub fn main() void {
    const @"identifier with 😃 in it" = Enum.@"source 😃 fixAll";
    const argh = @"identifier with 😃 in it";
    _ = argh;

    switch (@"identifier with 😃 in it") {
        .source => print("abc\n", .{}),
        .@"source 😃 fixAll" => print("abcd--\n", .{}),
    }
}

For a tokens based approach feel free to peek at the commits in
https://github.com/llogick/zls/tree/llogick/at-ident-syntax-fixes
https://github.com/llogick/zls/tree/dev

Cheers

@Sekky61
Copy link
Contributor Author

Sekky61 commented Sep 21, 2024

Oh, I read that issue before but I didn't remember it when I encountered it today. I will take a look at it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants