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

Struct init fields completion. #1075

Merged
merged 1 commit into from
Mar 21, 2023
Merged

Conversation

llogick
Copy link
Member

@llogick llogick commented Mar 17, 2023

Up for logic review and a fuzz (suggestions still welcome).

Caveat:

// has to be at root level, ie outside of fns
const MyStruct = path.to.my.MyStruct;
var a = MyStruct{. // now resolved

Note:
Nested structs completion, ie MyStruct{.fld = a, SomeOther{.dlf = 1}, .<complete here>}, is out of scope, use locals.

@leecannon leecannon added the pr:fuzz Attach to a PR to start fuzzing / continually fuzz (please do this before merging!) label Mar 17, 2023
@llogick llogick force-pushed the struct-init-compl branch 2 times, most recently from 9598bc3 to 41482af Compare March 17, 2023 22:40
@leecannon
Copy link
Member

leecannon commented Mar 21, 2023

This works well for the simple case but there are some very common cases that it does not support:

  • types returned by functions (any generic)
fn NonGenericGeneric() type {
    return struct {
        a: bool,
        b: bool,
    };
}

fn nonGenericGeneric() void {
    var r = NonGenericGeneric(){}; // no completion
}
  • completion within the container itself
const MyStruct = struct {
    a: bool,
    b: bool,
    
    fn inside() void {
        var s = MyStruct{}; // no completion 
    }
};

fn outside() void {
    var s = MyStruct{}; // completion
}
  • file level structs
// inside "MyStruct.zig"
a: bool,
b: bool,

// inside "main.zig"
const MyStruct = @import("MyStruct.zig");

fn topLevelStruct() void {
    var s = MyStruct{}; // no completion 
}
  • aliases
const MyStruct = struct {
    a: bool,
    b: bool,
};

const Alias = MyStruct;

fn alias() void {
    var s = Alias{}; // no completion
}

@leecannon
Copy link
Member

leecannon commented Mar 21, 2023

I'm okay merging this as is, as it does increases ZLS capabilities.
I'm just making note of things that can still be improved in this area.

@nullptrdevs any chance you can rebase?

@leecannon leecannon merged commit 9723a92 into zigtools:master Mar 21, 2023
@llogick
Copy link
Member Author

llogick commented Mar 21, 2023

Thank you for taking the time to review, I will look into all of these, but some initial thoughts:

Case 1: No idea where to start. Something something ComptimeInterpreter.
Case 2: The parser skips creating a node for MyStruct,

const MyStruct = struct {
    a: bool,
    b: bool,
    
    fn inside() void {
        var s = MyStruct{.}; // no completion 
    }
};

fn outside() void {
    var s = MyStruct{}; // completion
    MyStruct. // no completion here either
}

(Custom parser when?) Custom parse tokens looking for same .identifier, MyStruct, then parse .identifiers that might be fields(look for a .colon).
Case 3: This would be priority No.1.
Case 4: How common is this (worth the cpu cycles 😀 )?

@llogick llogick deleted the struct-init-compl branch March 21, 2023 22:01
@Vaxeral
Copy link

Vaxeral commented Apr 4, 2023

There is a bug, struct member completions fail to appear if the struct is defined after it is used.

Screenshot from 2023-04-04 14-06-56

Screenshot from 2023-04-04 14-07-08

@Vaxeral
Copy link

Vaxeral commented Apr 4, 2023

I am curious to know why this happens.
On the bright site the lsp does show the structure definition when i hover over it. That is good enough for me. :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:fuzz Attach to a PR to start fuzzing / continually fuzz (please do this before merging!)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants