chore: enforce consistency between variable getters and setters

core/src/main/java/ai/timefold/solver/core/impl/domain/common/ReflectionHelper.java

@@ -136,7 +136,7 @@ public static Method getDeclaredGetterMethod(Class<?> containingClass, String pr
         var baseClass = containingClass;
         while (baseClass != null) {
             try {
-                return containingClass.getDeclaredMethod(getterName);
+                return baseClass.getDeclaredMethod(getterName);
             } catch (NoSuchMethodException e) {
                 baseClass = baseClass.getSuperclass();
             }
@@ -158,12 +158,12 @@ public static Method getDeclaredGetterMethod(Class<?> containingClass, String pr
      * @param methodName never null
      * @return sometimes null
      */
-    public static Method getDeclaredMethod(Class<?> containingClass, String methodName) {
+    public static Method getDeclaredMethod(Class<?> containingClass, String methodName, Class<?>... parameterTypes) {
         var baseClass = containingClass;
 
         while (baseClass != null) {
             try {
-                return containingClass.getDeclaredMethod(methodName);
+                return baseClass.getDeclaredMethod(methodName, parameterTypes);
             } catch (NoSuchMethodException e) {
                 baseClass = baseClass.getSuperclass();
             }
@@ -233,6 +233,17 @@ public static Method getSetterMethod(Class<?> containingClass, Class<?> property
         }
     }
 
+    /**
+     * @param containingClass never null
+     * @param propertyType never null
+     * @param propertyName never null
+     * @return null if it doesn't exist
+     */
+    public static Method getDeclaredSetterMethod(Class<?> containingClass, Class<?> propertyType, String propertyName) {
+        String setterName = PROPERTY_MUTATOR_PREFIX + capitalizePropertyName(propertyName);
+        return getDeclaredMethod(containingClass, setterName, propertyType);
+    }
+
     /**
      * @param containingClass never null
      * @param propertyName never null

core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessor.java

@@ -4,7 +4,9 @@
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.lang.reflect.Type;
+import java.util.function.IntPredicate;
 
 import ai.timefold.solver.core.impl.domain.common.ReflectionHelper;
 
@@ -34,38 +36,73 @@ public ReflectionBeanPropertyMemberAccessor(Method getterMethod, boolean getterO
         } catch (IllegalAccessException e) {
             throw new IllegalStateException("""
                     Impossible state: method (%s) not accessible.
-                    %s
-                    """
-                    .strip()
+                    %s"""
                     .formatted(getterMethod, MemberAccessorFactory.CLASSLOADER_NUDGE_MESSAGE), e);
         }
         Class<?> declaringClass = getterMethod.getDeclaringClass();
         if (!ReflectionHelper.isGetterMethod(getterMethod)) {
-            throw new IllegalArgumentException("The getterMethod (" + getterMethod + ") is not a valid getter.");
+            throw new IllegalArgumentException("The getterMethod (%s) is not a valid getter."
+                    .formatted(getterMethod));
         }
         propertyType = getterMethod.getReturnType();
         propertyName = ReflectionHelper.getGetterPropertyName(getterMethod);
         if (getterOnly) {
             setterMethod = null;
             setterMethodHandle = null;
         } else {
-            setterMethod = ReflectionHelper.getSetterMethod(declaringClass, getterMethod.getReturnType(), propertyName);
-            if (setterMethod != null) {
-                try {
-                    setterMethod.setAccessible(true); // Performance hack by avoiding security checks
-                    this.setterMethodHandle = lookup.unreflect(setterMethod)
-                            .asFixedArity();
-                } catch (IllegalAccessException e) {
-                    throw new IllegalStateException("""
-                            Impossible state: method (%s) not accessible.
-                            %s
-                            """
-                            .strip()
-                            .formatted(setterMethod, MemberAccessorFactory.CLASSLOADER_NUDGE_MESSAGE), e);
+            setterMethod = ReflectionHelper.getDeclaredSetterMethod(declaringClass, getterMethod.getReturnType(), propertyName);
+            if (setterMethod == null) {
+                throw new IllegalArgumentException("The getterMethod (%s) does not have a matching setterMethod on class (%s)."
+                        .formatted(getterMethod.getName(), declaringClass.getCanonicalName()));
+            }
+            var getterAccess = AccessModifier.forMethod(getterMethod);
+            var setterAccess = AccessModifier.forMethod(setterMethod);
+            if (getterAccess != setterAccess) {
+                throw new IllegalArgumentException(
+                        "The getterMethod (%s) has access modifier (%s) which does not match the setterMethod (%s) access modifier (%s) on class (%s)."
+                                .formatted(getterMethod.getName(), getterAccess, setterMethod.getName(), setterAccess,
+                                        declaringClass.getCanonicalName()));
+            }
+            try {
+                setterMethod.setAccessible(true); // Performance hack by avoiding security checks
+                this.setterMethodHandle = lookup.unreflect(setterMethod)
+                        .asFixedArity();
+            } catch (IllegalAccessException e) {
+                throw new IllegalStateException("""
+                        Impossible state: method (%s) not accessible.
+                        %s"""
+                        .formatted(setterMethod, MemberAccessorFactory.CLASSLOADER_NUDGE_MESSAGE), e);
+            }
+        }
+    }
+
+    private enum AccessModifier {
+        PUBLIC("public", Modifier::isPublic),
+        PROTECTED("protected", Modifier::isProtected),
+        PACKAGE_PRIVATE("package-private", modifier -> false),
+        PRIVATE("private", Modifier::isPrivate);
+
+        final String name;
+        final IntPredicate predicate;
+
+        AccessModifier(String name, IntPredicate predicate) {
+            this.name = name;
+            this.predicate = predicate;
+        }
+
+        public static AccessModifier forMethod(Method method) {
+            var modifiers = method.getModifiers();
+            for (var accessModifier : AccessModifier.values()) {
+                if (accessModifier.predicate.test(modifiers)) {
+                    return accessModifier;
                 }
-            } else {
-                setterMethodHandle = null;
             }
+            return PACKAGE_PRIVATE;
+        }
+
+        @Override
+        public String toString() {
+            return name;
         }
     }
 

core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java

@@ -1,6 +1,5 @@
 package ai.timefold.solver.core.impl.domain.entity.descriptor;
 
-import static ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory.MemberAccessorType.FIELD_OR_GETTER_METHOD;
 import static ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory.MemberAccessorType.FIELD_OR_GETTER_METHOD_WITH_SETTER;
 import static ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory.MemberAccessorType.FIELD_OR_READ_METHOD;
 import static ai.timefold.solver.core.impl.domain.entity.descriptor.EntityDescriptorValidator.assertNotMixedInheritance;
@@ -46,7 +45,6 @@
 import ai.timefold.solver.core.config.util.ConfigUtils;
 import ai.timefold.solver.core.impl.domain.common.ReflectionHelper;
 import ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessor;
-import ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory;
 import ai.timefold.solver.core.impl.domain.policy.DescriptorPolicy;
 import ai.timefold.solver.core.impl.domain.solution.descriptor.ProblemScaleTracker;
 import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor;
@@ -323,44 +321,27 @@ private void processValueRangeProviderAnnotation(DescriptorPolicy descriptorPoli
         }
     }
 
+    @SuppressWarnings("unchecked")
     private void processPlanningVariableAnnotation(MutableInt variableDescriptorCounter, DescriptorPolicy descriptorPolicy,
             Member member) {
-        var variableAnnotationClass = ConfigUtils.extractAnnotationClass(
-                member, VARIABLE_ANNOTATION_CLASSES);
+        var variableAnnotationClass = ConfigUtils.extractAnnotationClass(member, VARIABLE_ANNOTATION_CLASSES);
         if (variableAnnotationClass != null) {
-            MemberAccessorFactory.MemberAccessorType memberAccessorType;
-            if (variableAnnotationClass.equals(ShadowVariable.class)) {
-                // Need to check only the single annotation version,
-                // since supplier variable can only be used with a single
-                // annotation.
-                ShadowVariable annotation;
-                if (member instanceof Field field) {
-                    annotation = field.getAnnotation(ShadowVariable.class);
-                } else if (member instanceof Method method) {
-                    annotation = method.getAnnotation(ShadowVariable.class);
-                } else {
-                    throw new IllegalStateException(
-                            "Member must be a field or a method, but was (%s).".formatted(member.getClass().getSimpleName()));
-                }
-                if (annotation == null) {
-                    throw new IllegalStateException("Impossible state: cannot get %s annotation on a %s annotated member (%s)."
-                            .formatted(ShadowVariable.class.getSimpleName(), ShadowVariable.class.getSimpleName(), member));
-                }
-                if (!annotation.supplierName().isEmpty()) {
-                    memberAccessorType = FIELD_OR_GETTER_METHOD_WITH_SETTER;
-                } else {
-                    memberAccessorType = FIELD_OR_GETTER_METHOD;
-                }
-            } else if (variableAnnotationClass.equals(CustomShadowVariable.class)
-                    || variableAnnotationClass.equals(ShadowVariable.List.class)
-                    || variableAnnotationClass.equals(PiggybackShadowVariable.class)
-                    || variableAnnotationClass.equals(CascadingUpdateShadowVariable.class)) {
-                memberAccessorType = FIELD_OR_GETTER_METHOD;
+            Object annotation;
+            if (member instanceof Field field) {
+                annotation = field.getAnnotation(variableAnnotationClass);
+            } else if (member instanceof Method method) {
+                annotation = method.getAnnotation(variableAnnotationClass);
             } else {
-                memberAccessorType = FIELD_OR_GETTER_METHOD_WITH_SETTER;
+                throw new IllegalStateException("Member must be a field or a method, but was (%s)."
+                        .formatted(member.getClass().getSimpleName()));
             }
-            var memberAccessor = descriptorPolicy.getMemberAccessorFactory().buildAndCacheMemberAccessor(member,
-                    memberAccessorType, variableAnnotationClass, descriptorPolicy.getDomainAccessType());
+            if (annotation == null) {
+                throw new IllegalStateException("Impossible state: cannot get annotation on a %s-annotated member (%s)."
+                        .formatted(variableAnnotationClass, member));
+            }
+            var memberAccessor = descriptorPolicy.getMemberAccessorFactory()
+                    .buildAndCacheMemberAccessor(member, FIELD_OR_GETTER_METHOD_WITH_SETTER, variableAnnotationClass,
+                            descriptorPolicy.getDomainAccessType());
             registerVariableAccessor(variableDescriptorCounter.intValue(), variableAnnotationClass, memberAccessor);
             variableDescriptorCounter.increment();
         }
@@ -695,10 +676,6 @@ public GenuineVariableDescriptor<Solution_> getGenuineVariableDescriptor(String
         return shadowVariableLoopedDescriptor;
     }
 
-    public boolean hasAnyGenuineVariables() {
-        return !effectiveGenuineVariableDescriptorMap.isEmpty();
-    }
-
     public boolean hasBothGenuineListAndBasicVariables() {
         if (!isGenuine()) {
             return false;
@@ -732,7 +709,7 @@ public boolean hasAnyGenuineListVariables() {
     }
 
     public boolean isGenuine() {
-        return hasAnyGenuineVariables();
+        return !effectiveGenuineVariableDescriptorMap.isEmpty();
     }
 
     public ListVariableDescriptor<Solution_> getGenuineListVariableDescriptor() {

core/src/main/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetConstraintFactory.java

@@ -135,7 +135,7 @@ public <Stream_ extends BavetAbstractConstraintStream<Solution_>> Stream_ share(
     public @NonNull <A> UniConstraintStream<A> from(@NonNull Class<A> fromClass) {
         assertValidFromType(fromClass);
         var entityDescriptor = solutionDescriptor.findEntityDescriptor(fromClass);
-        if (entityDescriptor != null && entityDescriptor.hasAnyGenuineVariables()) {
+        if (entityDescriptor != null && entityDescriptor.isGenuine()) {
             var predicate = (Predicate<A>) entityDescriptor.getIsInitializedPredicate();
             return share(new BavetForEachUniConstraintStream<>(this, fromClass, predicate, RetrievalSemantics.LEGACY));
         } else {

core/src/test/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessorTest.java

@@ -1,10 +1,13 @@
 package ai.timefold.solver.core.impl.domain.common.accessor;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
 
 import ai.timefold.solver.core.api.domain.variable.PlanningVariable;
 import ai.timefold.solver.core.testdomain.TestdataEntity;
 import ai.timefold.solver.core.testdomain.TestdataValue;
+import ai.timefold.solver.core.testdomain.invalid.gettersetter.TestdataDifferentGetterSetterVisibilityEntity;
+import ai.timefold.solver.core.testdomain.invalid.gettersetter.TestdataInvalidGetterEntity;
 
 import org.junit.jupiter.api.Test;
 
@@ -26,4 +29,31 @@ void methodAnnotatedEntity() throws NoSuchMethodException {
         assertThat(e1.getValue()).isSameAs(v2);
     }
 
+    @Test
+    void getterSetterVisibilityDoesNotMatch() {
+        assertThatCode(() -> new ReflectionBeanPropertyMemberAccessor(
+                TestdataDifferentGetterSetterVisibilityEntity.class.getDeclaredMethod("getValue1")))
+                .hasMessageContainingAll("getterMethod (getValue1)",
+                        "has access modifier (public)",
+                        "not match the setterMethod (setValue1)",
+                        "access modifier (private)",
+                        "on class (ai.timefold.solver.core.testdomain.invalid.gettersetter.TestdataDifferentGetterSetterVisibilityEntity)");
+        assertThatCode(() -> new ReflectionBeanPropertyMemberAccessor(
+                TestdataDifferentGetterSetterVisibilityEntity.class.getDeclaredMethod("getValue2")))
+                .hasMessageContainingAll("getterMethod (getValue2)",
+                        "has access modifier (package-private)",
+                        "not match the setterMethod (setValue2)",
+                        "access modifier (protected)",
+                        "on class (ai.timefold.solver.core.testdomain.invalid.gettersetter.TestdataDifferentGetterSetterVisibilityEntity)");
+    }
+
+    @Test
+    void setterMissing() {
+        assertThatCode(() -> new ReflectionBeanPropertyMemberAccessor(
+                TestdataInvalidGetterEntity.class.getDeclaredMethod("getValueWithoutSetter")))
+                .hasMessageContainingAll("getterMethod (getValueWithoutSetter)",
+                        "does not have a matching setterMethod",
+                        "on class (ai.timefold.solver.core.testdomain.invalid.gettersetter.TestdataInvalidGetterEntity)");
+    }
+
 }

core/src/test/java/ai/timefold/solver/core/testdomain/invalid/gettersetter/TestdataDifferentGetterSetterVisibilityEntity.java

@@ -0,0 +1,32 @@
+package ai.timefold.solver.core.testdomain.invalid.gettersetter;
+
+import ai.timefold.solver.core.api.domain.entity.PlanningEntity;
+import ai.timefold.solver.core.api.domain.variable.PlanningVariable;
+import ai.timefold.solver.core.testdomain.TestdataValue;
+
+@PlanningEntity
+public class TestdataDifferentGetterSetterVisibilityEntity {
+
+    private TestdataValue value1;
+    private TestdataValue value2;
+
+    @PlanningVariable
+    public TestdataValue getValue1() {
+        return value1;
+    }
+
+    private void setValue1(TestdataValue value1) {
+        this.value1 = value1;
+    }
+
+    @PlanningVariable
+    TestdataValue getValue2() {
+        return value2;
+    }
+
+    @PlanningVariable
+    protected TestdataValue setValue2(TestdataValue value2) {
+        return value2;
+    }
+
+}

core/src/test/java/ai/timefold/solver/core/testdomain/invalid/gettersetter/TestdataInvalidGetterEntity.java

@@ -0,0 +1,17 @@
+package ai.timefold.solver.core.testdomain.invalid.gettersetter;
+
+import ai.timefold.solver.core.api.domain.entity.PlanningEntity;
+import ai.timefold.solver.core.api.domain.variable.PlanningVariable;
+import ai.timefold.solver.core.testdomain.TestdataValue;
+
+@PlanningEntity
+public class TestdataInvalidGetterEntity {
+
+    @PlanningVariable
+    private TestdataValue valueWithoutSetter;
+
+    public TestdataValue getValueWithoutSetter() {
+        return valueWithoutSetter;
+    }
+
+}

core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableBigDecimalScoreSolution.java

@@ -75,7 +75,7 @@ public void setEntityList(List<TestdataEntity> entityList) {
     }
 
     @PlanningScore(bendableHardLevelsSize = 1, bendableSoftLevelsSize = 2)
-    BendableBigDecimalScore getScore() {
+    public BendableBigDecimalScore getScore() {
         return score;
     }
 

core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableLongScoreSolution.java

@@ -75,7 +75,7 @@ public void setEntityList(List<TestdataEntity> entityList) {
     }
 
     @PlanningScore(bendableHardLevelsSize = 1, bendableSoftLevelsSize = 2)
-    BendableLongScore getScore() {
+    public BendableLongScore getScore() {
         return score;
     }
 

core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataBendableScoreSolution.java

@@ -75,7 +75,7 @@ public void setEntityList(List<TestdataEntity> entityList) {
     }
 
     @PlanningScore(bendableHardLevelsSize = 1, bendableSoftLevelsSize = 2)
-    BendableScore getScore() {
+    public BendableScore getScore() {
         return score;
     }
 

core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftBigDecimalScoreSolution.java

@@ -77,7 +77,7 @@ public void setEntityList(List<TestdataEntity> entityList) {
     }
 
     @PlanningScore
-    HardMediumSoftBigDecimalScore getScore() {
+    public HardMediumSoftBigDecimalScore getScore() {
         return score;
     }
 

core/src/test/java/ai/timefold/solver/core/testdomain/score/TestdataHardMediumSoftLongScoreSolution.java

@@ -75,7 +75,7 @@ public void setEntityList(List<TestdataEntity> entityList) {
     }
 
     @PlanningScore
-    HardMediumSoftLongScore getScore() {
+    public HardMediumSoftLongScore getScore() {
         return score;
     }