feat: update incomplete-signature-doc to also error on non-global functions#975
feat: update incomplete-signature-doc to also error on non-global functions#975dquixote-jr wants to merge 3 commits intoEmmyLuaLs:mainfrom
Conversation
Summary of ChangesHello, 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! This pull request enhances the code analysis capabilities by extending the Highlights
🧠 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
Using Gemini Code AssistThe 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
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 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
|
There was a problem hiding this comment.
Code Review
This pull request extends the incomplete-signature-doc check to also report missing documentation for non-global functions. The implementation in incomplete_signature_doc.rs appears correct, and the refactoring to move the code variable definition is a good improvement. However, I've found a couple of issues in the accompanying tests. Some test cases seem to have inverted assertions, incorrectly asserting that a diagnostic is not found when it should be. Please see my detailed comments on the test file.
crates/emmylua_code_analysis/src/diagnostic/test/incomplete_signature_doc_test.rs
Show resolved
Hide resolved
|
Please fix the check |
|
done |
|
@CppCXY 需要考虑对闭包跟一些简单函数的影响,先别合 |
|
If we want to extend the diagnosis to be effective for non-global variables, then we need to handle some simple cases. local function a() -- No function parameters should trigger a warning.
end
function b(a) -- issue warning
end
local c = function(a) -- A warning should be issued.
end |
|
Added tests for the first 2, part 3 is broken right now as well, it doesn't work for global functions either, e.g. for the following: Type checker does not issue any warnings. So I'm not sure if fixing that is supposed to be part of this change. Should be a different PR. |
| ws.enable_full_diagnostic(); | ||
|
|
||
| // TODO: closures are not detected as functions in the semantic model | ||
| // assert!(!ws.check_code_for( |
There was a problem hiding this comment.
If you don't know how to handle it, you can ask AI for help.
There was a problem hiding this comment.
Does it have to be this PR?
Closures are not detected for global functions either. So I'm not sure if fixing that needs to be done right now.
The functionality that I am adding is unrelated to fixing the function detection.
relates - #964