WG012: flag MutableSideEffect with reference-equality value type

CHANGELOG.md

@@ -20,6 +20,27 @@ project adheres to [Semantic Versioning](https://semver.org/) (see
 
 ### Added
 
+- **WG012 — MutableSideEffect with reference-equality value type** (Determinism). Flags
+  `Workflow.mutableSideEffect(id, valueClass, updateFunction, func)` calls whose value type
+  relies on inherited, identity-based `Object.equals()` — since `func` constructs a new
+  instance every call, `equals()` never reports "unchanged," so every call appends a new
+  history event regardless of whether the logical value actually changed. This is WoGu's
+  first genuinely new declarative rule *type*: `mutable-side-effect-equality`, alongside
+  the existing `forbidden-method`. Unlike every prior rule, the violation isn't "this call
+  happened" but "this call happened with an argument whose *resolved type* lacks value
+  equality," so `RuleDefinition` gains a `valueTypeArgumentIndex` field, and a new
+  `ValueBasedEqualityArgumentTarget` (`wogu-temporal.callgraph`) inspects the named
+  argument's `Class<T>` literal via the symbol solver — treating a Java record, a type with
+  its own or an inherited non-`Object` `equals()`, or an unresolvable type (a dependency
+  outside WoGu's own classpath) all as safe, the same false-negative-preferring default
+  every other rule uses for unresolvable code. `docs/rules/WG012.md` documents it.
+- Fixed `SourceRootParser` silently failing to parse `record` declarations and
+  pattern-matching `instanceof` — both valid since Java 16, and routine in code written
+  against this project's own Java 17 baseline — because its `ParserConfiguration` never set
+  a `LanguageLevel` and fell back to JavaParser's more conservative default. Every rule's
+  analysis was silently incomplete against any file using either construct; both
+  `ParserConfiguration` instances (the main one, and the one `JavaParserTypeSolver` uses
+  internally for cross-file resolution) now request `JAVA_17` explicitly.
 - **WG011 — I/O or blocking calls inside Workflow.sideEffect()** (Determinism). Flags
   network, file, JDBC, and reflection calls reachable from inside a
   `Workflow.sideEffect(...)`/`Workflow.mutableSideEffect(...)` callback — a real

docs/rules/WG012.md

@@ -0,0 +1,123 @@
+# WG012 — MutableSideEffect with reference-equality value type
+
+| | |
+|---|---|
+| **Rule ID** | WG012 |
+| **Category** | Determinism ([WG001–WG099](../../CONTRIBUTING.md#rule-numbering)) |
+| **Severity** | WARNING |
+| **Engine** | Temporal Java SDK |
+| **Since** | 1.0.0 |
+| **Auto Fix** | Not Available |
+
+## Problem
+
+A `Workflow.mutableSideEffect(id, valueClass, updateFunction, func)` call's `valueClass`
+argument names a type that relies on inherited, identity-based `Object.equals()` instead of
+a real, value-based one.
+
+## Why this matters
+
+`Workflow.mutableSideEffect()` records a new history event only when its `updateFunction`
+reports the value as changed — that's the entire point of the API over a plain
+`sideEffect()`: skip recording (and skip re-running `func`) when nothing actually changed.
+The common `updateFunction` pattern is `(oldValue, newValue) -> !oldValue.equals(newValue)`,
+which delegates the "did it change" question straight to `equals()`.
+
+A value type with no `equals()` override falls back to `Object`'s default, which compares
+object identity, not field values. Since `func` constructs a *new* instance on every call,
+two logically-identical values are still two different objects — `equals()` always returns
+`false`, `updateFunction` always reports "changed," and every single call appends a new
+`MarkerRecorded` event to workflow history regardless of whether the underlying value moved
+at all. `mutableSideEffect()` silently degrades into `sideEffect()` with extra bookkeeping
+overhead, and a workflow that calls it frequently accumulates history far faster than its
+author likely expects.
+
+## Bad Example
+
+```java
+public class PaymentWorkflowImpl implements PaymentWorkflow {
+  @Override
+  public String fetchCurrentPrice() {
+    Price price = Workflow.mutableSideEffect(
+        "price",
+        Price.class,
+        (oldValue, newValue) -> !oldValue.equals(newValue),
+        () -> pricingService.currentPrice());
+    return price.toString();
+  }
+}
+
+class Price {
+  // No equals()/hashCode() override — every call is reported as "changed."
+  double amount;
+}
+```
+
+## Good Example
+
+```java
+public class PaymentWorkflowImpl implements PaymentWorkflow {
+  @Override
+  public String fetchCurrentPrice() {
+    Price price = Workflow.mutableSideEffect(
+        "price",
+        Price.class,
+        (oldValue, newValue) -> !oldValue.equals(newValue),
+        () -> pricingService.currentPrice());
+    return price.toString();
+  }
+}
+
+record Price(double amount) {
+  // A record: the language always synthesizes a value-based equals()/hashCode(), so
+  // updateFunction correctly reports "unchanged" once the price stabilizes.
+}
+```
+
+A hand-written `equals()`/`hashCode()` pair (or a primitive/JDK wrapper type like
+`Integer`/`String`) is equally correct — the rule only cares that *some* value-based
+equality exists, not how it's implemented.
+
+## Recommended Fix
+
+Use a primitive, a record, or a type with a proper value-based `equals()`/`hashCode()` for
+the `mutableSideEffect` value type. WG012 checks the value type's resolved `equals()`, not
+the `updateFunction`, so rewriting only the `updateFunction`'s comparison logic (e.g. to
+compare fields directly instead of calling `equals()`) does not suppress this diagnostic —
+the value type itself still needs real equality for WoGu to recognize it as safe.
+
+## References
+
+- [Temporal documentation: Workflow determinism](https://docs.temporal.io/workflows#deterministic-constraints)
+- [Temporal Java SDK: `Workflow.mutableSideEffect()`](https://javadoc.io/doc/io.temporal/temporal-sdk/latest/io/temporal/workflow/Workflow.html)
+
+## False Positives
+
+Unlike WG001–WG011, WG012 isn't a call-graph reachability rule — it's a `Class<T>`
+argument type check. WoGu resolves the value type via its symbol solver: a Java record is
+always treated as safe (the language guarantees a synthesized value-based `equals()`); a
+type declaring its own `equals(Object)`, or inheriting one from a resolvable ancestor other
+than `java.lang.Object`, is also treated as safe.
+
+A type WoGu cannot resolve — one that lives in a compiled dependency rather than the
+project's own source, since `SourceRootParser` never includes the target project's compiled
+classpath — is treated as "cannot determine," not a violation. This is a deliberate
+false-negative-preferring choice, the same one every other rule in this catalog makes for
+an unresolvable call: WG012 will not flag a well-known JDK type it happens to resolve via
+reflection (`Integer`, `String`, and similar all have real `equals()` and are correctly
+recognized as safe), but a type from a third-party library not on WoGu's own classpath will
+silently not be checked either way.
+
+WG012 only inspects the `valueClass` argument; it never looks at `updateFunction`. An
+`updateFunction` written to compare fields directly instead of calling `equals()` may well
+be correct at runtime, but WoGu has no way to verify that a given lambda actually implements
+value equality correctly (a trivial or inverted comparator would look the same
+syntactically), so this is not a check WG012 attempts — the diagnostic will still fire
+until the value type itself has real equality.
+
+If you believe WG012 is flagging or missing something incorrectly, please open an issue
+with a minimal reproduction.
+
+## Since Version
+
+1.0.0

sample-temporal-project/pom.xml

@@ -19,12 +19,13 @@
     calls into four service classes under com.example.wogu.sample.services, each grouped by
     the kind of non-determinism it demonstrates: PaymentService (UUID.randomUUID() for
     WG001, Thread.sleep() for WG002, System.currentTimeMillis() for WG003, a network call
-    inside Workflow.sideEffect() for WG011), RandomnessService (Math.random() for WG004;
-    new Random()/nextInt() for WG005; ThreadLocalRandom.current() for WG006; new
+    inside Workflow.sideEffect() for WG011, a Workflow.mutableSideEffect() value type with
+    no equals() override for WG012), RandomnessService (Math.random() for WG004; new
+    Random()/nextInt() for WG005; ThreadLocalRandom.current() for WG006; new
     SecureRandom()/nextBytes() for WG007), ConfigurationService (System.getenv() for WG008,
     System.getProperty() for WG009), and ConcurrencyService
     (Executors.newFixedThreadPool() for WG010). 'mvn verify' intentionally fails here and
-    writes target/wogu/index.html showing all thirteen violations (WG005 and WG007 each
+    writes target/wogu/index.html showing all fourteen violations (WG005 and WG007 each
     report twice: once for the constructor, once for the instance method called on it).
 
     PaymentWorkflowImpl also calls a real Activity (PaymentActivityImpl, via its
@@ -41,7 +42,10 @@
     shows the other side of that same coin: WG011 fires precisely because the call is inside
     a side effect, since sideEffect() makes I/O safe from determinism problems but not from
     the availability problems of running unretried, unbounded network calls on the workflow
-    thread.
+    thread. PaymentService.refreshCachedBalance() demonstrates a different Workflow API
+    entirely: mutableSideEffect()'s updateFunction depends on CachedBalance.equals(), which
+    is never overridden, so WG012 fires because every call is reported as "changed" even
+    when the underlying balance hasn't moved.
 
     This module is intentionally excluded from the repo's default Maven reactor (see the
     root pom's 'with-samples' profile), since it fails by design. Run it explicitly:

sample-temporal-project/src/main/java/com/example/wogu/sample/PaymentWorkflowImpl.java

@@ -22,6 +22,7 @@ public String processPayment(String accountId) {
     long processedAt = paymentService.recordTimestamp();
     String auditId = paymentService.recordAuditId();
     paymentService.fetchAccountTier();
+    paymentService.refreshCachedBalance();
 
 
     randomnessService.rollDiscount();

sample-temporal-project/src/main/java/com/example/wogu/sample/services/CachedBalance.java

@@ -0,0 +1,13 @@
+package com.example.wogu.sample.services;
+
+public class CachedBalance {
+
+  // Intentionally no equals()/hashCode() override — this is what WG012 flags on the
+  // Workflow.mutableSideEffect() call in PaymentService.refreshCachedBalance(): every call
+  // is reported as "changed" regardless of whether the amount actually moved.
+  double amount;
+
+  CachedBalance(double amount) {
+    this.amount = amount;
+  }
+}

sample-temporal-project/src/main/java/com/example/wogu/sample/services/PaymentService.java

@@ -47,4 +47,17 @@ public String fetchAccountTier() {
       }
     });
   }
+
+  public double refreshCachedBalance() {
+    // Intentional WoGu demo violation (WG012): CachedBalance (see CachedBalance.java) has
+    // no equals()/hashCode() override, so updateFunction's !oldValue.equals(newValue) is
+    // always true regardless of whether the balance actually changed, and every call
+    // appends a new history event.
+    CachedBalance balance = Workflow.mutableSideEffect(
+        "balance",
+        CachedBalance.class,
+        (oldValue, newValue) -> !oldValue.equals(newValue),
+        () -> new CachedBalance(100.0));
+    return balance.amount;
+  }
 }

wogu-temporal/src/main/java/dev/wogu/temporal/ForbiddenMethodRule.java

@@ -100,7 +100,12 @@ private static CallTarget toMethodCallTarget(String qualifiedMethodReference) {
     return new StaticMethodCallTarget(className, methodName);
   }
 
-  private static ExecutionContext toExecutionContext(String name) {
+  /**
+   * Package-private (not {@code private}) so {@link MutableSideEffectEqualityRule} can
+   * reuse this same {@code ExecutionContext} name-to-constant conversion for its own
+   * {@code suppressedContexts}/{@code requiredContexts} instead of duplicating it.
+   */
+  static ExecutionContext toExecutionContext(String name) {
     try {
       return ExecutionContext.valueOf(name);
     } catch (IllegalArgumentException e) {

wogu-temporal/src/main/java/dev/wogu/temporal/MutableSideEffectEqualityRule.java

@@ -0,0 +1,91 @@
+package dev.wogu.temporal;
+
+import com.github.javaparser.ast.body.MethodDeclaration;
+import dev.wogu.api.Rule;
+import dev.wogu.api.ValidationContext;
+import dev.wogu.api.Violation;
+import dev.wogu.temporal.callgraph.CallGraphAnalyzer;
+import dev.wogu.temporal.callgraph.CallTarget;
+import dev.wogu.temporal.callgraph.ContextEntryPoint;
+import dev.wogu.temporal.callgraph.ExecutionContext;
+import dev.wogu.temporal.callgraph.ValueBasedEqualityArgumentTarget;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+
+/**
+ * The {@code mutable-side-effect-equality} declarative rule type: flags a call to the
+ * single method named in a {@link RuleDefinition}'s {@code methods} list whose
+ * {@code Class<T>} argument at {@code valueTypeArgumentIndex} names a type relying on
+ * inherited, identity-based {@code equals()}.
+ *
+ * <p>This is the generic engine behind WG012 ({@code Workflow.mutableSideEffect()}'s value
+ * type) and any future "this argument's resolved type must have real equality" rule of the
+ * same shape — none of which need a dedicated Java class. Adding one is purely a matter of
+ * adding a YAML definition with {@code type: mutable-side-effect-equality},
+ * {@code methods: [the one method]}, and {@code valueTypeArgumentIndex} under
+ * {@code src/main/resources/rules}; this class builds the matching
+ * {@link ValueBasedEqualityArgumentTarget} and reuses {@link TemporalRuleSupport} for the
+ * actual call-graph traversal, suppression, and violation building, exactly like
+ * {@link ForbiddenMethodRule} does for its own shape — including honoring the same
+ * {@code suppressedContexts}/{@code requiredContexts} fields, via the same
+ * {@link ForbiddenMethodRule#toExecutionContext} conversion.
+ */
+final class MutableSideEffectEqualityRule implements TemporalRule {
+
+  private final Rule metadata;
+  private final CallTarget target;
+  private final String message;
+  private final String suggestedFix;
+  private final Set<ExecutionContext> suppressedContexts;
+  private final Set<ExecutionContext> requiredContexts;
+
+  MutableSideEffectEqualityRule(RuleDefinition definition) {
+    this.metadata = definition.toRule();
+    if (definition.methods().size() != 1) {
+      throw new IllegalArgumentException(
+          "Rule '" + definition.id() + "' of type 'mutable-side-effect-equality' must declare exactly one "
+              + "method, found: " + definition.methods());
+    }
+    if (definition.valueTypeArgumentIndex() == null) {
+      throw new IllegalArgumentException(
+          "Rule '" + definition.id() + "' of type 'mutable-side-effect-equality' requires "
+              + "'valueTypeArgumentIndex'");
+    }
+    this.target =
+        new ValueBasedEqualityArgumentTarget(definition.methods().get(0), definition.valueTypeArgumentIndex());
+    this.message = definition.description();
+    this.suggestedFix = definition.replacement();
+    this.suppressedContexts =
+        definition.suppressedContexts().stream().map(ForbiddenMethodRule::toExecutionContext).collect(Collectors.toUnmodifiableSet());
+    this.requiredContexts =
+        definition.requiredContexts().stream().map(ForbiddenMethodRule::toExecutionContext).collect(Collectors.toUnmodifiableSet());
+  }
+
+  @Override
+  public Rule metadata() {
+    return metadata;
+  }
+
+  @Override
+  public List<Violation> evaluate(
+      ValidationContext context,
+      List<ScannedWorkflowClass> workflowClasses,
+      CallGraphAnalyzer callGraph,
+      Predicate<MethodDeclaration> activityBoundary,
+      List<ContextEntryPoint> contextEntryPoints) {
+    return TemporalRuleSupport.findViolations(
+        workflowClasses,
+        callGraph,
+        target,
+        metadata,
+        context,
+        message,
+        suggestedFix,
+        activityBoundary,
+        contextEntryPoints,
+        suppressedContexts,
+        requiredContexts);
+  }
+}

wogu-temporal/src/main/java/dev/wogu/temporal/RuleDefinition.java

@@ -44,6 +44,10 @@
  *     exists specifically to flag I/O reachable only from inside a
  *     {@code Workflow.sideEffect(...)} callback, needs this: {@code suppressedContexts}
  *     alone can only exclude a context, never require one.
+ * @param valueTypeArgumentIndex for {@code mutable-side-effect-equality} rules (see
+ *     {@link dev.wogu.temporal.callgraph.ValueBasedEqualityArgumentTarget}): 0-based index
+ *     of the {@code Class<T>} argument, on the single method named in {@link #methods()},
+ *     whose resolved type must have real value-based equality; {@code null} for other types
  * @param tags free-form labels, reserved for future use (not yet rendered anywhere)
  */
 record RuleDefinition(
@@ -61,6 +65,7 @@ record RuleDefinition(
     List<String> constructors,
     List<String> suppressedContexts,
     List<String> requiredContexts,
+    Integer valueTypeArgumentIndex,
     List<String> tags) {
 
   RuleDefinition {

wogu-temporal/src/main/java/dev/wogu/temporal/RuleDefinitionLoader.java

@@ -84,6 +84,7 @@ RuleDefinition parse(String yaml, String sourceName) {
         stringList(data, "constructors"),
         stringList(data, "suppressedContexts"),
         stringList(data, "requiredContexts"),
+        optionalInt(data, "valueTypeArgumentIndex"),
         stringList(data, "tags"));
   }
 
@@ -115,6 +116,11 @@ private static List<String> stringList(Map<String, Object> data, String key) {
     return ((List<Object>) value).stream().map(Object::toString).toList();
   }
 
+  private static Integer optionalInt(Map<String, Object> data, String key) {
+    Object value = data.get(key);
+    return value == null ? null : Integer.valueOf(value.toString());
+  }
+
   private List<String> listYamlResourceNames() {
     List<String> names = new ArrayList<>();
     try {

wogu-temporal/src/main/java/dev/wogu/temporal/RuleRegistry.java

@@ -22,7 +22,9 @@
 final class RuleRegistry {
 
   private static final Map<String, Function<RuleDefinition, TemporalRule>> FACTORIES_BY_TYPE =
-      Map.of("forbidden-method", ForbiddenMethodRule::new);
+      Map.of(
+          "forbidden-method", ForbiddenMethodRule::new,
+          "mutable-side-effect-equality", MutableSideEffectEqualityRule::new);
 
   private RuleRegistry() {}