Skip to content

Add eval for anyMatch reject-if-any validation helpers #55

Description

@martinfrancois

Problem

The Java Streams skill should recognize small validation helpers where a loop only answers whether any element violates a predicate, then throws one common exception. In that shape, stream().anyMatch(...) with a named predicate can be clearer than a loop because the code states the domain rule directly: reject if any entry is invalid.

This is eval-worthy because the skill should not blindly replace every loop. It should recommend this shape only when all invalid elements are equivalent, the method does not need the offending element for diagnostics, short-circuiting is preserved, and the exception behavior remains unchanged.

Code before the prompt was executed

The implementation was functionally correct. It looped over top-level YAML front-matter entries and threw the same public workflow parse error when either a top-level key or value was null.

private static void rejectNullTopLevelEntries(LinkedHashMap<String, Object> parsed) {
    for (Map.Entry<String, Object> entry : parsed.entrySet()) {
        if (entry.getKey() == null || entry.getValue() == null) {
            throw new WorkflowException(
                    "workflow_parse_error", "Workflow front matter top-level keys and values must not be null");
        }
    }
}

Prompt that caused the implementation

A fuzzing hardening change added explicit validation for null top-level workflow front-matter entries. The useful implementation requirements were:

  • reject null top-level YAML keys and values;
  • preserve nested null config values;
  • keep the public error code stable;
  • keep the failure deterministic and non-crashing for fuzzed workflow input.

The original implementation used the loop above.

Later prompt that exposed the issue

A later maintainer prompt asked:

can the implementation of rejectNullTopLevelEntries(LinkedHashMap<String potentially be improved with the streams skill?

That prompt should cause the skill to review whether a stream improves readability without changing behavior.

Prompt-produced code before maintainer correction

There was no intermediate broken stream implementation. The original loop was correct, but it encoded an existence check imperatively.

private static void rejectNullTopLevelEntries(LinkedHashMap<String, Object> parsed) {
    for (Map.Entry<String, Object> entry : parsed.entrySet()) {
        if (entry.getKey() == null || entry.getValue() == null) {
            throw new WorkflowException(
                    "workflow_parse_error", "Workflow front matter top-level keys and values must not be null");
        }
    }
}

Why the prompt-produced code is bad

The original code is not bad in the sense of being incorrect. It is only slightly weaker as an expression of intent: it manually walks entries even though the only question is whether any entry has a null key or value.

The skill should avoid overstating this as a performance fix. The loop already short-circuits. The stream version is preferable here for readability because anyMatch expresses the existence check directly, and a named predicate keeps the null rule readable.

Behavior-equivalence analysis

The replacement is behavior-preserving because:

  • anyMatch short-circuits like the original loop;
  • the offending entry is not used in the error message, so not retaining it does not change diagnostics;
  • the same WorkflowException code and message are thrown;
  • iteration still covers only parsed.entrySet(), so nested null values remain allowed;
  • the method still performs validation before returning parsed config;
  • no output collection, ordering change, mutability change, parser change, or side effect is introduced.

A stream version would not be appropriate if the method needed to report the first invalid key, aggregate every invalid entry, mutate entries, or preserve a more complex branch-specific exception.

Maintainer-preferred code

private static void rejectNullTopLevelEntries(LinkedHashMap<String, Object> parsed) {
    if (parsed.entrySet().stream().anyMatch(WorkflowLoader::hasNullKeyOrValue)) {
        throw new WorkflowException(
                "workflow_parse_error", "Workflow front matter top-level keys and values must not be null");
    }
}

private static boolean hasNullKeyOrValue(Map.Entry<String, Object> entry) {
    return entry.getKey() == null || entry.getValue() == null;
}

Why the replacement is better

The replacement states the validation rule at the same level as the method name: reject if any top-level entry has a null key or value. The named predicate keeps the stream lambda as glue rather than embedding branching logic directly in the pipeline.

Desired eval behavior

  • Reward suggesting anyMatch for pure reject-if-any validation loops when all failures share one outcome.
  • Reward keeping the predicate in a named helper when it improves readability.
  • Reward explicitly saying the original loop is correct and already short-circuits.
  • Reward behavior-equivalence analysis for exception message, null handling, side effects, and nested values.
  • Reward keeping the loop when the invalid element is needed for diagnostics or when branches differ.

Anti-patterns the eval should reject

  • Replacing the loop with filter(...).findAny().get() or filter(...).toList().isEmpty().
  • Collecting invalid entries just to check whether any exist.
  • Throwing from inside a stream lambda with a multi-line block when a direct anyMatch plus one throw is clearer.
  • Changing the public exception code, message, or top-level-only null validation boundary.
  • Claiming a performance improvement without evidence.

Suggested eval name

anymatch-reject-if-any-validation

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions