Symbols are not generated/displayed correctly

[mika76]

It seems to show globals defined multiple times, and even with table keys eg

function _init()
    p1={}
end

function create()    
    p1.name="p1"
    p1.hp=100

end

function hit()
    p1.hp-=1
end

will show

The best, I think, would be to show just p1 once and be able to drill into it to see it's keys - and only show where they are defined

now to compare it to javascript, I converted the code as close as possible:

var p1 = { name: "", hp: 0 }

function _init() {
    p1 = {}
}
function create() {
    p1.name = "p1"
    p1.hp = 100
}

function hit() {
    p1.hp -= 1
}

this shows: and if I comment out the first line (definition of the global var) then any mention of p1 disappears.

I think showing globals defined in lua is actually a good idea, but it's too much to see it shown everywhere it's used. I think a hybrid approach would be better - so as mentioned above - show just once the symbol for p1 (linking to where it's defined first) and then drill into it to see any keys/properties/entries/fields mentioned (and only link to the first one)

[mika76]

@japhib I'd love to try my hand at making this better, but am struggling to debug the server and symbol creation. Could you help get me started? I tried choosing "client-server" and press F5 - it runs and seems to connect and opens vscode but no breakpoints work...

[japhib]

Yeah, I'd love to help you get started. The run configurations are mainly left over auto-generated ones from when I created the repo. I haven't gotten breakpoints to work when running the actual client; however, console.log works fine and will print to the extension output. So you could put in a console.log statement where you want to see the result -- e.g. inside the connection.onDocumentSymbol callback, or inside parseTextDocument() where the cache is populated by a call to documentSymbols.set(). (Both of those locations are inside the file server/src/server.ts)

[japhib]

The issue is most likely to be found in server/src/parser/symbols.ts, where we scan the parsed AST and construct a list of all the symbols.

In the screenshot you posted, I see there's 2 of p1.hp -- this corresponds to the 2 assignments to p1.hp -- p1.hp=100 and p1.hp-=1. It looks like in SymbolFinder.findSymbolsInAssignment(), we call another method called addSymbolForMemberExpressionAssignment(), which seems like it's probably where it's going wrong.

It'll probably be easier to debug the issue using a unit test, since then you can isolate the behavior and have it only parse the relevant code, without doing all the extra stuff it'll do by default when you run the entire client. Looks like the most relevant existing test for this is handles member expressions appropriately, in server/src/parser/test/symbols.test.ts, on line 280. Inside that test, there are some asserts that look like this:

    deepEquals(symbols, [
      { name: 'tbl', type: CodeSymbolType.GlobalVariable, children: [] },
      { name: 'tbl.some_key', type: CodeSymbolType.GlobalVariable, children: [] },

As you can see here, instead of looking for one symbol named tbl with a child named some_key, it explicitly tests for 2 separate keys, tbl and tbl.some_key. So, in test-driven-development format, you could change the assert to look something like this:

    deepEquals(symbols, [
      { name: 'tbl', type: CodeSymbolType.GlobalVariable, children: [
        { name: 'some_key', type: CodeSymbolType.GlobalVariable, children: [] },
      ] },

which will fail, and then work on getting that to pass.

Hopefully that's enough information to get your feet wet! Let me know if you have more questions.

[mika76]

Hey @japhib thanks a lot for your thorough answer. I've actually been playing around with it quite a bit and have made quite a bit of progress - even got F5 debugging to work (by totally breaking the build of course 😆) - anyway I'll work a bit more on it and post a pr so you can see what I've done.

Basically I'm trying to copy the way Typescript/Javascripts lsp shows symbols which would be a lot less "noisy" and have added signatures and details and even a rudimentary ldoc implementation. There's still a bit to do of course, and I have no tests for this stuff so I should probably do those too, but it's been fun so far. The lua parser/AST is amazing stuff!

Here's a screenshot of it:

[japhib]

Wow, that looks great!