WG013: flag catching Throwable or Error in workflow code

CHANGELOG.md

@@ -20,6 +20,22 @@ project adheres to [Semantic Versioning](https://semver.org/) (see
 
 ### Added
 
+- **WG013 — Catching Throwable or Error in workflow code** (Determinism). Flags
+  `catch (Throwable ...)`/`catch (Error ...)` reachable from a workflow entry point,
+  including a multi-catch's individual component types: the SDK uses unchecked `Error`
+  subclasses internally for cancellation and workflow-task-failure control flow, and a
+  bare `Throwable`/`Error` catch swallows those signals along with everything else. A third
+  new declarative rule type, `forbidden-catch-type`, alongside `forbidden-method` and
+  `mutable-side-effect-equality`: a `catch` clause's caught type is neither a method call
+  nor a constructor call, so it's not something any existing `CallTarget` can match.
+  `CallGraphAnalyzer` gains `findCaughtTypeMatches`, a new traversal method reusing all of
+  `findCallPaths`'s existing path/visiting/context bookkeeping (`recordMatch`,
+  `effectiveContext`, `resolveToSource`) without touching `search()` itself.
+  `TemporalRuleSupport.findViolations` is refactored to split its filter-and-convert step
+  into a reusable `toViolations(List<CallGraphMatch>, ...)`, so `ForbiddenCatchTypeRule`
+  shares the exact same `suppressedContexts`/`requiredContexts` handling
+  `ForbiddenMethodRule` uses instead of a third copy of that logic. `docs/rules/WG013.md`
+  documents it.
 - **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

docs/rules/WG013.md

