Merge pull request #454 from domaframework/fix/annotation-option-check-when-using-a-parent-property

Fix: Annotation Option Checks Respect Entity Inheritance

Author: xterao

.claude/guidelines/TEST_CASE_GUIDELINE.md

@@ -4,7 +4,7 @@ Follow the rules below when implementing test code.
 ## Code Inspection Functionality
 Implement tests for code inspection functionality using the following steps:
 
-### **Implementation of Test Case Classes**
+## Implementation of Test Case Classes
 - Implement test case classes in `@src/test/kotlin/org/domaframework/doma/intellij/inspection`.
 - Implement test cases for each subclass of **AbstractBaseJavaLocalInspectionTool**.
 - Name the test case class as `<InspectionToolName>InspectionTest`.
@@ -17,14 +17,31 @@ Implement tests for code inspection functionality using the following steps:
 Create the target Dao class or SQL file for inspection.
 Wrap the elements to be error-highlighted with the **<error>** tag and specify the error message to be displayed using the **descr** option.
 
-#### Test Data for Dao Classes
+**Test Data for Dao Classes**
 - Implement test data Dao classes in **Java**.
 - Annotate test data Dao classes with **@Dao**.
 - Place them under the appropriate subpackage in `@src/test/testData/src/main/java/doma/example/dao/inspection`.
 - Implement any other necessary classes as needed.
 
+**Entity Test Data**
+When creating test cases, prepare and verify the following Entity classes to test parent-child relationships:
+- Parent class annotated with @Entity
+- Subclass without @Entity annotation
+- Subclass annotated with @Entity
+
+Using the above, create the following test cases for features that use `PsiClass`:
+- Test cases using fields/methods defined in parent classes
+- Test cases using fields/methods defined in subclasses
+
+**Examples**
+- Test that bind variable definition inspection doesn't highlight errors when using fields defined in parent classes
+- Test that include/exclude option checking in DAO method annotations doesn't highlight errors when using fields defined in parent classes
+- Test that code completion shows fields/methods defined in parent classes as completion candidates
+
 #### Test Data for SQL Files
 - Create and place SQL files in a directory named after the corresponding Dao class under `@src/test/testData/src/main/resources/META-INF/doma/example/dao`.
