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

Fixed injects into child classes #2841

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Fixed injects into child classes #2841

merged 1 commit into from
Sep 10, 2024

Conversation

Luke100000
Copy link
Contributor

#2573

This change handles "cannot be injected" correctly if the field member is inherited.

---@class (exact) A
---@field test number
local a = {}

function a:init()
    --This is fine
    self.test = 0
end

---@class (exact) B : A
local b = {}

function b:init()
    --This is an injection and thus invalid
    self.test = 0

    --This is an actual injection and will remain invalid
    self.test2 = 0
end

Comment on lines +109 to +120
for _, src in ipairs(fields) do
if src.type == "setfield" then
local class = vm.getDefinedClass(uri, source)
if class then
for _, doc in ipairs(class:getSets(uri)) do
if vm.docHasAttr(doc, 'exact') then
return
end
end
end
end
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The forloop here seems can early break 🤔

  1. local class = vm.getDefinedClass(uri, source) doesn't depend on src, so it is a constant in this loop
  2. Now whether class is nil or not, it is unrelated to outer loop, and checking the whole logic once is enough:
    • if the class has exact attribute => early return
    • if the class is nil or has no exact attribute => early break
        for _, src in ipairs(fields) do
            if src.type == "setfield" then
                local class = vm.getDefinedClass(uri, source)
                if class then
                    for _, doc in ipairs(class:getSets(uri)) do
                        if vm.docHasAttr(doc, 'exact') then
                            return
                        end
                    end
                end
                break  <-- the break here, because the above logic only need to check once
            end
        end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, that's true, thanks. I will revisit this code (it's been a few months since I wrote that) and make a follow-up PR for that later.

@sumneko sumneko merged commit f0d98a2 into LuaLS:master Sep 10, 2024
10 of 11 checks passed
@sumneko
Copy link
Collaborator

sumneko commented Sep 10, 2024

Thank you!

@alex-courtis
Copy link

Thanks @Luke100000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants