-
Notifications
You must be signed in to change notification settings - Fork 0
Cannot resolve references from within argument-parameter elements #207
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
Cannot resolve references from within argument-parameter elements #207
Conversation
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 fixes reference resolution for elements nested within argument-parameter contexts by explicitly handling parameter-element classes and updates related test fixtures and DAO signatures.
- Enhanced SQL test files to cover
toString()calls and custom-number function calls - Introduced a new static helper method
getCustomNumberinProjectDetailand updated DAO signature to supplydetail - Extended Kotlin test expectations and added a check in
SqlPsiReferenceProviderforSqlElParameters
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| referenceStaticField.sql | Updated test SQL to include .toString() call and getCustomNumber usage |
| referenceEntityProperty.sql | Added conditional and loop variations for getCustomNumber resolution |
| ProjectDetail.java | Added new static getCustomNumber(String) method |
| ReferenceTestDao.java | Changed referenceStaticField to accept a ProjectDetail parameter |
| SqlReferenceTestCase.kt | Made test properties private, expanded expected mappings, and added debug logging |
| SqlPsiReferenceProvider.kt | Imported SqlElParameters and added a resolution case for parameter elements |
Comments suppressed due to low confidence (1)
src/test/testData/src/main/java/doma/example/dao/reference/ReferenceTestDao.java:17
- [nitpick] The new
detailparameter is not referenced in the associated SQL (referenceStaticField.sql), which may confuse maintainers. Either remove the unused parameter or update the SQL to use it.
ProjectDetail referenceStaticField(ProjectDetail detail);
src/test/testData/src/main/java/doma/example/entity/ProjectDetail.java
Outdated
Show resolved
Hide resolved
src/test/kotlin/org/domaframework/doma/intellij/reference/SqlReferenceTestCase.kt
Outdated
Show resolved
Hide resolved
0619ebb to
8058377
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 PR fixes a bug where elements nested inside argument-parameter blocks could not be reference-resolved by adding an explicit SqlElParameters check, and updates related tests and data to exercise the new logic.
- Add support for resolving EL ID expressions within argument-parameter elements in the reference provider
- Update SQL test scripts to cover
toString()calls and a newgetCustomNumbermethod - Extend the
ProjectDetailentity and adjust the DAO signature to support custom number logic
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/kotlin/org/domaframework/doma/intellij/reference/SqlPsiReferenceProvider.kt | Import SqlElParameters and add a branch to resolve ID expressions inside parameter elements |
| src/test/testData/src/main/resources/META-INF/doma/example/dao/reference/ReferenceTestDao/referenceStaticField.sql | Use .toString() on employeeId and add an OR clause with getCustomNumber |
| src/test/testData/src/main/resources/META-INF/doma/example/dao/reference/ReferenceTestDao/referenceEntityProperty.sql | Wrap custom-number expressions in an if directive and add nested calls in the loop |
| src/test/testData/src/main/java/doma/example/entity/ProjectDetail.java | Introduce projectName field and add getCustomNumber(String) method |
| src/test/testData/src/main/java/doma/example/dao/reference/ReferenceTestDao.java | Change referenceStaticField() to referenceStaticField(ProjectDetail detail) |
| src/test/kotlin/org/domaframework/doma/intellij/reference/SqlReferenceTestCase.kt | Update test mappings to include getCustomNumber, toString, isNotBlank, and field resolutions |
Comments suppressed due to low confidence (3)
src/test/kotlin/org/domaframework/doma/intellij/reference/SqlReferenceTestCase.kt:76
- [nitpick] The test mapping key
"null"is ambiguous; consider renaming it to clarify what scenario it covers (e.g.,emptyResolution).
"null" to listOf(null),
src/test/testData/src/main/java/doma/example/dao/reference/ReferenceTestDao.java:17
- The DAO method signature now includes a
detailparameter but the corresponding SQL template doesn’t reference it; consider removing the parameter or updating the SQL to use it.
ProjectDetail referenceStaticField(ProjectDetail detail);
src/main/kotlin/org/domaframework/doma/intellij/reference/SqlPsiReferenceProvider.kt:45
- [nitpick] Remove the blank line immediately after this branch to keep the
whenblock’s cases consistently formatted.
getParentClassPsiType(element, SqlElParameters::class.java) != null ->
src/test/testData/src/main/java/doma/example/entity/ProjectDetail.java
Outdated
Show resolved
Hide resolved
8058377 to
56a7892
Compare
Resolve a bug where elements nested inside argument-parameter elements could not be reference-resolved. In the branch that returns the reference-resolver class for parent elements, add an explicit check for the parameter-element class.