Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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 方式了 🙂

@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
# for free to join this conversation on GitHub. Already have an account? # 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