-
Notifications
You must be signed in to change notification settings - Fork 0
Fix code completion for static field access #226
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 code completion for static field access #226
Conversation
…ctive symbol handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances SQL code-completion in the Doma IntelliJ plugin by improving static field-access detection and parsing, and adds test coverage for these scenarios.
- Added new SQL test fixtures and DAO methods to cover static field completion before
@and after other elements - Refactored
SqlParameterCompletionProviderandStaticDirectiveHandlerto improve static field detection, unify comment parsing viaStringUtil, and centralize directive symbols - Extracted directive symbols into
DirectiveCompletionand introduced suffix constants inForDirectiveUtilfor cleaner for-directive handling
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/.../completeTopElementBeforeAtsign.sql | New test fixture for completion immediately before @ |
| src/test/.../completeStaticPropertyAfterOtherElement.sql | New test fixture for completion after another bind element |
| src/test/.../completePropertyAfterStaticPropertyCallWithMethodParameter.sql | Adjusted test fixture for static property call removal of suffix |
| SqlCompleteTestDao.java | Added DAO methods to drive the new completion tests |
| SqlCompleteTest.kt | Registered new SQL test files and updated expected suggestion lists |
| SqlParameterCompletionProvider.kt | Unified block-comment parsing, updated element filtering, and adjusted static-access logic |
| StaticDirectiveHandler.kt | Refined detection for class-name vs. static-field completions |
| StringUtil.kt | Introduced utility for block-comment stripping and class-name extraction |
| ForDirectiveUtil.kt | Added HAS_NEXT_PREFIX and INDEX_PREFIX constants |
| DirectiveCompletion.kt | Centralized directiveSymbols list |
| CleanElementText.kt | Delegated comment parsing to StringUtil |
| PsiStaticElement.kt | Switched to StringUtil.getSqlElClassText for FQCN parsing |
| PsiPatternUtil.kt | Updated to use DirectiveCompletion.directiveSymbols and improved comment matching logic |
Comments suppressed due to low confidence (2)
src/main/kotlin/org/domaframework/doma/intellij/contributor/sql/provider/SqlParameterCompletionProvider.kt:345
- [nitpick] The function name
getElementTypeByPrevSqlElClassWordsis quite long and may be hard to read. Consider renaming to something more concise likegetStaticFieldTypeFromFqdn.
private fun getElementTypeByPrevSqlElClassWords(
src/main/kotlin/org/domaframework/doma/intellij/common/util/StringUtil.kt:19
- Utility functions
getSqlElClassTextandreplaceBlockCommentStartEndare critical for parsing but have no dedicated unit tests. Consider adding tests to validate edge cases (e.g. missing delimiters, multiple@).
fun getSqlElClassText(text: String): String =
src/main/kotlin/org/domaframework/doma/intellij/common/sql/directive/StaticDirectiveHandler.kt
Outdated
Show resolved
Hide resolved
...n/org/domaframework/doma/intellij/contributor/sql/provider/SqlParameterCompletionProvider.kt
Outdated
Show resolved
Hide resolved
...n/org/domaframework/doma/intellij/contributor/sql/provider/SqlParameterCompletionProvider.kt
Show resolved
Hide resolved
44b2a80 to
690ad4e
Compare
690ad4e to
4cfd2a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request refines code completion for static field access by narrowing the target elements during completion and ensuring that class-name suggestions are not mistakenly triggered. Key changes include adjustments to the SQL test files to support new completion scenarios, updates to the provider logic in Kotlin for improved element filtering and handling, and minor refactoring across utility and directive classes.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| completeTopElementBeforeAtsign.sql | Added a test case for top element completion before an at-sign. |
| completeStaticPropertyAfterOtherElement.sql | Added a test case to trigger static property completion after another element. |
| completePropertyAfterStaticPropertyCallWithMethodParameter.sql | Updated completion test ensuring consistency after a static method call. |
| SqlCompleteTestDao.java | Added new DAO methods for testing the static field access scenarios. |
| SqlCompleteTest.kt | Extended the test suite to cover the new and refactored completion cases. |
| SqlParameterCompletionProvider.kt | Refactored filtering and processing of completion candidates with additional parameter handling. |
| StringUtil.kt, ForDirectiveUtil.kt, StaticDirectiveHandler.kt, DirectiveCompletion.kt, CleanElementText.kt, PsiStaticElement.kt, PsiPatternUtil.kt | Minor refactoring and usage of constants to improve code clarity and maintainability. |
Comments suppressed due to low confidence (1)
src/main/kotlin/org/domaframework/doma/intellij/common/sql/directive/StaticDirectiveHandler.kt:75
- [nitpick] Consider adding a comment explaining the regex pattern's intent for class package validation to aid future maintenance.
val elClassPattern = "^([a-zA-Z]*(\.)+)*$"
When identifying code-completion targets, ignore argument-parameter elements and retrieve only the preceding element list.
Ensure that even if a field-access element isn’t fully constructed mid-typing, string-based parsing still triggers static field-access code completion.