@@ -0,0 +1,126 @@
+# WG013 — Catching Throwable or Error in workflow code
+
+| | |
+|---|---|
+| **Rule ID** | WG013 |
+| **Category** | Determinism ([WG001–WG099](../../CONTRIBUTING.md#rule-numbering)) |
+| **Severity** | ERROR |
+| **Engine** | Temporal Java SDK |
+| **Since** | 1.0.0 |
+| **Auto Fix** | Not Available |
+
+## Problem
+
+A Temporal workflow implementation — or any code reachable from a workflow's entry point,
+however many method calls away — contains a `catch (Throwable ...)` or `catch (Error ...)`
+clause.
+
+## Why this matters
+
+The Temporal Java SDK uses unchecked `Error` subclasses internally as control-flow signals:
+for cancellation, and for the destroy path a workflow task takes when it fails
+deterministically and needs to safely stop and retry. Both are implemented as `Error`, not
+as a checked or ordinary runtime exception, specifically so application code's normal
+`catch (Exception ...)` handlers don't accidentally intercept them.
+
+`catch (Throwable ...)` and `catch (Error ...)` don't respect that boundary — they catch
+everything, including the SDK's own internal signals. A workflow that swallows them this way
+can silently fail to propagate a cancellation, or can limp forward in a corrupted state after
+a workflow task failure that was supposed to stop it cleanly. There is no legitimate reason
+for workflow code to catch either type directly.
+
+## Bad Example
+
+```java
+public class PaymentWorkflowImpl implements PaymentWorkflow {
+  @Override
+  public void processPayment(String accountId) {
+    try {
+      activities.chargeCard(accountId);
+    } catch (Throwable t) {
+      logger.warn("ignoring failure", t); // swallows Temporal's internal Error signals too
+    }
+  }
+}
+```
+
+This is flagged even when the `catch` is several methods away from the workflow entry point:
+
+```java
+public class PaymentWorkflowImpl implements PaymentWorkflow {
+  private final PaymentService paymentService = new PaymentService();
+
+  @Override
+  public void processPayment(String accountId) {
+    // WG013 still fires: WoGu's call-graph analysis follows this call into
+    // PaymentService.chargeCardSafely() and finds catch (Throwable) inside it.
+    paymentService.chargeCardSafely(accountId);
+  }
+}
+
+class PaymentService {
+  void chargeCardSafely(String accountId) {
+    try {
+      activities.chargeCard(accountId);
+    } catch (Throwable t) {
+      // swallowed
+    }
+  }
+}
+```
+
+A multi-catch is checked component by component, so `catch (IOException | Throwable t)` is
+flagged for its `Throwable` component even though `IOException` alone would be fine.
+
+## Good Example
+
+```java
+public class PaymentWorkflowImpl implements PaymentWorkflow {
+  @Override
+  public void processPayment(String accountId) {
+    try {
+      activities.chargeCard(accountId);
+    } catch (ActivityFailure e) {
+      // handle the specific, expected failure
+    }
+  }
+}
+```
+
+## Recommended Fix
+
+Catch specific checked or application exceptions (`ActivityFailure`, your own business
+exceptions) instead of `Throwable` or `Error`. If you need a catch-all for logging or
+cleanup purposes, catch `Exception` — never `Throwable`, and never `Error`.
+
+## References
+
+- [Temporal documentation: Workflow determinism](https://docs.temporal.io/workflows#deterministic-constraints)
+
+## False Positives
+
+WG013 uses the same call-graph analysis as WG001–WG012: starting from a workflow's
+entry-point method(s), it follows every method call that can be resolved to another
+method's source within the same project, however many hops deep, looking for a `catch`
+clause matching `java.lang.Throwable` or `java.lang.Error` along the way — including inside
+a `Workflow.sideEffect(...)`/`Workflow.mutableSideEffect(...)` callback, since the danger
+(swallowing an internal SDK signal on the workflow thread) applies there just as much as in
+ordinary workflow code; unlike WG001, WG013 declares no `suppressedContexts`.
+
+As with every call-graph-based rule, this analysis intentionally stops, without reporting
+anything past that point, when it reaches a call it cannot resolve to source it can see —
+most notably, a call into an **Activity** implementation (reached through the Activity's
+interface, which has no body to look inside, so the implementation's real `catch` clauses
+are never reached), a call into a compiled dependency, or a call resolved only through
+reflection or dynamic dispatch.
+
+This means WG013 can produce **false negatives** (a real violation hidden behind a call it
+can't see into) but is designed to avoid **false positives** — it will not flag a `catch`
+clause for any type other than `Throwable`/`Error` themselves; catching a subclass of
+`Exception`, or any other checked/unchecked exception type, is untouched. If you believe
+WG013 is flagging or missing something incorrectly, please open an issue with a minimal
+reproduction.
+
+## Since Version
+
+1.0.0

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

@@ -0,0 +1,80 @@
+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.CallGraphMatch;
+import dev.wogu.temporal.callgraph.ContextEntryPoint;
+import dev.wogu.temporal.callgraph.ExecutionContext;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+
+/**
+ * The {@code forbidden-catch-type} declarative rule type: flags a {@code catch} clause,
+ * reachable from a workflow entry point, whose caught type matches one of a
+ * {@link RuleDefinition}'s {@code catchTypes}.
+ *
+ * <p>This is the generic engine behind WG013 ({@code catch (Throwable)}/{@code catch
+ * (Error)}) and any future "flag catching this exception type" rule — none of which need a
+ * dedicated Java class. Adding one is purely a matter of adding a YAML definition with
+ * {@code type: forbidden-catch-type} and a {@code catchTypes} list under
+ * {@code src/main/resources/rules}; this class reads it, calls
+ * {@link CallGraphAnalyzer#findCaughtTypeMatches}, and reuses
+ * {@link TemporalRuleSupport#toViolations} for the same suppressed/required-context
+ * filtering and violation-building {@link ForbiddenMethodRule} uses for its own shape — the
+ * only real difference is which {@link CallGraphAnalyzer} method supplies the matches,
+ * since a {@code catch} clause's type isn't a method call or constructor a
+ * {@link dev.wogu.temporal.callgraph.CallTarget} can match.
+ */
+final class ForbiddenCatchTypeRule implements TemporalRule {
+
+  private final Rule metadata;
+  private final List<String> catchTypes;
+  private final String message;
+  private final String suggestedFix;
+  private final Set<ExecutionContext> suppressedContexts;
+  private final Set<ExecutionContext> requiredContexts;
+
+  ForbiddenCatchTypeRule(RuleDefinition definition) {
+    this.metadata = definition.toRule();
+    if (definition.catchTypes().isEmpty()) {
+      throw new IllegalArgumentException(
+          "Rule '" + definition.id() + "' of type 'forbidden-catch-type' requires at least one entry in "
+              + "'catchTypes'");
+    }
+    this.catchTypes = definition.catchTypes();
+    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) {
+    List<CallGraphMatch> matches = new ArrayList<>();
+    for (ScannedWorkflowClass workflowClass : workflowClasses) {
+      for (MethodDeclaration entryPoint : workflowClass.entryPoints()) {
+        matches.addAll(callGraph.findCaughtTypeMatches(entryPoint, catchTypes, activityBoundary, contextEntryPoints));
+      }
+    }
+    return TemporalRuleSupport.toViolations(
+        matches, metadata, context, message, suggestedFix, suppressedContexts, requiredContexts);
+  }
+}

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

@@ -48,6 +48,9 @@
  *     {@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 catchTypes for {@code forbidden-catch-type} rules: fully qualified names of
+ *     exception types that must not be caught in code reachable from a workflow entry
+ *     point, e.g. {@code "java.lang.Throwable"}; empty for other types
  * @param tags free-form labels, reserved for future use (not yet rendered anywhere)
  */
 record RuleDefinition(
@@ -66,6 +69,7 @@ record RuleDefinition(
     List<String> suppressedContexts,
     List<String> requiredContexts,
     Integer valueTypeArgumentIndex,
+    List<String> catchTypes,
     List<String> tags) {
 
   RuleDefinition {
@@ -83,6 +87,7 @@ record RuleDefinition(
     constructors = List.copyOf(Objects.requireNonNull(constructors, "constructors"));
     suppressedContexts = List.copyOf(Objects.requireNonNull(suppressedContexts, "suppressedContexts"));
     requiredContexts = List.copyOf(Objects.requireNonNull(requiredContexts, "requiredContexts"));
+    catchTypes = List.copyOf(Objects.requireNonNull(catchTypes, "catchTypes"));
     tags = List.copyOf(Objects.requireNonNull(tags, "tags"));
   }
 

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

@@ -85,6 +85,7 @@ RuleDefinition parse(String yaml, String sourceName) {
         stringList(data, "suppressedContexts"),
         stringList(data, "requiredContexts"),
         optionalInt(data, "valueTypeArgumentIndex"),
+        stringList(data, "catchTypes"),
         stringList(data, "tags"));
   }
 

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

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

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

@@ -98,22 +98,44 @@ static List<Violation> findViolations(
       List<ContextEntryPoint> contextEntryPoints,
       Set<ExecutionContext> suppressedContexts,
       Set<ExecutionContext> requiredContexts) {
-    List<Violation> violations = new ArrayList<>();
+    List<CallGraphMatch> matches = new ArrayList<>();
     for (ScannedWorkflowClass workflowClass : workflowClasses) {
       for (MethodDeclaration entryPoint : workflowClass.entryPoints()) {
         for (CallTarget target : targets) {
-          for (CallGraphMatch match : callGraph.findCallPaths(entryPoint, target, activityBoundary, contextEntryPoints)) {
-            if (suppressedContexts.contains(match.executionContext())) {
-              continue;
-            }
-            if (!requiredContexts.isEmpty() && !requiredContexts.contains(match.executionContext())) {
-              continue;
-            }
-            violations.add(toViolation(rule, context, match, message, suggestedFix));
-          }
+          matches.addAll(callGraph.findCallPaths(entryPoint, target, activityBoundary, contextEntryPoints));
         }
       }
     }
+    return toViolations(matches, rule, context, message, suggestedFix, suppressedContexts, requiredContexts);
+  }
+
+  /**
+   * Filters {@code matches} down to the ones {@code suppressedContexts}/{@code requiredContexts}
+   * allow, and converts what's left into {@link Violation}s. Split out from
+   * {@link #findViolations(List, CallGraphAnalyzer, List, Rule, ValidationContext, String,
+   * String, Predicate, List, Set, Set)} so a rule whose matches don't come from
+   * {@link CallGraphAnalyzer#findCallPaths} at all — e.g. {@link ForbiddenCatchTypeRule}'s
+   * {@link CallGraphAnalyzer#findCaughtTypeMatches} — can still reuse the same
+   * suppression/required-context semantics and violation shape instead of duplicating them.
+   */
+  static List<Violation> toViolations(
+      List<CallGraphMatch> matches,
+      Rule rule,
+      ValidationContext context,
+      String message,
+      String suggestedFix,
+      Set<ExecutionContext> suppressedContexts,
+      Set<ExecutionContext> requiredContexts) {
+    List<Violation> violations = new ArrayList<>();
+    for (CallGraphMatch match : matches) {
+      if (suppressedContexts.contains(match.executionContext())) {
+        continue;
+      }
+      if (!requiredContexts.isEmpty() && !requiredContexts.contains(match.executionContext())) {
+        continue;
+      }
+      violations.add(toViolation(rule, context, match, message, suggestedFix));
+    }
     return violations;
   }