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

fix: bind variables with @class using tail comments #3044

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

tomlau10
Copy link
Contributor

fixes #2673

這個是我上年最初接觸和使用 luals 時遇到的一個問題
當時由於找到繞過方式用 @type,也就解決了

但是最近自某版本起 @type 修正成會檢查該 class 的所有 missing field 而導致報錯。。。

  • 要麼用 disable 方式忽略部分報錯
  • 要麼改回用 @class

後者好像合理一些
換句話還是得解決最根本的問題:
連續多行用 tail comment 寫 @class 時,無法正確 bind 到 variable

最初由於不了解 luals 代碼庫,所以也沒有深究原因
現在算是稍為熟悉一點了,很快找到 fix 方式了 🙂

changelog.md Outdated Show resolved Hide resolved
script/parser/luadoc.lua Outdated Show resolved Hide resolved
@tomlau10 tomlau10 force-pushed the fix/bind_class_with_tail_comment branch from 8d982f6 to 762850d Compare January 17, 2025 01:26
@tomlau10
Copy link
Contributor Author

tomlau10 commented Jan 17, 2025

should be fixed now @C3pa


But I just found some edge cases...

  • my code just specifically check that if a tail comment @class is followed by another @class, then the 1st one should be binded to the left hand side variable
  • however if the next line is not @class, then the binding will be incorrect 🤦‍♂️

in the following example:

local ClassA ---@class A
local strVar ---@type string
  • the @class A will incorrectly bind to strVar
  • but what I expect is to bind with the ClassA variable
  • this problem persists without my PR though

I am thinking we have to come up with a fix to resolve it once and for all 🤔
I think the root cause is that when checking tail comment, currently it will just skip @class and @field here:

elseif isTailComment(text, doc) and doc.type ~= "doc.class" and doc.type ~= "doc.field" then
bindDocWithSources(sources, binded)
binded = nil

  • removing doc.class in this condition fixes both mine and the newly identified issue
  • i.e. if @class is inside a tail comment, always bind to the left hand side variable
  • but I am not sure if it would cause other side effects (I didn't found any in my 2 projects)

What do you think? 🤔
With the new suggested change, then we don't even need to add the elseif case below.

@tomlau10 tomlau10 marked this pull request as draft January 17, 2025 01:44
@tomlau10 tomlau10 force-pushed the fix/bind_class_with_tail_comment branch from 762850d to 3ce90e0 Compare January 19, 2025 13:50
@tomlau10
Copy link
Contributor Author

hi @emmericp, sorry to bother you

I'm trying to fix the issue mentioned in #2673, where continuous @class tail comment annotation is not working.
(see examples in the issue)

And after git blaming, I found that it is caused by the change in your PR #2502:
https://github.com/LuaLS/lua-language-server/pull/2502/files#diff-1a85305731532da4bf3f750c6ddc3e48793c76b8544c75d04708f673fcd4b3a8R2008

I'm not sure why @class is excluded if it is in tail comment when processing the bind doc to variable logic 😕
So I don't know if the change in this PR would break your original use case.
Would you mind having a review in this PR?

Thanks

@tomlau10 tomlau10 marked this pull request as ready for review January 20, 2025 12:14
@sumneko sumneko merged commit be499af into LuaLS:master Jan 21, 2025
11 checks passed
@emmericp
Copy link
Contributor

I'm not sure why @Class is excluded if it is in tail comment when processing the bind doc to variable logic 😕

To be honest: I don't remember the reason behind that logic

@tomlau10 tomlau10 deleted the fix/bind_class_with_tail_comment branch January 23, 2025 12:12
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.

inline @class does not work properly without empty line in between
4 participants