Address additional errors found in native rules implementation (#533)

Issues resolved:

- a protovalidate rule on a repeated google.protobuf.Int32Value caused a
panic
- a string.min_len on a map<string, google.protobuf.StringValue> was
ignored
- order of rule violations did not match validate.proto
- unique wasn't respected on a repeated google.protobuf.Int32Value
- custom CEL rules are evaluated twice if there's a standard rule as
well

Tests expanded to cover these cases.

Author: jonbodner-buf

src/main/java/build/buf/protovalidate/EvaluatorBuilder.java

@@ -15,6 +15,7 @@
 package build.buf.protovalidate;
 
 import build.buf.protovalidate.exceptions.CompilationException;
+import build.buf.validate.EnumRules;
 import build.buf.validate.FieldPath;
 import build.buf.validate.FieldPathElement;
 import build.buf.validate.FieldRules;
@@ -308,7 +309,6 @@ private void buildValue(
       processWrapperRules(fieldDescriptor, fieldRules, valueEvaluator);
       processStandardRules(fieldDescriptor, fieldRules, valueEvaluator);
       processAnyRules(fieldDescriptor, fieldRules, valueEvaluator);
-      processEnumRules(fieldDescriptor, fieldRules, valueEvaluator);
       processMapRules(fieldDescriptor, fieldRules, valueEvaluator);
       processRepeatedRules(fieldDescriptor, fieldRules, valueEvaluator);
     }
@@ -456,7 +456,13 @@ private void processWrapperRules(
       ValueEvaluator unwrapped =
           new ValueEvaluator(
               valueEvaluatorEval.getDescriptor(), valueEvaluatorEval.getNestedRule());
-      buildValue(fieldDescriptor.getMessageType().findFieldByName("value"), fieldRules, unwrapped);
+      // Only the type rules apply to the inner value; the outer pipeline already
+      // handled the rest (cel, cel_expression, ...), which would otherwise run twice.
+      FieldRules innerRules =
+          FieldRules.newBuilder()
+              .setField(expectedWrapperDescriptor, fieldRules.getField(expectedWrapperDescriptor))
+              .build();
+      buildValue(fieldDescriptor.getMessageType().findFieldByName("value"), innerRules, unwrapped);
       valueEvaluatorEval.append(unwrapped);
     }
 
@@ -466,14 +472,38 @@ private void processStandardRules(
 
       // If this is a wrapper field, just return. Wrapper fields are handled by
       // processWrapperRules and their unwrapped values are passed through the process gauntlet.
-      if (fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE) {
+      // A list of wrappers still needs its list-level rules (min_items, unique).
+      if (fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE
+          && (!fieldDescriptor.isRepeated() || valueEvaluatorEval.hasNestedRule())) {
         FieldDescriptor expectedWrapperDescriptor =
             DescriptorMappings.expectedWrapperRules(fieldDescriptor.getMessageType().getFullName());
         if (expectedWrapperDescriptor != null) {
           return;
         }
       }
 
+      // defined_only has its own evaluator; keep it in validate.proto order,
+      // between const and the remaining enum rules.
+      EnumRules enumRules = fieldRules.getEnum();
+      if (fieldDescriptor.getJavaType() == FieldDescriptor.JavaType.ENUM
+          && enumRules.getDefinedOnly()) {
+        if (enumRules.hasConst()) {
+          FieldRules constRules =
+              FieldRules.newBuilder()
+                  .setEnum(EnumRules.newBuilder().setConst(enumRules.getConst()))
+                  .build();
+          appendStandardRules(fieldDescriptor, constRules, valueEvaluatorEval);
+          fieldRules = fieldRules.toBuilder().setEnum(enumRules.toBuilder().clearConst()).build();
+        }
+        valueEvaluatorEval.append(
+            new EnumEvaluator(valueEvaluatorEval, fieldDescriptor.getEnumType().getValues()));
+      }
+      appendStandardRules(fieldDescriptor, fieldRules, valueEvaluatorEval);
+    }
+
+    private void appendStandardRules(
+        FieldDescriptor fieldDescriptor, FieldRules fieldRules, ValueEvaluator valueEvaluatorEval)
+        throws CompilationException {
       // Try native rule evaluators when opted in. Any rule covered natively is cleared on the
       // residual builder so CEL only compiles what's left; rules without a native implementation
       // remain on the residual and CEL handles them.
@@ -510,18 +540,6 @@ private void processAnyRules(
               fieldRules.getAny().getNotInList()));
     }
 
-    private void processEnumRules(
-        FieldDescriptor fieldDescriptor, FieldRules fieldRules, ValueEvaluator valueEvaluatorEval) {
-      if (fieldDescriptor.getJavaType() != FieldDescriptor.JavaType.ENUM) {
-        return;
-      }
-      if (fieldRules.getEnum().getDefinedOnly()) {
-        Descriptors.EnumDescriptor enumDescriptor = fieldDescriptor.getEnumType();
-        valueEvaluatorEval.append(
-            new EnumEvaluator(valueEvaluatorEval, enumDescriptor.getValues()));
-      }
-    }
-
     private void processMapRules(
         FieldDescriptor fieldDescriptor, FieldRules fieldRules, ValueEvaluator valueEvaluatorEval)
         throws CompilationException {

src/main/java/build/buf/protovalidate/NumericRulesEvaluator.java

@@ -227,6 +227,16 @@ public List<RuleViolation.Builder> evaluate(Value val, boolean failFast) {
       }
     }
 
+    if (lowerKind != LowerBound.NONE || upperKind != UpperBound.NONE) {
+      RuleViolation.Builder rangeViolation = buildRangeViolation(val, actual);
+      if (rangeViolation != null) {
+        violations = RuleBase.add(violations, rangeViolation);
+        if (failFast) {
+          return base.done(violations);
+        }
+      }
+    }
+
     if (!inVals.isEmpty() && !containsValue(inVals, actual)) {
       violations =
           RuleBase.add(
@@ -269,16 +279,6 @@ public List<RuleViolation.Builder> evaluate(Value val, boolean failFast) {
       }
     }
 
-    if (lowerKind != LowerBound.NONE || upperKind != UpperBound.NONE) {
-      RuleViolation.Builder rangeViolation = buildRangeViolation(val, actual);
-      if (rangeViolation != null) {
-        violations = RuleBase.add(violations, rangeViolation);
-        if (failFast) {
-          return base.done(violations);
-        }
-      }
-    }
-
     return base.done(violations);
   }
 

src/main/java/build/buf/protovalidate/RuleCache.java

@@ -33,6 +33,7 @@
 import dev.cel.runtime.CelRuntime.Program;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -113,8 +114,7 @@ List<CompiledProgram> compile(
     }
     Message message = resolved.message;
     List<CelRule> completeProgramList = new ArrayList<>();
-    for (Map.Entry<FieldDescriptor, Object> entry : message.getAllFields().entrySet()) {
-      FieldDescriptor ruleFieldDesc = entry.getKey();
+    for (FieldDescriptor ruleFieldDesc : sortedRuleFields(message)) {
       List<CelRule> programList =
           compileRule(fieldDescriptor, forItems, resolved.setOneof, ruleFieldDesc, message);
       if (programList == null) continue;
@@ -134,6 +134,16 @@ List<CompiledProgram> compile(
     return Collections.unmodifiableList(programs);
   }
 
+  // getAllFields orders by field number, but violations follow validate.proto declaration order
+  // (string.len is field 19); extensions come last, by field number.
+  private static List<FieldDescriptor> sortedRuleFields(Message message) {
+    List<FieldDescriptor> fields = new ArrayList<>(message.getAllFields().keySet());
+    fields.sort(
+        Comparator.comparing(FieldDescriptor::isExtension)
+            .thenComparingInt(field -> field.isExtension() ? field.getNumber() : field.getIndex()));
+    return fields;
+  }
+
   private @Nullable List<CelRule> compileRule(
       FieldDescriptor fieldDescriptor,
       boolean forItems,

src/main/java/build/buf/protovalidate/Rules.java

@@ -62,12 +62,10 @@ private Rules() {}
       if (scalar == null) {
         return null;
       }
-      // When processWrapperRules recurses with the inner "value" field, the ValueEvaluator's
-      // descriptor is still the OUTER wrapper field. Detect that and wrap the scalar evaluator
-      // so it unwraps the wrapper Message at evaluation time before delegating.
-      FieldDescriptor outerDescriptor = valueEvaluator.getDescriptor();
-      if (outerDescriptor != null
-          && outerDescriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE) {
+      // For wrapper WKTs, fieldDescriptor is the inner "value" field but the runtime value is the
+      // wrapper message. valueEvaluator.getDescriptor() is null for list items and map values.
+      if (DescriptorMappings.expectedWrapperRules(fieldDescriptor.getContainingType().getFullName())
+          != null) {
         return new WrappedValueEvaluator(fieldDescriptor, scalar);
       }
       return scalar;

src/main/java/build/buf/protovalidate/StringRulesEvaluator.java

@@ -564,6 +564,15 @@ public List<RuleViolation.Builder> evaluate(Value val, boolean failFast) {
     String strVal = (String) val.rawValue();
     List<RuleViolation.Builder> violations = null;
 
+    if (constVal != null && !strVal.equals(constVal)) {
+      violations =
+          RuleBase.add(
+              violations,
+              NativeViolations.newViolation(
+                  CONST_SITE, null, "must equal `" + constVal + "`", val, constVal));
+      if (failFast) return base.done(violations);
+    }
+
     if (exactLen != null || minLen != null || maxLen != null) {
       long runeCount = strVal.codePointCount(0, strVal.length());
       violations = applyLength(violations, val, runeCount, failFast);
@@ -580,15 +589,6 @@ public List<RuleViolation.Builder> evaluate(Value val, boolean failFast) {
       }
     }
 
-    if (constVal != null && !strVal.equals(constVal)) {
-      violations =
-          RuleBase.add(
-              violations,
-              NativeViolations.newViolation(
-                  CONST_SITE, null, "must equal `" + constVal + "`", val, constVal));
-      if (failFast) return base.done(violations);
-    }
-
     if (pattern != null && !pattern.matches(strVal)) {
       violations =
           RuleBase.add(