add classes for each parser.object type

[lewis6991]

Now LuaLS can narrow types based on literal fields (see #2864), I've began to break out parser.object into smaller classes that contain the exact fields for each.

This results in a lot of type warnings for script/vm/* that may be difficult to resolve.

Is this something that is worth pursuing or would it be preferred to maintain parser.object are a large class with lots of optional fields?

[tomlau10]

I've began to break out parser.object into smaller classes that contain the exact fields for each.

wow~ this is great idea and great work 👍 If this can be completed, then the auto completion for each object type will be much more accurate.

This results in a lot of type warnings for script/vm/* that may be difficult to resolve.

I just checked out this PR and did a full workspace diagnostics, currently there are like 12 warnings related to script/vm/*.
edit: I just found out that it's because I have turned on both type.weakNilCheck and type.weakUnionCheck in my settings.json 😇 After I turn it off, now there are 21 warnings related to script/vm/*

need-check-nil

Most of them are just need-check-nil and maybe resolved by ---@cast <var> -?: https://luals.github.io/wiki/annotations/#cast

• For example the one in script/vm/compiler.lua#L1181

            if ref.parent.type ~= 'call' then
                goto continue
            end
            local caller = ref.parent   ---@cast caller -?
            if not caller.args then     -- will fix `need-nil-check` for `caller`
                goto continue
            end

• However as you may have guessed, ref.parent should actually be inferred as parser.object.call, but now it's throwing warning because it is inferred as parser.object only. I think this is because narrow types based on literal fields is implemented to trace one level of variable only (for performance reason I guess?) 😕
  • if obj.type == 'A' then => will trace the infer type for obj ✅
  • but ‼️ if obj.nested.type == 'A' => will NOT trace the infer type for obj.nested ❌
  • That means the narrow types based on literal fields function cannot cover all the cases
• In order make use of your type narrow by literal field, the above code piece needs to be rewritten as:

            local refParent = ref.parent
            if not refParent or refParent.type ~= 'call' then
                goto continue
            end
            local caller = refParent  -- now `caller` will be of `parser.object.call` type
            if not caller.args then   -- no more warnings
                goto continue
            end

undefined-field (bug?)

Another warning type that I've seen is undefined-field, but I think it is a false positive case which is a bug (?) 😳
It is the one in script/vm/tracer.lua#L248:

                if cast.extends then
                    node:clear()
                    node:merge(vm.compileNode(cast.extends))
                end

• Here cast is of type parser.object (the alias of all parser.object.*), and the hover preview shows it has the extends field
• And I don't know why it is still complaining... so probably a bug in undefined-field check?

---

Is this something that is worth pursuing

First of all thanks again for your effort contributing to LuaLS, which is a great tool. ❤️ But to be honest, I doubt that this refactor is worth pursuing. Because as you may know 4.0.0 rewrite is on the way, and when it comes it will just replace this project's git history. So all your effort might be gone at that time ☹️ #2865 (comment)

[tomlau10]

Another warning type that I've seen is undefined-field, but I think it is a false positive case which is a bug (?) 😳
It is the one in script/vm/tracer.lua#L248:

I think I know why... the narrow types based on literal fields feature is doing more works than someone would have expected 😇

lua-language-server/script/vm/tracer.lua

Lines 232 to 252 in 6ba0c93

	for _, cast in ipairs(action.casts) do
	if cast.mode == '+' then
	if cast.optional then
	node:addOptional()
	end
	if cast.extends then
	node:merge(vm.compileNode(cast.extends))
	end
	elseif cast.mode == '-' then
	if cast.optional then
	node:removeOptional()
	end
	if cast.extends then
	node:removeNode(vm.compileNode(cast.extends))
	end
	else
	if cast.extends then
	node:clear()
	node:merge(vm.compileNode(cast.extends))
	end
	end

• in this for loop cast is original a parser.object, which is the union of all parser.object.*
• but then the if cast.mode == '+' on line 233 actually matches the paser.object.other's field mode
  => causing this if branch to infer cast as parser.object.other

---@class parser.object.other : parser.object.base
---...
---@field mode?                 '+' | '-'

• so in the else branch on line 248, cast actually becomes paser.object minus parser.object.other
  => now it doesn't have parser.object.other, so it is complaining that it cannot find the extends field ...

[lewis6991]

so in the else branch on line 248, cast actually becomes paser.object minus parser.object.other
=> now it doesn't have parser.object.other, so it is complaining that it cannot find the extends field ...

That looks like a bug. The narrow shouldn't narrow @field field lit1 | lit2, it should just narrow fields that a pure literals, to avoid this exact problem.

[lewis6991]

Because as you may know 4.0.0 rewrite is on the way, and when it comes it will just replace this project's git history.

Looks like you are right. The 4.0 branch already has separated AST classes: 54d6c28#diff-23e30c69ec9b9d4cbd4b4820a36c729e86f8621179a102abc1a30cad16334dcf

EDIT: fixed in #2871

[lewis6991]

Ok won't bother with this and will wait for 4.0.

I'll focus my contributions to things that add tests, since those can be ported forward.