Skip to content

fix(parser): 修复文档参数绑定的重复问题#3349

Open
ushen-pyj wants to merge 1 commit intoLuaLS:masterfrom
ushen-pyj:master
Open

fix(parser): 修复文档参数绑定的重复问题#3349
ushen-pyj wants to merge 1 commit intoLuaLS:masterfrom
ushen-pyj:master

Conversation

@ushen-pyj
Copy link
Contributor

我们为大部分函数代码使用了astHelper.addParamTypeDoc,大部分情况下是没问题的,但是少部分情况,我们想要手动写注释覆盖虚拟注释

在plugin运行时,我没有办法知道这里是否有注释,因为plugin运行发生在更早之前

所以目前如果plugin加了virtual comment, 手动加的---@param就无法生效

我想在bind结束后检查这一情况

大部分情况下这是一次O(n)的检查

新增 docsDedupe 函数,用于在绑定文档时检测并移除重复的参数文档。当同一参数存在多个文档时,优先保留非虚拟文档,确保生成的文档绑定唯一且准确。
@gemini-code-assist
Copy link

Summary of Changes

Hello @ushen-pyj, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

此拉取请求旨在解决 LuaDoc 解析器中参数文档绑定重复的问题。通过引入一个去重机制,确保手动编写的参数注释能够正确覆盖由插件生成的虚拟注释,从而提高文档的准确性和可控性。

Highlights

  • 修复文档参数绑定重复问题: 引入了一个新的去重逻辑,以解决当插件生成虚拟注释时,手动添加的 @param 注释无法生效的问题。现在,手动注释将优先于虚拟注释。

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • changelog.md
    • FIX Deduplicate documentation bindings for parameters
  • script/parser/luadoc.lua
    • 新增 docsDedupe 函数,用于处理参数文档的去重逻辑。
    • bindDocs 函数中调用 docsDedupe,以在绑定过程结束后应用去重规则。
    • 优化了参数文档的优先级,确保手动注释优先于虚拟注释。
Activity
  • 没有提供关于此拉取请求的任何评论、审查或活动信息。
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

此拉取请求旨在通过在绑定后删除重复的文档参数来解决问题。虽然方向是正确的,但 docsDedupe 函数中的当前实现存在一个错误:当两个参数文档具有相同的 virtual 状态(均为虚拟或均非虚拟)时,它不会删除重复项。这会导致重复的参数文档仍然存在。此外,该函数的性能在最坏情况下为 O(N^2),对于具有许多参数的函数可能会很慢。我提供了一个修复错误的建议,并指出可以考虑更高效的实现。

Comment on lines +2220 to +2225
if old.virtual and not doc.virtual then
removeByValue(source.bindDocs, old)
elseif not old.virtual and doc.virtual then
removeByValue(source.bindDocs, doc)
doc = old
end

Choose a reason for hiding this comment

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

high

old.virtualdoc.virtual 状态相同时(都为 true 或都为 false),当前逻辑不会移除任何一个重复的参数文档,这会导致重复的文档仍然存在。你应该处理这种情况,例如,保留文件中先出现的文档(索引较低的那个)。

此外,removeByValue 函数每次调用都会遍历 bindDocs,导致此去重逻辑在最坏情况下的时间复杂度为 O(N^2)。对于有很多参数的函数,这可能会成为性能瓶颈。

我建议修改逻辑以处理所有重复情况,并考虑一个更高效的 O(N) 实现来构建一个新的 bindDocs 列表,而不是在迭代时从中移除元素。

                        if old.virtual and not doc.virtual then
                            removeByValue(source.bindDocs, old)
                        elseif not old.virtual and doc.virtual then
                            removeByValue(source.bindDocs, doc)
                            doc = old
                        else -- both virtual or both not virtual, keep the one with lower index ('doc')
                            removeByValue(source.bindDocs, old)
                        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.

同时为virtual或者同时都不是virtual,这是人为写出来的,可以自己避免,正常使用不会这么写,给一个函数加两个一模一样的virtual,或者在一个函数写两个一模一样的@param注释。

最快的情况下O(N^2),通常是为每一个参数都加了虚拟注解,又重新写了每一个参数的真实注解,这种情况我认为也是不合理使用导致的,我们为某种规则的函数,根据规定用plugin定义了参数,说明是大部分都是正确的才会这样用,也就是大部分都不需要重写param

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.

1 participant