tree-sitter-grammars / tree-sitter-odin

Odin grammar for tree-sitter
MIT License
19 stars 4 forks source link

Does not recognize closing quotes for strings in several scenarios #7

Closed GoNZooo closed 9 months ago

GoNZooo commented 11 months ago

Lately I've noticed that most of my projects are having syntax highlighting issues with strings being highlighted as not closed despite having clear end markers. I didn't notice it being at all this bad until recently, which I think coincides with an upgrade (unexpected downgrade) of the nvim-treesitter plugin which bundles this repository as the Odin parser by default.

Some examples:

    testing.expect_value(t, s, "foo")
    testing.expect(
        t,
        slice.equal(remaining_arguments, expected_arguments),
        fmt.tprintf("Expected remaining arguments to equal %v, got: %v", remaining_arguments),
    )

    i: int
    expected_arguments = []string{}
    i, remaining_arguments, error = parse_arguments_as_type({"123"}, int, context.allocator)
    testing.expect_value(t, error, nil)
    testing.expect_value(t, i, 123)
    testing.expect(
        t,
        slice.equal(remaining_arguments, expected_arguments),
        fmt.tprintf("Expected remaining arguments to equal %v, got: %v", remaining_arguments),
    )

image

What actually seems to close the example above is yet another missed closing quote:

    ts: TestStruct
    expected_arguments = []string{"rest", "of", "arguments"}
    ts, remaining_arguments, error = parse_arguments_as_type(
         {
            "-2",
            "123",
            "--field-one",
            "foo",
            "--no-tag",
            "123.456",
            "--field-three",
            "true",
            "rest",
            "of",
            "arguments",
        },
        TestStruct,
        context.allocator,
    )
    testing.expect_value(t, error, nil)
    testing.expect_value(
        t,
        ts,
        TestStruct{field_one = "foo", field_two = 123, field_three = true, no_tag = 123.456},
    )

image

@(test, private = "package")
test_parse_argument_as_type :: proc(t: ^testing.T) {
    context.logger = log.create_console_logger()

    tid: typeid

    tid = string
    bytes, error := parse_argument_as_type("foo", tid, context.allocator)
    s := mem.reinterpret_copy(string, raw_data(bytes))
    testing.expect_value(t, error, nil)
    testing.expect(t, s == "foo", fmt.tprintf("Expected 'foo', got '%s'", s))

    tid = int
    bytes, error = parse_argument_as_type("123", tid, context.allocator)
    i := mem.reinterpret_copy(int, raw_data(bytes))
    testing.expect_value(t, error, nil)
    testing.expect(t, i == 123, fmt.tprintf("Expected 123, got %d", i))

image

The above one goes on for several procedures and type definitions until we reach this:

@(test, private = "package")
test_struct_decoding_info :: proc(t: ^testing.T) {
    cli_info, allocator_error := struct_decoding_info(TestStruct)
    if allocator_error != nil {
        fmt.panicf("Allocator error: %s", allocator_error)
    }
    fields := []FieldCliInfo {
         {
            name = "field_one",
            type = string,
            cli_short_name = "1",
            cli_long_name = "field-one",
            offset = 0,
            required = false,
            size = 16,
        },
         {
            name = "field_two",
            type = int,
            cli_short_name = "2",
            cli_long_name = "field-two",
            offset = 16,
            required = true,
            size = 8,
        },
         {
            name = "field_three",
            type = bool,
            cli_short_name = "",
            cli_long_name = "field-three",
            offset = 24,
            required = true,
            size = 1,
        },

image

amaanq commented 11 months ago

I can't reproduce with your snippets - I need a full reproducer else this isn't actionable

GoNZooo commented 11 months ago

You can clone https://github.com/GoNZooo/odin-cli to see it fail in action if you want. Here is d165dbee27617dab2653e38737d96ede1030d14f and the same areas, by the way, confirming that it was caused by your fix for v1.0.1:

image

MineBill commented 11 months ago

I think i found the possible cause of this issue. Opening the project OP provided, odin-cli, and then using a tool like Playground will show numerous errors, with the first one appearing at this line: image After some debugging it looks like the parser has trouble with comas on the return statement, when they are in new lines: image image

Here is the snippet which causes this error:

package main

test :: proc() -> (int, int, int) {
    return 1,
        1, 1,
}

This particular comma placement appears multiple times OPs' project and my guess is it causes more and more parse errors to happen until the issue becomes more visible with the strings.

GoNZooo commented 11 months ago

It should be noted that this formatting is the default formatting of odinfmt, the formatter that is used in ols, the defacto standard language server, so it would definitely be useful for this formatting to not break the parser.

amaanq commented 9 months ago

Sorry about the delay, this is now fixed and I've added support for the new stuff odin seemingly has added, like or_continue, or_break, and some tidying up