+- When creating test cases for bind variables, prepare test data that checks instance field methods, static field methods, field methods of elements defined in loop directives, custom functions, and built-in functions.
+  For cases combining multiple variables, ensure comprehensive coverage of all variable combination patterns.
 
 ### Reference
 For actual implementation examples using Doma, refer to the [Doma GitHub repository](https://github.com/domaframework/doma/tree/master/integration-test-java/src/main/java/org/seasar/doma/it/dao).

CLAUDE.md

@@ -107,6 +107,19 @@ Feature Package
     └── AnAction subclass
 ```
 
+### Common
+
+**Accessing `PsiClass` Members**
+When retrieving fields and methods from `PsiClass`, use `allFields` and `allMethods` instead of `fields` and `methods`.
+This is necessary to include members defined in parent classes.
+
+Alternatively, you can use [PsiParentClass](src/main/kotlin/org/domaframework/doma/intellij/common/psi/PsiParentClass.kt)
+to achieve equivalent functionality with `findField()` and `findMethod()`.
+
+**Separating Complex Logic**
+Each feature requires implementing corresponding classes following the IntelliJ platform conventions.
+Complex logic should not be implemented directly in these corresponding classes but delegated to separate classes (Processors or Handlers).
+
 ### Actions
 Action functionality for navigating between DAO files and SQL files
 
@@ -171,6 +184,10 @@ Code inspection functionality for DAO methods and DOMA directives in SQL
   - [ValidationResult](src/main/kotlin/org/domaframework/doma/intellij/common/validation/result): Classes that provide error messages and highlights for code inspection results
     - **Class Naming Rules**: Use `ValidationResult` as a suffix. Name classes according to the message resources to be displayed
 
+**Coding Rule**
+For code inspection features, always implement `InspectionTool` and `Visitor` as separate classes.
+When the logic within `Visitor` becomes complex, implement separate Processor classes for main processing or Handler classes for error highlighting.
+
 ### Completion
 Code completion functionality for DOMA directive syntax in SQL
 - **[Feature Package](src/main/kotlin/org/domaframework/doma/intellij/contributor/sql)**

src/main/kotlin/org/domaframework/doma/intellij/common/psi/PsiPatternUtil.kt

@@ -157,7 +157,7 @@ object PsiPatternUtil {
         targetType: IElementType?,
     ): MutableList<PsiElement> {
         var prevElement = PsiTreeUtil.prevLeaf(element, true)
-        var prevElements = mutableListOf<PsiElement>()
+        val prevElements = mutableListOf<PsiElement>()
         while (prevElement != null &&
             prevElement !is PsiWhiteSpace &&
             prevElement.elementType != targetType &&

src/main/kotlin/org/domaframework/doma/intellij/common/util/TypeUtil.kt

@@ -89,8 +89,7 @@ object TypeUtil {
             returnTypeClass?.getClassAnnotation(DomaClassName.ENTITY.className) ?: return false
         return entity.let { entity ->
             AnnotationUtil.getBooleanAttributeValue(entity, "immutable") == true
-        } == true ||
-            returnTypeClass.isRecord == true
+        } || returnTypeClass.isRecord
     }
 
     /**

src/main/kotlin/org/domaframework/doma/intellij/inspection/dao/processor/option/DaoAnnotationOptionParameterCheckProcessor.kt

@@ -109,14 +109,12 @@ class DaoAnnotationOptionParameterCheckProcessor(
         arrayValues.map { fields ->
             val valueFields = fields.text.replace("\"", "").split(".")
             var searchParamType: PsiType = entityClass.psiClassType
+            var prevFieldVal = valueFields.first()
             var searchParamClass: PsiClass? = project.getJavaClazz(searchParamType)
-            var hasError = false
 
-            valueFields.forEachIndexed { index, field ->
-                val currentField =
-                    searchParamClass
-                        ?.fields
-                        ?.find { property -> isOptionTargetProperty(property, field, project) }
+            valueFields.forEachIndexed { _, field ->
+                // Error when specifying a property not defined in the Entity or the Embeddable.
+                val currentField = getMatchFields(searchParamClass).find { f -> isOptionTargetProperty(f, field, project) }
                 // Given that the first `searchParamType` is assumed to contain the type of  Entity class,
                 // checking the index for a primitive type is unnecessary.
                 if (searchParamType is PsiPrimitiveType) {
@@ -125,13 +123,13 @@ class DaoAnnotationOptionParameterCheckProcessor(
                         fields,
                         shortName,
                         fields.text.replace("\"", ""),
-                        field,
+                        prevFieldVal,
                         optionName,
                     ).highlightElement(holder)
-                    hasError = true
                     return@map
                 } else {
                     if (currentField != null) {
+                        prevFieldVal = currentField.nameIdentifier.text
                         searchParamType = currentField.type
                         searchParamClass = project.getJavaClazz(searchParamType)
                     } else {
@@ -143,7 +141,6 @@ class DaoAnnotationOptionParameterCheckProcessor(
                             searchParamClass?.name ?: "Unknown",
                             getTargetOptionProperties(searchParamClass),
                         ).highlightElement(holder)
-                        hasError = true
                         return@map
                     }
                 }
@@ -171,16 +168,28 @@ class DaoAnnotationOptionParameterCheckProcessor(
                 .filter { it is PsiLiteralExpression }
         }
 
+    private fun getMatchFields(paramClass: PsiClass?): List<PsiField> =
+        paramClass?.allFields?.filter { f ->
+            isValidMatchField(f)
+        } ?: emptyList()
+
+    /**
+     * Helper method to encapsulate field validation logic for getMatchFields.
+     */
+    private fun isValidMatchField(f: PsiField): Boolean {
+        val parentClass = f.parent as? PsiClass
+        val isEntityOrEmbeddable = parentClass?.isEntity() == true || parentClass?.isEmbeddable() == true
+        val isBaseOrEmbeddableType = TypeUtil.isBaseOrOptionalWrapper(f.type) || TypeUtil.isEmbeddable(f.type, project)
+        return isEntityOrEmbeddable && isBaseOrEmbeddableType
+    }
+
     private fun getTargetOptionProperties(paramClass: PsiClass?) =
-        paramClass?.fields?.filter { isOptionTargetProperty(it, it.name, project) }?.joinToString(", ") { it.name.substringAfter(":") }
-            ?: "No fields found"
+        getMatchFields(paramClass).joinToString(", ") { it.name.substringAfter(":") }
 
-    private fun getEmbeddableProperties(embeddableClass: PsiClass?) =
-        embeddableClass
-            ?.fields
-            ?.filter { !TypeUtil.isEntity(it.type, project) && !TypeUtil.isEmbeddable(it.type, project) }
-            ?.joinToString(", ") { it.name }
-            ?: "No properties found"
+    /**
+     * If the last field access is Embeddable, get its property list
+     */
+    private fun getEmbeddableProperties(embeddableClass: PsiClass?) = getMatchFields(embeddableClass).joinToString(", ") { it.name }
 
     private fun isOptionTargetProperty(
         field: PsiField,
@@ -189,7 +198,7 @@ class DaoAnnotationOptionParameterCheckProcessor(
     ): Boolean =
         (
             field.name == optionPropertyName && (
-                !TypeUtil.isEntity(field.type, project) ||
+                TypeUtil.isBaseOrOptionalWrapper(field.type) ||
                     TypeUtil.isEmbeddable(field.type, project)
             )
         )

src/test/kotlin/org/domaframework/doma/intellij/inspection/dao/AnnotationOptionParameterInspectionTest.kt

@@ -0,0 +1,48 @@
+/*
+ * Copyright Doma Tools Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.domaframework.doma.intellij.inspection.dao
+
+import org.domaframework.doma.intellij.DomaSqlTest
+import kotlin.test.Ignore
+
+/**
+ * Test class for annotation option parameter inspection.
+ * Tests include/exclude options with parent class properties.
+ */
+@Ignore
+class AnnotationOptionParameterInspectionTest : DomaSqlTest() {
+    /**
+     * Since error highlight tags for annotation options cannot be set in the test data, verify manually.
+     * There are no automated test cases, so perform manual checks using the following as a reference.
+     *
+     * Here is an updated summary of the test case coverage based on the revised method documentation. This can be used as a test case overview document.
+     * Relation: [AnnotationOptionTestInValidDao]
+     * - Error check when specifying fields not defined in the parameter type with `include` option.
+     * - Error check when specifying fields not defined in the parameter type with `exclude` option.
+     * - Error check for specifying fields not defined in immutable Entity with `MultiInsert` (also for fields not defined in parameter type).
+     * - Error check for specifying fields not defined in mutable Entity with `MultiInsert`.
+     * - Error check for specifying fields not defined in the parameter type with batch annotations.
+     * - Error when ending with an embedded property.
+     * - Error when specifying incorrect properties in an embedded class.
+     * - Error check for invalid field specification in `Returning` option.
+     * - Error check for invalid field specification in `Returning` option for mutable Entity.
+     * - Error check for specifying fields not defined in embedded property.
+     * - Error when specifying further properties from a primitive type.
+     * - Error check for specifying parent class properties in subclass with `@Entity`.
+     * - Error check for specifying parent class properties in subclass without `@Entity`.
+     * - Error check for specifying fields from a parent class that is not an Entity.
+     */
+}